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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.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 5c1fd5a4638528d45f63ab6f54e568683b93b967..f61f2347605a0b4250e99d3ae487654c4f3938c9 100644 (file)
--- 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 677bb2e019a0eddf8683e5eb8985713fd1ef5b0a..77da9bb294599c787133102753772c21a00edd0a 100644 (file)
--- 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")
        }