Skip to content

Conversation

yuhan6665
Copy link
Member

Latest go golang/go@e282cbb
Note: 32d3673 is mine commit that try to minimize files copied from golang

yuhan6665 and others added 30 commits May 3, 2025 12:55
For #69536

Change-Id: Idcef2411c1fd4da302412609f28e438839e8aa4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/624736
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
For #69536

Change-Id: I2d7b6e7b9932d0f0f582a5ab0bb871395dc2a1e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/626675
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
The new implementation encodes the key schedule into the type system,
which is actually nicer than what we had before.

For #69536

Change-Id: Iddab62c2aae40bc2425a155443576bb9b7aafe03
Reviewed-on: https://go-review.googlesource.com/c/go/+/626836
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Commit-Queue: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
This will make it easier to support multiple sizes if needed.

Change-Id: I47495559fdbbf678fd98421ad6cb28172e5c810d
Reviewed-on: https://go-review.googlesource.com/c/go/+/621977
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Change-Id: I3feacb044caa15ac9bbfc11f5d90bebf8a505510
Reviewed-on: https://go-review.googlesource.com/c/go/+/621980
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Russ Cox <rsc@golang.org>
In preparation for introducing ML-KEM-1024.

Aside from the constants at the top, all other changes were automated.

Change-Id: I0fafce9a776c7b0b9179be1c858709cabf60e80f
Reviewed-on: https://go-review.googlesource.com/c/go/+/621981
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
In the process, replace out-of-module imports with their FIPS versions.

For #69536

Change-Id: I83e900b7c38ecf760382e5dca7fd0b1eaa5a5589
Reviewed-on: https://go-review.googlesource.com/c/go/+/626879
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
For #69536

Change-Id: If2477c5249a7c7db45c1af05e715ae0b61e7d940
Reviewed-on: https://go-review.googlesource.com/c/go/+/626837
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
If Be and Le stand for big-endian and little-endian,
then they should be BE and LE.

Change-Id: I723e3962b8918da84791783d3c547638f1c9e8a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/627376
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Consolidates handling of FIPS 140-3 considerations for the tls package.
Considerations specific to certificates are now handled in tls instead
of x509 to limit the area-of-effect of FIPS as much as possible.
Boringcrypto specific prefixes are renamed as appropriate.

For #69536

Co-authored-by: Filippo Valsorda <filippo@golang.org>
Change-Id: I1b1fef83c3599e4c9b98ad81db582ac93253030b
Reviewed-on: https://go-review.googlesource.com/c/go/+/629675
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I07e7c8eaa5bd4bac0d576b2f2f4cd3f81b0b77a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/630055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Adds support for server-side ECH.

We make a couple of implementation decisions that are not completely
in-line with the spec. In particular, we don't enforce that the SNI
matches the ECHConfig public_name, and we implement a hybrid
shared/backend mode (rather than shared or split mode, as described in
Section 7). Both of these match the behavior of BoringSSL.

The hybrid server mode will either act as a shared mode server, where-in
the server accepts "outer" client hellos and unwraps them before
processing the "inner" hello, or accepts bare "inner" hellos initially.
This lets the server operate either transparently as a shared mode
server, or a backend server, in Section 7 terminology. This seems like
the best implementation choice for a TLS library.

Fixes #68500

Change-Id: Ife69db7c1886610742e95e76b0ca92587e6d7ed4
Reviewed-on: https://go-review.googlesource.com/c/go/+/623576
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
receipient -> recipient

Change-Id: I9ed5937acf0f3808283e35221f8b4f41408eee7c
GitHub-Last-Rev: 0ed5ff7
GitHub-Pull-Request: #73131
Reviewed-on: https://go-review.googlesource.com/c/go/+/662175
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Jorropo <jorropo.pgm@gmail.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
This makes three related changes that work particularly well together
and would require significant extra work to do separately: it replaces
X25519Kyber768Draft00 with X25519MLKEM768, it makes CurvePreferences
ordering crypto/tls-selected, and applies a preference to PQ key
exchange methods over key shares (to mitigate downgrades).

TestHandshakeServerUnsupportedKeyShare was removed because we are not
rejecting unsupported key shares anymore (nor do we select them, and
rejecting them actively is a MAY). It would have been nice to keep the
test to check we still continue successfully, but testClientHelloFailure
is broken in the face of any server-side behavior which requires writing
any other messages back to the client, or reading them.

Updates #69985
Fixes #69393

