From 2611d81dc82ba18bb9dd45afce9a412b0b821913 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Wed, 19 Feb 2025 10:30:50 -0500 Subject: [PATCH] crypto/tls: ignore TLS 1.3 user canceled alerts MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Roland Shoemaker --- src/crypto/tls/bogo_config.json | 2 +- src/crypto/tls/conn.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json index e1ace2398e..0ca65a6c3b 100644 --- a/src/crypto/tls/bogo_config.json +++ b/src/crypto/tls/bogo_config.json @@ -47,12 +47,12 @@ "*DTLS*": "No DTLS", "SendEmptyRecords*": "crypto/tls doesn't implement spam protections", "SendWarningAlerts*": "crypto/tls doesn't implement spam protections", + "SendUserCanceledAlerts-TooMany-TLS13": "crypto/tls doesn't implement spam protections", "TooManyKeyUpdates": "crypto/tls doesn't implement spam protections (TODO: I think?)", "KyberNotEnabledByDefaultInClients": "crypto/tls intentionally enables it", "JustConfiguringKyberWorks": "we always send a X25519 key share with Kyber", "KyberKeyShareIncludedSecond": "we always send the Kyber key share first", "KyberKeyShareIncludedThird": "we always send the Kyber key share first", - "SendUserCanceledAlerts*": "TODO may be a real bug?", "GREASE-Server-TLS13": "TODO ???", "GarbageCertificate*": "TODO ask davidben, alertDecode vs alertBadCertificate", "SendBogusAlertType": "sending wrong alert type", diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go index bdbc2bde41..8163328d3f 100644 --- a/src/crypto/tls/conn.go +++ b/src/crypto/tls/conn.go @@ -724,6 +724,15 @@ func (c *Conn) readRecordOrCCS(expectChangeCipherSpec bool) error { return c.in.setErrorLocked(io.EOF) } if c.vers == VersionTLS13 { + // 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). + if alert(data[1]) == alertUserCanceled { + // Like TLS 1.2 alertLevelWarning alerts, we drop the record and retry. + return c.retryReadRecord(expectChangeCipherSpec) + } return c.in.setErrorLocked(&net.OpError{Op: "remote error", Err: alert(data[1])}) } switch data[0] { -- 2.50.0