From: Filippo Valsorda Date: Fri, 23 May 2025 18:28:36 +0000 (+0200) Subject: crypto/tls: ensure the ECDSA curve matches the signature algorithm X-Git-Tag: go1.25rc2~2^2 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=2f653a5a9e9112ff64f1392ff6e1d404aaf23e8c;p=gostls13.git 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 Auto-Submit: Filippo Valsorda TryBot-Bypass: Filippo Valsorda Reviewed-by: David Chase Reviewed-by: Daniel McCarney --- diff --git a/src/crypto/tls/auth.go b/src/crypto/tls/auth.go index f5de7b3069..7169e47105 100644 --- 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 b269d4b670..cf316718c8 100644 --- 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 4f4966904f..7018bb2336 100644 --- 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 dbd6ff2c4f..a52bc76a0d 100644 --- 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") }