commit 2b8415e8ff324cd4a322670a5a1b6cb8906ebcfd
author Steven Valdez <svaldez@google.com> Thu Jun 30 13:27:23 2016 -0400
committer David Benjamin <davidben@google.com> Fri Jul 01 19:01:33 2016 +0000
tree 48b12efcca80cc72a424f69848095a613685cfe0
parent 0ba87732c659b92cbf879c09e18b8ae1f33422f0

Move the Digest/Sign split for SignatureAlgorithms to a lower level.

In order to delay the digest of the handshake transcript and unify
around message-based signing callbacks, a copy of the transcript is kept
around until we are sure there is no certificate authentication.

This removes support for SSL_PRIVATE_KEY_METHOD as a client in SSL 3.0.

Change-Id: If8999a19ca021b4ff439319ab91e2cd2103caa64
Reviewed-on: https://boringssl-review.googlesource.com/8561
Reviewed-by: David Benjamin <davidben@google.com>

ssl/handshake_client.c

diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index b4123dd..e3cd102 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -880,11 +880,10 @@
     goto f_err;
   }
 
-  /* If doing a full handshake with TLS 1.2, the server may request a client
-   * certificate which requires hashing the handshake transcript under a
-   * different hash. Otherwise, the handshake buffer may be released. */
-  if (ssl->hit || ssl3_protocol_version(ssl) < TLS1_2_VERSION ||
-      !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
+  /* If doing a full handshake, the server may request a client certificate
+   * which requires hashing the handshake transcript. Otherwise, the handshake
+   * buffer may be released. */
+  if (ssl->hit || !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
     ssl3_free_handshake_buffer(ssl);
   }
 
@@ -1120,7 +1119,6 @@
 }
 
 static int ssl3_get_server_key_exchange(SSL *ssl) {
-  EVP_MD_CTX md_ctx;
   int al, ok;
   EVP_PKEY *pkey = NULL;
   DH *dh = NULL;
@@ -1158,7 +1156,6 @@
 
   uint32_t alg_k = ssl->s3->tmp.new_cipher->algorithm_mkey;
   uint32_t alg_a = ssl->s3->tmp.new_cipher->algorithm_auth;
-  EVP_MD_CTX_init(&md_ctx);
 
   if (alg_a & SSL_aPSK) {
     CBS psk_identity_hint;
@@ -1336,21 +1333,25 @@
       goto f_err;
     }
 
-    const EVP_MD *md = tls12_get_hash(signature_algorithm);
-    if (md == NULL) {
-      al = SSL_AD_ILLEGAL_PARAMETER;
-      OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SIGNATURE_TYPE);
+    CBB transcript;
+    uint8_t *transcript_data;
+    size_t transcript_len;
+    if (!CBB_init(&transcript, 2*SSL3_RANDOM_SIZE + CBS_len(&parameter)) ||
+        !CBB_add_bytes(&transcript, ssl->s3->client_random, SSL3_RANDOM_SIZE) ||
+        !CBB_add_bytes(&transcript, ssl->s3->server_random, SSL3_RANDOM_SIZE) ||
+        !CBB_add_bytes(&transcript, CBS_data(&parameter), CBS_len(&parameter)) ||
+        !CBB_finish(&transcript, &transcript_data, &transcript_len)) {
+      CBB_cleanup(&transcript);
+      al = SSL_AD_INTERNAL_ERROR;
+      OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
       goto f_err;
     }
-    int sig_ok = EVP_DigestVerifyInit(&md_ctx, NULL, md, NULL, pkey) &&
-                 EVP_DigestVerifyUpdate(&md_ctx, ssl->s3->client_random,
-                                        SSL3_RANDOM_SIZE) &&
-                 EVP_DigestVerifyUpdate(&md_ctx, ssl->s3->server_random,
-                                        SSL3_RANDOM_SIZE) &&
-                 EVP_DigestVerifyUpdate(&md_ctx, CBS_data(&parameter),
-                                        CBS_len(&parameter)) &&
-                 EVP_DigestVerifyFinal(&md_ctx, CBS_data(&signature),
-                                       CBS_len(&signature));
+
+    int sig_ok = ssl_public_key_verify(
+        ssl, CBS_data(&signature), CBS_len(&signature), signature_algorithm,
+        pkey, transcript_data, transcript_len);
+    OPENSSL_free(transcript_data);
+
 #if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
     sig_ok = 1;
     ERR_clear_error();
@@ -1372,7 +1373,6 @@
     }
   }
   EVP_PKEY_free(pkey);
-  EVP_MD_CTX_cleanup(&md_ctx);
   return 1;
 
 f_err:
@@ -1382,7 +1382,6 @@
   DH_free(dh);
   EC_POINT_free(srvr_ecpoint);
   EC_KEY_free(ecdh);
-  EVP_MD_CTX_cleanup(&md_ctx);
   return -1;
 }
 
@@ -1840,23 +1839,44 @@
     goto err;
   }
 
-  size_t sig_len;
+  size_t sig_len = max_sig_len;
   enum ssl_private_key_result_t sign_result;
   if (ssl->state == SSL3_ST_CW_CERT_VRFY_A) {
-    /* Compute the digest. In TLS 1.1 and below, the digest type is also
-     * selected here. */
-    uint8_t digest[EVP_MAX_MD_SIZE];
-    size_t digest_len;
-    if (!ssl3_cert_verify_hash(ssl, digest, &digest_len, signature_algorithm)) {
-      goto err;
+    /* The SSL3 construction for CertificateVerify does not decompose into a
+     * single final digest and signature, and must be special-cased. */
+    if (ssl3_protocol_version(ssl) == SSL3_VERSION) {
+      if (ssl->cert->key_method != NULL) {
+        OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL_FOR_CUSTOM_KEY);
+        goto err;
+      }
+
+      uint8_t digest[EVP_MAX_MD_SIZE];
+      size_t digest_len;
+      if (!ssl3_cert_verify_hash(ssl, digest, &digest_len,
+                                 signature_algorithm)) {
+        goto err;
+      }
+
+      sign_result = ssl_private_key_success;
+
+      EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new(ssl->cert->privatekey, NULL);
+      if (pctx == NULL ||
+          !EVP_PKEY_sign_init(pctx) ||
+          !EVP_PKEY_sign(pctx, ptr, &sig_len, digest, digest_len)) {
+        EVP_PKEY_CTX_free(pctx);
+        sign_result = ssl_private_key_failure;
+        goto err;
+      }
+      EVP_PKEY_CTX_free(pctx);
+    } else {
+      sign_result = ssl_private_key_sign(
+          ssl, ptr, &sig_len, max_sig_len, signature_algorithm,
+          (const uint8_t *)ssl->s3->handshake_buffer->data,
+          ssl->s3->handshake_buffer->length);
     }
 
     /* The handshake buffer is no longer necessary. */
     ssl3_free_handshake_buffer(ssl);
-
-    /* Sign the digest. */
-    sign_result = ssl_private_key_sign(ssl, ptr, &sig_len, max_sig_len,
-                                       signature_algorithm, digest, digest_len);
   } else {
     assert(ssl->state == SSL3_ST_CW_CERT_VRFY_B);
     sign_result =