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>
diff --git a/ssl/internal.h b/ssl/internal.h
index 65b05f8..c971bab 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -490,6 +490,11 @@
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 @@
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 e71f82d..cca2f06 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 @@
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 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_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 @@
*
* 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 @@
? 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 @@
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 @@
/* 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 2fe7a90..5279a5d 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2511,27 +2511,6 @@
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_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 @@
}
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 023de3a..1965091 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -896,6 +896,10 @@
// 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 7857b1d..4eb2ce1 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -570,7 +570,7 @@
}
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 @@
}
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 @@
// 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 a5bfed7..d04486f 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -749,7 +749,7 @@
}
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 0c9dc35..96870f3 100644
--- a/ssl/test/runner/key_agreement.go
+++ b/ssl/test/runner/key_agreement.go
@@ -64,7 +64,7 @@
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 @@
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 @@
}
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 5e424ca..aa86ea7 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4676,7 +4676,6 @@
{"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 @@
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,
- },
- },
- flags: []string{
- "-cert-file", path.Join(*resourceDir, getShimCertificate(alg.cert)),
- "-key-file", path.Join(*resourceDir, getShimKey(alg.cert)),
- "-enable-all-curves",
- },
- expectedPeerSignatureAlgorithm: alg.id,
- })
+ for _, ver := range tlsVersions {
+ if ver.version < VersionTLS12 {
+ continue
+ }
- 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,
- },
- },
- 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",
- },
- })
+ // ecdsa_sha1 does not exist in TLS 1.3.
+ if ver.version == VersionTLS13 && alg.id == signatureECDSAWithSHA1 {
+ continue
+ }
- 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,
+ 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,
+ },
},
- 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{
+ testType: serverTest,
+ name: "SigningHash-ClientAuth-Verify" + suffix,
+ config: Config{
+ MaxVersion: ver.version,
+ Certificates: []Certificate{getRunnerCertificate(alg.cert)},
+ SignatureAlgorithms: []signatureAlgorithm{
+ alg.id,
+ },
},
- 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{
- "-expect-peer-signature-algorithm", strconv.Itoa(int(alg.id)),
- "-enable-all-curves",
- },
- })
+ })
+
+ 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,
+ },
+ },
+ 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{
+ 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,
+ },
+ },
+ 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 @@
},
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 cf2d91d..ec1738e 100644
--- a/ssl/test/runner/sign.go
+++ b/ssl/test/runner/sign.go
@@ -7,6 +7,7 @@
import (
"crypto"
"crypto/ecdsa"
+ "crypto/elliptic"
"crypto/md5"
"crypto/rsa"
"crypto/sha1"
@@ -24,7 +25,7 @@
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 @@
continue
}
- signer, err := getSigner(version, key, sigAlg)
+ signer, err := getSigner(version, key, config, sigAlg)
if err != nil {
continue
}
@@ -50,7 +51,7 @@
}
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 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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)
}
