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>

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)
 	}

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",

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")
 	}

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")
 		}