Move TLS 1.2 key exchange fields to SSL_HANDSHAKE.

SSL_HANDSHAKE is dropped after the handshake, so I've removed the logic
around smaller sizes. It's much simpler when we can use CBS_stow and
CBB_finish without extra bounds-checking.

Change-Id: Idafaa5d69e171aed9a8759f3d44e52cb01c40f39
Reviewed-on: https://boringssl-review.googlesource.com/11567
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/ssl.h

@@ -4355,18 +4355,6 @@ typedef struct ssl3_state_st {
     /* peer_signature_algorithm is the signature algorithm used to authenticate
      * the peer, or zero if not applicable. */
     uint16_t peer_signature_algorithm;
-
-    /* ecdh_ctx is the current ECDH instance. */
-    SSL_ECDH_CTX ecdh_ctx;
-
-    /* peer_key is the peer's ECDH key. */
-    uint8_t *peer_key;
-    uint16_t peer_key_len;
-
-    /* server_params stores the ServerKeyExchange parameters to be signed while
-     * the signature is being computed. */
-    uint8_t *server_params;
-    uint32_t server_params_len;
   } tmp;
 
   /* new_session is the new mutable session being established by the current

ssl/handshake_client.c

@@ -1213,18 +1213,13 @@ static int ssl3_get_server_key_exchange(SSL *ssl) {
       goto err;
     }
 
-    SSL_ECDH_CTX_init_for_dhe(&ssl->s3->tmp.ecdh_ctx, dh);
+    SSL_ECDH_CTX_init_for_dhe(&ssl->s3->hs->ecdh_ctx, dh);
     dh = NULL;
 
     /* Save the peer public key for later. */
