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>
