Until we've gotten it fully working, we should not mint any of these
SSL_SESSIONs, to avoid constraining future versions of our client code.
Notably, if any of our TLS 1.3 clients today serialized sessions, we
would need to rev the serialization format. Without opting into 0-RTT, a
TLS 1.3 client will create SSL_SESSIONs tagged as 0-RTT-capable but
missing important fields (ALPN, etc.). When that serialized session
makes its way to a future version of our client code, it would disagree
with the server about the ALPN value stored in the ticket and cause
interop failures.
I believe the only client code enabling TLS 1.3 right now is Chrome, and
the window is small, so it should be fine. But fix this now before it
becomes a problem.
Change-Id: Ie2b109f8d158017a6f3b4cb6169050d38a66b31c
Reviewed-on: https://boringssl-review.googlesource.com/13342
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.
This change removes the old version and the TLS cipher suites that used
it.
BUG=chromium:682816
Change-Id: I2345d6db83441691fe0c1ab6d7c6da4d24777849
Reviewed-on: https://boringssl-review.googlesource.com/13203
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit def9b46801.
(I should have uploaded a new version before sending to the commit queue.)
Change-Id: Iaead89c8d7fc1f56e6294d869db9238b467f520a
Reviewed-on: https://boringssl-review.googlesource.com/13202
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>
Before RFC 7539 we had a ChaCha20-Poly1305 cipher suite that had a 64/64
nonce/counter split (as DJB's original ChaCha20 did). RFC 7539 changed
that to 96/32 and we've supported both for some time.
This change removes the old version and the TLS cipher suites that used
it.
Change-Id: Icd9c2117c657f3aa6df55990c618d562194ef0e8
Reviewed-on: https://boringssl-review.googlesource.com/13201
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This gives coverage over needing to fragment something over multiple
records.
Change-Id: I2373613608ef669358d48f4e12f68577fa5a40dc
Reviewed-on: https://boringssl-review.googlesource.com/13101
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.3 doesn't support renegotiation in the first place, but so callers
don't report TLS 1.3 servers as missing it, always report it as
(vacuously) protected against this bug.
BUG=chromium:680281
Change-Id: Ibfec03102b2aec7eaa773c331d6844292e7bb685
Reviewed-on: https://boringssl-review.googlesource.com/13046
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>
08b65f4e31 introduced a memory leak and
also got enums confused. Also fix a codepath that was missing an error
code.
Thanks to OSS-Fuzz which appears to have found it in a matter of hours.
Change-Id: Ia9e926c28a01daab3e6154d363d0acda91209a22
Reviewed-on: https://boringssl-review.googlesource.com/13104
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This adds support for setting 0-RTT mode on tickets minted by
BoringSSL, allowing for testing of the initial handshake knowledge.
BUG=76
Change-Id: Ic199842c03b5401ef122a537fdb7ed9e9a5c635a
Reviewed-on: https://boringssl-review.googlesource.com/12740
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 write path for TLS is going to need some work. There are some fiddly
cases when there is a write in progress. Start adding tests to cover
this logic.
Later I'm hoping we can extend this flag so it drains the unfinished
write and thus test the interaction of read/write paths in 0-RTT. (We
may discover 1-RTT keys while we're in the middle of writing data.)
Change-Id: Iac2c417e4b5e84794fb699dd7cbba26a883b64ef
Reviewed-on: https://boringssl-review.googlesource.com/13049
Reviewed-by: Adam Langley <agl@google.com>
Upstream accidentally started rejecting server-sent point formats in
https://github.com/openssl/openssl/issues/2133. Our own test coverage
here is also lacking, so flesh it out.
Change-Id: I99059558bd28d3a540c9687649d6db7e16579d29
Reviewed-on: https://boringssl-review.googlesource.com/12979
Reviewed-by: Steven Valdez <svaldez@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 extension will be used to test whether
https://github.com/tlswg/tls13-spec/pull/762 is deployable against
middleboxes. For simplicity, it is mutually exclusive with 0-RTT. If
client and server agree on the extension, TLS 1.3 records will use the
format in the PR rather than what is in draft 18.
BUG=119
Change-Id: I1372ddf7b328ddf73d496df54ac03a95ede961e1
Reviewed-on: https://boringssl-review.googlesource.com/12684
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 asserting on the shim-side error code.
Change-Id: Idc08c253a7723a2a7fd489da761a56c72f7a3b96
Reviewed-on: https://boringssl-review.googlesource.com/12923
Reviewed-by: Adam Langley <agl@google.com>
It should probably have a TLS 1.3 in the name to be clear that's what
it's testing.
Change-Id: I50b5f503a8038715114136179bde83e7da064e9b
Reviewed-on: https://boringssl-review.googlesource.com/12961
Reviewed-by: Steven Valdez <svaldez@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>
There are no longer any consumers of these APIs.
These were useful back when the CBC vs. RC4 tradeoff varied by version
and it was worth carefully tuning this cutoff. Nowadays RC4 is
completely gone and there's no use in configuring these anymore.
To avoid invalidating the existing ssl_ctx_api corpus and requiring it
regenerated, I've left the entries in there. It's probably reasonable
for new API fuzzers to reuse those slots.
Change-Id: I02bf950e3828062341e4e45c8871a44597ae93d5
Reviewed-on: https://boringssl-review.googlesource.com/12880
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The loop is getting a little deeply nested and hard to read.
Change-Id: I3a99fba54c2f352850b83aef91ab72d5d9aabfb8
Reviewed-on: https://boringssl-review.googlesource.com/12685
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>
Also fix the error code. It's a missing extension, not an unexpected
one.
Change-Id: I48e48c37e27173f6d7ac5e993779948ead3706f2
Reviewed-on: https://boringssl-review.googlesource.com/12683
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>
So we can report it cleanly out of DevTools, it should behave like
SSL_get_curve_id and be reported on resumption too.
BUG=chromium:658905
Change-Id: I0402e540a1e722e09eaebadf7fb4785d8880c389
Reviewed-on: https://boringssl-review.googlesource.com/12694
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>
Also test that TLS 1.3 can be resumed at a different curve.
Change-Id: Ic58e03ad858c861958b7c934813c3e448fb2829c
Reviewed-on: https://boringssl-review.googlesource.com/12692
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>
Nothing calls this anymore. DHE is nearly gone. This unblocks us from
making key_exchange_info only apply to the curve.
Change-Id: I3099e7222a62441df6e01411767d48166a0729b1
Reviewed-on: https://boringssl-review.googlesource.com/12691
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: Ie947ab176d10feb709c6e135d5241c6cf605b8e8
Reviewed-on: https://boringssl-review.googlesource.com/12700
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 was useful when we were transitioning NPN off in Chromium, but now
there are no callers remaining.
Change-Id: Ic619613d6d475eea6bc258c4a90148f129ea4a81
Reviewed-on: https://boringssl-review.googlesource.com/12637
Reviewed-by: Steven Valdez <svaldez@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>
Change-Id: Ida26e32a700c68e1899f9f6ccff73e2fa5252313
Reviewed-on: https://boringssl-review.googlesource.com/12633
Commit-Queue: Adam Langley <agl@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>
Change-Id: I1e28ba84de59336cab432d1db3dd9c6023909081
Reviewed-on: https://boringssl-review.googlesource.com/12625
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>
Although we ignore all but the first identity, keep clients honest by
parsing the whole thing. Also explicitly check that the binder and
identity counts match.
Change-Id: Ib9c4caae18398360f3b80f8db1b22d4549bd5746
Reviewed-on: https://boringssl-review.googlesource.com/12469
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=101
Change-Id: Ia1edbccee535b0bc3a0e18465286d5bcca240035
Reviewed-on: https://boringssl-review.googlesource.com/12470
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 change causes SSL_CTX_set_signed_cert_timestamp_list to check the
SCT list for shallow validity before allowing it to be set.
Change-Id: Ib8a1fe185224ff02ed4ce53a0109e60d934e96b3
Reviewed-on: https://boringssl-review.googlesource.com/12401
Commit-Queue: 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>
Reviewed-by: David Benjamin <davidben@google.com>
Previously the option to retain only the SHA-256 hash of client
certificates could only be set at the |SSL_CTX| level. This change makes
|SSL| objects inherit the setting from the |SSL_CTX|, but allows it to
be overridden on a per-|SSL| basis.
Change-Id: Id435934af3d425d5f008d2f3b9751d1d0884ee55
Reviewed-on: https://boringssl-review.googlesource.com/12182
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 changes our resumption strategy. Before, we would negotiate ciphers
only on fresh handshakes. On resumption, we would blindly use whatever
was in the session.
Instead, evaluate cipher suite preferences on every handshake.
Resumption requires that the saved cipher suite match the one that would
have been negotiated anyway. If client or server preferences changed
sufficiently, we decline the session.
This is much easier to reason about (we always pick the best cipher
suite), simpler, and avoids getting stuck under old preferences if
tickets are continuously renewed. Notably, although TLS 1.2 ticket
renewal does not work in practice, TLS 1.3 will renew tickets like
there's no tomorrow.
It also means we don't need dedicated code to avoid resuming a cipher
which has since been disabled. (That dedicated code was a little odd
anyway since the mask_k, etc., checks didn't occur. When cert_cb was
skipped on resumption, one could resume without ever configuring a
certificate! So we couldn't know whether to mask off RSA or ECDSA cipher
suites.)
Add tests which assert on this new arrangement.
BUG=116
Change-Id: Id40d851ccd87e06c46c6ec272527fd8ece8abfc6
Reviewed-on: https://boringssl-review.googlesource.com/11847
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 simplifies a little code around EMS and PSK KE modes, but requires
tweaking the SNI code.
The extensions that are more tightly integrated with the handshake are
still processed inline for now. It does, however, require an extra state
in 1.2 so the asynchronous session callback does not cause extensions to
be processed twice. Tweak a test enforce this.
This and a follow-up to move cert_cb before resumption are done in
preparation for resolving the cipher suite before resumption and only
resuming on match.
Note this has caller-visible effects:
- The legacy SNI callback happens before resumption.
- The ALPN callback happens before resumption.
- Custom extension ClientHello parsing callbacks also cannot depend on
resumption state.
- The DoS protection callback now runs after all the extension callbacks
as it is documented to be called after the resumption decision.
BUG=116
Change-Id: I1281a3b61789b95c370314aaed4f04c1babbc65f
Reviewed-on: https://boringssl-review.googlesource.com/11845
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>
As a client, we must tolerate this to avoid interoperability failures
with allowed server behaviors.
BUG=117
Change-Id: I9c40a2a048282e2e63ab5ee1d40773fc2eda110a
Reviewed-on: https://boringssl-review.googlesource.com/12311
Reviewed-by: David Benjamin <davidben@google.com>
When debugging a flaky test, it's useful to be able to run a given test
over and over.
Change-Id: I1a7b38792215550b242eb8238214d873d41becb6
Reviewed-on: https://boringssl-review.googlesource.com/12301
Reviewed-by: David Benjamin <davidben@google.com>
The draft 18 implementation did not compute scts_requested correctly. As
a result, it always believed SCTs were requested. Fix this and add tests
for unsolicited OCSP responses and SCTs at all versions.
Thanks to Daniel Hirche for the report.
Change-Id: Ifc59c5c4d7edba5703fa485c6c7a4055b15954b4
Reviewed-on: https://boringssl-review.googlesource.com/12305
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>
Having that logic in two different places is a nuisance when we go to
add new checks like resumption stuff. Along the way, this adds missing
tests for the ClientHello cipher/session consistency check. (We'll
eventually get it for free once the cipher/resumption change is
unblocked, but get this working in the meantime.)
This also fixes a bug where the session validity checks happened in the
wrong order relative to whether tickets_supported or renew_ticket was
looked at. Fix that by lifting that logic closer to the handshake.
Change-Id: I3f4b59cfe01064f9125277dc5834e62a36e64aae
Reviewed-on: https://boringssl-review.googlesource.com/12230
Reviewed-by: Adam Langley <agl@google.com>
We missed that the TLS 1.3 code was inconsistent with the TLS 1.2 code.
Only on the server did we push an error code. But consistency between
client and server is probably worthwhile so, fix the 1.2 code to match
for now.
Change-Id: I17952c72048697dc66eacf0f144a66ced9cb3be8
Reviewed-on: https://boringssl-review.googlesource.com/12260
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>
Certificate chain with intermediate taken from Chromium's tests. Though
it doesn't really matter because the runner tests don't verify
certificates.
BUG=70
Change-Id: I46fd1d4be0f371b5bfd43370b97d2c8053cfad60
Reviewed-on: https://boringssl-review.googlesource.com/12261
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We used to enforce after the version was set, but stopped enforcing with
TLS 1.3. NSS enforces the value for encrypted records, which makes sense
and avoids the problems gating it on have_version. Add tests for this.
Change-Id: I7fb5f94ab4a22e8e3b1c14205aa934952d671727
Reviewed-on: https://boringssl-review.googlesource.com/12143
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 AEAD-level coverage for these, but we should also test this in
the TLS stack, and at maximum size per upstream's CVE-2016-7054.
Change-Id: I1f4ad0356e793d6a3eefdc2d55a9c7e05ea08261
Reviewed-on: https://boringssl-review.googlesource.com/12187
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
TLS 1.3 ciphers are now always enabled and come with a hard-coded
preference order.
BUG=110
Change-Id: Idd9cb0d75fb6bf2676ecdee27d88893ff974c4a3
Reviewed-on: https://boringssl-review.googlesource.com/12025
Reviewed-by: Adam Langley <agl@google.com>
They will get very confused about which key they're using. Any caller
using exporters must either (a) leave renegotiation off or (b) be very
aware of when renegotiations happen anyway. (You need to somehow
coordinate with the peer about which epoch's exporter to use.)
Change-Id: I921ad01ac9bdc88f3fd0f8283757ce673a47ec75
Reviewed-on: https://boringssl-review.googlesource.com/12003
Reviewed-by: Adam Langley <agl@google.com>
The existing tests for this codepath require us to reconfigure the shim.
This will not work when TLS 1.3 cipher configuration is detached from
the old cipher language. It also doesn't hit codepaths like sessions
containing a TLS 1.3 version but TLS 1.2 cipher.
Instead, add some logic to the runner to rewrite tickets and build tests
out of that.
Change-Id: I57ac5d49c3069497ed9aaf430afc65c631014bf6
Reviewed-on: https://boringssl-review.googlesource.com/12024
Reviewed-by: Adam Langley <agl@google.com>
This change is based on interpreting TLS 1.3 draft 18.
Change-Id: I727961aff2f7318bcbbc8bf6d62b7d6ad3e62da9
Reviewed-on: https://boringssl-review.googlesource.com/11921
Reviewed-by: David Benjamin <davidben@google.com>
BUG=chromium:659593
Change-Id: I73a4751609b85df7cd40f0f60dc3f3046a490940
Reviewed-on: https://boringssl-review.googlesource.com/11861
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>
On the client we'll leave it off by default until the change has made it
through Chrome's release process. For TLS 1.3, there is no existing
breakage risk, so always do it. This saves us the trouble of having to
manually turn it on in servers.
See [0] for a data point of someone getting it wrong.
[0] https://hg.mozilla.org/projects/nss/rev/9dbc21b1c3cc
Change-Id: I74daad9e7efd2040e9d66d72d558b31f145e6c4c
Reviewed-on: https://boringssl-review.googlesource.com/11680
Reviewed-by: Adam Langley <agl@google.com>
BUG=103
Change-Id: I9a49fbaf66af73978ce264d27926f483e1e44766
Reviewed-on: https://boringssl-review.googlesource.com/11620
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>
Channel ID for TLS 1.3 uses the same digest construction as
CertificateVerify. This message is signed with the Channel ID key and
put in the same handshake message (with the same format) as in TLS 1.2.
BUG=103
Change-Id: Ia5b2dffe5a39c39db0cecb0aa6bdc328e53accc2
Reviewed-on: https://boringssl-review.googlesource.com/11420
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>
{sha1, ecdsa} is virtually nonexistent. {sha512, ecdsa} is pointless
when we only accept P-256 and P-384. See Chromium Intent thread here:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/kWwLfeIQIBM/9chGZ40TCQAJ
This tweaks the signature algorithm logic slightly so that sign and
verify preferences are separate.
BUG=chromium:655318
Change-Id: I1097332600dcaa38e62e4dffa0194fb734c6df3f
Reviewed-on: https://boringssl-review.googlesource.com/11621
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>
