This CL adds an initial implementation of the ECH server, with pieces of
the client in BoGo as necessary for testing. In particular, the server
supports ClientHelloInner compression with ech_outer_extensions. When
ECH decryption fails, it can send retry_configs back to the client.
This server passes the "ech-accept" and "ech-reject" test cases in
tls-interop-runner[0] when tested against both the cloudflare-go and nss
clients. For reproducibility, I started with the main branch at commit
707604c262d8bcf3e944ed1d5a675077304732ce and updated the endpoint's
script to pass the server's ECHConfig and private key to the boringssl
tool.
Follow-up CLs will update HPKE to the latest draft and catch us up to
draft-10.
[0]: https://github.com/xvzcf/tls-interop-runner
Bug: 275
Change-Id: I49be35af46d1fd5dd9c62252f07d0bae179381ab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45285
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We currently construct finishedHash fairly late, after we've resolved
HelloRetryRequest. As a result, we need to defer some of the transcript
operations across a large chunk of code.
This is a remnant of earlier iterations of TLS 1.3, when
HelloRetryRequest didn't tell us the cipher suite yet. Now the cipher
suite is known earlier and we can construct the finishedHash object
immediately. In doing so, move HRR handling inside doTLS13Handshake().
This keeps more of TLS 1.3 bits together and allows us to maintain the
HRR bits of the handshake closer to the rest of HRR processing. This
will be useful for ECH which complicates this part of the process with
an inner and outer ClientHello. Finally, this adds a missing check that
the HRR and SH cipher suites match.
Change-Id: Iec149eb5c648973325b190f8a0622c9196bf3a29
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46630
Reviewed-by: Adam Langley <agl@google.com>
The challenge field, at least per our implementation and OpenSSL, may be
either left-padded or truncated to form the ClientHello random. Test
both cases, as well as an exact match.
Change-Id: Icdedf899ef483225d8ed20580ad15818b5e52e91
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46631
Reviewed-by: Adam Langley <agl@google.com>
The client handshake currently defers creating the finishedHash and
writing things into the transcript, which is a little annoying for ECH.
In preparation for simplifying that, one nuisance is that we retain both
hello and helloBytes, across a long span of code. helloBytes is *almost*
the same as hello.marshal() except:
- When we send a V2ClientHello, helloBytes records that we serialized
the ClientHello completely differently.
- For the JDK11 workaround tests, helloBytes records that we swapped out
the ClientHello entirely.
- By the time we finally write helloBytes into the transcript, hello may
have been updated to the second ClientHello.
This CL resolves the first two issues. It replaces the v2ClientHelloMsg
with an option when serializing the clientHelloMsg, and it has the
ClientHello replacement function return a clientHelloMsg instead of a
[]byte. (This is a little weird because we're conflating parsed and
constructed ClientHellos, but ah well.)
A follow-up CL will remove the differed transcript bits and we'll
actually be able to drop helloBytes.
Change-Id: Ib82ac216604e2c4bf421277e57aa5fd3b4cef161
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46629
Reviewed-by: Adam Langley <agl@google.com>
Also avoid unnecessarily stashing a copy of the serialized old
ClientHello.
Change-Id: I699299f0ce767ba059fbb08e8f2140793a649322
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46628
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We only need to implement enough of SSL 3.0 to test that the shim does
not.
Change-Id: I25cb48e407f1bc458bbdb3544b9df9fdfbc3d9c9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46625
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Per the comment in writeClientHash, we should writeClientHash before
writeRecord to get the sequence numbers right. Some of the client HRR
bits are still wrong, but I'll fix those as part of tidying up the HRR
path in a later commit.
(This doesn't actually matter because only DTLS uses sequence numbers,
and we don't support DTLS 1.3.)
Change-Id: I4cbc671f524d56c7f970b5ec0bceeb2641625d15
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46624
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This is a remnant of when we had various pre-standard TLS 1.3 variants.
runner's logic is now built-in.
Change-Id: I72a2fcef9a94e82fa39fe4be9d60ddd329d212ce
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46604
Reviewed-by: Adam Langley <agl@google.com>
This avoids looking up and passing around the cipherSuite object
everywhere. We don't serialize ClientSessionState and, if we did, we can
simply do the lookup at parsing time.
Change-Id: Ice06e4da6b23ff32988597100e8aaa11b82f23ad
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46565
Reviewed-by: Adam Langley <agl@google.com>
This avoids duplicating some code in client and server. It should also
clean up some ECH test code, which needs to juggle a pair of transcripts
for a brief window.
Change-Id: I4db11119e34b56453f01b5890060b8d4129a25b9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46564
Reviewed-by: Adam Langley <agl@google.com>
Omitting the extension means we'll never issue tickets, but if the
client were to offer a ticket anyway, RFC8446 4.2.9 says we MUST reject
the ClientHello. It's not clear on what alert to use, but
missing_extension is probably appropriate.
Thanks to Ben Kaduk for pointing this out.
Change-Id: Ie5c720eac9dd2e1a27ba8a13c59b707c109eaa4e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46464
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This reverts commit a3437c09c7. There was
a miscommunication and it does not seem like we currently need this. If
that changes later, it's in Git and we can bring it back easily.
Change-Id: Ibbce29df2258a2d893d725ab3ee6fd78c5b6cb00
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46286
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
See draft-davidben-tls13-pkcs1-00. The code point is disabled by default
and must be configured in SSL_set_verify_algorithm_prefs and
SSL_set_signing_algorithm_prefs. It is also only defined for TLS 1.3
client certificates and otherwise ignored.
This required reworking the tests a bit since this is the first
signature algorithm that's disabled by default, and the first algorithm
that behaves differently between client and server.
Change-Id: Iac4aa96a4963cbc33688c252e958a572c5c3b511
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46187
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This mirrors a change on the C side. Sessions may store the master
secret (main secret as of draft-ietf-tls-rfc8446bis-01) in TLS 1.2 or
the resumption PSK in TLS 1.3, so giving it any description other than
plain 'secret' isn't even accurate.
(Doing this separately from the rfc8446bis names since it's a bit less
mechanical.)
Change-Id: Iaf2b72fe298f17eeb4f4957cfd78b0015c3a9d89
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45824
Reviewed-by: Adam Langley <agl@google.com>
A linter was complaining about some instance, so just fix the lot of it.
Change-Id: I7e23cbada6e42da887d740b84a05de9f104a86ab
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/45284
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
IETF QUIC draft 33 is replacing the TLS extension
codepoint for QUIC transport parameters from 0xffa5
to 57. To support multiple versions of Chrome, we
need to support both codepoints in BoringSSL. This
CL adds support for the new codepoint in a way that
can be enabled on individual connections.
Note that when BoringSSL is not in QUIC mode, it
will error if it sees the new codepoint as a server
but it will ignore the legacy codepoint as that could
be a different private usage of that codepoint.
Change-Id: I314f8f0b169cedd96eeccc42b44153e97044388c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44704
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This changes the format of the mock QUIC transport to include an
explicit encryption level, matching real QUIC a bit better. In
particular, we need that extra data to properly skip rejected early data
on the shim side. (On the runner, we manage it by synchronizing with the
TLS stack. Still, the levels make it a bit more accurate.)
Testing sending and receiving of actual early data is not very relevant
in QUIC since application I/O is external, but this allows us to more
easily run the same tests in TLS and QUIC.
Along the way, improve error-reporting in mock_quick_transport.cc so
it's easier to diagnose record-level mismatches.
Change-Id: I96175a4023134b03d61dac089f8e7ff4eb627933
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44988
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
This CL implements the backend server behavior described in Section 7.2
of draft-ietf-tls-esni-09.
Bug: 275
Change-Id: I2e162673ce564db0cb75fc9b71ef11ed15037f4b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43924
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(Original CL by svaldez, reworked by davidben.)
Change-Id: I8570808fa5e96a1c9e6e03c4877039a22e73254f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42404
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This CL replaces clientHelloMsg's npnAfterAlpn and pskBinderFirst fields
with a new field: prefixExtensions. The extensions in prefixExtensions
are tried first when marshalling clientHelloMsg.
The ability to control extensions' marshalling order will make it
simpler to implement the "outer_extensions" behavior defined in
draft-ietf-tls-esni-07.
Bug: 275
Change-Id: Ib6dcc1e6fa0281f312cb65a9e204415c3f3ef2c6
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43064
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Daniel McArdle
David Benjamin
David Schinazi
Steven Valdez
Nick Harper
Adam Langley