-    size_t peer_key_len;
-    if (!CBS_stow(&dh_Ys, &ssl->s3->tmp.peer_key, &peer_key_len)) {
+    if (!CBS_stow(&dh_Ys, &ssl->s3->hs->peer_key, &ssl->s3->hs->peer_key_len)) {
       goto err;
     }
-    /* |dh_Ys| was initialized with CBS_get_u16_length_prefixed, so peer_key_len
-     * fits in a uint16_t. */
-    assert(sizeof(ssl->s3->tmp.peer_key_len) == 2 && peer_key_len <= 0xffff);
-    ssl->s3->tmp.peer_key_len = (uint16_t)peer_key_len;
   } else if (alg_k & SSL_kECDHE) {
     /* Parse the server parameters. */
     uint8_t group_type;
@@ -1248,17 +1243,12 @@ static int ssl3_get_server_key_exchange(SSL *ssl) {
     }
 
     /* Initialize ECDH and save the peer public key for later. */
-    size_t peer_key_len;
-    if (!SSL_ECDH_CTX_init(&ssl->s3->tmp.ecdh_ctx, group_id) ||
-        !CBS_stow(&point, &ssl->s3->tmp.peer_key, &peer_key_len)) {
+    if (!SSL_ECDH_CTX_init(&ssl->s3->hs->ecdh_ctx, group_id) ||
+        !CBS_stow(&point, &ssl->s3->hs->peer_key, &ssl->s3->hs->peer_key_len)) {
       goto err;
     }
-    /* |point| was initialized with CBS_get_u8_length_prefixed, so peer_key_len
-     * fits in a uint16_t. */
-    assert(sizeof(ssl->s3->tmp.peer_key_len) == 2 && peer_key_len <= 0xffff);
-    ssl->s3->tmp.peer_key_len = (uint16_t)peer_key_len;
   } else if (alg_k & SSL_kCECPQ1) {
-    SSL_ECDH_CTX_init_for_cecpq1(&ssl->s3->tmp.ecdh_ctx);
+    SSL_ECDH_CTX_init_for_cecpq1(&ssl->s3->hs->ecdh_ctx);
     CBS key;
     if (!CBS_get_u16_length_prefixed(&server_key_exchange, &key)) {
       al = SSL_AD_DECODE_ERROR;
@@ -1266,14 +1256,9 @@ static int ssl3_get_server_key_exchange(SSL *ssl) {
       goto f_err;
     }
 
-    size_t peer_key_len;
-    if (!CBS_stow(&key, &ssl->s3->tmp.peer_key, &peer_key_len)) {
+    if (!CBS_stow(&key, &ssl->s3->hs->peer_key, &ssl->s3->hs->peer_key_len)) {
       goto err;
     }
-    /* |key| was initialized with CBS_get_u16_length_prefixed, so peer_key_len
-     * fits in a uint16_t. */
-    assert(sizeof(ssl->s3->tmp.peer_key_len) == 2 && peer_key_len <= 0xffff);
-    ssl->s3->tmp.peer_key_len = (uint16_t)peer_key_len;
   } else if (!(alg_k & SSL_kPSK)) {
     al = SSL_AD_UNEXPECTED_MESSAGE;
     OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
@@ -1620,15 +1605,15 @@ static int ssl3_send_client_key_exchange(SSL *ssl) {
   } else if (alg_k & (SSL_kECDHE|SSL_kDHE|SSL_kCECPQ1)) {
     /* Generate a keypair and serialize the public half. */
     CBB child;
-    if (!SSL_ECDH_CTX_add_key(&ssl->s3->tmp.ecdh_ctx, &body, &child)) {
+    if (!SSL_ECDH_CTX_add_key(&ssl->s3->hs->ecdh_ctx, &body, &child)) {
       goto err;
     }
 
     /* Compute the premaster. */
     uint8_t alert;
-    if (!SSL_ECDH_CTX_accept(&ssl->s3->tmp.ecdh_ctx, &child, &pms, &pms_len,
-                             &alert, ssl->s3->tmp.peer_key,
-                             ssl->s3->tmp.peer_key_len)) {
+    if (!SSL_ECDH_CTX_accept(&ssl->s3->hs->ecdh_ctx, &child, &pms, &pms_len,
+                             &alert, ssl->s3->hs->peer_key,
+                             ssl->s3->hs->peer_key_len)) {
       ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
       goto err;
     }
@@ -1637,9 +1622,10 @@ static int ssl3_send_client_key_exchange(SSL *ssl) {
     }
 
     /* The key exchange state may now be discarded. */
-    SSL_ECDH_CTX_cleanup(&ssl->s3->tmp.ecdh_ctx);
-    OPENSSL_free(ssl->s3->tmp.peer_key);
-    ssl->s3->tmp.peer_key = NULL;
+    SSL_ECDH_CTX_cleanup(&ssl->s3->hs->ecdh_ctx);
+    OPENSSL_free(ssl->s3->hs->peer_key);
+    ssl->s3->hs->peer_key = NULL;
+    ssl->s3->hs->peer_key_len = 0;
   } else if (alg_k & SSL_kPSK) {
     /* For plain PSK, other_secret is a block of 0s with the same length as
      * the pre-shared key. */

ssl/handshake_server.c

@@ -1050,13 +1050,13 @@ static int ssl3_send_server_key_exchange(SSL *ssl) {
         goto err;
       }
 
-      SSL_ECDH_CTX_init_for_dhe(&ssl->s3->tmp.ecdh_ctx, dh);
+      SSL_ECDH_CTX_init_for_dhe(&ssl->s3->hs->ecdh_ctx, dh);
       if (!CBB_add_u16_length_prefixed(&cbb, &child) ||
           !BN_bn2cbb_padded(&child, BN_num_bytes(params->p), params->p) ||
           !CBB_add_u16_length_prefixed(&cbb, &child) ||
           !BN_bn2cbb_padded(&child, BN_num_bytes(params->g), params->g) ||
           !CBB_add_u16_length_prefixed(&cbb, &child) ||
-          !SSL_ECDH_CTX_offer(&ssl->s3->tmp.ecdh_ctx, &child)) {
+          !SSL_ECDH_CTX_offer(&ssl->s3->hs->ecdh_ctx, &child)) {
         goto err;
       }
     } else if (alg_k & SSL_kECDHE) {
@@ -1070,39 +1070,35 @@ static int ssl3_send_server_key_exchange(SSL *ssl) {
       ssl->s3->new_session->key_exchange_info = group_id;
 
       /* Set up ECDH, generate a key, and emit the public half. */
-      if (!SSL_ECDH_CTX_init(&ssl->s3->tmp.ecdh_ctx, group_id) ||
+      if (!SSL_ECDH_CTX_init(&ssl->s3->hs->ecdh_ctx, group_id) ||
           !CBB_add_u8(&cbb, NAMED_CURVE_TYPE) ||
           !CBB_add_u16(&cbb, group_id) ||
           !CBB_add_u8_length_prefixed(&cbb, &child) ||
-          !SSL_ECDH_CTX_offer(&ssl->s3->tmp.ecdh_ctx, &child)) {
+          !SSL_ECDH_CTX_offer(&ssl->s3->hs->ecdh_ctx, &child)) {
         goto err;
       }
     } else if (alg_k & SSL_kCECPQ1) {
-      SSL_ECDH_CTX_init_for_cecpq1(&ssl->s3->tmp.ecdh_ctx);
+      SSL_ECDH_CTX_init_for_cecpq1(&ssl->s3->hs->ecdh_ctx);
       if (!CBB_add_u16_length_prefixed(&cbb, &child) ||
-          !SSL_ECDH_CTX_offer(&ssl->s3->tmp.ecdh_ctx, &child)) {
+          !SSL_ECDH_CTX_offer(&ssl->s3->hs->ecdh_ctx, &child)) {
         goto err;
       }
     } else {
       assert(alg_k & SSL_kPSK);
     }
 
-    size_t len;
-    if (!CBB_finish(&cbb, &ssl->s3->tmp.server_params, &len) ||
-        len > 0xffffffffu) {
-      OPENSSL_free(ssl->s3->tmp.server_params);
-      ssl->s3->tmp.server_params = NULL;
+    if (!CBB_finish(&cbb, &ssl->s3->hs->server_params,
+                    &ssl->s3->hs->server_params_len)) {
       goto err;
     }
-    ssl->s3->tmp.server_params_len = (uint32_t)len;
   }
 
   /* Assemble the message. */
   CBB body;
   if (!ssl->method->init_message(ssl, &cbb, &body,
                                  SSL3_MT_SERVER_KEY_EXCHANGE) ||
-      !CBB_add_bytes(&body, ssl->s3->tmp.server_params,
-                     ssl->s3->tmp.server_params_len)) {
+      !CBB_add_bytes(&body, ssl->s3->hs->server_params,
+                     ssl->s3->hs->server_params_len)) {
     goto err;
   }
 
@@ -1141,11 +1137,11 @@ static int ssl3_send_server_key_exchange(SSL *ssl) {
       uint8_t *transcript_data;
       size_t transcript_len;
       if (!CBB_init(&transcript,
-                    2*SSL3_RANDOM_SIZE + ssl->s3->tmp.server_params_len) ||
+                    2*SSL3_RANDOM_SIZE + ssl->s3->hs->server_params_len) ||
           !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, ssl->s3->tmp.server_params,
-                         ssl->s3->tmp.server_params_len) ||
+          !CBB_add_bytes(&transcript, ssl->s3->hs->server_params,
+                         ssl->s3->hs->server_params_len) ||
           !CBB_finish(&transcript, &transcript_data, &transcript_len)) {
         CBB_cleanup(&transcript);
         OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
@@ -1181,9 +1177,9 @@ static int ssl3_send_server_key_exchange(SSL *ssl) {
     goto err;
   }
 
-  OPENSSL_free(ssl->s3->tmp.server_params);
-  ssl->s3->tmp.server_params = NULL;
-  ssl->s3->tmp.server_params_len = 0;
+  OPENSSL_free(ssl->s3->hs->server_params);
+  ssl->s3->hs->server_params = NULL;
+  ssl->s3->hs->server_params_len = 0;
 
   ssl->state = SSL3_ST_SW_KEY_EXCH_C;
   return ssl->method->write_message(ssl);
@@ -1571,7 +1567,7 @@ static int ssl3_get_client_key_exchange(SSL *ssl) {
   } else if (alg_k & (SSL_kECDHE|SSL_kDHE|SSL_kCECPQ1)) {
     /* Parse the ClientKeyExchange. */
     CBS peer_key;
-    if (!SSL_ECDH_CTX_get_key(&ssl->s3->tmp.ecdh_ctx, &client_key_exchange,
+    if (!SSL_ECDH_CTX_get_key(&ssl->s3->hs->ecdh_ctx, &client_key_exchange,
                               &peer_key) ||
         CBS_len(&client_key_exchange) != 0) {
       al = SSL_AD_DECODE_ERROR;
@@ -1581,7 +1577,7 @@ static int ssl3_get_client_key_exchange(SSL *ssl) {
 
     /* Compute the premaster. */
     uint8_t alert;
-    if (!SSL_ECDH_CTX_finish(&ssl->s3->tmp.ecdh_ctx, &premaster_secret,
+    if (!SSL_ECDH_CTX_finish(&ssl->s3->hs->ecdh_ctx, &premaster_secret,
                              &premaster_secret_len, &alert, CBS_data(&peer_key),
                              CBS_len(&peer_key))) {
       al = alert;
@@ -1589,7 +1585,7 @@ static int ssl3_get_client_key_exchange(SSL *ssl) {
     }
 
     /* The key exchange state may now be discarded. */
-    SSL_ECDH_CTX_cleanup(&ssl->s3->tmp.ecdh_ctx);
+    SSL_ECDH_CTX_cleanup(&ssl->s3->hs->ecdh_ctx);
   } else if (alg_k & SSL_kPSK) {
     /* For plain PSK, other_secret is a block of 0s with the same length as the
      * pre-shared key. */

ssl/internal.h

@@ -919,7 +919,7 @@ struct ssl_handshake_st {
     uint16_t received;
   } custom_extensions;
 
-  /* ecdh_ctx is the active client ECDH offer in TLS 1.3. */
+  /* ecdh_ctx is the current ECDH instance. */
   SSL_ECDH_CTX ecdh_ctx;
 
   /* retry_group is the group ID selected by the server in HelloRetryRequest in
@@ -949,6 +949,15 @@ struct ssl_handshake_st {
   uint16_t *peer_supported_group_list;
   size_t peer_supported_group_list_len;
 
+  /* peer_key is the peer's ECDH key for a TLS 1.2 client. */
+  uint8_t *peer_key;
+  size_t peer_key_len;
+
+  /* server_params, in TLS 1.2, stores the ServerKeyExchange parameters to be
+   * signed while the signature is being computed. */
+  uint8_t *server_params;
+  size_t server_params_len;
+
   /* session_tickets_sent, in TLS 1.3, is the number of tickets the server has
    * sent. */
   uint8_t session_tickets_sent;

ssl/s3_both.c

@@ -150,6 +150,8 @@ void ssl_handshake_free(SSL_HANDSHAKE *hs) {
   OPENSSL_cleanse(hs->secret, sizeof(hs->secret));
   OPENSSL_cleanse(hs->traffic_secret_0, sizeof(hs->traffic_secret_0));
   SSL_ECDH_CTX_cleanup(&hs->ecdh_ctx);
+  OPENSSL_free(hs->peer_key);
+  OPENSSL_free(hs->server_params);
   OPENSSL_free(hs->key_share_bytes);
   OPENSSL_free(hs->public_key);
   OPENSSL_free(hs->peer_sigalgs);

ssl/s3_lib.c

@@ -203,9 +203,6 @@ void ssl3_free(SSL *ssl) {
   ssl3_cleanup_key_block(ssl);
   ssl_read_buffer_clear(ssl);
   ssl_write_buffer_clear(ssl);
-  SSL_ECDH_CTX_cleanup(&ssl->s3->tmp.ecdh_ctx);
-  OPENSSL_free(ssl->s3->tmp.peer_key);
-  OPENSSL_free(ssl->s3->tmp.server_params);
 
   SSL_SESSION_free(ssl->s3->new_session);
   SSL_SESSION_free(ssl->s3->established_session);