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>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index dde36f9..34eac9a 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4355,18 +4355,6 @@
/* 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
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 5bde567..a917e02 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -1213,18 +1213,13 @@
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 @@
}
/* 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 @@
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 @@
} 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 @@
}
/* 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. */
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 99df871..83e09d3 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1050,13 +1050,13 @@
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 @@
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 @@
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 @@
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 @@
} 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 @@
/* 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 @@
}
/* 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. */
diff --git a/ssl/internal.h b/ssl/internal.h
index 591e6ab..0243ac4 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -919,7 +919,7 @@
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 @@
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;
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 3ed3793..e0d97dc 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -150,6 +150,8 @@
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);
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 1515f94..69d3a9d 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -203,9 +203,6 @@
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);
