Generalize invalid signature tests and run at all versions.

TLS 1.3 will go through very different code than everything else. Even
SSL 3.0 is somewhat special-cased now. Move the invalid signature tests
there and run at all versions.

Change-Id: Idd0ee9aac2939c0c8fd9af2ea7b4a22942121c60
Reviewed-on: https://boringssl-review.googlesource.com/8775
Reviewed-by: David Benjamin <davidben@google.com>

ssl/test/runner/common.go

@@ -426,13 +426,9 @@ const (
 )
 
 type ProtocolBugs struct {
-	// InvalidSKXSignature specifies that the signature in a
+	// InvalidSignature specifies that the signature in a ServerKeyExchange
-	// ServerKeyExchange message should be invalid.
+	// or CertificateVerify message should be invalid.
-	InvalidSKXSignature bool
+	InvalidSignature bool
-
-	// InvalidCertVerifySignature specifies that the signature in a
-	// CertificateVerify message should be invalid.
-	InvalidCertVerifySignature bool
 
 	// SendCurve, if non-zero, causes the ServerKeyExchange message to use
 	// the specified curve ID rather than the negotiated one.

ssl/test/runner/handshake_client.go

@@ -808,13 +808,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 		}
 
 		if c.vers > VersionSSL30 {
-			msg := hs.finishedHash.buffer
+			certVerify.signature, err = signMessage(c.vers, privKey, c.config, certVerify.signatureAlgorithm, hs.finishedHash.buffer)
-			if c.config.Bugs.InvalidCertVerifySignature {
-				msg = make([]byte, len(hs.finishedHash.buffer))
-				copy(msg, hs.finishedHash.buffer)
-				msg[0] ^= 0x80
-			}
-			certVerify.signature, err = signMessage(c.vers, privKey, c.config, certVerify.signatureAlgorithm, msg)
 			if err == nil && c.config.Bugs.SendSignatureAlgorithm != 0 {
 				certVerify.signatureAlgorithm = c.config.Bugs.SendSignatureAlgorithm
 			}
@@ -826,7 +820,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 				err = errors.New("unsupported signature type for client certificate")
 			} else {
 				digest := hs.finishedHash.hashForClientCertificateSSL3(hs.masterSecret)
-				if c.config.Bugs.InvalidCertVerifySignature {
+				if c.config.Bugs.InvalidSignature {
 					digest[0] ^= 0x80
 				}
 				certVerify.signature, err = rsa.SignPKCS1v15(c.config.rand(), rsaKey, crypto.MD5SHA1, digest)

ssl/test/runner/key_agreement.go

@@ -397,10 +397,6 @@ func (ka *signedKeyAgreement) signParameters(config *Config, cert *Certificate,
 	msg = append(msg, hello.random...)
 	msg = append(msg, params...)
 
-	if config.Bugs.InvalidSKXSignature {
-		msg[0] ^= 0x80
-	}
-
 	var sigAlg signatureAlgorithm
 	var err error
 	if ka.version >= VersionTLS12 {

ssl/test/runner/runner.go

@@ -1050,63 +1050,6 @@ func bigFromHex(hex string) *big.Int {
 
 func addBasicTests() {
 	basicTests := []testCase{
-		{
-			name: "BadRSASignature",
-			config: Config{
-				// TODO(davidben): Add a TLS 1.3 version of this.
-				MaxVersion:   VersionTLS12,
-				CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
-				Bugs: ProtocolBugs{
-					InvalidSKXSignature: true,
-				},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_SIGNATURE:",
-		},
-		{
-			name: "BadECDSASignature",
-			config: Config{
-				// TODO(davidben): Add a TLS 1.3 version of this.
-				MaxVersion:   VersionTLS12,
-				CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
-				Bugs: ProtocolBugs{
-					InvalidSKXSignature: true,
-				},
-				Certificates: []Certificate{ecdsaP256Certificate},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_SIGNATURE:",
-		},
-		{
-			testType: serverTest,
-			name:     "BadRSASignature-ClientAuth",
-			config: Config{
-				// TODO(davidben): Add a TLS 1.3 version of this.
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					InvalidCertVerifySignature: true,
-				},
-				Certificates: []Certificate{rsaCertificate},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_SIGNATURE:",
-			flags:         []string{"-require-any-client-certificate"},
-		},
-		{
-			testType: serverTest,
-			name:     "BadECDSASignature-ClientAuth",
-			config: Config{
-				// TODO(davidben): Add a TLS 1.3 version of this.
-				MaxVersion: VersionTLS12,
-				Bugs: ProtocolBugs{
-					InvalidCertVerifySignature: true,
-				},
-				Certificates: []Certificate{ecdsaP256Certificate},
-			},
-			shouldFail:    true,
-			expectedError: ":BAD_SIGNATURE:",
-			flags:         []string{"-require-any-client-certificate"},
-		},
 		{
 			name: "NoFallbackSCSV",
 			config: Config{
@@ -4774,17 +4717,36 @@ var testSignatureAlgorithms = []struct {
 	{"RSA-PSS-SHA256", signatureRSAPSSWithSHA256, testCertRSA},
 	{"RSA-PSS-SHA384", signatureRSAPSSWithSHA384, testCertRSA},
 	{"RSA-PSS-SHA512", signatureRSAPSSWithSHA512, testCertRSA},
+	// Tests for key types prior to TLS 1.2.
+	{"RSA", 0, testCertRSA},
+	{"ECDSA", 0, testCertECDSAP256},
 }
 
 const fakeSigAlg1 signatureAlgorithm = 0x2a01
 const fakeSigAlg2 signatureAlgorithm = 0xff01
 
 func addSignatureAlgorithmTests() {
+	// Not all ciphers involve a signature. Advertise a list which gives all
+	// versions a signing cipher.
+	signingCiphers := []uint16{
+		TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+		TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+		TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+		TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+		TLS_DHE_RSA_WITH_AES_128_CBC_SHA,
+	}
+
 	// Make sure each signature algorithm works. Include some fake values in
 	// the list and ensure they're ignored.
 	for _, alg := range testSignatureAlgorithms {
 		for _, ver := range tlsVersions {
-			if ver.version < VersionTLS12 {
+			if (ver.version < VersionTLS12) != (alg.id == 0) {
+				continue
+			}
+
+			// TODO(davidben): Support ECDSA in SSL 3.0 in Go for testing
+			// or remove it in C.
+			if ver.version == VersionSSL30 && alg.cert != testCertRSA {
 				continue
 			}
 
@@ -4857,11 +4819,8 @@ func addSignatureAlgorithmTests() {
 				testType: serverTest,
 				name:     "ServerAuth-Sign" + suffix,
 				config: Config{
-					MaxVersion: ver.version,
+					MaxVersion:   ver.version,
-					CipherSuites: []uint16{
+					CipherSuites: signingCiphers,
-						TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-						TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
-					},
 					VerifySignatureAlgorithms: []signatureAlgorithm{
 						fakeSigAlg1,
 						alg.id,
@@ -4883,10 +4842,7 @@ func addSignatureAlgorithmTests() {
 				config: Config{
 					MaxVersion:   ver.version,
 					Certificates: []Certificate{getRunnerCertificate(alg.cert)},
-					CipherSuites: []uint16{
+					CipherSuites: signingCiphers,
-						TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-						TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
-					},
 					SignSignatureAlgorithms: []signatureAlgorithm{
 						alg.id,
 					},
@@ -4902,6 +4858,47 @@ func addSignatureAlgorithmTests() {
 				shouldFail:    shouldFail,
 				expectedError: verifyError,
 			})
+
+			if !shouldFail {
+				testCases = append(testCases, testCase{
+					testType: serverTest,
+					name:     "ClientAuth-InvalidSignature" + suffix,
+					config: Config{
+						MaxVersion:   ver.version,
+						Certificates: []Certificate{getRunnerCertificate(alg.cert)},
+						SignSignatureAlgorithms: []signatureAlgorithm{
+							alg.id,
+						},
+						Bugs: ProtocolBugs{
+							InvalidSignature: true,
+						},
+					},
+					flags: []string{
+						"-require-any-client-certificate",
+						"-enable-all-curves",
+					},
+					shouldFail:    true,
+					expectedError: ":BAD_SIGNATURE:",
+				})
+
+				testCases = append(testCases, testCase{
+					name: "ServerAuth-InvalidSignature" + suffix,
+					config: Config{
+						MaxVersion:   ver.version,
+						Certificates: []Certificate{getRunnerCertificate(alg.cert)},
+						CipherSuites: signingCiphers,
+						SignSignatureAlgorithms: []signatureAlgorithm{
+							alg.id,
+						},
+						Bugs: ProtocolBugs{
+							InvalidSignature: true,
+						},
+					},
+					flags:         []string{"-enable-all-curves"},
+					shouldFail:    true,
+					expectedError: ":BAD_SIGNATURE:",
+				})
+			}
 		}
 	}
 

ssl/test/runner/sign.go

@@ -51,6 +51,13 @@ func selectSignatureAlgorithm(version uint16, key crypto.PrivateKey, config *Con
 }
 
 func signMessage(version uint16, key crypto.PrivateKey, config *Config, sigAlg signatureAlgorithm, msg []byte) ([]byte, error) {
+	if config.Bugs.InvalidSignature {
+		newMsg := make([]byte, len(msg))
+		copy(newMsg, msg)
+		newMsg[0] ^= 0x80
+		msg = newMsg
+	}
+
 	signer, err := getSigner(version, key, config, sigAlg)
 	if err != nil {
 		return nil, err