crypto/tls: don't advertise TLS 1.2-only sigAlgs in TLS 1.3

If a ClientHello only supports TLS 1.3, or if a CertificateRequest is
sent after selecting TLS 1.3, we should not advertise TLS 1.2-only
signature_algorithms like PKCS#1 v1.5 or SHA-1.

However, since crypto/x509 still supports PKCS#1 v1.5, and a direct
CertPool match might not care about the signature in the certificate at
all, start sending a separate signature_algorithms_cert extension to
indicate support for PKCS#1 v1.5 and SHA-1 in certificates.

We were already correctly rejecting these algorithms if the peer
selected them in a TLS 1.3 connection.

Updates #72883

Change-Id: I6a6a4656ab60e1b7fb20fdedc32604dc156953ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/658215
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>

auth.go

@@ -220,7 +220,7 @@ func signatureSchemesForCertificate(version uint16, cert *Certificate) []Signatu
 
 	// Filter out any unsupported signature algorithms, for example due to
 	// FIPS 140-3 policy, or any downstream changes to defaults.go.
-	supportedAlgs := supportedSignatureAlgorithms()
+	supportedAlgs := supportedSignatureAlgorithms(version)
 	sigAlgs = slices.DeleteFunc(sigAlgs, func(sigAlg SignatureScheme) bool {
 		return !isSupportedSignatureAlgorithm(sigAlg, supportedAlgs)
 	})

common.go

