This throws me off every time.
Change-Id: I19848927fe821f7656dea0343361d70dae4007c9
Reviewed-on: https://boringssl-review.googlesource.com/23445
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>
After much procrastinating, we finally moved Chromium to the new stuff.
We can now delete this. This is a breaking change for
SSL_PRIVATE_KEY_METHOD consumers, but it should be trivial (remove some
unused fields in the struct). I've bumped BORINGSSL_API_VERSION to ease
any multi-sided changes that may be needed.
Change-Id: I9fe562590ad938bcb4fcf9af0fadeff1d48745fb
Reviewed-on: https://boringssl-review.googlesource.com/23224
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Change-Id: I82f92019dccfaf927f7180a5af53c9ffae111861
Reviewed-on: https://boringssl-review.googlesource.com/23145
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 were only running a random subset of TLS 1.3 tests with variants and
let a lot of bugs through as a result.
- HelloRetryRequest-EmptyCookie wasn't actually testing what we were
trying to test.
- The second HelloRetryRequest detection needs tweaks in draft-22.
- The empty HelloRetryRequest logic can't be based on non-empty
extensions in draft-22.
- We weren't sending ChangeCipherSpec correctly in HRR or testing it
right.
- Rework how runner reads ChangeCipherSpec by setting a flag which
affects the next readRecord. This cuts down a lot of cases and works
correctly if the client didn't send early data. (In that case, we
don't flush CCS until EndOfEarlyData and runner deadlocks waiting for
the ChangeCipherSpec to arrive.)
Change-Id: I559c96ea3a8b350067e391941231713c6edb2f78
Reviewed-on: https://boringssl-review.googlesource.com/23125
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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>
Change-Id: I9da9734625d1d9d2c783830d8b4aecd34f51acc6
Reviewed-on: https://boringssl-review.googlesource.com/23124
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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>
Error paths must always have OPENSSL_PUT_ERROR.
Change-Id: I0ed8c8288484a4ea69ec58317064ad3cd90ddd64
Reviewed-on: https://boringssl-review.googlesource.com/23104
Reviewed-by: Steven Valdez <svaldez@chromium.org>
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>
The current PR says the sender only skips it during the handshake. Add a
test that we got this right.
Change-Id: Ib27eb942f11d955b8a24e32321efe474037f5254
Reviewed-on: https://boringssl-review.googlesource.com/23024
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
See https://github.com/tlswg/tls13-spec/pull/1083. We misread the
original text spec, but it turns out the original spec text required
senders have version-specific maximum send fragments. The PR fixes this
off-by-one issue. Align with the new spec text uniformly.
This is a wire format change for our existing drafts *only if* records
have padding. We don't currently send padding, so this is fine. Unpadded
records continue to be capped at 2^14 bytes of plaintext (or 2^14+1
bytes of TLSInnerPlaintext structure).
Change-Id: I01017cfd13162504bb163dd59afd74aff0896cc4
Reviewed-on: https://boringssl-review.googlesource.com/23004
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>
Change-Id: I1a0f264cbfa0eb5d4adac96d0fc24fa342f2b6a3
Reviewed-on: https://boringssl-review.googlesource.com/22946
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>
This introduces a wire change to Experiment2/Experiment3 over 0RTT, however
as there is never going to be a 0RTT deployment with Experiment2/Experiment3,
this is valid.
Change-Id: Id541d195cbc4bbb3df7680ae2a02b53bb8ae3eab
Reviewed-on: https://boringssl-review.googlesource.com/22744
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
We end up writing these switch cases everywhere. Let consumers decompose
these a bit. The original thought was folks should write switch-cases so
they handle everything they support, but that's a pain. As long as
algorithm preferences are always configured, we can still add new
dimensions because folks won't be asked to sign algorithms that depend
on dimensions they don't understand.
Change-Id: I3dd7f067f2c55212f0201876546bc70fee032bcf
Reviewed-on: https://boringssl-review.googlesource.com/22524
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Node's default settings spell P-256 as prime256v1. This comes from
OpenSSL additionally allowing the long and short names of each curve's
NID. This works out to one additional name per curve for the ones we
support. To avoid depending on the giant OID table, this replicates the
names in libssl.
Change-Id: I456a2db6939eb6745e5a9d2f12cf6886e6265b9f
Reviewed-on: https://boringssl-review.googlesource.com/22545
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>
Change-Id: I46686aea9b68105cfe70a11db0e88052781e179c
Reviewed-on: https://boringssl-review.googlesource.com/22164
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
RC4 is dead and gone. This trims away the suiteNoDTLS flag.
Change-Id: I1ddc5d0811ad8cfb073e6e3c73100240bc649615
Reviewed-on: https://boringssl-review.googlesource.com/22469
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>
This avoids needing to keep track of which rules do and don't need it.
Change-Id: Id086b0622305f7f4acd3892f5d24d8e0c970febb
Reviewed-on: https://boringssl-review.googlesource.com/22468
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>
RC4 is gone. The only remaining exception was the dumb SSL_eNULL cipher,
which works fine in DTLS. It doesn't seem worth the trouble to retain
this special-case.
Change-Id: I31023b71192808e4d21e82109255dc4d6d381df8
Reviewed-on: https://boringssl-review.googlesource.com/22467
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>
This reverts commit 75d43b5785. Chatting
with EKR, there is some reason to believe that doing this might cause
more middlebox issues. Since we're still in the middle of working
towards viable deployment in the first place, revert this.
We can experiment with this later. I should have arranged for this to be
controlled more carefully anyway.
Change-Id: I0c8bf578f9d7364e913894e1bf3c2b8123dfd770
Reviewed-on: https://boringssl-review.googlesource.com/22204
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>
Both of these changes have stuck in Chrome for quite a while now. Let's
clear them.
Change-Id: I13094451be2584ecaaf6b60eedefb7212b7bcde2
Reviewed-on: https://boringssl-review.googlesource.com/22226
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 never implemented psk_ke, so there's no need to define the constant.
Change-Id: I6e52596e1a2cf0b3db5e7cd96db6836f4290bf0b
Reviewed-on: https://boringssl-review.googlesource.com/22144
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>
This doesn't matter in so far as runner is not a real TLS
implementation, but it should enforce what there is to enforce just to
keep BoringSSL honest.
Bug: 80
Change-Id: I68940c33712d34a2437dc4dee31342e7f0f57c23
Reviewed-on: https://boringssl-review.googlesource.com/22069
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 does not affect TLS 1.2 (beyond Channel ID or NPN) but, in TLS 1.3,
we send several encrypted handshake messages in a row. For the server,
this means 66 wasted bytes in TLS 1.3. Since OpenSSL has otherwise used
one record per message since the beginning and unencrypted overhead is
less interesting, leave that behavior as-is for the time being. (This
isn't the most pressing use of the breakage budget.) But TLS 1.3 is new,
so get this tight from the start.
Change-Id: I64dbd590a62469d296e1f10673c14bcd0c62919a
Reviewed-on: https://boringssl-review.googlesource.com/22068
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We enforce that servers don't send bogus ALPN values, so consumers may
assume that SSL_get0_alpn_selected won't have anything terribly weird.
To maintain that invariant in the face of folks whose ALPN preferences
change (consider a persisted session cache), we should decline to offer
0-RTT if early_alpn would have been rejected by the check anyway.
Change-Id: Ic3a9ba4041d5d4618742eb05e27033525d96ade1
Reviewed-on: https://boringssl-review.googlesource.com/22067
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is in preparation for giving DTLS_STATE one.
Change-Id: I3dfeeaad2d20c547d8e65d739bd0ad5bc1acf74a
Reviewed-on: https://boringssl-review.googlesource.com/22065
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>
new_*_len can just be computed rather than maintained as state.
Change-Id: If097ee9e68d8791fcfeb69052151faf0134c7c52
Reviewed-on: https://boringssl-review.googlesource.com/21948
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 finally clears most of the SSL_clear special-cases.
Change-Id: I00fc240ccbf13f4290322845f585ca6f5786ad80
Reviewed-on: https://boringssl-review.googlesource.com/21947
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
As with SSLTranscript before, we temporarily need some nastiness in
SSL3_STATE, but this is in preparation of giving SSL3_STATE a
constructor and destructor.
Change-Id: Ifc0ce34fdcd8691d521d8ea03ff5e83dad43b4a3
Reviewed-on: https://boringssl-review.googlesource.com/21944
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>
Now that we've gotten everything, test this by just making bssl_shim run
all errors twice. The manual tests added to ssl_test.cc may now be
removed.
Bug: 206
Change-Id: Iefa0eae83ba59b476e6b6c6f0f921d5d1b72cbfb
Reviewed-on: https://boringssl-review.googlesource.com/21886
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
While a fairly small hook, open_close_notify is pretty weird. It
processes things at the record level and not above. Notably, this will
break if it skips past a TLS 1.3 KeyUpdate.
Instead, it can share the core part of SSL_read/SSL_peek, with slight
tweaks to post-handshake processing. Note this does require some tweaks
to that code. Notably, to retain the current semantics that SSL_shutdown
does not call funny callbacks, we suppress tickets.
Change-Id: Ia0cbd0b9f4527f1b091dd2083a5d8c7efb2bac65
Reviewed-on: https://boringssl-review.googlesource.com/21885
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Ideally we'd put this deep in the record layer, but sending alerts
currently awkwardly sets the field early, so we can't quite lock it out
this deep down.
This is mostly a sanity-check, but a later CL will fix SSL_shutdown's
post-handshake message processing, so this will help catch errors there.
Change-Id: I78e627c19547dbcdc85fb168795240d692baf031
Reviewed-on: https://boringssl-review.googlesource.com/21884
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This gets us closer to exposing BIO-free APIs. The next step is probably
to make the experimental bssl::OpenRecord function call a split out core
of ssl_read_impl.
Change-Id: I4acebb43f708df8c52eb4e328da8ae3551362fb9
Reviewed-on: https://boringssl-review.googlesource.com/21865
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
With this change, it should now always be the case that rr->length is
zero on entry to ssl3_read_message. This will let us detach everything
but application data from rr. This pushes some init_buf invariants down
into tls_open_record so we don't need to maintain them everywhere.
Change-Id: I206747434e0a9603eea7d19664734fd16fa2de8e
Reviewed-on: https://boringssl-review.googlesource.com/21524
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Enough were to make record processing idempotent (we either consume a
record or we don't), but some errors would cause us to keep processing
records when we should get stuck.
This leaves errors in the layer between the record bits and the
handshake. I'm hoping that will be easier to resolve once they do not
depend on BIO, at which point the checks added in this CL may move
around.
Bug: 206
Change-Id: I6b177079388820335e25947c5bd736451780ab8f
Reviewed-on: https://boringssl-review.googlesource.com/21366
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We'll probably want to either move or add additional checks later, but
meanwhile this gets more code on the BIO-free side of the divide.
Change-Id: I3e2b570cdf1d70a262d952c20fd2d76ff4f70dd0
Reviewed-on: https://boringssl-review.googlesource.com/21365
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Ultimately the ssl_buffer_* code will be above SSL_PROTOCOL_METHOD, so
having the processing be analogous is simpler. This also means that DTLS
can surface errors out of dtls_open_record without the caller reading an
extra record.
Bug: 206
Change-Id: Ic1cb3a884763c8e875e1129b1cda226f72bc95b7
Reviewed-on: https://boringssl-review.googlesource.com/21364
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We only support non-blocking BIOs for DTLS as of
https://boringssl-review.googlesource.com/13945. This logic is a remnant
of that. It should not be necessary. All users of DTLSv1_get_timeout
call DTLSv1_handle_timeout. This gets it out of the way for
dtls_open_record calls which don't use dtls1_get_record.
We can restore it elsewhere if necessary, but I don't think we need it.
Change-Id: Idb737868358e4b59ad3cb2c994c7084ffcdb3709
Reviewed-on: https://boringssl-review.googlesource.com/21349
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This removes the last place where non-app-data hooks leave anything
uncomsumed in rrec. (There is still a place where non-app-data hooks see
a non-empty rrec an entrance. read_app_data calls into read_handshake.
That'll be fixed in a later patch in this series.)
This should not change behavior, though some error codes may change due
to some processing happening in a slightly different order.
Since we do this in a few places, this adds a BUF_MEM_append with tests.
Change-Id: I9fe1fc0103e47f90e3c9f4acfe638927aecdeff6
Reviewed-on: https://boringssl-review.googlesource.com/21345
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
RSABadValueTooLong should have the true one as a suffix, not a prefix,
so that the version check still works. Also do the padding manually to
catch a few other bad padding cases. This is sufficient coverage so that
disabling any one comparison in the padding check flags some failure.
Change-Id: Ibcad284e5ecee3e995f43101c09e4cf7694391e9
Reviewed-on: https://boringssl-review.googlesource.com/21904
Reviewed-by: Steven Valdez <svaldez@google.com>
Application records may be packed with other application data records or
with handshake records. We also were never testing CCS and handshake
being packed together. Implement this by moving the packing logic to the
bottom of BoGo's DTLS record layer.
Change-Id: Iabc14ec4ce7b99ed1f923ce9164077efe948c7a0
Reviewed-on: https://boringssl-review.googlesource.com/21844
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>
To make sure I don't break it later on.
Change-Id: I0a326800593cd3196efaf2ec9f4042935ecf8eb8
Reviewed-on: https://boringssl-review.googlesource.com/21864
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
I think that's the last of the ssl3_ prefix being used for common
functions.
Change-Id: Id83e6f2065c3765931250bd074f6ebf1fc251696
Reviewed-on: https://boringssl-review.googlesource.com/21347
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
These are common between TLS and DTLS so should not have the ssl3_
prefix. (TLS-only stuff should really have a tls_ prefix, but we still
have a lot of that one.)
This also fixes a stray reference to ssl3_send_client_key_exchange..
Change-Id: Ia05b360aa090ab3b5f075d5f80f133cbfe0520d4
Reviewed-on: https://boringssl-review.googlesource.com/21346
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
It's no longer needed in the public header at all, now that we've hidden
the SSL_CTX struct.
Change-Id: I2fc6ddbeb52f000487627b433b9cdd7a4cde37a8
Reviewed-on: https://boringssl-review.googlesource.com/21684
Reviewed-by: Steven Valdez <svaldez@google.com>
GCC 7.2.0 (in Release builds) can't figure out that |type| is always
set:
../ssl/tls_record.cc: In function ‘bssl::OpenRecordResult bssl::OpenRecord(SSL*, bssl::Span<unsigned char>*, size_t*, uint8_t*, bssl::Span<unsigned char>)’:
../ssl/tls_record.cc:595:44: error: ‘type’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (type != SSL3_RT_APPLICATION_DATA && type != SSL3_RT_ALERT) {
cc1plus: all warnings being treated as errors
Change-Id: I1ca9683a18d89097288018f48b50991bce185da8
Reviewed-on: https://boringssl-review.googlesource.com/21724
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
The handshake should be generic between TLS and DTLS.
Change-Id: I6feb2f013dd5d771f206750653ab9d117d7ea716
Reviewed-on: https://boringssl-review.googlesource.com/21348
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
This frees us up to make SSL_CTX a C++ type and avoids a lot of
protrusions of otherwise private types into the global namespace.
Bug: 6
Change-Id: I8a0624a53a4d26ac4a483fa270c39ecdd07459ee
Reviewed-on: https://boringssl-review.googlesource.com/21584
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>
On some Chrome builds on Windows (including the official builds that we
ship) there are dynamic initializers for kNamedGroups in chrome.dll and
chrome_child.dll. Tagging this array with constexpr is guaranteed to
avoid this.
Bug: chromium:341941
Change-Id: I0e4ea0665b8ed9640b76b709dd300416be49e59e
Reviewed-on: https://boringssl-review.googlesource.com/21564
Reviewed-by: Bruce Dawson <brucedawson@google.com>
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>
This doesn't particularly matter but is more consistent with DTLS and
avoids the callback being potentially called from two places.
Change-Id: I2f57ca94d2d532c56f37a0bac7000c15b3b4b520
Reviewed-on: https://boringssl-review.googlesource.com/21344
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
We usually use read/write rather than recv/send to describe the two
sides.
Change-Id: Ie3ac8c52c59ea9a5143f56b894f58cecd351dc7d
Reviewed-on: https://boringssl-review.googlesource.com/21304
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Clients need not accept CertificateRequest. We don't, have no intention
to, and post-handshake auth now requires an extension.
Change-Id: I2160c89e4a6988a7d743052b588d8aa2598ffabf
Reviewed-on: https://boringssl-review.googlesource.com/21305
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
The only difference is whether there's an alert to send back, but we'll
need to allow an "error without alert" in several cases anyway:
1. If the server sees an HTTP request or garbage instead of a
ClientHello, it shouldn't send an alert.
2. Resurfaced errors.
Just make zero signal no alert for now. Later on, I'm thinking we might
just want to put the alert into the outgoing buffer and make it further
uniform.
This also gives us only one error state to keep track of rather than
two.
Bug: 206
Change-Id: Ia821d9f89abd2ca6010e8851220d4e070bc42fa1
Reviewed-on: https://boringssl-review.googlesource.com/21286
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
This is analogous to the Go stack's handshakeErr field. Since it's quite
common for callers to run two I/O operations in parallel[*] like
SSL_read and SSL_write (or SSL_read and SSL_do_handshake for client
0-RTT). Accordingly, the new handshake state machine jams itself up on
handshake error, but to fully work with such callers, we should also
replay the error state.
This doesn't yet catch all cases (there are some parts of the read flow
which need to be fixed). Those will be resolved in later changes.
[*] Not actually in parallel, of course, but logically in parallel on a
non-blocking socket.
Bug: 206
Change-Id: I5a4d37a258b9e3fc555b732938b0528b839650f8
Reviewed-on: https://boringssl-review.googlesource.com/21285
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Thanks to Dimitar Vlahovski for pointing this out.
Change-Id: I417f52ec6c3e950bdab6079962b29976fb75c029
Reviewed-on: https://boringssl-review.googlesource.com/21324
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>
Change-Id: I815f9fa77e08f72b0130ea9ef0dda751bf2ed7a6
Reviewed-on: https://boringssl-review.googlesource.com/20826
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Bug: 132
Change-Id: I1d6cd1dd7470a3f64ec91b954042ed3f8c6b561e
Reviewed-on: https://boringssl-review.googlesource.com/20825
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
This roughly aligns with absl::Span<T>::subspan.
Bug: 132
Change-Id: Iaf29418c1b10e2d357763dec90b6cb1371b86c3b
Reviewed-on: https://boringssl-review.googlesource.com/20824
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Although we are derived from 1.0.2, we mimic 1.1.0 in some ways around
our FOO_up_ref functions and opaque libssl types. This causes some
difficulties when porting third-party code as any OPENSSL_VERSION_NUMBER
checks for 1.1.0 APIs we have will be wrong.
Moreover, adding accessors without changing OPENSSL_VERSION_NUMBER can
break external projects. It is common to implement a compatibility
version of an accessor under #ifdef as a static function. This then
conflicts with our headers if we, unlike OpenSSL 1.0.2, have this
function.
This change switches OPENSSL_VERSION_NUMBER to 1.1.0 and atomically adds
enough accessors for software with 1.1.0 support already. The hope is
this will unblock hiding SSL_CTX and SSL_SESSION, which will be
especially useful with C++-ficiation. The cost is we will hit some
growing pains as more 1.1.0 consumers enter the ecosystem and we
converge on the right set of APIs to import from upstream.
It does not remove any 1.0.2 APIs, so we will not require that all
projects support 1.1.0. The exception is APIs which changed in 1.1.0 but
did not change the function signature. Those are breaking changes.
Specifically:
- SSL_CTX_sess_set_get_cb is now const-correct.
- X509_get0_signature is now const-correct.
For C++ consumers only, this change temporarily includes an overload
hack for SSL_CTX_sess_set_get_cb that keeps the old callback working.
This is a workaround for Node not yet supporting OpenSSL 1.1.0.
The version number is set at (the as yet unreleased) 1.1.0g to denote
that this change includes https://github.com/openssl/openssl/pull/4384.
Bug: 91
Change-Id: I5eeb27448a6db4c25c244afac37f9604d9608a76
Reviewed-on: https://boringssl-review.googlesource.com/10340
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
I was just passing by.
Change-Id: I0212b4a1a3fd2ad24d7157181cd55a92263a3727
Reviewed-on: https://boringssl-review.googlesource.com/20904
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>
Querying versions is a bit of a mess between DTLS and TLS and variants
and friends. Add SSL_SESSION_is_single_use which informs the caller
whether the session should be single-use.
Bug: chromium:631988
Change-Id: I745d8a5dd5dc52008fe99930d81fed7651b92e4e
Reviewed-on: https://boringssl-review.googlesource.com/20844
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>
SSL_CTX_sessions is the only think making us expose LHASH as public API
and nothing uses it. Nothing can use it anyway as it's not thread-safe.
I haven't actually removed it yet since SSL_CTX is public, but once the
types are opaque, we could trim the number of symbols ssl.h pulls in
with some work.
Relatedly, fix thread safety of SSL_CTX_sess_number.
Change-Id: I75a6c93509d462cd5ed3ce76c587f0d1e7cd0797
Reviewed-on: https://boringssl-review.googlesource.com/20804
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>
The function has exactly one caller. Also add some comments.
Change-Id: I1566aed625449c91f25a777f5a4232d236019ed7
Reviewed-on: https://boringssl-review.googlesource.com/20673
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: 132
Change-Id: I710dbd4906bb7a8b971831be0121df5b78e4f9e0
Reviewed-on: https://boringssl-review.googlesource.com/20672
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>
This adds a CBBFinishArray helper since we need to do that fairly often.
Bug: 132
Change-Id: I7ec0720de0e6ea31caa90c316041bb5f66661cd3
Reviewed-on: https://boringssl-review.googlesource.com/20671
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>
This adds a CopyFrom companion to Init as a replacement for CBS_stow.
Bug: 132
Change-Id: I4d77291b07552bd2286a09f8ba33655d6d97c853
Reviewed-on: https://boringssl-review.googlesource.com/20670
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>
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>
The ticket encryption key is rotated automatically once every 24 hours,
unless a key has been configured manually (i.e. using
|SSL_CTX_set_tlsext_ticket_keys|) or one of the custom ticket encryption
methods is used.
Change-Id: I0dfff28b33e58e96b3bbf7f94dcd6d2642f37aec
Reviewed-on: https://boringssl-review.googlesource.com/18924
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This loosens the earlier restriction to match Channel ID. Both may be
configured and offered, but the server is obligated to select only one
of them. This aligns with the current tokbind + 0-RTT draft where the
combination is signaled by a separate extension.
Bug: 183
Change-Id: I786102a679999705d399f0091f76da236be091c2
Reviewed-on: https://boringssl-review.googlesource.com/19124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Other projects are starting to use them. Having two APIs for the same
thing is silly, so deprecate all our old ones.
Change-Id: Iaf6b6995bc9e4b624140d5c645000fbf2cb08162
Reviewed-on: https://boringssl-review.googlesource.com/19064
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
CertificateVerify must be sent after a non-empty Certificate msg for:
1) TLS1.2 client
2) TLS1.3 client and server
This CL adds tests for those use cases.
Change-Id: I696e9dd74dcd523c6f8868a4fb9ada28fd67746d
Reviewed-on: https://boringssl-review.googlesource.com/19044
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>
Use SSL_SESSION_get_digest instead of the lower level function where
applicable. Also, remove the failure case (Ivan Maidanski points out in
https://android-review.googlesource.com/c/337852/1/src/ssl/t1_enc.c that
this unreachable codepath is a memory leak) by passing in an SSL_CIPHER
to make it more locally obvious that other values are impossible.
Change-Id: Ie624049d47ab0d24f32b405390d6251c7343d7d6
Reviewed-on: https://boringssl-review.googlesource.com/19024
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>
This is analogous to needing to test that Finished is enforced in False
Start.
Change-Id: I168a72ac51b0f75156aaf6ccc9724ae66ce1e734
Reviewed-on: https://boringssl-review.googlesource.com/18986
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This allows us to fix another consumer that directly accesses SSL_CTX.
I've made ssl_test use it for test coverage, though we're okay with
ssl_test depending on ssl/internal.h.
Bug: 6
Change-Id: I464325e3faa9f0194bbd357fbb31a996afc0c2e1
Reviewed-on: https://boringssl-review.googlesource.com/18964
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
These should use the shim/runner combined setting.
Change-Id: Iad6abb4e76f6e5accef446696aa4132073eca06a
Reviewed-on: https://boringssl-review.googlesource.com/18984
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Rather than init_msg/init_num, there is a get_message function which
either returns success or try again. This function does not advance the
current message (see the previous preparatory change). It only completes
the current one if necessary.
Being idempotent means it may be freely placed at the top of states
which otherwise have other asychronous operations. It also eases
converting the TLS 1.2 state machine. See
https://docs.google.com/a/google.com/document/d/11n7LHsT3GwE34LAJIe3EFs4165TI4UR_3CqiM9LJVpI/edit?usp=sharing
for details.
The read_message hook (later to be replaced by something which doesn't
depend on BIO) intentionally does not finish the handshake, only "makes
progress". A follow-up change will align both TLS and DTLS on consuming
one handshake record and always consuming the entire record (so init_buf
may contain trailing data). In a few places I've gone ahead and
accounted for that case because it was more natural to do so.
This change also removes a couple pointers of redundant state from every
socket.
Bug: 128
Change-Id: I89d8f3622d3b53147d69ee3ac34bb654ed044a71
Reviewed-on: https://boringssl-review.googlesource.com/18806
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>
WebRTC will need this (probably among other things) to lose crypto/x509
at some point.
Bug: chromium:706445
Change-Id: I988e7300c4d913986b6ebbd1fa4130548dde76a4
Reviewed-on: https://boringssl-review.googlesource.com/18904
Reviewed-by: David Benjamin <davidben@google.com>
With on_handshake_complete, this can be managed internally by the TLS
code. The next commit will add a ton more calls to this function.
Change-Id: I91575d3e4bfcccbbe492017ae33c74b8cc1d1340
Reviewed-on: https://boringssl-review.googlesource.com/18865
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Instead, the DTLS driver can detect these states implicitly based on
when we write flights and when the handshake completes. When we flush a
new flight, the peer has enough information to send their reply, so we
start a timer. When we begin assembling a new flight, we must have
received the final message in the peer's flight. (If there are
asynchronous events between, we may stop the timer later, but we may
freely stop the timer anytime before we next try to read something.)
The only place this fails is if we were the last to write a flight,
we'll have a stray timer. Clear it in a handshake completion hook.
Change-Id: I973c592ee5721192949a45c259b93192fa309edb
Reviewed-on: https://boringssl-review.googlesource.com/18864
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>
Checking the record type returned by the |tls_open_record| call only
makes sense if that call was successful.
Change-Id: Ib4bebd2b1198c7def513d9fba3653524c17a6e68
Reviewed-on: https://boringssl-review.googlesource.com/18884
Reviewed-by: Adam Langley <agl@google.com>
reuse_message and V2ClientHellos each caused messages to be
double-reported.
Change-Id: I8722a3761ede272408ac9cf8e1b2ce383911cc6f
Reviewed-on: https://boringssl-review.googlesource.com/18764
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>
This would only come up if the peer didn't pack records together, but
it's free to handle. Notably OpenSSL has a bug where it does not pack
retransmits together.
Change-Id: I0927d768f6b50c62bacdd82bd1c95396ed503cf3
Reviewed-on: https://boringssl-review.googlesource.com/18724
Reviewed-by: David Benjamin <davidben@google.com>
There are still a ton of them, almost exclusively complaints that
function declaration and definitions have different parameter names. I
just fixed a few randomly.
Change-Id: I1072f3dba8f63372cda92425aa94f4aa9e3911fa
Reviewed-on: https://boringssl-review.googlesource.com/18706
Reviewed-by: Steven Valdez <svaldez@google.com>
Bug: 132
Change-Id: I75d6ce5a2256a4b464ca6a9378ac6b63a9bd47e2
Reviewed-on: https://boringssl-review.googlesource.com/18644
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>
placement new requires operator new (size_t, void*) to be defined, which
requires pulling in the <new> header.
Change-Id: Ibaa8f3309b03129958f201d32de8afcfafed70f6
Reviewed-on: https://boringssl-review.googlesource.com/18664
Reviewed-by: David Benjamin <davidben@google.com>
The first line of bssl::New is invalid in LLVM CFI as we are casting a
pointer to T before the object is constructed. Instead, we should leave
it as void* and only use it as a T* afterward being constructed.
Bug: chromium:750445
Change-Id: I0ae60c2a7e541b45bc0155dd8f359b662f561dcc
Reviewed-on: https://boringssl-review.googlesource.com/18684
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>
Similarly, add EVP_AEAD_CTX_tag_len which computes the exact tag length
for required by EVP_AEAD_CTX_seal_scatter.
Change-Id: I069b0ad16fab314fd42f6048a3c1dc45e8376f7f
Reviewed-on: https://boringssl-review.googlesource.com/18324
Reviewed-by: Adam Langley <agl@google.com>
I'm not that fast when debugging.
Change-Id: I37a120a77e9a35ac5255ad760513b983f83d9bd7
Reviewed-on: https://boringssl-review.googlesource.com/18605
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>
Pushing entries onto a stack when handling malloc failures is a
nuisance. sk_push only takes ownership on success. PushToStack smooths
that over with a UniquePtr.
Bug: 132
Change-Id: I4f0a9eee86dda7453f128c33d3a71b550beb25e9
Reviewed-on: https://boringssl-review.googlesource.com/18468
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
My original plan here was to make STACK_OF(T) expand to a template so
the inner type were extractable. Unfortunately, we cannot sanely make
STACK_OF(T) expand to a different type in C and C++ even across
compilation units because UBSan sometimes explodes. This is nuts, but so
it goes.
Instead, use StackTraits to extract the STACK_OF(T) parameters and
define an iterator type.
Bug: 189
Change-Id: I64f5173b34b723ec471f7a355ff46b04f161386a
Reviewed-on: https://boringssl-review.googlesource.com/18467
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
It returns false for incomplete types (or is undefined prior to C++14),
so other instantiations can get confused. Instead, require an explicit
kAllowUniquePtr toggle.
I tried using sizeof(T) to SFINAE-detect an incomplete type but ran into
MSVC issues, I think
https://connect.microsoft.com/VisualStudio/feedback/details/820390/vc-sizeof-doesnt-work-as-expected-in-sfinae-context
Though it seems this also may cause ODR violations if different
compilation units disagree on whether a type is complete. This is all a
mess, so just do the boring thing.
Bug: 132
Change-Id: I6f2d47499f16e75f62629c76f43a5329e91c6daf
Reviewed-on: https://boringssl-review.googlesource.com/18464
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
OpenSSL allows spaces, commas and semi-colons to be used as separators
in cipher strings, in addition to the usual colons.
This change documents that spaces cannot be used in equal-preference
groups and forbids these alternative separators in strict mode.
Change-Id: I3879e25aed54539c281511627e6a282e9463bdc3
Reviewed-on: https://boringssl-review.googlesource.com/18424
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This allows us to avoid omitting all the silly abort() flags in
reasonable downstreams like Chromium, while the holdouts are fixed. It
also means that we still get the compiler checking that we've
implemented all pure virtuals in some build configurations, which we'll
put on a bot somewhere.
Bug: 132
Change-Id: If500749f7100bb22bb8e828e8ecf38a992ae9fe5
Reviewed-on: https://boringssl-review.googlesource.com/18406
Reviewed-by: Steven Valdez <svaldez@google.com>
SSL_get0_peer_certificates is documented to return NULL if the peer was
anonymous, but it actually returns a non-NULL empty list (except in SSL
3.0 where the Certificate message and thus ssl_parse_cert_chain is
skipped).
Make the implementation match the documentation.
Change-Id: Ib3e25d2155f316cc5e9eb3ab7f74b78e08b8a86b
Reviewed-on: https://boringssl-review.googlesource.com/18226
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>
Looks like they're using the pool now.
Change-Id: Ieeb1cacb9cb039d35ff091bc9742262f0fc5b146
Reviewed-on: https://boringssl-review.googlesource.com/18364
Reviewed-by: Adam Langley <agl@google.com>
This is a C++ interface for encrypting and decrypting TLS application
data records in-place, wrapping the existing C API in tls_record.cc.
Also add bssl::Span, a non-owning reference to a contiguous array of
elements which can be used as a common interface over contiguous
container types (like std::vector), pointer-length-pairs, arrays, etc.
Change-Id: Iaa2ca4957cde511cb734b997db38f54e103b0d92
Reviewed-on: https://boringssl-review.googlesource.com/18104
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Short-term, we will need to use these macros and build without RTTI when
defining any virtual base class. Long-term, it would be good to remove
these constraints, but it will require some downstream work.
Bug: 132
Change-Id: I3bc65bb12d7653978612b7d1bf06f772a2f3b1cd
Reviewed-on: https://boringssl-review.googlesource.com/18344
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
SSLECDHContext has the acronyms problem, so I went with SSLKeyShare to
match the TLS 1.3 terminology. It's also a little shorter. Accept and
Finish, for now, take raw output pointers in anticipation of some
bssl::Array and maybe bssl::CleansedArray types.
Bug: 132
Change-Id: I427c7c0eac95704f3ad093676c504c2848f5acb9
Reviewed-on: https://boringssl-review.googlesource.com/18265
Reviewed-by: Steven Valdez <svaldez@google.com>
Originally GREASE was a client-only thing but, in TLS 1.3, we send some
bogus extensions in NewSessionTicket and CertificateRequest. Sampling
from the client_random works fine, but better to use our own entropy
rather than the peer's.
Change-Id: Ic7317eb75a9024c677fcde8e62c73aff380294e4
Reviewed-on: https://boringssl-review.googlesource.com/18144
Reviewed-by: Steven Valdez <svaldez@google.com>
I started by switching a couple fields to SSL_HANDSHAKE and then kept
following transitive bits.
Bug: 132
Change-Id: I640dadd3558615fa38c7e8498d4efe7449b0658f
Reviewed-on: https://boringssl-review.googlesource.com/18245
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
SSL_HANDSHAKE is large so I have not attempted to fully switch it to
scopers in this CL. This is just a preparatory step so that we can start
switching its fields to scopers.
(I also anticipate we'll want a bssl::Array<uint8_t> to replace the
pointer/length pairs.)
Bug: 132
Change-Id: I1538d3fc7f9c7385cd8c44a7b99b5c76e8a8768c
Reviewed-on: https://boringssl-review.googlesource.com/18244
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
The previous attempt around the 'struct ssl_st' compatibility mess
offended OSS-Fuzz and UBSan because one compilation unit passed a
function pointer with ssl_st* and another called it with
bssl::SSLConnection*.
Linkers don't retain such types, of course, but to silence this alert,
instead make C-visible types be separate from the implementation and
subclass the public type. This does mean we risk polluting the symbol
namespace, but hopefully the compiler is smart enough to inline the
visible struct's constructor and destructor.
Bug: 132
Change-Id: Ia75a89b3a22a202883ad671a630b72d0aeef680e
Reviewed-on: https://boringssl-review.googlesource.com/18224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Clear out some of the easy cases.
Bug: 132
Change-Id: Icd5c246cb6bec4a96c72eccd6569235c3d030ebd
Reviewed-on: https://boringssl-review.googlesource.com/18204
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>
This adds several utilities as replacements for new and delete and makes
bssl::UniquePtr work with our private types.
Later work can convert more incrementally. I did this one more
aggressively to see how it'd work. Unfortunately, in doing so, I needed
to remove the NULL SSL_AEAD_CTX "method" receiver trick to appease
clang. The null cipher is now represented by a concrete SSL_AEAD_CTX.
The long-lived references to SSL_AEAD_CTX are not yet in types with
constructors, so they still bare Delete rather than UniquePtr for now.
Though this does mean we may be able to move the sequence number into
SSLAEADContext later which is one less object for DTLS to carry around.
Bug: 132
Change-Id: I506b404addafb692055d5709b0ca6d5439a4e6be
Reviewed-on: https://boringssl-review.googlesource.com/18164
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This is horrible, but everything else I tried was worse. The goal with
this CL is to take the extern "C" out of ssl/internal.h and move most
symbols to namespace bssl, so we can start using C++ helpers and
destructors without worry.
Complications:
- Public API functions must be extern "C" and match their declaration in
ssl.h, which is unnamespaced. C++ really does not want you to
interleave namespaced and unnamespaced things. One can actually write
a namespaced extern "C" function, but this means, from C++'s
perspective, the function is namespaced. Trying to namespace the
public header would worked but ended up too deep a rabbithole.
- Our STACK_OF macros do not work right in namespaces.
- The typedefs for our exposed but opaque types are visible in the
header files and copied into consuming projects as forward
declarations. We ultimately want to give SSL a destructor, but
clobbering an unnamespaced ssl_st::~ssl_st seems bad manners.
- MSVC complains about ambiguous names if one typedefs SSL to bssl::SSL.
This CL opts for:
- ssl/*.cc must begin with #define BORINGSSL_INTERNAL_CXX_TYPES. This
informs the public headers to create forward declarations which are
compatible with our namespaces.
- For now, C++-defined type FOO ends up at bssl::FOO with a typedef
outside. Later I imagine we'll rename many of them.
- Internal functions get namespace bssl, so we stop worrying about
stomping the tls1_prf symbol. Exported C functions are stuck as they
are. Rather than try anything weird, bite the bullet and reorder files
which have a mix of public and private functions. I expect that over
time, the public functions will become fairly small as we move logic
to more idiomatic C++.
Files without any public C functions can just be written normally.
- To avoid MSVC troubles, some bssl types are renamed to CPlusPlusStyle
in advance of them being made idiomatic C++.
Bug: 132
Change-Id: Ic931895e117c38b14ff8d6e5a273e868796c7581
Reviewed-on: https://boringssl-review.googlesource.com/18124
Reviewed-by: David Benjamin <davidben@google.com>
This is needed to switch Chromium's SSLServerSocket and parts of
Conscrypt to CRYPTO_BUFFER.
Bug: 54
Change-Id: Iacd417970607bc1a162057676b576956a3bdfa3f
Reviewed-on: https://boringssl-review.googlesource.com/17965
Reviewed-by: Adam Langley <agl@google.com>
This also serves as a certificate verification callback for
CRYPTO_BUFFER-based consumers. Remove the silly
SSL_CTX_i_promise_to_verify_certs_after_the_handshake placeholder.
Bug: 54, chromium:347402
Change-Id: I4c6b445cb9cd7204218acb2e5d1625e6f37aff6f
Reviewed-on: https://boringssl-review.googlesource.com/17964
Reviewed-by: David Benjamin <davidben@google.com>
In some configurations, Clang will warn about all unannotated
fall-throughs in C++. This change adds the needed annotation for Clang
in the single place where we appear to have this.
Change-Id: I25a9069e659ce278d3cd24bf46f667324b3d5146
Reviewed-on: https://boringssl-review.googlesource.com/18024
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Record splitting is a send-side only behaviour and supporting it in
fuzzer mode was messy.
Change-Id: I406d2cc77f1d83ed2039a85b95acdfbc815f5a44
Reviewed-on: https://boringssl-review.googlesource.com/17944
Reviewed-by: Adam Langley <agl@google.com>
Clang for Windows does not like OPENSSL_COMPILE_ASSERT inside a function
in C++. It complains that the struct is unused. I think we worked around
this in C previously by making it expand to C11 _Static_assert when
available.
But libssl is now C++ and assumes a C++11-capable compiler. Use real
static_assert.
Bug: 132
Change-Id: I6aceb95360244bd2c80d194b80676483abb60519
Reviewed-on: https://boringssl-review.googlesource.com/17924
Reviewed-by: Adam Langley <agl@google.com>
This plumbs EVP_AEAD_CTX_seal_scatter all the way through to
tls_record.c, so we can add a new zero-copy record sealing method on top
of the existing code.
Change-Id: I01fdd88abef5442dc16605ea31b29b4b1231c073
Reviewed-on: https://boringssl-review.googlesource.com/17684
Reviewed-by: Adam Langley <agl@google.com>
Due to SSL 3.0 legacy, TLS 1.0 through 1.2 allow ClientHello and
ServerHello messages to omit the extensions field altogether, rather
than write an empty field. We broke this in
https://boringssl-review.googlesource.com/c/17704/ when we needed to a
second ServerHello parsing path.
Fix this and add some regression tests to explicitly test both the
omitted and empty extensions ClientHello and ServerHello cases.
Bug: chromium:743218
Change-Id: I8297ba608570238e19f12ea44a9fe2fe9d881d28
Reviewed-on: https://boringssl-review.googlesource.com/17904
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This is in an attempt to debug the Mac flakiness. The timestamps will
hopefully help narrow down the order of operations here.
Bug: 199
Change-Id: I8b8dd7222e3a57a8b055b8bc1b7731334e0fcdf0
Reviewed-on: https://boringssl-review.googlesource.com/17886
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>
This was broken when we added the API to SSL.
Change-Id: I92d4330b0d70f655c9a9ad33898d6b84704e915c
Reviewed-on: https://boringssl-review.googlesource.com/17884
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This implements PR #1051
(https://github.com/tlswg/tls13-spec/pull/1051).
Local experiments were not able to replicate the claims in the PR, but
implement this anyway for comparison purposes.
Change-Id: Ic9baf5e671f9a44565020466a553dd08f5ec0f1b
Reviewed-on: https://boringssl-review.googlesource.com/17844
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>
I suspect this won't actually tell us much useful w.r.t. the Mac test
flakes, but we may as well print what we can get.
Change-Id: I4931f6000648c4bd955a132b54351ff83d6b6273
Reviewed-on: https://boringssl-review.googlesource.com/17804
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>
And, with that, stage one is complete. ssl/internal.h may include C++.
Bug: 132
Change-Id: I0cb89f0ed5f4be36632a50744a80321595dc921c
Reviewed-on: https://boringssl-review.googlesource.com/17768
Reviewed-by: Steven Valdez <svaldez@google.com>
This leaves just the TLS 1.3 handshake code.
Bug: 132
Change-Id: I2bd87b0ecd0ae7d6ea1302bc62c67aec5ca1dccb
Reviewed-on: https://boringssl-review.googlesource.com/17767
Reviewed-by: Steven Valdez <svaldez@google.com>
Bug: 132
Change-Id: I2b0c87262a5a529ea264ea8ce2d11c2dba0ec1c8
Reviewed-on: https://boringssl-review.googlesource.com/17766
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
In the process, merge the old canary function back in.
Bug: 132
Change-Id: Ib455320ecea67c839d0b4ac3882669d24f832b74
Reviewed-on: https://boringssl-review.googlesource.com/17765
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Bug: 132
Change-Id: I0b83bb05082aa6dad8c15f906cebc2d4f2d5216b
Reviewed-on: https://boringssl-review.googlesource.com/17764
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
The EC_POINT munging is sufficiently heavy on the goto err that I went
ahead and tidied it up.
Bug: 132
Change-Id: I7a3b3b3f166e39e4559acec834dd8e1ea9ac8620
Reviewed-on: https://boringssl-review.googlesource.com/17747
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>
