Skip to content

Commit

Permalink
Move TLS 1.2 key exchange fields to SSL_HANDSHAKE.
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@davidben @agl
davidben authored and agl committed Oct 9, 2016
1 parent 43612b6 commit a4c8ff0
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 67 deletions.
12 changes: 0 additions & 12 deletions include/openssl/ssl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 14 additions & 28 deletions ssl/handshake_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -1248,32 +1243,22 @@ 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;
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
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);
Expand Down Expand Up @@ -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;
}
Expand All @@ -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. */
Expand Down
42 changes: 19 additions & 23 deletions ssl/handshake_server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -1581,15 +1577,15 @@ 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;
goto f_err;
}

/* 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. */
Expand Down
11 changes: 10 additions & 1 deletion ssl/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions ssl/s3_both.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions ssl/s3_lib.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit a4c8ff0

Please sign in to comment.