Test that signature_algorithm preferences are enforced.

Both on the client and the server.

Change-Id: I9892c6dbbb29938154aba4f53b10e8b5231f9c47
Reviewed-on: https://boringssl-review.googlesource.com/4071
Reviewed-by: Adam Langley <agl@google.com>

ssl/test/runner/common.go

@@ -675,6 +675,10 @@ type ProtocolBugs struct {
 	// IgnorePeerCipherPreferences, if true, causes the peer's cipher
 	// preferences to be ignored.
 	IgnorePeerCipherPreferences bool
+
+	// IgnorePeerSignatureAlgorithmPreferences, if true, causes the peer's
+	// signature algorithm preferences to be ignored.
+	IgnorePeerSignatureAlgorithmPreferences bool
 }
 
 func (c *Config) serverInit() {

ssl/test/runner/handshake_client.go

@@ -6,7 +6,6 @@ package main
 
 import (
 	"bytes"
-	"crypto"
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rsa"
@@ -580,33 +579,39 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 			hasSignatureAndHash: c.vers >= VersionTLS12,
 		}
 
+		// Determine the hash to sign.
+		var signatureType uint8
+		switch c.config.Certificates[0].PrivateKey.(type) {
+		case *ecdsa.PrivateKey:
+			signatureType = signatureECDSA
+		case *rsa.PrivateKey:
+			signatureType = signatureRSA
+		default:
+			c.sendAlert(alertInternalError)
+			return errors.New("unknown private key type")
+		}
+		if c.config.Bugs.IgnorePeerSignatureAlgorithmPreferences {
+			certReq.signatureAndHashes = c.config.signatureAndHashesForClient()
+		}
+		certVerify.signatureAndHash, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.signatureAndHashes, c.config.signatureAndHashesForClient(), signatureType)
+		if err != nil {
+			c.sendAlert(alertInternalError)
+			return err
+		}
+		digest, hashFunc, err := hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash, hs.masterSecret)
+		if err != nil {
+			c.sendAlert(alertInternalError)
+			return err
+		}
+
 		switch key := c.config.Certificates[0].PrivateKey.(type) {
 		case *ecdsa.PrivateKey:
-			certVerify.signatureAndHash, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.signatureAndHashes, signatureECDSA)
-			if err != nil {
-				break
-			}
-			var digest []byte
-			digest, _, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash, hs.masterSecret)
-			if err != nil {
-				break
-			}
 			var r, s *big.Int
 			r, s, err = ecdsa.Sign(c.config.rand(), key, digest)
 			if err == nil {
 				signed, err = asn1.Marshal(ecdsaSignature{r, s})
 			}
 		case *rsa.PrivateKey:
-			certVerify.signatureAndHash, err = hs.finishedHash.selectClientCertSignatureAlgorithm(certReq.signatureAndHashes, signatureRSA)
-			if err != nil {
-				break
-			}
-			var digest []byte
-			var hashFunc crypto.Hash
-			digest, hashFunc, err = hs.finishedHash.hashForClientCertificate(certVerify.signatureAndHash, hs.masterSecret)
-			if err != nil {
-				break
-			}
 			signed, err = rsa.SignPKCS1v15(c.config.rand(), key, hashFunc, digest)
 		default:
 			err = errors.New("unknown private key type")

ssl/test/runner/handshake_server.go

