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>

include/openssl/ssl.h

@@ -1069,15 +1069,16 @@ enum ssl_private_key_result_t {
 
 /* ssl_private_key_method_st (aka |SSL_PRIVATE_KEY_METHOD|) describes private
  * key hooks. This is used to off-load signing operations to a custom,
- * potentially asynchronous, backend. */
+ * potentially asynchronous, backend. Metadata about the key such as the type
+ * and size are parsed out of the certificate.
+ *
+ * TODO(davidben): This API has a number of legacy hooks. Remove the last
+ * consumer of |sign_digest| and trim it. */
 struct ssl_private_key_method_st {
-  /* 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. */
+  /* type is ignored and should be NULL. */
   int (*type)(SSL *ssl);
 
-  /* max_signature_len returns the maximum length of a signature signed by the
-   * key used by |ssl|. This must be a constant value for a given |ssl|. */
+  /* max_signature_len is ignored and should be NULL. */
   size_t (*max_signature_len)(SSL *ssl);
 
   /* sign signs the message |in| in using the specified signature algorithm. On

ssl/handshake_client.c

@@ -1527,7 +1527,7 @@ static int ssl3_send_client_certificate(SSL_HANDSHAKE *hs) {
     }
   }
 
-  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 @@ static int ssl3_send_cert_verify(SSL_HANDSHAKE *hs) {
   }
 
   /* 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)) {

ssl/handshake_server.c

@@ -695,11 +695,11 @@ static void ssl_get_compatible_server_ciphers(SSL_HANDSHAKE *hs,
   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 @@ static int ssl3_select_certificate(SSL_HANDSHAKE *hs) {
     }
   }
 
-  if (!ssl->ctx->x509_method->ssl_auto_chain_if_needed(ssl)) {
+  if (!ssl_on_certificate_selected(hs)) {
     return -1;
   }
 
@@ -1241,7 +1241,7 @@ static int ssl3_send_server_key_exchange(SSL_HANDSHAKE *hs) {
     }
 
     /* 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 @@ static int ssl3_get_client_key_exchange(SSL_HANDSHAKE *hs) {
    * |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 @@ static int ssl3_get_client_key_exchange(SSL_HANDSHAKE *hs) {
     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;

ssl/internal.h

@@ -542,22 +542,16 @@ enum ssl_open_record_t ssl_process_alert(SSL *ssl, uint8_t *out_alert,
 
 /* 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 @@ enum ssl_private_key_result_t ssl_private_key_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
+/* 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 @@ int ssl_public_key_verify(
 
 /* 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_add_client_CA_list(SSL *ssl, CBB *cbb);
 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 @@ struct ssl_handshake_st {
   /* 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;
 

ssl/s3_both.c

@@ -184,6 +184,7 @@ void ssl_handshake_free(SSL_HANDSHAKE *hs) {
 
   OPENSSL_free(hs->hostname);
   EVP_PKEY_free(hs->peer_pubkey);
+  EVP_PKEY_free(hs->local_pubkey);
   OPENSSL_free(hs);
 }
 

ssl/ssl_cert.c

@@ -861,6 +861,25 @@ int ssl_check_leaf_certificate(SSL_HANDSHAKE *hs, EVP_PKEY *pkey,
   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;

ssl/ssl_privkey.c

@@ -298,40 +298,6 @@ int ssl_has_private_key(const SSL *ssl) {
   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 @@ enum ssl_private_key_result_t ssl_private_key_complete(SSL *ssl, uint8_t *out,
   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 @@ int ssl_private_key_supports_signature_algorithm(SSL *ssl,
       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 @@ int ssl_private_key_supports_signature_algorithm(SSL *ssl,
      * 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;
     }
 

ssl/t1_lib.c

@@ -3313,17 +3313,17 @@ int tls1_choose_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t *out) {
   /* 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;
-    }
-    if (ssl_is_ecdsa_key_type(type)) {
-      *out = SSL_SIGN_ECDSA_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;
     }
-    OPENSSL_PUT_ERROR(SSL, SSL_R_NO_COMMON_SIGNATURE_ALGORITHMS);
-    return 0;
   }
 
   const uint16_t *sigalgs = cert->sigalgs;
@@ -3350,7 +3350,7 @@ int tls1_choose_signature_algorithm(SSL_HANDSHAKE *hs, uint16_t *out) {
     /* 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;
     }
 

ssl/test/bssl_shim.cc

@@ -286,23 +286,6 @@ static bssl::UniquePtr<STACK_OF(X509_NAME)> DecodeHexX509Names(
   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 ssl_private_key_result_t AsyncPrivateKeyComplete(
 }
 
 static const SSL_PRIVATE_KEY_METHOD g_async_private_key_method = {
-    AsyncPrivateKeyType,
-    AsyncPrivateKeyMaxSignatureLen,
+    nullptr /* type */,
+    nullptr /* max_signature_len */,
     AsyncPrivateKeySign,
     nullptr /* sign_digest */,
     AsyncPrivateKeyDecrypt,

ssl/tls13_both.c

@@ -549,7 +549,7 @@ enum ssl_private_key_result_t tls13_add_certificate_verify(SSL_HANDSHAKE *hs,
 
   /* 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) ||

ssl/tls13_client.c

@@ -525,7 +525,7 @@ static enum ssl_hs_wait_t do_send_client_certificate(SSL_HANDSHAKE *hs) {
     }
   }
 
-  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;
   }