Change-Id: I58de76f5b8742a9bd4543fd7907c48e038507b19
Reviewed-on: https://go-review.googlesource.com/c/go/+/630775
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I5d4baa454487a810f138334b613b412a5273311e
Reviewed-on: https://go-review.googlesource.com/c/go/+/631875
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Change-Id: I2b62fb37ae390c42682354eaa2a9d03159563b6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/637179
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Per FIPS 203 (https://csrc.nist.gov/pubs/fips/203/final), the order of return values should be sharedKey, ciphertext. This commit simply swaps those return values and updates any consumers of the Encapsulate() method to respect the new order.

Fixes #70950

Change-Id: I2a0d605e3baf7fe69510d60d3d35bbac18f883c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/638376
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Change-Id: Ie36a19ed6d6922e68f98e43745a417a24f8a7828
GitHub-Last-Rev: 6fb32e3
GitHub-Pull-Request: #71060
Reviewed-on: https://go-review.googlesource.com/c/go/+/639215
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
When ECH is rejected, properly take retry configs from the encrypted
extensions message. Also fix the bogo shim to properly test for this
behavior.

We should properly map the full BoringSSL -> Go errors so that we don't
run into a similar failure in the future, but this is left for a follow
up CL.

Fixes #70915

Change-Id: Icc1878ff6f87df059e7b83e0a431f50f1fea833c
Reviewed-on: https://go-review.googlesource.com/c/go/+/638583
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The spec indicates that if a client sends an invalid ECHClientHello.type
in ClientHelloOuter, the server will abort the handshake with a
decode_error alert.

Define errInvalidECHExt for invalid ECHClientHello.type. If parseECHExt
returns an errInvalidECHExt error, Conn now sends an illegal_parameter
alert.

Fixes #71061.

Change-Id: I240241fe8bbe3e77d6ad1af989794647bfa2ff87
GitHub-Last-Rev: 3d6c233
GitHub-Pull-Request: #71062
Reviewed-on: https://go-review.googlesource.com/c/go/+/639235
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Change-Id: I9ef075bbb0e3c65f3c2a9d49e599ef50b18aa9be
Reviewed-on: https://go-review.googlesource.com/c/go/+/639535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: I6a6a465612cf76d148b9758ee3fcdc8606497830
Reviewed-on: https://go-review.googlesource.com/c/go/+/648835
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Updates #71706

Change-Id: Id689ec476eb3f76500dbf59d716a4376749de700
Reviewed-on: https://go-review.googlesource.com/c/go/+/650719
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Make the errors we return when parsing an ECHConfig slightly more
verbose.

Fixes #71706

Change-Id: Id138fd9defec71ce492a490a71af4981cb9ede51
Reviewed-on: https://go-review.googlesource.com/c/go/+/650720
Auto-Submit: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
See Implementation Guidance D.Q.

Change-Id: I6a6a465607da94f2bb249934f0561ae04a55e7b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/650575
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
While not clearly motivated by normative language in RFC 8446 it seems
clear that an empty opaque ticket value is non-operable, and so we
should reject it with an appropriate alert/error.

This allows removing the SendEmptySessionTicket-TLS13 BoGo test from the
bogo excluded tests configuration.

Fixes #70513
Updates #72006

Change-Id: I589b34e86fb1eb27a349a230e920c22284597cde
Reviewed-on: https://go-review.googlesource.com/c/go/+/650735
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
When encountering alertUserCanceled in a TLS 1.3 handshake, ignore the
alert and retry reading a record. This matches existing logic for how
TLS 1.2 alertLevelWarning alerts are handled.

For broader context, TLS 1.3 removed warning-level alerts except for
alertUserCanceled (RFC 8446, § 6.1). Since at least one major
implementation (https://bugs.openjdk.org/browse/JDK-8323517)
misuses this alert, many TLS stacks now ignore it outright when seen in
a TLS 1.3 handshake (e.g. BoringSSL, NSS, Rustls).

With the crypto/tls behaviour changed to match peer implementations we
can now enable the "SendUserCanceledAlerts-TLS13" BoGo test.

"SendUserCanceledAlerts-TooMany-TLS13" remains ignored, because like
"SendWarningAlerts*" fixing the test requires some general spam
protocol message enhancements be done first.

Updates #72006

Change-Id: I570c1fa674b5a4760836c514d35ee17f746fe28d
Reviewed-on: https://go-review.googlesource.com/c/go/+/650716
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
For malformed client/server certificates in a TLS handshake send
a decode_error alert, matching BoringSSL behaviour.

Previously crypto/tls used a bad_certificate alert for this purpose.
The TLS specification is imprecise enough to allow this to be considered
a spec. justified choice, but since all other places in the protocol
encourage using decode_error for structurally malformed messages we may
as well do the same here and get some extra cross-impl consistency for
free.

This also allows un-ignoring the BoGo
GarbageCertificate-[Client|Server]-[TLS12|TLS13] tests.

Updates #72006

Change-Id: Ide45ba1602816e71c3289a60e77587266c3b9036
Reviewed-on: https://go-review.googlesource.com/c/go/+/652995
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
yuhan6665 added 15 commits May 10, 2025 15:44
If we weren't resuming an existing session, and we constructed a TLS 1.3
compatible client hello, ensure the server doesn't echo back the
made up compatibility session ID if we end up handshaking for TLS 1.2.

As part of an effort to make the initial stages of a TLS 1.3 handshake
compatible with TLS 1.2 middleboxes, TLS 1.3 requires that the client
hello contain a non-empty legacy_session_id value. For anti-ossification
purposes it's recommended this ID be randomly generated. This is the
strategy the crypto/tls package takes.

When we follow this approach, but then end up negotiating TLS 1.2, the
server should not have echoed back that random ID to us. It's impossible
for the server to have had a session with a matching ID and so it is
misbehaving and it's prudent for our side to abort the handshake.

See RFC 8446 Section 4.1.2 for more detail:
  https://www.rfc-editor.org/rfc/rfc8446#section-4.1.2

Adopting this behaviour allows un-ignoring the BoGo
EchoTLS13CompatibilitySessionID testcase.

Updates #72006

Change-Id: I1e52075177a13a7aa103b45498eae38d8a4c34b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/652997
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Partially reverts CL 587296, restoring the Go+BoringCrypto 1.23 behavior
in terms of supported curves.

Updates #71757

Change-Id: I6a6a465651a8407056fd0fae091d10a945b37997
Reviewed-on: https://go-review.googlesource.com/c/go/+/657095
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
This required adding a new field to SessionState for TLS 1.0–1.2, since
the key exchange is not repeated on resumption. The additional field is
unfortunately not backwards compatible because current Go versions check
that the encoding has no extra data at the end, but will cause
cross-version tickets to be ignored. Relaxed that so we can add fields
in a backwards compatible way the next time.

For the cipher suite, we check that the session's is still acceptable
per the Config. That would arguably make sense here, too: if a Config
for example requires PQ, we should reject resumptions of connections
that didn't use PQ. However, that only applies to pre-TLS 1.3
connections, since in TLS 1.3 we always do a fresh key exchange on
resumption. Since PQ is the only main differentiator between key
exchanges (aside from off-by-default non-PFS RSA, which are controlled
by the cipher suite in TLS 1.0–1.2) and it's PQ-only, we can skip that
check.

Fixes #67516

Change-Id: I6a6a465681a6292edf66c7b8df8f4aba4171a76b
Reviewed-on: https://go-review.googlesource.com/c/go/+/653315
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Cleaned up a lot of the plumbing to make it consistently follow this
logic: clone the preference order; filter by user preference; filter by
FIPS policy. There should be no behavior changes.

Updates #71757

Change-Id: I6a6a4656eb02e56d079f0a22f98212275a400000
Reviewed-on: https://go-review.googlesource.com/c/go/+/657096
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
We are going to stick to BoringSSL's policy for Go+BoringCrypto, but
when using the native FIPS 140-3 module we can allow Ed25519, ML-KEM,
and P-521.

NIST SP 800-52r2 is stricter, but it only applies to some entities, so
they can restrict the profile with Config.

Fixes #71757

Change-Id: I6a6a4656eb02e56d079f0a22f98212275a40a679
Reviewed-on: https://go-review.googlesource.com/c/go/+/650576
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
Previously, the code only checked supportedVersions[0] for TLS 1.3
However, Chromium-based
browsers may list TLS 1.3 at different positions, causing ECH failures.
This fix:
    Iterates through supportedVersions to accept connections as long as TLS 1.3 is present.
    Improves ECH compatibility, ensuring Chrome, Edge, and other browsers work properly.

Fixes #71642

Change-Id: I32f4219fb6654d5cc22c7f33497c6142c0acb4f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/648015
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
If GetConfigForClient returns a tls.Config that has
SessionTicketsDisabled set, the TLS server handshake currently leaves
the Config's internal RWMutex read locked after calculating the
ticketKeys to use for the handshake.

Change-Id: I07e450a0d2edda9e80f51fc2c20af633aa271684
GitHub-Last-Rev: 693d7ac
GitHub-Pull-Request: #68607
Reviewed-on: https://go-review.googlesource.com/c/go/+/601335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Roland Shoemaker <roland@golang.org>
For consistency, prefer crypto/hkdf over crypto/internal/fips140/hkdf.
Both should have the same behavior given the constrained use of HKDF
in TLS.

Change-Id: Ia982b9f7a6ea66537d748eb5ecae1ac1eade68a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/658217
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
This change makes debugging easier if the server handshake fails because
the client only offers unsupported algorithms.

Change-Id: I7daac173a16af2e073aec3d9b59709560f540c6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/631555
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Replace the usage of runtime.SetFinalizer with runtime.AddCleanup in
the certificate cache.

Updates #70907

Change-Id: Ieab6ff88dbc4083f11c1b475f11bd61521dbc638
Reviewed-on: https://go-review.googlesource.com/c/go/+/664275
Auto-Submit: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Previously if the clientHandshakeState for the TLS 1.2 client code
encountered a server helo message that contained a compression method
other than compressionNone, we would emit an unexpected message alert.

Instead, it seems more appropriate to return an illegal parameter alert.
The server hello message _was_ expected, it just contained a bad
parameter option.

Making this change also allows enabling the InvalidCompressionMethod
bogo test.

Updates #72006

Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/669155
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Unlike in earlier TLS versions, in TLS 1.3 when processing a server
hello the legacy_compression_method MUST have the value 0. It is no
longer a parameter that offers a choice of compression method.

With this in mind, it seems more appropriate to return a decode error
when we encounter a non-zero compression method in a server hello
message. We haven't found a parameter value we reject, we've found
a message that doesn't decode according to its specification.

Making this change also aligns with BoringSSL and allows enabling the
TLS13-HRR-InvalidCompressionMethod bogo test.

Updates #72006

Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/669156
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
If a client or server explicitly offers point formats, and the point
formats don't include the uncompressed format, then error. This matches
BoringSSL and Rustls behaviour and allows enabling the
PointFormat-Client-MissingUncompressed bogo test.

Updates #72006

Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/669157
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously for protocol versions older than TLS 1.3 our server handshake
implementation sent an alertBadCertificate alert in the case where the
server TLS config indicates a client cert is required and none was
received.

This commit updates the relevant logic to instead send
alertHandshakeFailure in these circumstances.

For TLS 1.2, RFC 5246 §7.4.6 unambiguously describes this as the correct
alert:
  If the client does not send any certificates, the
  server MAY at its discretion either continue the handshake without
  client authentication, or respond with a fatal handshake_failure
  alert.

The TLS 1.1 and 1.0 specs also describe using this alert (RFC 4346 §7.4.6
and RFC 2246 §7.4.6) both say:
  If client authentication is required by the server for the handshake
  to continue, it may respond with a fatal handshake failure alert.

Making this correction also allows enabling the
RequireAnyClientCertificate-TLS1* bogo tests.

Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/671195
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
If the client hello legacy version is >= TLS 1.3, and no
supported_versions extension is sent, negotiate TLS 1.2 or lower when
supported.

On the topic of supported version negotiation RFC 8446 4.2.1 indicates
TLS 1.3 implementations MUST send a supported_versions extension with
a list of their supported protocol versions. The crypto/tls package
enforces this when the client hello legacy version indicates TLS 1.3
(0x0304), aborting the handshake with an alertMissingExtension alert if
no supported_versions were received.

However, section 4.2.1 indicates different behaviour should be used when
the extension is not present and TLS 1.2 or prior are supported:

  If this extension is not present, servers which are compliant with
  this specification and which also support TLS 1.2 MUST negotiate
  TLS 1.2 or prior as specified in [RFC5246], even if
  ClientHello.legacy_version is 0x0304 or later.

This commit updates the client hello processing logic to allow this
behaviour. If no supported_versions extension was received we ignore the
legacy version being >= TLS 1.3 and instead negotiate a lower supported
version if the server configuration allows.

This fix in turn allows enabling the BoGo ClientHelloVersionTooHigh,
MinorVersionTolerance, and MajorVersionTolerance tests.

Updates #72006
Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/671235
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@RPRX
Copy link
Member

RPRX commented May 12, 2025

感谢 @yuhan6665 的辛苦工作!

@yuhan6665 yuhan6665 deleted the go124 branch May 13, 2025 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants