crypto/tls: ensure the ECDSA curve matches the signature algorithm

Change-Id: I6a6a4656c1b47ba6bd652d4da18922cb6b80a8ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/675836
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
TryBot-Bypass: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>

diff --git a/src/crypto/tls/auth.go b/src/crypto/tls/auth.go
index f5de7b306940df18f854d46ec0ae8c281901b170..7169e471056afa38820dab913f32e6d296f970ec 100644 (file)
--- a/src/crypto/tls/auth.go
+++ b/src/crypto/tls/auth.go
@@ -163,73 +163,64 @@ var rsaSignatureSchemes = []struct {
        {PKCS1WithSHA1, 15 + crypto.SHA1.Size() + 11},
 }
 
-// signatureSchemesForCertificate returns the list of supported SignatureSchemes
-// for a given certificate, based on the public key and the protocol version,
-// and optionally filtered by its explicit SupportedSignatureAlgorithms.
-func signatureSchemesForCertificate(version uint16, cert *Certificate) []SignatureScheme {
-       priv, ok := cert.PrivateKey.(crypto.Signer)
-       if !ok {
-               return nil
-       }
-
-       var sigAlgs []SignatureScheme
-       switch pub := priv.Public().(type) {
+func signatureSchemesForPublicKey(version uint16, pub crypto.PublicKey) []SignatureScheme {
+       switch pub := pub.(type) {
        case *ecdsa.PublicKey:
-               if version != VersionTLS13 {
+               if version < VersionTLS13 {
                        // In TLS 1.2 and earlier, ECDSA algorithms are not
                        // constrained to a single curve.
-                       sigAlgs = []SignatureScheme{
+                       return []SignatureScheme{
                                ECDSAWithP256AndSHA256,
                                ECDSAWithP384AndSHA384,
                                ECDSAWithP521AndSHA512,
                                ECDSAWithSHA1,
                        }
-                       break
                }
                switch pub.Curve {
                case elliptic.P256():
-                       sigAlgs = []SignatureScheme{ECDSAWithP256AndSHA256}
+                       return []SignatureScheme{ECDSAWithP256AndSHA256}
                case elliptic.P384():
-                       sigAlgs = []SignatureScheme{ECDSAWithP384AndSHA384}
+                       return []SignatureScheme{ECDSAWithP384AndSHA384}
                case elliptic.P521():
-                       sigAlgs = []SignatureScheme{ECDSAWithP521AndSHA512}
+                       return []SignatureScheme{ECDSAWithP521AndSHA512}
                default:
                        return nil
                }
        case *rsa.PublicKey:
                size := pub.Size()
-               sigAlgs = make([]SignatureScheme, 0, len(rsaSignatureSchemes))
+               sigAlgs := make([]SignatureScheme, 0, len(rsaSignatureSchemes))
                for _, candidate := range rsaSignatureSchemes {
                        if size >= candidate.minModulusBytes {
                                sigAlgs = append(sigAlgs, candidate.scheme)
                        }
                }
+               return sigAlgs
        case ed25519.PublicKey:
-               sigAlgs = []SignatureScheme{Ed25519}
+               return []SignatureScheme{Ed25519}
        default:
                return nil
        }
-
-       if cert.SupportedSignatureAlgorithms != nil {
-               sigAlgs = slices.DeleteFunc(sigAlgs, func(sigAlg SignatureScheme) bool {
-                       return !isSupportedSignatureAlgorithm(sigAlg, cert.SupportedSignatureAlgorithms)
-               })
-       }
-
-       // Filter out any unsupported signature algorithms, for example due to
-       // FIPS 140-3 policy, tlssha1=0, or protocol version.
-       sigAlgs = slices.DeleteFunc(sigAlgs, func(sigAlg SignatureScheme) bool {
-               return isDisabledSignatureAlgorithm(version, sigAlg, false)
-       })
-
-       return sigAlgs
 }
 
 // selectSignatureScheme picks a SignatureScheme from the peer's preference list
 // that works with the selected certificate. It's only called for protocol
 // versions that support signature algorithms, so TLS 1.2 and 1.3.
 func selectSignatureScheme(vers uint16, c *Certificate, peerAlgs []SignatureScheme) (SignatureScheme, error) {
-       supportedAlgs := signatureSchemesForCertificate(vers, c)
+       priv, ok := c.PrivateKey.(crypto.Signer)
+       if !ok {
+               return 0, unsupportedCertificateError(c)
+       }
+       supportedAlgs := signatureSchemesForPublicKey(vers, priv.Public())
+       if c.SupportedSignatureAlgorithms != nil {
+               supportedAlgs = slices.DeleteFunc(supportedAlgs, func(sigAlg SignatureScheme) bool {
+                       return !isSupportedSignatureAlgorithm(sigAlg, c.SupportedSignatureAlgorithms)
+               })
+       }
+       // Filter out any unsupported signature algorithms, for example due to
+       // FIPS 140-3 policy, tlssha1=0, or protocol version.
+       supportedAlgs = slices.DeleteFunc(supportedAlgs, func(sigAlg SignatureScheme) bool {
+               return isDisabledSignatureAlgorithm(vers, sigAlg, false)
+       })
        if len(supportedAlgs) == 0 {
                return 0, unsupportedCertificateError(c)
        }

diff --git a/src/crypto/tls/bogo_config.json b/src/crypto/tls/bogo_config.json
index b269d4b67085bb83957d45654bff06007b4fc406..cf316718c80e892fd1a366e4631420fc6690437b 100644 (file)
--- a/src/crypto/tls/bogo_config.json
+++ b/src/crypto/tls/bogo_config.json
@@ -49,8 +49,6 @@
         "ServerAuth-SHA1-Fallback*": "We don't support SHA-1 in TLS 1.2 (without tlssha1=1), so we fail if there are no signature_algorithms",
 
         "Agree-Digest-SHA256": "We select signature algorithms in peer preference order. We should consider changing this.",
-        "ECDSACurveMismatch-Verify-TLS13": "We don't enforce the curve when verifying. This is a bug. We need to fix this.",
-        "*-Verify-ECDSA_P224_SHA256-TLS13": "Side effect of the bug above. BoGo sends a P-256 sigAlg with a P-224 key, and we allow it.",
 
         "V2ClientHello-*": "We don't support SSLv2",
         "SendV2ClientHello*": "We don't support SSLv2",

diff --git a/src/crypto/tls/handshake_client_tls13.go b/src/crypto/tls/handshake_client_tls13.go
index 4f4966904f59f450a4e4b356e8567f1cd80079d3..7018bb2336b8f3f01cab57c14ad3abc23c64b590 100644 (file)
--- a/src/crypto/tls/handshake_client_tls13.go
+++ b/src/crypto/tls/handshake_client_tls13.go
@@ -677,7 +677,8 @@ func (hs *clientHandshakeStateTLS13) readServerCertificate() error {
        // See RFC 8446, Section 4.4.3.
        // We don't use hs.hello.supportedSignatureAlgorithms because it might
        // include PKCS#1 v1.5 and SHA-1 if the ClientHello also supported TLS 1.2.
-       if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms(c.vers)) {
+       if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms(c.vers)) ||
+               !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, signatureSchemesForPublicKey(c.vers, c.peerCertificates[0].PublicKey)) {
                c.sendAlert(alertIllegalParameter)
                return errors.New("tls: certificate used with invalid signature algorithm")
        }

diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go
index dbd6ff2c4f4d9465ffcdaa5714ab51d9988b9479..a52bc76a0d1a9feafcb8b52ca70e2601a6c91d2c 100644 (file)
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -1098,7 +1098,8 @@ func (hs *serverHandshakeStateTLS13) readClientCertificate() error {
                // See RFC 8446, Section 4.4.3.
                // We don't use certReq.supportedSignatureAlgorithms because it would
                // require keeping the certificateRequestMsgTLS13 around in the hs.
-               if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms(c.vers)) {
+               if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms(c.vers)) ||
+                       !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, signatureSchemesForPublicKey(c.vers, c.peerCertificates[0].PublicKey)) {
                        c.sendAlert(alertIllegalParameter)
                        return errors.New("tls: client certificate used with invalid signature algorithm")
                }