crypto/tls: ignore TLS 1.3 user canceled alerts

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>

diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json
index e1ace2398e0baf2e9b92b0fb6e0175cd0bc46dc6..0ca65a6c3b6c336e634bbea1b8b5b7bd96fa12a2 100644 (file)
--- 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 bdbc2bde416ac06f6cdd8f24ee55ff419f2fb8b0..8163328d3f3df5c24167a9e6d1f3dbf9882e498d 100644 (file)
--- 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] {