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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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 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>
TLS 1.3 deployment is currently blocked by buggy middleboxes
throughout the ecosystem. As an experiment to better understand these bugs
and the problems they are causing, implement TLS 1.3 variants with
alternate encodings. These are still the same protocol, only encoded
slightly differently. We will use what we learn from these experiments to
guide the TLS 1.3 deployment strategy and proposals to the IETF, if any.
These experiments only target the basic 1-RTT TLS 1.3 handshake. Based on
what we learn from this experiment, we may try future variations to
explore 0-RTT and HelloRetryRequest.
When enabled, the server supports all TLS 1.3 variants while the client
is configured to use a particular variant.
Change-Id: I532411d1abc41314dc76acce0246879b754b4c61
Reviewed-on: https://boringssl-review.googlesource.com/17327
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: I90286da596d5822d4cfedf40995d80cf76adaf97
Reviewed-on: https://boringssl-review.googlesource.com/17536
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Otherwise the fuzzer gets stuck at renegotiations.
Bug: 104
Change-Id: If37f9ab165d06e37bfc5c423fba35edaabed293b
Reviewed-on: https://boringssl-review.googlesource.com/17532
Reviewed-by: David Benjamin <davidben@google.com>
This is in preparation for supporting multiple TLS 1.3 variants.
Change-Id: Ia2caf984f576f1b9e5915bdaf6ff952c8be10417
Reviewed-on: https://boringssl-review.googlesource.com/17526
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
SSL_set_max_proto_version(TLS1_3_DRAFT_VERSION) worked unintentionally.
Fix that. Also add an error when it fails.
Change-Id: I1048fede7b163e1c170e17bf4370b468221a7077
Reviewed-on: https://boringssl-review.googlesource.com/17525
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=76
Change-Id: If58a73da38e46549fd55f84a9104e2dfebfda43f
Reviewed-on: https://boringssl-review.googlesource.com/14164
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@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>
The C side no longer supports DHE, so there is no longer a need for the
Go side to anymore.
Change-Id: I5084177becd369779a4008a41f4838cb31adcfde
Reviewed-on: https://boringssl-review.googlesource.com/15664
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>
Also remove TODO about post-handshake authentication. The only sensible
way to handle unexpected post-handshake authentication is a fatal error
(dropping them would cause a deadlock), and we treat all post-handshake
authentication as unexpected.
BUG=74
Change-Id: Ic92035b26ddcbcf25241262ce84bcc57b736b7a7
Reviewed-on: https://boringssl-review.googlesource.com/14744
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>
There was a case we were not covering.
Change-Id: Ia8bc1b73f5db3d18afc3cdcfa249867784c3dcd2
Reviewed-on: https://boringssl-review.googlesource.com/14824
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 only works at TLS 1.2 and above as, before TLS 1.2, there is no way
to advertise support for Ed25519 or negotiate the correct signature
algorithm. Add tests for this accordingly.
For now, this is disabled by default on the verifying side but may be
enabled per SSL_CTX. Notably, projects like Chromium which use an
external verifier may need changes elsewhere before they can enable it.
(On the signing side, we can assume that if the caller gave us an
Ed25519 certificate, they mean for us to use it.)
BUG=187
Change-Id: Id25b0a677dcbe205ddd26d8dbba11c04bb520756
Reviewed-on: https://boringssl-review.googlesource.com/14450
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
These will be used to test the C implementation.
BUG=187
Change-Id: If397eaa51885c8140a63c5f731ce58a8ad6949aa
Reviewed-on: https://boringssl-review.googlesource.com/14452
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
BUG=76
Change-Id: Ie894ea5d327f88e66b234767de437dbe5c67c41d
Reviewed-on: https://boringssl-review.googlesource.com/12960
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=76
Change-Id: I43672ee82a50f8fe706a5d607ef774a6e96db252
Reviewed-on: https://boringssl-review.googlesource.com/14379
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>
Once 0-RTT data is added to the current 0-RTT logic, the server will
trigger a write when processing incoming data via SSL_read. This means
SSL_read will block on transport write, which is something we've not
tried to avoid far (assuming no renegotiation).
The specification allows for tickets to be sent at half-RTT by
predicting the client Finished. By doing this we both get the tickets on
the wire sooner and avoid confusing I/O patterns. Moreover, we
anticipate we will need this mode for one of the QUIC stateless reject
patterns.
This is tested by always processing NewSessionTickets in the
ExpectHalfRTTData path on 0-RTT connections. As not other
implementations using BoGo may not do this, this is configurable via the
shim config.
BUG=76
Change-Id: Ia0f56ae63f15078ff1cacceba972d2b99001947f
Reviewed-on: https://boringssl-review.googlesource.com/14371
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: 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>
This adds support on the server and client to accept data-less early
data. The server will still fail to parse early data with any
contents, so this should remain disabled.
BUG=76
Change-Id: Id85d192d8e0360b8de4b6971511b5e8a0e8012f7
Reviewed-on: https://boringssl-review.googlesource.com/12921
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'll measure this value to guide what tolerance to use in the 0-RTT
anti-replay mechanism. This also fixes a bug where we were previously
minting ticket_age_add-less tickets on the server. Add a check to reject
all those tickets.
BUG=113
Change-Id: I68e690c0794234234e0d0500b4b9a7f79aea641e
Reviewed-on: https://boringssl-review.googlesource.com/14068
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>
Due to middlebox and ecosystem intolerance, short record headers are going to
be unsustainable to deploy.
BUG=119
Change-Id: I20fee79dd85bff229eafc6aeb72e4f33cac96d82
Reviewed-on: https://boringssl-review.googlesource.com/14044
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 allows us to move the code from Chrome into BoringSSL itself.
BUG=126
Change-Id: I04b4f63008a6de0a58dd6c685c78e9edd06deda6
Reviewed-on: https://boringssl-review.googlesource.com/14028
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>
In honor of CVE-2016-9244. Although that particular bug BoGo was already
testing since it uses 16 bytes here.
The empty session ID case is particularly worth testing to make sure we
don't get confused somewhere. RFC 5077 allows clients to offer tickets
with no session ID. This is absurd since the client then has no way of
detecting resumption except by lookahead. We'll never do this as a
client, but should handle it correctly as a server.
Change-Id: I49695d19f03c4efdef43749c07372d590a010cda
Reviewed-on: https://boringssl-review.googlesource.com/13740
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
The SNI extension may be ACKed by the server. This is kind of pointless,
but make sure we cover these codepaths.
Change-Id: I14b25ab865dd6e35a30f11ebc9027a1518bbeed9
Reviewed-on: https://boringssl-review.googlesource.com/13633
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: I878dfb9f5d3736c3ec0d5fa39052cca58932dbb7
Reviewed-on: https://boringssl-review.googlesource.com/12981
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: I38cd04fa40edde4e4dd31fdc16bbf92985430198
Reviewed-on: https://boringssl-review.googlesource.com/12702
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>
In TLS 1.2, resumption's benefits are more-or-less subsumed by False
Start. TLS 1.2 resumption lifetime is bounded by how much traffic we are
willing to encrypt without fresh key material, so the lifetime is short.
Renewal uses the same key, so we do not allow it to increase lifetimes.
In TLS 1.3, resumption unlocks 0-RTT. We do not implement psk_ke, so
resumption incorporates fresh key material into both encrypted traffic
(except for early data) and renewed tickets. Thus we are both more
willing to and more interested in longer lifetimes for tickets. Renewal
is also not useless. Thus in TLS 1.3, lifetime is bound separately by
the lifetime of a given secret as a psk_dhe_ke authenticator and the
lifetime of the online signature which authenticated the initial
handshake.
This change maintains two lifetimes on an SSL_SESSION: timeout which is
the renewable lifetime of this ticket, and auth_timeout which is the
non-renewable cliff. It also separates the TLS 1.2 and TLS 1.3 timeouts.
The old session timeout defaults and configuration apply to TLS 1.3, and
we define new ones for TLS 1.3.
Finally, this makes us honor the NewSessionTicket timeout in TLS 1.3.
It's no longer a "hint" in 1.3 and there's probably value in avoiding
known-useless 0-RTT offers.
BUG=120
Change-Id: Iac46d56e5a6a377d8b88b8fa31f492d534cb1b85
Reviewed-on: https://boringssl-review.googlesource.com/13503
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I44202457841f06a899e140f78ae8afa7ac720283
Reviewed-on: https://boringssl-review.googlesource.com/12600
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 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>
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>
