From fd605450a7be429efe68aed2271fbd3d40818f8e Mon Sep 17 00:00:00 2001
From: Daniel McCarney <daniel@binaryparadox.net>
Date: Thu, 8 May 2025 15:22:41 -0400
Subject: [PATCH] crypto/tls: fix TLS <1.3 client cert required alert
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
---
src/crypto/tls/bogo_config.json | 4 ----
src/crypto/tls/handshake_server.go | 4 ++--
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json
index 5c1fd5a4638..f61f2347605 100644
--- a/src/crypto/tls/bogo_config.json
+++ b/src/crypto/tls/bogo_config.json
@@ -60,16 +60,12 @@
"CurveID-Resume*": "unexposed curveID is not stored in the ticket yet",
"BadRSAClientKeyExchange-4": "crypto/tls doesn't check the version number in the premaster secret - see processClientKeyExchange comment",
"BadRSAClientKeyExchange-5": "crypto/tls doesn't check the version number in the premaster secret - see processClientKeyExchange comment",
-
"CheckLeafCurve": "TODO: first pass, this should be fixed",
"DisabledCurve-HelloRetryRequest-TLS13": "TODO: first pass, this should be fixed",
"UnsupportedCurve": "TODO: first pass, this should be fixed",
"SupportTicketsWithSessionID": "TODO: first pass, this should be fixed",
"NoNullCompression-TLS12": "TODO: first pass, this should be fixed",
"KeyUpdate-RequestACK": "TODO: first pass, this should be fixed",
- "RequireAnyClientCertificate-TLS1": "TODO: first pass, this should be fixed",
- "RequireAnyClientCertificate-TLS11": "TODO: first pass, this should be fixed",
- "RequireAnyClientCertificate-TLS12": "TODO: first pass, this should be fixed",
"ClientHelloVersionTooHigh": "TODO: first pass, this should be fixed",
"MinorVersionTolerance": "TODO: first pass, this should be fixed",
"IgnoreClientVersionOrder": "TODO: first pass, this should be fixed",
diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index 677bb2e019a..77da9bb2945 100644
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -898,7 +898,7 @@ func (hs *serverHandshakeState) sendFinished(out []byte) error {
}
// processCertsFromClient takes a chain of client certificates either from a
-// Certificates message and verifies them.
+// certificateMsg message or a certificateMsgTLS13 message and verifies them.
func (c *Conn) processCertsFromClient(certificate Certificate) error {
certificates := certificate.Certificate
certs := make([]*x509.Certificate, len(certificates))
@@ -921,7 +921,7 @@ func (c *Conn) processCertsFromClient(certificate Certificate) error {
if c.vers == VersionTLS13 {
c.sendAlert(alertCertificateRequired)
} else {
- c.sendAlert(alertBadCertificate)
+ c.sendAlert(alertHandshakeFailure)
}
return errors.New("tls: client didn't provide a certificate")
}
--
GitLab