Skip to content
Snippets Groups Projects
Commit fd605450 authored by Daniel McCarney's avatar Daniel McCarney
Browse files

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: default avatarCherry Mui <cherryyz@google.com>
Reviewed-by: default avatarRoland Shoemaker <roland@golang.org>
parent 97eab214
No related branches found
No related tags found
No related merge requests found
......@@ -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",
......
......@@ -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")
}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment