Enforce ECDSA curve matching in TLS 1.3.

Implement in both C and Go. To test this, route config into all the
sign.go functions so we can expose bugs to skip the check.

Unfortunately, custom private keys are going to be a little weird since
we can't check their curve type. We may need to muse on what to do here.
Perhaps the key type bit should return an enum that includes the curve?
It's weird because, going forward, hopefully all new key types have
exactly one kind of signature so key type == sig alg == sig alg prefs.

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

9 files changed, 289 insertions, 141 deletions

diff --git a/ssl/internal.h b/ssl/internal.h
index 65b05f8e..c971babe 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -490,6 +490,11 @@ enum ssl_private_key_result_t ssl_private_key_decrypt(
 enum ssl_private_key_result_t ssl_private_key_decrypt_complete(
     SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out);
 
+/* ssl_private_key_supports_signature_algorithm returns one if |ssl|'s private
+ * key supports |signature_algorithm| and zero otherwise. */
+int ssl_private_key_supports_signature_algorithm(SSL *ssl,
+                                                 uint16_t signature_algorithm);
+
 /* ssl_public_key_verify verifies that the |signature| is valid for the public
  * key |pkey| and input |in|, using the |signature_algorithm| specified. */
 int ssl_public_key_verify(
@@ -497,6 +502,7 @@ int ssl_public_key_verify(
     uint16_t signature_algorithm, EVP_PKEY *pkey,
     const uint8_t *in, size_t in_len);
 
+
 /* Custom extensions */
 
 /* ssl_custom_extension (a.k.a. SSL_CUSTOM_EXTENSION) is a structure that
diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index e71f82d8..cca2f063 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -58,6 +58,8 @@
 
 #include <limits.h>
 
+#include <openssl/ec.h>
+#include <openssl/ec_key.h>
 #include <openssl/err.h>
 #include <openssl/evp.h>
 #include <openssl/mem.h>
@@ -452,18 +454,22 @@ static int ssl_verify_rsa_pkcs1(SSL *ssl, const uint8_t *signature,
   return ret;
 }
 
-static int is_ecdsa(const EVP_MD **out_md, uint16_t sigalg) {
+static int is_ecdsa(int *out_curve, const EVP_MD **out_md, uint16_t sigalg) {
   switch (sigalg) {
     case SSL_SIGN_ECDSA_SHA1:
+      *out_curve = NID_undef;
       *out_md = EVP_sha1();
       return 1;
     case SSL_SIGN_ECDSA_SECP256R1_SHA256:
+      *out_curve = NID_X9_62_prime256v1;
       *out_md = EVP_sha256();
       return 1;
     case SSL_SIGN_ECDSA_SECP384R1_SHA384:
+      *out_curve = NID_secp384r1;
       *out_md = EVP_sha384();
       return 1;
     case SSL_SIGN_ECDSA_SECP521R1_SHA512:
+      *out_curve = NID_secp521r1;
       *out_md = EVP_sha512();
       return 1;
     default:
@@ -472,8 +478,22 @@ static int is_ecdsa(const EVP_MD **out_md, uint16_t sigalg) {
 }
 
 static int ssl_sign_ecdsa(SSL *ssl, uint8_t *out, size_t *out_len,
-                          size_t max_out, const EVP_MD *md, const uint8_t *in,
-                          size_t in_len) {
+                          size_t max_out, int curve, const EVP_MD *md,
+                          const uint8_t *in, size_t in_len) {
+  EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(ssl->cert->privatekey);
+  if (ec_key == NULL) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SIGNATURE_TYPE);
+    return 0;
+  }
+
+  /* In TLS 1.3, the curve is also specified by the signature algorithm. */
+  if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION &&
+      (curve == NID_undef ||
+       EC_GROUP_get_curve_name(EC_KEY_get0_group(ec_key)) != curve)) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SIGNATURE_TYPE);
+    return 0;
+  }
+
   EVP_MD_CTX ctx;
   EVP_MD_CTX_init(&ctx);
   *out_len = max_out;
@@ -485,9 +505,18 @@ static int ssl_sign_ecdsa(SSL *ssl, uint8_t *out, size_t *out_len,
 }
 
 static int ssl_verify_ecdsa(SSL *ssl, const uint8_t *signature,
-                            size_t signature_len, const EVP_MD *md,
+                            size_t signature_len, int curve, const EVP_MD *md,
                             EVP_PKEY *pkey, const uint8_t *in, size_t in_len) {
-  if (pkey->type != EVP_PKEY_EC) {
+  EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(pkey);
+  if (ec_key == NULL) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SIGNATURE_TYPE);
+    return 0;
+  }
+
+  /* In TLS 1.3, the curve is also specified by the signature algorithm. */
+  if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION &&
+      (curve == NID_undef ||
+       EC_GROUP_get_curve_name(EC_KEY_get0_group(ec_key)) != curve)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SIGNATURE_TYPE);
     return 0;
   }
@@ -510,8 +539,9 @@ enum ssl_private_key_result_t ssl_private_key_sign(
      *
      * TODO(davidben): Switch SSL_PRIVATE_KEY_METHOD to message-based APIs. */
     const EVP_MD *md;
+    int curve;
     if (!is_rsa_pkcs1(&md, signature_algorithm) &&
-        !is_ecdsa(&md, signature_algorithm)) {
+        !is_ecdsa(&curve, &md, signature_algorithm)) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL_FOR_CUSTOM_KEY);
       return ssl_private_key_failure;
     }
@@ -532,8 +562,9 @@ enum ssl_private_key_result_t ssl_private_key_sign(
                ? ssl_private_key_success
                : ssl_private_key_failure;
   }
-  if (is_ecdsa(&md, signature_algorithm)) {
-    return ssl_sign_ecdsa(ssl, out, out_len, max_out, md, in, in_len)
+  int curve;
+  if (is_ecdsa(&curve, &md, signature_algorithm)) {
+    return ssl_sign_ecdsa(ssl, out, out_len, max_out, curve, md, in, in_len)
                ? ssl_private_key_success
                : ssl_private_key_failure;
   }
@@ -556,8 +587,9 @@ int ssl_public_key_verify(SSL *ssl, const uint8_t *signature,
     return ssl_verify_rsa_pkcs1(ssl, signature, signature_len, md, pkey, in,
                                 in_len);
   }
-  if (is_ecdsa(&md, signature_algorithm)) {
-    return ssl_verify_ecdsa(ssl, signature, signature_len, md, pkey, in,
+  int curve;
+  if (is_ecdsa(&curve, &md, signature_algorithm)) {
+    return ssl_verify_ecdsa(ssl, signature, signature_len, curve, md, pkey, in,
                             in_len);
   }
 
@@ -593,3 +625,32 @@ enum ssl_private_key_result_t ssl_private_key_decrypt_complete(
   /* Only custom keys may be asynchronous. */
   return ssl->cert->key_method->decrypt_complete(ssl, out, out_len, max_out);
 }
+
+int ssl_private_key_supports_signature_algorithm(SSL *ssl,
+                                                 uint16_t signature_algorithm) {
+  const EVP_MD *md;
+  if (is_rsa_pkcs1(&md, signature_algorithm)) {
+    return ssl_private_key_type(ssl) == EVP_PKEY_RSA;
+  }
+
+  int curve;
+  if (is_ecdsa(&curve, &md, signature_algorithm)) {
+    if (ssl_private_key_type(ssl) != EVP_PKEY_EC) {
+      return 0;
+    }
+
+    /* For non-custom keys, also check the curve matches. Custom private keys
+     * must instead configure the signature algorithms accordingly. */
+    if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION &&
+        ssl->cert->key_method == NULL) {
+      EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(ssl->cert->privatekey);
+      if (curve == NID_undef ||
+          EC_GROUP_get_curve_name(EC_KEY_get0_group(ec_key)) != curve) {
+        return 0;
+      }
+    }
+    return 1;
+  }
+
+  return 0;
+}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 2fe7a90e..5279a5dc 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2511,27 +2511,6 @@ done:
   return ret;
 }
 
-/* tls12_get_pkey_type returns the EVP_PKEY type corresponding to TLS signature
- * algorithm |sigalg|. It returns -1 if the type is unknown. */
-static int tls12_get_pkey_type(uint16_t sigalg) {
-  switch (sigalg) {
-    case SSL_SIGN_RSA_PKCS1_SHA1:
-    case SSL_SIGN_RSA_PKCS1_SHA256:
-    case SSL_SIGN_RSA_PKCS1_SHA384:
-    case SSL_SIGN_RSA_PKCS1_SHA512:
-      return EVP_PKEY_RSA;
-
-    case SSL_SIGN_ECDSA_SHA1:
-    case SSL_SIGN_ECDSA_SECP256R1_SHA256:
-    case SSL_SIGN_ECDSA_SECP384R1_SHA384:
-    case SSL_SIGN_ECDSA_SECP521R1_SHA512:
-      return EVP_PKEY_EC;
-
-    default:
-      return -1;
-  }
-}
-
 int tls1_parse_peer_sigalgs(SSL *ssl, const CBS *in_sigalgs) {
   /* Extension ignored for inappropriate versions */
   if (ssl3_protocol_version(ssl) < TLS1_2_VERSION) {
@@ -2579,13 +2558,12 @@ int tls1_parse_peer_sigalgs(SSL *ssl, const CBS *in_sigalgs) {
 
 int tls1_choose_signature_algorithm(SSL *ssl, uint16_t *out) {
   CERT *cert = ssl->cert;
-  int type = ssl_private_key_type(ssl);
   size_t i, j;
 
   /* Before TLS 1.2, the signature algorithm isn't negotiated as part of the
    * handshake. It is fixed at MD5-SHA1 for RSA and SHA1 for ECDSA. */
   if (ssl3_protocol_version(ssl) < TLS1_2_VERSION) {
-    if (type == EVP_PKEY_RSA) {
+    if (ssl_private_key_type(ssl) == EVP_PKEY_RSA) {
       *out = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
     } else {
       *out = SSL_SIGN_ECDSA_SHA1;
@@ -2615,14 +2593,17 @@ int tls1_choose_signature_algorithm(SSL *ssl, uint16_t *out) {
   }
 
   for (i = 0; i < sigalgs_len; i++) {
+    uint16_t sigalg = sigalgs[i];
+    /* SSL_SIGN_RSA_PKCS1_MD5_SHA1 is an internal value and should never be
+     * negotiated. */
+    if (sigalg == SSL_SIGN_RSA_PKCS1_MD5_SHA1 ||
+        !ssl_private_key_supports_signature_algorithm(ssl, sigalgs[i])) {
+      continue;
+    }
+
     for (j = 0; j < peer_sigalgs_len; j++) {
-      uint16_t signature_algorithm = peer_sigalgs[j];
-      /* SSL_SIGN_RSA_PKCS1_MD5_SHA1 is an internal value and should never be
-       * negotiated. */
-      if (signature_algorithm != SSL_SIGN_RSA_PKCS1_MD5_SHA1 &&
-          signature_algorithm == sigalgs[i] &&
-          tls12_get_pkey_type(signature_algorithm) == type) {
-        *out = signature_algorithm;
+      if (sigalg == peer_sigalgs[j]) {
+        *out = sigalg;
         return 1;
       }
     }
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 023de3a9..19650913 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -896,6 +896,10 @@ type ProtocolBugs struct {
 	// SendSignatureAlgorithm, if non-zero, causes all signatures to be sent
 	// with the given signature algorithm rather than the one negotiated.
 	SendSignatureAlgorithm signatureAlgorithm
+
+	// SkipECDSACurveCheck, if true, causes all ECDSA curve checks to be
+	// skipped.
+	SkipECDSACurveCheck bool
 }
 
 func (c *Config) serverInit() {
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 7857b1d1..4eb2ce16 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -570,7 +570,7 @@ func (hs *clientHandshakeState) doTLS13Handshake() error {
 		}
 
 		input := hs.finishedHash.certificateVerifyInput(serverCertificateVerifyContextTLS13)
-		err = verifyMessage(c.vers, leaf.PublicKey, certVerifyMsg.signatureAlgorithm, input, certVerifyMsg.signature)
+		err = verifyMessage(c.vers, leaf.PublicKey, c.config, certVerifyMsg.signatureAlgorithm, input, certVerifyMsg.signature)
 		if err != nil {
 			return err
 		}
@@ -768,7 +768,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
 		}
 
 		if certVerify.hasSignatureAlgorithm {
-			certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.vers, privKey, certReq.signatureAlgorithms, c.config.signatureAlgorithmsForClient())
+			certVerify.signatureAlgorithm, err = selectSignatureAlgorithm(c.vers, privKey, c.config, certReq.signatureAlgorithms, c.config.signatureAlgorithmsForClient())
 			if err != nil {
 				c.sendAlert(alertInternalError)
 				return err
@@ -1254,7 +1254,7 @@ findCert:
 		// Ensure the private key supports one of the advertised
 		// signature algorithms.
 		if certReq.hasSignatureAlgorithm {
-			if _, err := selectSignatureAlgorithm(c.vers, chain.PrivateKey, certReq.signatureAlgorithms, c.config.signatureAlgorithmsForClient()); err != nil {
+			if _, err := selectSignatureAlgorithm(c.vers, chain.PrivateKey, c.config, certReq.signatureAlgorithms, c.config.signatureAlgorithmsForClient()); err != nil {
 				continue
 			}
 		}
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index a5bfed7f..d04486f7 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -749,7 +749,7 @@ func (hs *serverHandshakeState) doFullHandshake() error {
 		}
 
 		if c.vers > VersionSSL30 {
-			err = verifyMessage(c.vers, pub, sigAlg, hs.finishedHash.buffer, certVerify.signature)
+			err = verifyMessage(c.vers, pub, c.config, sigAlg, hs.finishedHash.buffer, certVerify.signature)
 		} else {
 			// SSL 3.0's client certificate construction is
 			// incompatible with signatureAlgorithm.
diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go
index 0c9dc358..96870f3a 100644
--- a/ssl/test/runner/key_agreement.go
+++ b/ssl/test/runner/key_agreement.go
@@ -64,7 +64,7 @@ func (ka *rsaKeyAgreement) generateServerKeyExchange(config *Config, cert *Certi
 
 	var sigAlg signatureAlgorithm
 	if ka.version >= VersionTLS12 {
-		sigAlg, err = selectSignatureAlgorithm(ka.version, cert.PrivateKey, clientHello.signatureAlgorithms, config.signatureAlgorithmsForServer())
+		sigAlg, err = selectSignatureAlgorithm(ka.version, cert.PrivateKey, config, clientHello.signatureAlgorithms, config.signatureAlgorithmsForServer())
 		if err != nil {
 			return nil, err
 		}
@@ -404,7 +404,7 @@ func (ka *signedKeyAgreement) signParameters(config *Config, cert *Certificate,
 	var sigAlg signatureAlgorithm
 	var err error
 	if ka.version >= VersionTLS12 {
-		sigAlg, err = selectSignatureAlgorithm(ka.version, cert.PrivateKey, clientHello.signatureAlgorithms, config.signatureAlgorithmsForServer())
+		sigAlg, err = selectSignatureAlgorithm(ka.version, cert.PrivateKey, config, clientHello.signatureAlgorithms, config.signatureAlgorithmsForServer())
 		if err != nil {
 			return nil, err
 		}
@@ -488,7 +488,7 @@ func (ka *signedKeyAgreement) verifyParameters(config *Config, clientHello *clie
 	}
 	sig = sig[2:]
 
-	return verifyMessage(ka.version, cert.PublicKey, sigAlg, msg, sig)
+	return verifyMessage(ka.version, cert.PublicKey, config, sigAlg, msg, sig)
 }
 
 // ecdheRSAKeyAgreement implements a TLS key agreement where the server
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 5e424cac..aa86ea77 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4676,7 +4676,6 @@ var testSignatureAlgorithms = []struct {
 	{"RSA-PKCS1-SHA384", signatureRSAPKCS1WithSHA384, testCertRSA},
 	{"RSA-PKCS1-SHA512", signatureRSAPKCS1WithSHA512, testCertRSA},
 	{"ECDSA-SHA1", signatureECDSAWithSHA1, testCertECDSAP256},
-	// TODO(davidben): Enforce curve matching in TLS 1.3 and test.
 	{"ECDSA-P256-SHA256", signatureECDSAWithP256AndSHA256, testCertECDSAP256},
 	{"ECDSA-P384-SHA384", signatureECDSAWithP384AndSHA384, testCertECDSAP384},
 	{"ECDSA-P521-SHA512", signatureECDSAWithP521AndSHA512, testCertECDSAP521},
@@ -4688,93 +4687,103 @@ const fakeSigAlg2 signatureAlgorithm = 0xff01
 func addSignatureAlgorithmTests() {
 	// Make sure each signature algorithm works. Include some fake values in
 	// the list and ensure they're ignored.
-	//
-	// TODO(davidben): Test each of these against both TLS 1.2 and TLS 1.3.
 	for _, alg := range testSignatureAlgorithms {
-		testCases = append(testCases, testCase{
-			name: "SigningHash-ClientAuth-Sign-" + alg.name,
-			config: Config{
-				MaxVersion: VersionTLS12,
-				// SignatureAlgorithms is shared, so we must
-				// configure a matching server certificate too.
-				Certificates: []Certificate{getRunnerCertificate(alg.cert)},
-				ClientAuth:   RequireAnyClientCert,
-				SignatureAlgorithms: []signatureAlgorithm{
-					fakeSigAlg1,
-					alg.id,
-					fakeSigAlg2,
+		for _, ver := range tlsVersions {
+			if ver.version < VersionTLS12 {
+				continue
+			}
+
+			// ecdsa_sha1 does not exist in TLS 1.3.
+			if ver.version == VersionTLS13 && alg.id == signatureECDSAWithSHA1 {
+				continue
+			}
+
+			suffix := "-" + alg.name + "-" + ver.name
+			testCases = append(testCases, testCase{
+				name: "SigningHash-ClientAuth-Sign" + suffix,
+				config: Config{
+					MaxVersion: ver.version,
+					// SignatureAlgorithms is shared, so we must
+					// configure a matching server certificate too.
+					Certificates: []Certificate{getRunnerCertificate(alg.cert)},
+					ClientAuth:   RequireAnyClientCert,
+					SignatureAlgorithms: []signatureAlgorithm{
+						fakeSigAlg1,
+						alg.id,
+						fakeSigAlg2,
+					},
 				},
-			},
-			flags: []string{
-				"-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)),
-				"-key-file", path.Join(*resourceDir, getShimKey(alg.cert)),
-				"-enable-all-curves",
-			},
-			expectedPeerSignatureAlgorithm: alg.id,
-		})
+				flags: []string{
+					"-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)),
+					"-key-file", path.Join(*resourceDir, getShimKey(alg.cert)),
+					"-enable-all-curves",
+				},
+				expectedPeerSignatureAlgorithm: alg.id,
+			})
 
-		testCases = append(testCases, testCase{
-			testType: serverTest,
-			name:     "SigningHash-ClientAuth-Verify-" + alg.name,
-			config: Config{
-				MaxVersion:   VersionTLS12,
-				Certificates: []Certificate{getRunnerCertificate(alg.cert)},
-				SignatureAlgorithms: []signatureAlgorithm{
-					alg.id,
+			testCases = append(testCases, testCase{
+				testType: serverTest,
+				name:     "SigningHash-ClientAuth-Verify" + suffix,
+				config: Config{
+					MaxVersion:   ver.version,
+					Certificates: []Certificate{getRunnerCertificate(alg.cert)},
+					SignatureAlgorithms: []signatureAlgorithm{
+						alg.id,
+					},
 				},
-			},
-			flags: []string{
-				"-require-any-client-certificate",
-				"-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)),
-				// SignatureAlgorithms is shared, so we must
-				// configure a matching server certificate too.
-				"-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)),
-				"-key-file", path.Join(*resourceDir, getShimKey(alg.cert)),
-				"-enable-all-curves",
-			},
-		})
+				flags: []string{
+					"-require-any-client-certificate",
+					"-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)),
+					// SignatureAlgorithms is shared, so we must
+					// configure a matching server certificate too.
+					"-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)),
+					"-key-file", path.Join(*resourceDir, getShimKey(alg.cert)),
+					"-enable-all-curves",
+				},
+			})
 
-		testCases = append(testCases, testCase{
-			testType: serverTest,
-			name:     "SigningHash-ServerKeyExchange-Sign-" + alg.name,
-			config: Config{
-				MaxVersion: VersionTLS12,
-				CipherSuites: []uint16{
-					TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-					TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+			testCases = append(testCases, testCase{
+				testType: serverTest,
+				name:     "SigningHash-ServerKeyExchange-Sign" + suffix,
+				config: Config{
+					MaxVersion: ver.version,
+					CipherSuites: []uint16{
+						TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+						TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+					},
+					SignatureAlgorithms: []signatureAlgorithm{
+						fakeSigAlg1,
+						alg.id,
+						fakeSigAlg2,
+					},
 				},
-				SignatureAlgorithms: []signatureAlgorithm{
-					fakeSigAlg1,
-					alg.id,
-					fakeSigAlg2,
+				flags: []string{
+					"-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)),
+					"-key-file", path.Join(*resourceDir, getShimKey(alg.cert)),
+					"-enable-all-curves",
 				},
-			},
-			flags: []string{
-				"-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)),
-				"-key-file", path.Join(*resourceDir, getShimKey(alg.cert)),
-				"-enable-all-curves",
-			},
-			expectedPeerSignatureAlgorithm: alg.id,
-		})
+				expectedPeerSignatureAlgorithm: alg.id,
+			})
 
-		testCases = append(testCases, testCase{
-			name: "SigningHash-ServerKeyExchange-Verify-" + alg.name,
-			config: Config{
-				MaxVersion:   VersionTLS12,
-				Certificates: []Certificate{getRunnerCertificate(alg.cert)},
-				CipherSuites: []uint16{
-					TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-					TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+			testCases = append(testCases, testCase{
+				name: "SigningHash-ServerKeyExchange-Verify" + suffix,
+				config: Config{
+					MaxVersion:   ver.version,
+					Certificates: []Certificate{getRunnerCertificate(alg.cert)},
+					CipherSuites: []uint16{
+						TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+						TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+					},
+					SignatureAlgorithms: []signatureAlgorithm{
+						alg.id,
+					},
 				},
-				SignatureAlgorithms: []signatureAlgorithm{
-					alg.id,
+				flags: []string{
+					"-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)),
+					"-enable-all-curves",
 				},
-			},
-			flags: []string{
-				"-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)),
-				"-enable-all-curves",
-			},
-		})
+			})
+		}
 	}
 
 	// Test that algorithm selection takes the key type into account.
@@ -5063,6 +5072,75 @@ func addSignatureAlgorithmTests() {
 		},
 		flags: []string{"-p384-only"},
 	})
+
+	// In TLS 1.2, the ECDSA curve is not in the signature algorithm.
+	testCases = append(testCases, testCase{
+		name: "ECDSACurveMismatch-Verify-TLS12",
+		config: Config{
+			MaxVersion:   VersionTLS12,
+			CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+			Certificates: []Certificate{ecdsaP256Certificate},
+			SignatureAlgorithms: []signatureAlgorithm{
+				signatureECDSAWithP384AndSHA384,
+			},
+		},
+	})
+
+	// In TLS 1.3, the ECDSA curve comes from the signature algorithm.
+	testCases = append(testCases, testCase{
+		name: "ECDSACurveMismatch-Verify-TLS13",
+		config: Config{
+			MaxVersion:   VersionTLS13,
+			CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+			Certificates: []Certificate{ecdsaP256Certificate},
+			SignatureAlgorithms: []signatureAlgorithm{
+				signatureECDSAWithP384AndSHA384,
+			},
+			Bugs: ProtocolBugs{
+				SkipECDSACurveCheck: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":WRONG_SIGNATURE_TYPE:",
+	})
+
+	// Signature algorithm selection in TLS 1.3 should take the curve into
+	// account.
+	testCases = append(testCases, testCase{
+		testType: serverTest,
+		name:     "ECDSACurveMismatch-Sign-TLS13",
+		config: Config{
+			MaxVersion:   VersionTLS13,
+			CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+			SignatureAlgorithms: []signatureAlgorithm{
+				signatureECDSAWithP384AndSHA384,
+				signatureECDSAWithP256AndSHA256,
+			},
+		},
+		flags: []string{
+			"-cert-file", path.Join(*resourceDir, ecdsaP256CertificateFile),
+			"-key-file", path.Join(*resourceDir, ecdsaP256KeyFile),
+		},
+		expectedPeerSignatureAlgorithm: signatureECDSAWithP256AndSHA256,
+	})
+
+	// ecdsa_sha1 cannot be negotiated in TLS 1.3.
+	testCases = append(testCases, testCase{
+		name: "NoECDSAWithSHA1-TLS13",
+		config: Config{
+			MaxVersion:   VersionTLS13,
+			CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
+			Certificates: []Certificate{ecdsaP256Certificate},
+			SignatureAlgorithms: []signatureAlgorithm{
+				signatureECDSAWithSHA1,
+			},
+			Bugs: ProtocolBugs{
+				SkipECDSACurveCheck: true,
+			},
+		},
+		shouldFail:    true,
+		expectedError: ":WRONG_SIGNATURE_TYPE:",
+	})
 }
 
 // timeouts is the retransmit schedule for BoringSSL. It doubles and
diff --git a/ssl/test/runner/sign.go b/ssl/test/runner/sign.go
index cf2d91dc..ec1738e8 100644
--- a/ssl/test/runner/sign.go
+++ b/ssl/test/runner/sign.go
@@ -7,6 +7,7 @@ package runner
 import (
 	"crypto"
 	"crypto/ecdsa"
+	"crypto/elliptic"
 	"crypto/md5"
 	"crypto/rsa"
 	"crypto/sha1"
@@ -24,7 +25,7 @@ type signer interface {
 	verifyMessage(key crypto.PublicKey, msg, sig []byte) error
 }
 
-func selectSignatureAlgorithm(version uint16, key crypto.PrivateKey, peerSigAlgs, ourSigAlgs []signatureAlgorithm) (signatureAlgorithm, error) {
+func selectSignatureAlgorithm(version uint16, key crypto.PrivateKey, config *Config, peerSigAlgs, ourSigAlgs []signatureAlgorithm) (signatureAlgorithm, error) {
 	// 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
@@ -37,7 +38,7 @@ func selectSignatureAlgorithm(version uint16, key crypto.PrivateKey, peerSigAlgs
 			continue
 		}
 
-		signer, err := getSigner(version, key, sigAlg)
+		signer, err := getSigner(version, key, config, sigAlg)
 		if err != nil {
 			continue
 		}
@@ -50,7 +51,7 @@ func selectSignatureAlgorithm(version uint16, key crypto.PrivateKey, peerSigAlgs
 }
 
 func signMessage(version uint16, key crypto.PrivateKey, config *Config, sigAlg signatureAlgorithm, msg []byte) ([]byte, error) {
-	signer, err := getSigner(version, key, sigAlg)
+	signer, err := getSigner(version, key, config, sigAlg)
 	if err != nil {
 		return nil, err
 	}
@@ -58,8 +59,8 @@ func signMessage(version uint16, key crypto.PrivateKey, config *Config, sigAlg s
 	return signer.signMessage(key, config, msg)
 }
 
-func verifyMessage(version uint16, key crypto.PublicKey, sigAlg signatureAlgorithm, msg, sig []byte) error {
-	signer, err := getSigner(version, key, sigAlg)
+func verifyMessage(version uint16, key crypto.PublicKey, config *Config, sigAlg signatureAlgorithm, msg, sig []byte) error {
+	signer, err := getSigner(version, key, config, sigAlg)
 	if err != nil {
 		return err
 	}
@@ -110,14 +111,25 @@ func (r *rsaPKCS1Signer) verifyMessage(key crypto.PublicKey, msg, sig []byte) er
 }
 
 type ecdsaSigner struct {
-	// TODO(davidben): In TLS 1.3, ECDSA signatures must match curves as
-	// well. Pass in a curve to enforce in 1.3 alone.
-	hash crypto.Hash
+	version uint16
+	config  *Config
+	curve   elliptic.Curve
+	hash    crypto.Hash
+}
+
+func (e *ecdsaSigner) isCurveValid(curve elliptic.Curve) bool {
+	if e.config.Bugs.SkipECDSACurveCheck {
+		return true
+	}
+	if e.version <= VersionTLS12 {
+		return true
+	}
+	return e.curve != nil && curve == e.curve
 }
 
 func (e *ecdsaSigner) supportsKey(key crypto.PrivateKey) bool {
-	_, ok := key.(*ecdsa.PrivateKey)
-	return ok
+	ecdsaKey, ok := key.(*ecdsa.PrivateKey)
+	return ok && e.isCurveValid(ecdsaKey.Curve)
 }
 
 func maybeCorruptECDSAValue(n *big.Int, typeOfCorruption BadValue, limit *big.Int) *big.Int {
@@ -143,6 +155,9 @@ func (e *ecdsaSigner) signMessage(key crypto.PrivateKey, config *Config, msg []b
 	if !ok {
 		return nil, errors.New("invalid key type for ECDSA")
 	}
+	if !e.isCurveValid(ecdsaKey.Curve) {
+		return nil, errors.New("invalid curve for ECDSA")
+	}
 
 	h := e.hash.New()
 	h.Write(msg)
@@ -163,6 +178,9 @@ func (e *ecdsaSigner) verifyMessage(key crypto.PublicKey, msg, sig []byte) error
 	if !ok {
 		return errors.New("invalid key type for ECDSA")
 	}
+	if !e.isCurveValid(ecdsaKey.Curve) {
+		return errors.New("invalid curve for ECDSA")
+	}
 
 	ecdsaSig := new(ecdsaSignature)
 	if _, err := asn1.Unmarshal(sig, ecdsaSig); err != nil {
@@ -180,14 +198,14 @@ func (e *ecdsaSigner) verifyMessage(key crypto.PublicKey, msg, sig []byte) error
 	return nil
 }
 
-func getSigner(version uint16, key interface{}, sigAlg signatureAlgorithm) (signer, error) {
+func getSigner(version uint16, key interface{}, config *Config, sigAlg signatureAlgorithm) (signer, error) {
 	// TLS 1.1 and below use legacy signature algorithms.
 	if version < VersionTLS12 {
 		switch key.(type) {
 		case *rsa.PrivateKey, *rsa.PublicKey:
 			return &rsaPKCS1Signer{crypto.MD5SHA1}, nil
 		case *ecdsa.PrivateKey, *ecdsa.PublicKey:
-			return &ecdsaSigner{crypto.SHA1}, nil
+			return &ecdsaSigner{version, config, nil, crypto.SHA1}, nil
 		default:
 			return nil, errors.New("unknown key type")
 		}
@@ -206,13 +224,13 @@ func getSigner(version uint16, key interface{}, sigAlg signatureAlgorithm) (sign
 	case signatureRSAPKCS1WithSHA512:
 		return &rsaPKCS1Signer{crypto.SHA512}, nil
 	case signatureECDSAWithSHA1:
-		return &ecdsaSigner{crypto.SHA1}, nil
+		return &ecdsaSigner{version, config, nil, crypto.SHA1}, nil
 	case signatureECDSAWithP256AndSHA256:
-		return &ecdsaSigner{crypto.SHA256}, nil
+		return &ecdsaSigner{version, config, elliptic.P256(), crypto.SHA256}, nil
 	case signatureECDSAWithP384AndSHA384:
-		return &ecdsaSigner{crypto.SHA384}, nil
+		return &ecdsaSigner{version, config, elliptic.P384(), crypto.SHA384}, nil
 	case signatureECDSAWithP521AndSHA512:
-		return &ecdsaSigner{crypto.SHA512}, nil
+		return &ecdsaSigner{version, config, elliptic.P521(), crypto.SHA512}, nil
 	default:
 		return nil, fmt.Errorf("unsupported signature algorithm %04x", sigAlg)
 	}