They are exactly the same structure. Doing it in CBS allows us to switch
bssl::Span to absl::Span or a standard std::span in the future.
Bug: 132
Change-Id: Ibc96673c23233d557a1dd4d8768d2659d7a4ca0c
Reviewed-on: https://boringssl-review.googlesource.com/20669
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
There seems to be a GCC bug that requires kDefaultGroups having an
explicit cast, but this is still much nicer than void(const uint16_t **,
size_t *) functions.
Bug: 132
Change-Id: Id586d402ca0b8a01370353ff17295e71ee219ff3
Reviewed-on: https://boringssl-review.googlesource.com/20668
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
An Array<T> is an owning Span<T>. It's similar to absl::FixedArray<T>
but plays well with OPENSSL_malloc and doesn't implement inlining. With
OPENSSL_cleanse folded into OPENSSL_free, we could go nuts with
UniquePtr<uint8_t>, but having the pointer and length tied together is
nice for other reasons. Notably, Array<T> plays great with Span<T>.
Also switch the other parameter to a Span.
Bug: 132
Change-Id: I4cdcf810cf2838208c8ba9fcc6215c1e369dffb8
Reviewed-on: https://boringssl-review.googlesource.com/20667
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Rather than use those weird bitmasks, just pass an evp_aead_direction_t
and figure it out from there.
Change-Id: Ie52c6404bd0728d7d1ef964a3590d9ba0843c1d6
Reviewed-on: https://boringssl-review.googlesource.com/20666
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
draft-ietf-quic-tls needs access to the cipher's PRF hash to size its
keys correctly.
Change-Id: Ie4851f990e5e1be724f262f608f7195f7ca837ca
Reviewed-on: https://boringssl-review.googlesource.com/20624
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Fixes failed compile with [-Werror=implicit-fallthrough=], which is
default on gcc-7.x on distributions like fedora.
Enabling no implicit fallthrough for more than just clang as well to
catch this going forward.
Change-Id: I6cd880dac70ec126bd7812e2d9e5ff804d32cadd
Signed-off-by: Vincent Batts <vbatts@redhat.com>
Reviewed-on: https://boringssl-review.googlesource.com/20564
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
base.h pulls in all the forward declarations, so this isn't needed. We
should also remove bio.h and buf.h, but cURL seems to depend on those.
Code search suggests this one is okay though.
case:yes content:\bHMAC content:openssl/ssl.h -content:openssl/hmac.h
Change-Id: Id91686bd134649245855025940bc17f82823c734
Reviewed-on: https://boringssl-review.googlesource.com/20364
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Further testing suggests the behavior is slightly different than I
originally thought.
Change-Id: I3df6b3425dbb551e374159566ca969347d72a306
Reviewed-on: https://boringssl-review.googlesource.com/20284
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The Java client implementation of the 3SHAKE mitigation incorrectly
rejects initial handshakes when all of the following are true:
1. The ClientHello offered a session.
2. The session was successfully resumed previously.
3. The server declines the session.
4. The server sends a certificate with a different SAN list than in the
previous session.
(Note the 3SHAKE mitigation is to reject certificates changes on
renegotiation, while Java's logic applies to initial handshakes as
well.)
The end result is long-lived Java clients break on some certificate
rotations. Fingerprint Java clients and decline all offered sessions.
This avoids (2) while still introducing new sessions to clear any
existing problematic sessions.
See also b/65323005.
Change-Id: Ib2b84c69b5ecba285ffb8c4d03de5626838d794e
Reviewed-on: https://boringssl-review.googlesource.com/20184
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Bug: 124
Change-Id: Iff02be9df2806572e6d3f860b448f598f85778c3
Reviewed-on: https://boringssl-review.googlesource.com/20107
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
There's a lot of duplicated code between the two. This is in preparation
for adding two more of these fuzzers, this time for DTLS.
Bug: 124
Change-Id: I8ca2a02d599e2c88e30838d04b7cf07d4221aa76
Reviewed-on: https://boringssl-review.googlesource.com/20106
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Found with libFuzzer.
Bug: chromium:763097
Change-Id: I806bcfc714c0629ff7f725e37f4c0045d4ec7ac6
Reviewed-on: https://boringssl-review.googlesource.com/20105
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We forgot to reset that value.
Change-Id: Ic869cb61da332983cc40223cbbdf23b455dd9766
Reviewed-on: https://boringssl-review.googlesource.com/20084
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The new_session_cb callback should not be run if SSL_SESS_CACHE_CLIENT
is off.
Change-Id: I1ab320f33688f186b241d95c81775331a5c5b1a1
Reviewed-on: https://boringssl-review.googlesource.com/20065
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Right now we report the per-connection value during the handshake and
the per-session value after the handshake. This also trims our tickets
slightly by removing a largely unused field from SSL_SESSION.
Putting it on SSL_HANDSHAKE would be better, but sadly a number of
bindings-type APIs expose it after the handshake.
Change-Id: I6a1383f95da9b1b141b9d6adadc05ee1e458a326
Reviewed-on: https://boringssl-review.googlesource.com/20064
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Allocations by |OPENSSL_malloc| are prefixed with their length.
|OPENSSL_free| zeros the allocation before calling free(), eliminating
the need for a separate call to |OPENSSL_cleanse| for sensitive data.
This change will be followed up by the cleanup in
https://boringssl-review.googlesource.com/c/boringssl/+/19824.
Change-Id: Ie272f07e9248d7d78af9aea81dacec0fdb7484c4
Reviewed-on: https://boringssl-review.googlesource.com/19544
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I4dea223825da4e4ab0bc789e738f470f5fe5d659
Reviewed-on: https://boringssl-review.googlesource.com/20044
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
By resolving Channel ID earlier, we can take advantage of
flight-by-flight writes.
Change-Id: I31265bda3390eb1faec976ac13d7a01ba5f6dd5f
Reviewed-on: https://boringssl-review.googlesource.com/19925
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This fixes a regression in Conscrypt added by
https://boringssl-review.googlesource.com/19144. SSL_get_session
otherwise attempts to return hs->new_session, but that has been released
at this point.
Change-Id: I55b41cbefb65b3ae3cfbfad72f6338bd66db3341
Reviewed-on: https://boringssl-review.googlesource.com/19904
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
For historical reasons, TLS allows ServerHellos (and ClientHellos)
without extensions to omit the extensions fields entirely.
https://github.com/openssl/openssl/pull/4296 reports this is even
necessary for compatibility with extension-less clients. We continue to
do so, but add a test for it anyway.
Change-Id: I63c2e3a5f298674eb21952fca6914dad07d7c245
Reviewed-on: https://boringssl-review.googlesource.com/19864
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
That's the last of it!
Change-Id: I93d1f5ab7e95b2ad105c34b24297a0bf77625263
Reviewed-on: https://boringssl-review.googlesource.com/19784
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
ctx->cached_x509_client_CA needs to be protected under a lock since
SSL_CTX_get_client_CA_list is a logically const operation. The fallback
in SSL_get_client_CA_list was not using this lock.
Change-Id: I2431218492d1a853cc1a59c0678b0b50cd9beab2
Reviewed-on: https://boringssl-review.googlesource.com/19765
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
That function actually got a little complicated after the CRYPTO_BUFFER
work.
Change-Id: Ib679a9f2bcc2c974fe059af49805b8200e77bd03
Reviewed-on: https://boringssl-review.googlesource.com/19764
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
The fuzzer should discover this instantly, but it's a sufficiently
important failure case (don't accidentally drop the certificate on the
floor or anything weird like that) that it's probably worth testing.
Change-Id: I684932c2e8a88fcf9b2318bf46980d312c66f6ef
Reviewed-on: https://boringssl-review.googlesource.com/19744
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Easy bit of test coverage.
Change-Id: I0362fca926d82869b512e3c40dc53d6dc771dfc8
Reviewed-on: https://boringssl-review.googlesource.com/19724
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Bug: 128
Change-Id: Ief3779b1c43dd34a154a0f1d2f94d0da756bc07a
Reviewed-on: https://boringssl-review.googlesource.com/19144
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
OpenSSL's API has a non-fatal "soft fail" mode (can we get rid of
this?), so we should set the flag even if config->verify_fail is true.
Change-Id: I5a2a3290b9bf45c682f3a629a8b6474b1090fc6e
Reviewed-on: https://boringssl-review.googlesource.com/19684
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Consumers have been switched to the new ones.
Change-Id: I7a8ec6308775a105a490882c97955daed12a2c2c
Reviewed-on: https://boringssl-review.googlesource.com/19605
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We have fancy -on-initial and -on-resume prefixes now that can apply to
every flag.
Change-Id: I6195a97f663ebc94db320ca35889c213c700a976
Reviewed-on: https://boringssl-review.googlesource.com/19666
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We currently forbid the server certificate from changing on
renegotiation. This means re-verifying the certificate is pointless and
indeed the callback being called again seems to surprise consumers more
than anything else.
Carry over the initial handshake's SCT lists and OCSP responses (don't
enforce they don't change since the server may have, say, picked up new
OCSP responses in the meantime), ignore new ones received on
renegotiation, and don't bother redoing verification.
For our purposes, TLS 1.2 renegotiation is an overcomplicated TLS 1.3
KeyUpdate + post-handshake auth. The server is not allowed to change
identity.
Bug: 126
Change-Id: I0dae85bcf243943b1a5a97fa4f30f100c9e6e41e
Reviewed-on: https://boringssl-review.googlesource.com/19665
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We do not call the new_session callback on renego, but a consumer using
SSL_get_session may still attempt to resume such a session. Leave the
not_resumable flag unset. Also document this renegotiation restriction.
Change-Id: I5361f522700b02edf5272ba5089c0777e5dafb09
Reviewed-on: https://boringssl-review.googlesource.com/19664
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
I messed up https://boringssl-review.googlesource.com/8883 and caused
both sides to believe they had sent the final Finished. Use next_message
to detect whether our last flight had a reply.
Change-Id: Ia4d8c8eefa818c9a69acc94d63c9c863293c3cf5
Reviewed-on: https://boringssl-review.googlesource.com/19604
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
They both can be moderately large. This should hopefully relieve a little
memory pressure from both connections to hosts which serve SCTs and
TLS 1.3's single-use tickets.
Change-Id: I034bbf057fe5a064015a0f554b3ae9ea7797cd4e
Reviewed-on: https://boringssl-review.googlesource.com/19584
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
SSL_state_string_long and SSL_state_string are often used for debugging
purposes. The latter's 6-letter codes are absurd, but
SSL_state_string_long is plausible. So we don't lose this when
converging state machines or switching to TLS 1.3, add this to TLS 1.3.
Bug: 128
Change-Id: Iec6529a4d9eddcf08bc9610137b4ccf9ea2681a6
Reviewed-on: https://boringssl-review.googlesource.com/19524
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Change-Id: I2fe57cd500e8408ae15164070afe4b081a5daab0
Reviewed-on: https://boringssl-review.googlesource.com/19404
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>