From: Daniel McCarney Date: Thu, 8 May 2025 19:22:41 +0000 (-0400) Subject: crypto/tls: fix TLS <1.3 client cert required alert X-Git-Tag: go1.25rc1~310 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=fd605450a7be429efe68aed2271fbd3d40818f8e;p=gostls13.git crypto/tls: fix TLS <1.3 client cert required alert 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 Reviewed-by: Cherry Mui Reviewed-by: Roland Shoemaker --- diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json index 5c1fd5a463..f61f234760 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 677bb2e019..77da9bb294 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") }