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>
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>
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>
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>
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>
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>
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>
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>
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>
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 #69985Fixes#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>
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>
Go 1.23 adds two new events to QUICConns: QUICStoreSessionEvent and
QUICResumeSessionEvent. We added a QUICConfig.EnableStoreSessionEvent
flag to control whether the store-session event is provided or not,
because receiving this event requires additional action from the caller:
the session must be explicitly stored with QUICConn.StoreSession.
We did not add a control for whether the resume-session event is
provided, because this event requires no action and the caller is
expected to ignore unknown events.
However, we never documented the expectation that callers ignore
unknown events, and quic-go produces an error when receiving an
unexpected event. So change the EnableStoreSessionEvent flag to
apply to both new events.
Fixes#68124
For #63691
Change-Id: I84af487e52b3815f7b648e09884608f8915cd645
Reviewed-on: https://go-review.googlesource.com/c/go/+/594475
Reviewed-by: Marten Seemann <martenseemann@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Just like for tls.Config.GetCertificate the http.Server.ServeTLS method
should be checking tls.Config.GetConfigForClient before trying top open
the specified certFile/keyFile.
This was previously fixed for crypto/tls when using tls.Listen in
CL205059, but the same change for net/http was missed. I've added a
comment src/crypto/tls/tls.go in the relevant section in the hope that
any future changes of a similar nature consider will consider updating
net/http as needed as well.
Change-Id: I312303bc497d92aa2f4627fe2620c70779cbcc99
GitHub-Last-Rev: 6ed29a9
GitHub-Pull-Request: #66795
Reviewed-on: https://go-review.googlesource.com/c/go/+/578396
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Add linknames for most modules with ≥50 dependents.
Add linknames for a few other modules that we know
are important but are below 50.
Remove linknames from badlinkname.go that do not merit
inclusion (very small number of dependents).
We can add them back later if the need arises.
Fixes#67401. (For now.)
Change-Id: I1e49fec0292265256044d64b1841d366c4106002
Reviewed-on: https://go-review.googlesource.com/c/go/+/587756
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Bypass: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
yuhan6665