Skip to content

Commit

Permalink
crypto/tls: improve client auth failure alerts
Browse files Browse the repository at this point in the history
This change makes it easier for clients to debug mutual TLS connection failures. Currently, there are a few situations where invalid client auth leads to a generic "bad certificate" alert. 3 specific situations have a more appropriate TLS alert code, based on the alert descriptions in the appendix of both RFC5246 and RFC8446.
  1. The server is configured to require client auth, but no client cert was provided; the appropriate alert is "certificate required". This applies only to TLS 1.3, which first defined the certificate_required alert code.
  2. The client provided a cert that was signed by an authority that is not in the server's trusted set of CAs; the appropriate alert is "unknown certificate authority".
  3. The client provided an expired (or not yet valid) cert; the appropriate alert is "expired certificate".
Otherwise, we still fall back to "bad certificate".

Fixes #52113

Change-Id: I7d5860fe911cad8a1615f16bfe488a37e936dc36
GitHub-Last-Rev: 34eeab5
GitHub-Pull-Request: #53251
Reviewed-on: https://go-review.googlesource.com/c/go/+/410496
Reviewed-by: Michael Knyszek <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Roland Shoemaker <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
  • Loading branch information
anitgandhi authored and gopherbot committed Jan 20, 2023
1 parent 8b79c41 commit 62a9948
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions src/crypto/tls/handshake_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,11 @@ func (c *Conn) processCertsFromClient(certificate Certificate) error {
}

if len(certs) == 0 && requiresClientCert(c.config.ClientAuth) {
c.sendAlert(alertBadCertificate)
if c.vers == VersionTLS13 {
c.sendAlert(alertCertificateRequired)
} else {
c.sendAlert(alertBadCertificate)
}
return errors.New("tls: client didn't provide a certificate")
}

Expand All @@ -830,7 +834,14 @@ func (c *Conn) processCertsFromClient(certificate Certificate) error {

chains, err := certs[0].Verify(opts)
if err != nil {
c.sendAlert(alertBadCertificate)
var errCertificateInvalid x509.CertificateInvalidError
if errors.As(err, &x509.UnknownAuthorityError{}) {
c.sendAlert(alertUnknownCA)
} else if errors.As(err, &errCertificateInvalid) && errCertificateInvalid.Reason == x509.Expired {
c.sendAlert(alertCertificateExpired)
} else {
c.sendAlert(alertBadCertificate)
}
return &CertificateVerificationError{UnverifiedCertificates: certs, Err: err}
}

Expand Down

0 comments on commit 62a9948

Please sign in to comment.