Widen SSL_PRIVATE_KEY_METHOD types to include the curve name.
This makes custom private keys and EVP_PKEYs symmetric again. There is
no longer a requirement that the caller pre-filter the configured
signing prefs.
Also switch EVP_PKEY_RSA to NID_rsaEncryption. These are identical, but
if some key types are to be NIDs, we should make them all NIDs.
Change-Id: I82ea41c27a3c57f4c4401ffe1ccad406783e4c64
Reviewed-on: https://boringssl-review.googlesource.com/8785
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 7b4b349..75ea320 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -980,8 +980,12 @@
/* SSL_PRIVATE_KEY_METHOD describes private key hooks. This is used to off-load
* signing operations to a custom, potentially asynchronous, backend. */
typedef struct ssl_private_key_method_st {
- /* type returns either |EVP_PKEY_RSA| or |EVP_PKEY_EC| to denote the type of
- * key used by |ssl|. */
+ /* type returns the type of the key used by |ssl|. For RSA keys, return
+ * |NID_rsaEncryption|. For ECDSA keys, return |NID_X9_62_prime256v1|,
+ * |NID_secp384r1|, or |NID_secp521r1|, depending on the curve.
+ *
+ * Returning |EVP_PKEY_EC| for ECDSA keys is deprecated and may result in
+ * connection failures in TLS 1.3. */
int (*type)(SSL *ssl);
/* max_signature_len returns the maximum length of a signature signed by the
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index fbf935d..7cd8265 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1424,7 +1424,7 @@
size_t decrypt_len;
if (ssl->state == SSL3_ST_SR_KEY_EXCH_A) {
if (!ssl_has_private_key(ssl) ||
- ssl_private_key_type(ssl) != EVP_PKEY_RSA) {
+ ssl_private_key_type(ssl) != NID_rsaEncryption) {
al = SSL_AD_HANDSHAKE_FAILURE;
OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_RSA_CERTIFICATE);
goto f_err;
diff --git a/ssl/internal.h b/ssl/internal.h
index be7bde2..06148f1 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -468,6 +468,10 @@
* configured and zero otherwise. */
int ssl_has_private_key(const SSL *ssl);
+/* ssl_is_ecdsa_key_type returns one if |type| is an ECDSA key type and zero
+ * otherwise. */
+int ssl_is_ecdsa_key_type(int type);
+
/* ssl_private_key_* call the corresponding function on the
* |SSL_PRIVATE_KEY_METHOD| for |ssl|, if configured. Otherwise, they implement
* the operation with |EVP_PKEY|. */
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index ca52233..9842d96 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1905,10 +1905,11 @@
uint32_t mask_a = 0;
if (ssl->cert->x509 != NULL && ssl_has_private_key(ssl)) {
- if (ssl_private_key_type(ssl) == EVP_PKEY_RSA) {
+ int type = ssl_private_key_type(ssl);
+ if (type == NID_rsaEncryption) {
mask_k |= SSL_kRSA;
mask_a |= SSL_aRSA;
- } else if (ssl_private_key_type(ssl) == EVP_PKEY_EC) {
+ } else if (ssl_is_ecdsa_key_type(type)) {
/* An ECC certificate may be usable for ECDSA cipher suites depending on
* the key usage extension and on the client's group preferences. */
X509 *x = ssl->cert->x509;
diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c
index fec5d46..9678493 100644
--- a/ssl/ssl_rsa.c
+++ b/ssl/ssl_rsa.c
@@ -400,11 +400,32 @@
return ssl->cert->privatekey != NULL || ssl->cert->key_method != NULL;
}
+int ssl_is_ecdsa_key_type(int type) {
+ switch (type) {
+ /* TODO(davidben): Remove support for |EVP_PKEY_EC| key types. */
+ case EVP_PKEY_EC:
+ case NID_X9_62_prime256v1:
+ case NID_secp384r1:
+ case NID_secp521r1:
+ return 1;
+ default:
+ return 0;
+ }
+}
+
int ssl_private_key_type(SSL *ssl) {
if (ssl->cert->key_method != NULL) {
return ssl->cert->key_method->type(ssl);
}
- return EVP_PKEY_id(ssl->cert->privatekey);
+ switch (EVP_PKEY_id(ssl->cert->privatekey)) {
+ case EVP_PKEY_RSA:
+ return NID_rsaEncryption;
+ case EVP_PKEY_EC:
+ return EC_GROUP_get_curve_name(
+ EC_KEY_get0_group(EVP_PKEY_get0_EC_KEY(ssl->cert->privatekey)));
+ default:
+ return NID_undef;
+ }
}
size_t ssl_private_key_max_signature_len(SSL *ssl) {
@@ -716,31 +737,28 @@
uint16_t signature_algorithm) {
const EVP_MD *md;
if (is_rsa_pkcs1(&md, signature_algorithm)) {
- return ssl_private_key_type(ssl) == EVP_PKEY_RSA;
+ return ssl_private_key_type(ssl) == NID_rsaEncryption;
}
int curve;
if (is_ecdsa(&curve, &md, signature_algorithm)) {
- if (ssl_private_key_type(ssl) != EVP_PKEY_EC) {
+ int type = ssl_private_key_type(ssl);
+ if (!ssl_is_ecdsa_key_type(type)) {
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;
- }
+ /* Prior to TLS 1.3, ECDSA curves did not match the signature algorithm. */
+ if (ssl3_protocol_version(ssl) < TLS1_3_VERSION) {
+ return 1;
}
- return 1;
+
+ /* TODO(davidben): Remove support for EVP_PKEY_EC keys. */
+ return curve != NID_undef && (type == EVP_PKEY_EC || type == curve);
}
if (is_rsa_pss(&md, signature_algorithm)) {
if (ssl3_protocol_version(ssl) < TLS1_3_VERSION ||
- ssl_private_key_type(ssl) != EVP_PKEY_RSA) {
+ ssl_private_key_type(ssl) != NID_rsaEncryption) {
return 0;
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 4594649..9761dc9 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2596,12 +2596,17 @@
/* 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 (ssl_private_key_type(ssl) == EVP_PKEY_RSA) {
+ int type = ssl_private_key_type(ssl);
+ if (type == NID_rsaEncryption) {
*out = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
- } else {
- *out = SSL_SIGN_ECDSA_SHA1;
+ return 1;
}
- return 1;
+ if (ssl_is_ecdsa_key_type(type)) {
+ *out = SSL_SIGN_ECDSA_SHA1;
+ return 1;
+ }
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_COMMON_SIGNATURE_ALGORITHMS);
+ return 0;
}
const uint16_t *sigalgs;
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 3cfbeb3..c505f21 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -147,7 +147,16 @@
}
static int AsyncPrivateKeyType(SSL *ssl) {
- return EVP_PKEY_id(GetTestState(ssl)->private_key.get());
+ EVP_PKEY *key = GetTestState(ssl)->private_key.get();
+ switch (EVP_PKEY_id(key)) {
+ case EVP_PKEY_RSA:
+ return NID_rsaEncryption;
+ case EVP_PKEY_EC:
+ return EC_GROUP_get_curve_name(
+ EC_KEY_get0_group(EVP_PKEY_get0_EC_KEY(key)));
+ default:
+ return NID_undef;
+ }
}
static size_t AsyncPrivateKeyMaxSignatureLen(SSL *ssl) {
