Deprecate SSL_PRIVATE_KEY_METHOD type and max_signature_len.
Instead, extract it from the certificate, which is what everyone was
doing anyway. A follow-up change will take advantage of this cleanup to
deduplicate code between signing and verifying for which keys are good
for which signature algorithms.
BUG=188
Change-Id: Ic3f83a6477e8fa53e5e7233f4545f4d2c4b58d01
Reviewed-on: https://boringssl-review.googlesource.com/14565
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 7eddd35..3b053c7 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -1527,7 +1527,7 @@
}
}
- if (!ssl->ctx->x509_method->ssl_auto_chain_if_needed(ssl) ||
+ if (!ssl_on_certificate_selected(hs) ||
!ssl3_output_cert_chain(ssl)) {
return -1;
}
@@ -1739,7 +1739,7 @@
}
/* Set aside space for the signature. */
- const size_t max_sig_len = ssl_private_key_max_signature_len(ssl);
+ const size_t max_sig_len = EVP_PKEY_size(hs->local_pubkey);
uint8_t *ptr;
if (!CBB_add_u16_length_prefixed(&body, &child) ||
!CBB_reserve(&child, &ptr, max_sig_len)) {
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index cc1897f..f8c9705 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -695,11 +695,11 @@
uint32_t mask_a = 0;
if (ssl_has_certificate(ssl)) {
- int type = ssl_private_key_type(ssl);
- if (type == NID_rsaEncryption) {
+ int type = EVP_PKEY_id(hs->local_pubkey);
+ if (type == EVP_PKEY_RSA) {
mask_k |= SSL_kRSA;
mask_a |= SSL_aRSA;
- } else if (ssl_is_ecdsa_key_type(type)) {
+ } else if (type == EVP_PKEY_EC) {
mask_a |= SSL_aECDSA;
}
}
@@ -879,7 +879,7 @@
}
}
- if (!ssl->ctx->x509_method->ssl_auto_chain_if_needed(ssl)) {
+ if (!ssl_on_certificate_selected(hs)) {
return -1;
}
@@ -1241,7 +1241,7 @@
}
/* Add space for the signature. */
- const size_t max_sig_len = ssl_private_key_max_signature_len(ssl);
+ const size_t max_sig_len = EVP_PKEY_size(hs->local_pubkey);
uint8_t *ptr;
if (!CBB_add_u16_length_prefixed(&body, &child) ||
!CBB_reserve(&child, &ptr, max_sig_len)) {
@@ -1573,7 +1573,7 @@
* |premaster_secret_len|. */
if (alg_k & SSL_kRSA) {
/* Allocate a buffer large enough for an RSA decryption. */
- const size_t rsa_size = ssl_private_key_max_signature_len(ssl);
+ const size_t rsa_size = EVP_PKEY_size(hs->local_pubkey);
decrypt_buf = OPENSSL_malloc(rsa_size);
if (decrypt_buf == NULL) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
@@ -1584,7 +1584,7 @@
size_t decrypt_len;
if (hs->state == SSL3_ST_SR_KEY_EXCH_A) {
if (!ssl_has_private_key(ssl) ||
- ssl_private_key_type(ssl) != NID_rsaEncryption) {
+ EVP_PKEY_id(hs->local_pubkey) != EVP_PKEY_RSA) {
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 a6375a6..3ebac72 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -542,22 +542,16 @@
/* Private key operations. */
+typedef struct ssl_handshake_st SSL_HANDSHAKE;
+
/* ssl_has_private_key returns one if |ssl| has a private key
* 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|. */
-int ssl_private_key_type(SSL *ssl);
-
-size_t ssl_private_key_max_signature_len(SSL *ssl);
-
enum ssl_private_key_result_t ssl_private_key_sign(
SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
uint16_t signature_algorithm, const uint8_t *in, size_t in_len);
@@ -570,9 +564,9 @@
size_t *out_len,
size_t max_out);
-/* ssl_private_key_supports_signature_algorithm returns one if |ssl|'s private
+/* ssl_private_key_supports_signature_algorithm returns one if |hs|'s private
* key supports |signature_algorithm| and zero otherwise. */
-int ssl_private_key_supports_signature_algorithm(SSL *ssl,
+int ssl_private_key_supports_signature_algorithm(SSL_HANDSHAKE *hs,
uint16_t signature_algorithm);
/* ssl_public_key_verify verifies that the |signature| is valid for the public
@@ -585,8 +579,6 @@
/* Custom extensions */
-typedef struct ssl_handshake_st SSL_HANDSHAKE;
-
/* ssl_custom_extension (a.k.a. SSL_CUSTOM_EXTENSION) is a structure that
* contains information about custom-extension callbacks. */
struct ssl_custom_extension {
@@ -857,6 +849,11 @@
int ssl_check_leaf_certificate(SSL_HANDSHAKE *hs, EVP_PKEY *pkey,
const CRYPTO_BUFFER *leaf);
+/* ssl_on_certificate_selected is called once the certificate has been selected.
+ * It finalizes the certificate and initializes |hs->local_pubkey|. It returns
+ * one on success and zero on error. */
+int ssl_on_certificate_selected(SSL_HANDSHAKE *hs);
+
/* TLS 1.3 key derivation. */
@@ -1061,6 +1058,9 @@
/* hostname, on the server, is the value of the SNI extension. */
char *hostname;
+ /* local_pubkey is the public key we are authenticating as. */
+ EVP_PKEY *local_pubkey;
+
/* peer_pubkey is the public key parsed from the peer's leaf certificate. */
EVP_PKEY *peer_pubkey;
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index aff2154..e05a16e 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -184,6 +184,7 @@
OPENSSL_free(hs->hostname);
EVP_PKEY_free(hs->peer_pubkey);
+ EVP_PKEY_free(hs->local_pubkey);
OPENSSL_free(hs);
}
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 4722a66..87b60e9 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -861,6 +861,25 @@
return 1;
}
+int ssl_on_certificate_selected(SSL_HANDSHAKE *hs) {
+ SSL *const ssl = hs->ssl;
+ if (!ssl_has_certificate(ssl)) {
+ /* Nothing to do. */
+ return 1;
+ }
+
+ if (!ssl->ctx->x509_method->ssl_auto_chain_if_needed(ssl)) {
+ return 0;
+ }
+
+ CBS leaf;
+ CRYPTO_BUFFER_init_CBS(sk_CRYPTO_BUFFER_value(ssl->cert->chain, 0), &leaf);
+
+ EVP_PKEY_free(hs->local_pubkey);
+ hs->local_pubkey = ssl_cert_parse_pubkey(&leaf);
+ return hs->local_pubkey != NULL;
+}
+
static int set_signed_cert_timestamp_list(CERT *cert, const uint8_t *list,
size_t list_len) {
CBS sct_list;
diff --git a/ssl/ssl_privkey.c b/ssl/ssl_privkey.c
index 4006759..82842d0 100644
--- a/ssl/ssl_privkey.c
+++ b/ssl/ssl_privkey.c
@@ -298,40 +298,6 @@
return ssl->cert->privatekey != NULL || ssl->cert->key_method != NULL;
}
-int ssl_is_ecdsa_key_type(int type) {
- switch (type) {
- case NID_secp224r1:
- 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);
- }
- 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) {
- if (ssl->cert->key_method != NULL) {
- return ssl->cert->key_method->max_signature_len(ssl);
- }
- return EVP_PKEY_size(ssl->cert->privatekey);
-}
-
static int is_rsa_pkcs1(const EVP_MD **out_md, uint16_t sigalg) {
switch (sigalg) {
case SSL_SIGN_RSA_PKCS1_MD5_SHA1:
@@ -523,18 +489,19 @@
return ssl->cert->key_method->complete(ssl, out, out_len, max_out);
}
-int ssl_private_key_supports_signature_algorithm(SSL *ssl,
+int ssl_private_key_supports_signature_algorithm(SSL_HANDSHAKE *hs,
uint16_t signature_algorithm) {
+ SSL *const ssl = hs->ssl;
+ int type = EVP_PKEY_id(hs->local_pubkey);
const EVP_MD *md;
if (is_rsa_pkcs1(&md, signature_algorithm) &&
ssl3_protocol_version(ssl) < TLS1_3_VERSION) {
- return ssl_private_key_type(ssl) == NID_rsaEncryption;
+ return type == EVP_PKEY_RSA;
}
int curve;
if (is_ecdsa(&curve, &md, signature_algorithm)) {
- int type = ssl_private_key_type(ssl);
- if (!ssl_is_ecdsa_key_type(type)) {
+ if (type != EVP_PKEY_EC) {
return 0;
}
@@ -543,11 +510,13 @@
return 1;
}
- return curve != NID_undef && type == curve;
+ return curve != NID_undef &&
+ EC_GROUP_get_curve_name(EC_KEY_get0_group(
+ EVP_PKEY_get0_EC_KEY(hs->local_pubkey))) == curve;
}
if (is_rsa_pss(&md, signature_algorithm)) {
- if (ssl_private_key_type(ssl) != NID_rsaEncryption) {
+ if (type != EVP_PKEY_RSA) {
return 0;
}
@@ -557,7 +526,7 @@
* defined RSASSA-PSS algorithm, but 1024-bit RSA is slightly too large for
* SHA-512. 1024-bit RSA is sometimes used for test credentials, so check
* the size to fall back to another algorithm. */
- if (ssl_private_key_max_signature_len(ssl) < 2 * EVP_MD_size(md) + 2) {
+ if ((size_t)EVP_PKEY_size(hs->local_pubkey) < 2 * EVP_MD_size(md) + 2) {
return 0;
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 2acd422..e47937d 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -3313,17 +3313,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) {
- int type = ssl_private_key_type(ssl);
- if (type == NID_rsaEncryption) {
- *out = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
- return 1;
+ switch (EVP_PKEY_id(hs->local_pubkey)) {
+ case EVP_PKEY_RSA:
+ *out = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
+ return 1;
+ case EVP_PKEY_EC:
+ *out = SSL_SIGN_ECDSA_SHA1;
+ return 1;
+ default:
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_COMMON_SIGNATURE_ALGORITHMS);
+ return 0;
}
- 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 = cert->sigalgs;
@@ -3350,7 +3350,7 @@
/* 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])) {
+ !ssl_private_key_supports_signature_algorithm(hs, sigalgs[i])) {
continue;
}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index b60c809..f7bcb0f 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -286,23 +286,6 @@
return ret;
}
-static int AsyncPrivateKeyType(SSL *ssl) {
- 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) {
- return EVP_PKEY_size(GetTestState(ssl)->private_key.get());
-}
-
static ssl_private_key_result_t AsyncPrivateKeySign(
SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
uint16_t signature_algorithm, const uint8_t *in, size_t in_len) {
@@ -436,8 +419,8 @@
}
static const SSL_PRIVATE_KEY_METHOD g_async_private_key_method = {
- AsyncPrivateKeyType,
- AsyncPrivateKeyMaxSignatureLen,
+ nullptr /* type */,
+ nullptr /* max_signature_len */,
AsyncPrivateKeySign,
nullptr /* sign_digest */,
AsyncPrivateKeyDecrypt,
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index ec67cdc..be3c1b7 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -549,7 +549,7 @@
/* Sign the digest. */
CBB child;
- const size_t max_sig_len = ssl_private_key_max_signature_len(ssl);
+ const size_t max_sig_len = EVP_PKEY_size(hs->local_pubkey);
uint8_t *sig;
size_t sig_len;
if (!CBB_add_u16_length_prefixed(&body, &child) ||
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index 4c711e6..6de51e5 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -525,7 +525,7 @@
}
}
- if (!ssl->ctx->x509_method->ssl_auto_chain_if_needed(ssl) ||
+ if (!ssl_on_certificate_selected(hs) ||
!tls13_add_certificate(hs)) {
return ssl_hs_error;
}
