diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 843403b3..b474352d 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -920,11 +920,10 @@ err: return -1; } -/* ssl3_check_certificate_for_cipher returns one if |leaf| is a suitable server - * certificate type for |cipher|. Otherwise, it returns zero and pushes an error - * on the error queue. */ -static int ssl3_check_certificate_for_cipher(X509 *leaf, - const SSL_CIPHER *cipher) { +/* ssl3_check_leaf_certificate returns one if |leaf| is a suitable leaf server + * certificate for |ssl|. Otherwise, it returns zero and pushes an error on the + * error queue. */ +static int ssl3_check_leaf_certificate(SSL *ssl, X509 *leaf) { int ret = 0; EVP_PKEY *pkey = X509_get_pubkey(leaf); if (pkey == NULL) { @@ -932,6 +931,7 @@ static int ssl3_check_certificate_for_cipher(X509 *leaf, } /* Check the certificate's type matches the cipher. */ + const SSL_CIPHER *cipher = ssl->s3->tmp.new_cipher; int expected_type = ssl_cipher_get_key_type(cipher); assert(expected_type != EVP_PKEY_NONE); if (pkey->type != expected_type) { @@ -939,9 +939,9 @@ static int ssl3_check_certificate_for_cipher(X509 *leaf, goto err; } - /* TODO(davidben): This behavior is preserved from upstream. Should key usages - * be checked in other cases as well? */ if (cipher->algorithm_auth & SSL_aECDSA) { + /* TODO(davidben): This behavior is preserved from upstream. Should key + * usages be checked in other cases as well? */ /* This call populates the ex_flags field correctly */ X509_check_purpose(leaf, -1, 0); if ((leaf->ex_flags & EXFLAG_KUSAGE) && @@ -949,6 +949,11 @@ static int ssl3_check_certificate_for_cipher(X509 *leaf, OPENSSL_PUT_ERROR(SSL, SSL_R_ECC_CERT_NOT_FOR_SIGNING); goto err; } + + if (!tls1_check_ec_cert(ssl, leaf)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_ECC_CERT); + goto err; + } } ret = 1; @@ -1018,7 +1023,7 @@ int ssl3_get_server_certificate(SSL *s) { } X509 *leaf = sk_X509_value(sk, 0); - if (!ssl3_check_certificate_for_cipher(leaf, s->s3->tmp.new_cipher)) { + if (!ssl3_check_leaf_certificate(s, leaf)) { al = SSL_AD_ILLEGAL_PARAMETER; goto f_err; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 9a290283..2a3ba7f7 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -660,23 +660,6 @@ int tls12_check_peer_sigalg(SSL *ssl, const EVP_MD **out_md, int *out_alert, return 0; } - if (pkey->type == EVP_PKEY_EC) { - uint16_t curve_id; - uint8_t comp_id; - /* Check compression and curve matches extensions */ - if (!tls1_curve_params_from_ec_key(&curve_id, &comp_id, pkey->pkey.ec)) { - *out_alert = SSL_AD_INTERNAL_ERROR; - return 0; - } - - if (ssl->server && (!tls1_check_curve_id(ssl, curve_id) || - comp_id != TLSEXT_ECPOINTFORMAT_uncompressed)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE); - *out_alert = SSL_AD_ILLEGAL_PARAMETER; - return 0; - } - } - /* Check signature matches a type we sent */ sent_sigslen = tls12_get_psigalgs(ssl, &sent_sigs); for (i = 0; i < sent_sigslen; i += 2, sent_sigs += 2) { diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 07ba9f59..b309449c 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -42,6 +42,7 @@ #include #include #include +#include #include #include @@ -1223,6 +1224,12 @@ static bool DoExchange(ScopedSSL_SESSION *out_session, SSL_CTX *ssl_ctx, if (config->disable_npn) { SSL_set_options(ssl.get(), SSL_OP_DISABLE_NPN); } + if (config->p384_only) { + int nid = NID_secp384r1; + if (!SSL_set1_curves(ssl.get(), &nid, 1)) { + return false; + } + } int sock = Connect(config->port); if (sock == -1) { diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 6ab71cf2..65738715 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -1997,6 +1997,16 @@ func addBasicTests() { resumeSession: true, expectResumeRejected: true, }, + { + name: "CheckLeafCurve", + config: Config{ + CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256}, + Certificates: []Certificate{getECDSACertificate()}, + }, + flags: []string{"-p384-only"}, + shouldFail: true, + expectedError: ":BAD_ECC_CERT:", + }, } testCases = append(testCases, basicTests...) } diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 50e6b234..23b0879e 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -98,6 +98,7 @@ const Flag kBoolFlags[] = { { "-renegotiate-freely", &TestConfig::renegotiate_freely }, { "-renegotiate-ignore", &TestConfig::renegotiate_ignore }, { "-disable-npn", &TestConfig::disable_npn }, + { "-p384-only", &TestConfig::p384_only }, }; const Flag kStringFlags[] = { diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 9f295aeb..733e0a11 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -101,6 +101,7 @@ struct TestConfig { bool renegotiate_ignore = false; bool disable_npn = false; int expect_server_key_exchange_hash = 0; + bool p384_only = false; }; bool ParseConfig(int argc, char **argv, TestConfig *out_config);