@@ -193,6 +193,9 @@ func (hs *serverHandshakeState) readClientHello() (isResume bool, err error) {
 	if c.clientVersion < VersionTLS12 && len(hs.clientHello.signatureAndHashes) > 0 {
 		return false, fmt.Errorf("tls: client included signature_algorithms before TLS 1.2")
 	}
+	if config.Bugs.IgnorePeerSignatureAlgorithmPreferences {
+		hs.clientHello.signatureAndHashes = config.signatureAndHashesForServer()
+	}
 
 	c.vers, ok = config.mutualVersion(hs.clientHello.vers)
 	if !ok {

ssl/test/runner/key_agreement.go

@@ -56,7 +56,7 @@ func (ka *rsaKeyAgreement) generateServerKeyExchange(config *Config, cert *Certi
 
 	var tls12HashId uint8
 	if ka.version >= VersionTLS12 {
-		if tls12HashId, err = pickTLS12HashForSignature(signatureRSA, clientHello.signatureAndHashes); err != nil {
+		if tls12HashId, err = pickTLS12HashForSignature(signatureRSA, clientHello.signatureAndHashes, config.signatureAndHashesForServer()); err != nil {
 			return nil, err
 		}
 	}
@@ -212,20 +212,19 @@ func hashForServerKeyExchange(sigType, hashFunc uint8, version uint16, slices ..
 // pickTLS12HashForSignature returns a TLS 1.2 hash identifier for signing a
 // ServerKeyExchange given the signature type being used and the client's
 // advertized list of supported signature and hash combinations.
-func pickTLS12HashForSignature(sigType uint8, clientSignatureAndHashes []signatureAndHash) (uint8, error) {
-	if len(clientSignatureAndHashes) == 0 {
+func pickTLS12HashForSignature(sigType uint8, clientList, serverList []signatureAndHash) (uint8, error) {
+	if len(clientList) == 0 {
 		// If the client didn't specify any signature_algorithms
 		// extension then we can assume that it supports SHA1. See
 		// http://tools.ietf.org/html/rfc5246#section-7.4.1.4.1
 		return hashSHA1, nil
 	}
 
-	for _, sigAndHash := range clientSignatureAndHashes {
+	for _, sigAndHash := range clientList {
 		if sigAndHash.signature != sigType {
 			continue
 		}
-		switch sigAndHash.hash {
-		case hashSHA1, hashSHA256:
+		if isSupportedSignatureAndHash(sigAndHash, serverList) {
 			return sigAndHash.hash, nil
 		}
 	}
@@ -279,7 +278,7 @@ func (ka *signedKeyAgreement) signParameters(config *Config, cert *Certificate,
 	var tls12HashId uint8
 	var err error
 	if ka.version >= VersionTLS12 {
-		if tls12HashId, err = pickTLS12HashForSignature(ka.sigType, clientHello.signatureAndHashes); err != nil {
+		if tls12HashId, err = pickTLS12HashForSignature(ka.sigType, clientHello.signatureAndHashes, config.signatureAndHashesForServer()); err != nil {
 			return nil, err
 		}
 	}

ssl/test/runner/prf.go

@@ -323,14 +323,14 @@ func (h finishedHash) serverSum(masterSecret []byte) []byte {
 
 // selectClientCertSignatureAlgorithm returns a signatureAndHash to sign a
 // client's CertificateVerify with, or an error if none can be found.
-func (h finishedHash) selectClientCertSignatureAlgorithm(serverList []signatureAndHash, sigType uint8) (signatureAndHash, error) {
+func (h finishedHash) selectClientCertSignatureAlgorithm(serverList, clientList []signatureAndHash, sigType uint8) (signatureAndHash, error) {
 	if h.version < VersionTLS12 {
 		// Nothing to negotiate before TLS 1.2.
 		return signatureAndHash{signature: sigType}, nil
 	}
 
 	for _, v := range serverList {
-		if v.signature == sigType && v.hash == hashSHA256 {
+		if v.signature == sigType && isSupportedSignatureAndHash(v, clientList) {
 			return v, nil
 		}
 	}

ssl/test/runner/runner.go

@@ -2922,6 +2922,45 @@ func addSigningHashTests() {
 			},
 		},
 	})
+
+	// Test that hash preferences are enforced. BoringSSL defaults to
+	// rejecting MD5 signatures.
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "SigningHash-ClientAuth-Enforced",
+		config: Config{
+			Certificates: []Certificate{rsaCertificate},
+			SignatureAndHashes: []signatureAndHash{
+				{signatureRSA, hashMD5},
+				// Advertise SHA-1 so the handshake will
+				// proceed, but the shim's preferences will be
+				// ignored in CertificateVerify generation, so
+				// MD5 will be chosen.
+				{signatureRSA, hashSHA1},
+			},
+			Bugs: ProtocolBugs{
+				IgnorePeerSignatureAlgorithmPreferences: true,
+			},
+		},
+		flags:         []string{"-require-any-client-certificate"},
+		shouldFail:    true,
+		expectedError: ":WRONG_SIGNATURE_TYPE:",
+	})
+
+	testCases = append(testCases, testCase{
+		name: "SigningHash-ServerKeyExchange-Enforced",
+		config: Config{
+			CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+			SignatureAndHashes: []signatureAndHash{
+				{signatureRSA, hashMD5},
+			},
+			Bugs: ProtocolBugs{
+				IgnorePeerSignatureAlgorithmPreferences: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":WRONG_SIGNATURE_TYPE:",
+	})
 }
 
 // timeouts is the retransmit schedule for BoringSSL. It doubles and