@@ -1187,6 +1187,8 @@ var supportedVersions = []uint16{
 const roleClient = true
 const roleServer = false
 
+// supportedVersions returns the list of supported TLS versions, sorted from
+// highest to lowest (and hence also in preference order).
 func (c *Config) supportedVersions(isClient bool) []uint16 {
 	versions := make([]uint16, 0, len(supportedVersions))
 	for _, v := range supportedVersions {
@@ -1704,12 +1706,35 @@ func unexpectedMessageError(wanted, got any) error {
 	return fmt.Errorf("tls: received unexpected handshake message of type %T when waiting for %T", got, wanted)
 }
 
-// supportedSignatureAlgorithms returns the supported signature algorithms.
+// supportedSignatureAlgorithms returns the supported signature algorithms for
-func supportedSignatureAlgorithms() []SignatureScheme {
+// the given minimum TLS version, to advertise in ClientHello and
+// CertificateRequest messages.
+func supportedSignatureAlgorithms(minVers uint16) []SignatureScheme {
+	sigAlgs := defaultSupportedSignatureAlgorithms()
 	if fips140tls.Required() {
-		return allowedSupportedSignatureAlgorithmsFIPS
+		sigAlgs = slices.DeleteFunc(sigAlgs, func(s SignatureScheme) bool {
+			return !slices.Contains(allowedSignatureAlgorithmsFIPS, s)
+		})
 	}
-	return defaultSupportedSignatureAlgorithms
+	if minVers > VersionTLS12 {
+		sigAlgs = slices.DeleteFunc(sigAlgs, func(s SignatureScheme) bool {
+			sigType, sigHash, _ := typeAndHashFromSignatureScheme(s)
+			return sigType == signaturePKCS1v15 || sigHash == crypto.SHA1
+		})
+	}
+	return sigAlgs
+}
+
+// supportedSignatureAlgorithmsCert returns the supported algorithms for
+// signatures in certificates.
+func supportedSignatureAlgorithmsCert() []SignatureScheme {
+	sigAlgs := defaultSupportedSignatureAlgorithmsCert()
+	if fips140tls.Required() {
+		sigAlgs = slices.DeleteFunc(sigAlgs, func(s SignatureScheme) bool {
+			return !slices.Contains(allowedSignatureAlgorithmsFIPS, s)
+		})
+	}
+	return sigAlgs
 }
 
 func isSupportedSignatureAlgorithm(sigAlg SignatureScheme, supportedSignatureAlgorithms []SignatureScheme) bool {

defaults.go

@@ -23,11 +23,12 @@ func defaultCurvePreferences() []CurveID {
 	return []CurveID{X25519MLKEM768, X25519, CurveP256, CurveP384, CurveP521}
 }
 
-// defaultSupportedSignatureAlgorithms contains the signature and hash algorithms that
+// defaultSupportedSignatureAlgorithms returns the signature and hash algorithms that
-// the code advertises as supported in a TLS 1.2+ ClientHello and in a TLS 1.2+
+// the code advertises and supports in a TLS 1.2+ ClientHello and in a TLS 1.2+
 // CertificateRequest. The two fields are merged to match with TLS 1.3.
 // Note that in TLS 1.2, the ECDSA algorithms are not constrained to P-256, etc.
-var defaultSupportedSignatureAlgorithms = []SignatureScheme{
+func defaultSupportedSignatureAlgorithms() []SignatureScheme {
+	return []SignatureScheme{
 		PSSWithSHA256,
 		ECDSAWithP256AndSHA256,
 		Ed25519,
@@ -40,6 +41,33 @@ var defaultSupportedSignatureAlgorithms = []SignatureScheme{
 		ECDSAWithP521AndSHA512,
 		PKCS1WithSHA1,
 		ECDSAWithSHA1,
+	}
+}
+
+// defaultSupportedSignatureAlgorithmsCert returns the signature algorithms that
+// the code advertises as supported for signatures in certificates.
+//
+// We include all algorithms, including SHA-1 and PKCS#1 v1.5, because it's more
+// likely that something on our side will be willing to accept a *-with-SHA1
+// certificate (e.g. with a custom VerifyConnection or by a direct match with
+// the CertPool), than that the peer would have a better certificate but is just
+// choosing not to send it. crypto/x509 will refuse to verify important SHA-1
+// signatures anyway.
+func defaultSupportedSignatureAlgorithmsCert() []SignatureScheme {
+	return []SignatureScheme{
+		PSSWithSHA256,
+		ECDSAWithP256AndSHA256,
+		Ed25519,
+		PSSWithSHA384,
+		PSSWithSHA512,
+		PKCS1WithSHA256,
+		PKCS1WithSHA384,
+		PKCS1WithSHA512,
+		ECDSAWithP384AndSHA384,
+		ECDSAWithP521AndSHA512,
+		PKCS1WithSHA1,
+		ECDSAWithSHA1,
+	}
 }
 
 //var tlsrsakex = godebug.New("tlsrsakex")

defaults_boring.go

@@ -31,7 +31,7 @@ var (
 		CurveP384,
 		CurveP521,
 	}
-	allowedSupportedSignatureAlgorithmsFIPS = []SignatureScheme{
+	allowedSignatureAlgorithmsFIPS = []SignatureScheme{
 		PSSWithSHA256,
 		PSSWithSHA384,
 		PSSWithSHA512,

defaults_fips140.go

@@ -36,7 +36,7 @@ var (
 		CurveP384,
 		CurveP521,
 	}
-	allowedSupportedSignatureAlgorithmsFIPS = []SignatureScheme{
+	allowedSignatureAlgorithmsFIPS = []SignatureScheme{
 		PSSWithSHA256,
 		ECDSAWithP256AndSHA256,
 		Ed25519,

handshake_client.go

@@ -66,7 +66,10 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *keySharePrivateKeys, *echCli
 		return nil, nil, nil, errors.New("tls: no supported versions satisfy MinVersion and MaxVersion")
 	}
 
-	maxVersion := config.maxSupportedVersion(roleClient)
+	// Since supportedVersions is sorted in descending order, the first element
+	// is the maximum version and the last element is the minimum version.
+	maxVersion := supportedVersions[0]
+	minVersion := supportedVersions[len(supportedVersions)-1]
 
 	hello := &clientHelloMsg{
 		vers:                         maxVersion,
@@ -120,18 +123,20 @@ func (c *Conn) makeClientHello() (*clientHelloMsg, *keySharePrivateKeys, *echCli
 	}
 
 	if maxVersion >= VersionTLS12 {
-		hello.supportedSignatureAlgorithms = supportedSignatureAlgorithms()
+		hello.supportedSignatureAlgorithms = supportedSignatureAlgorithms(minVersion)
+		hello.supportedSignatureAlgorithmsCert = supportedSignatureAlgorithmsCert()
 	}
 	if testingOnlyForceClientHelloSignatureAlgorithms != nil {
 		hello.supportedSignatureAlgorithms = testingOnlyForceClientHelloSignatureAlgorithms
 	}
 
 	var keyShareKeys *keySharePrivateKeys
-	if hello.supportedVersions[0] == VersionTLS13 {
+	if maxVersion >= VersionTLS13 {
 		// Reset the list of ciphers when the client only supports TLS 1.3.
-		if len(hello.supportedVersions) == 1 {
+		if minVersion >= VersionTLS13 {
 			hello.cipherSuites = nil
 		}
+		
 		if fips140tls.Required() {
 			hello.cipherSuites = append(hello.cipherSuites, allowedCipherSuitesTLS13FIPS...)
 		} else if hasAESGCMHardwareSupport {

handshake_client_tls13.go

@@ -675,7 +675,9 @@ func (hs *clientHandshakeStateTLS13) readServerCertificate() error {
 	}
 
 	// See RFC 8446, Section 4.4.3.
-	if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms()) {
+	// 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)) {
 		c.sendAlert(alertIllegalParameter)
 		return errors.New("tls: certificate used with invalid signature algorithm")
 	}
@@ -684,8 +686,7 @@ func (hs *clientHandshakeStateTLS13) readServerCertificate() error {
 		return c.sendAlert(alertInternalError)
 	}
 	if sigType == signaturePKCS1v15 || sigHash == crypto.SHA1 {
-		c.sendAlert(alertIllegalParameter)
+		return c.sendAlert(alertInternalError)
-		return errors.New("tls: certificate used with invalid signature algorithm")
 	}
 	signed := signedMessage(sigHash, serverSignatureContext, hs.transcript)
 	if err := verifyHandshakeSignature(sigType, c.peerCertificates[0].PublicKey,

handshake_server.go

@@ -637,7 +637,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 		}
 		if c.vers >= VersionTLS12 {
 			certReq.hasSignatureAlgorithm = true
-			certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithms()
+			certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithms(c.vers)
 		}
 
 		// An empty list of certificateAuthorities signals to

handshake_server_tls13.go

@@ -917,7 +917,8 @@ func (hs *serverHandshakeStateTLS13) sendServerCertificate() error {
 		certReq := new(certificateRequestMsgTLS13)
 		certReq.ocspStapling = true
 		certReq.scts = true
-		certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithms()
+		certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithms(c.vers)
+		certReq.supportedSignatureAlgorithmsCert = supportedSignatureAlgorithmsCert()
 		if c.config.ClientCAs != nil {
 			certReq.certificateAuthorities = c.config.ClientCAs.Subjects()
 		}
@@ -1168,7 +1169,9 @@ func (hs *serverHandshakeStateTLS13) readClientCertificate() error {
 		}
 
 		// See RFC 8446, Section 4.4.3.
-		if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms()) {
+		// We don't use certReq.supportedSignatureAlgorithms because it would
+		// require keeping the certificateRequestMsgTLS13 around in the hs.
+		if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms(c.vers)) {
 			c.sendAlert(alertIllegalParameter)
 			return errors.New("tls: client certificate used with invalid signature algorithm")
 		}
@@ -1177,8 +1180,7 @@ func (hs *serverHandshakeStateTLS13) readClientCertificate() error {
 			return c.sendAlert(alertInternalError)
 		}
 		if sigType == signaturePKCS1v15 || sigHash == crypto.SHA1 {
-			c.sendAlert(alertIllegalParameter)
+			return c.sendAlert(alertInternalError)
-			return errors.New("tls: client certificate used with invalid signature algorithm")
 		}
 		signed := signedMessage(sigHash, clientSignatureContext, hs.transcript)
 		if err := verifyHandshakeSignature(sigType, c.peerCertificates[0].PublicKey,