Create the SSLKeyShare object in TLS 1.2 client ECDHE slightly later
We call Accept as a TLS 1.2 client and a TLS 1.3 server. In the latter,
we create an SSLKeyShare object, Accept, and immediately destroy it. In
the former, we create the SSLKeyShare object a couple steps before
actually using it.
It's equivalent to create the object just before Accept, so switch to
that. This change means that hs->key_shares now only ever contains
objects in between Offer and Finish. Or, in KEM terms, it only ever
contains KEM private keys. (SSLKeyShare objects are currently a little
confused about what kind of state they contain.)
Change-Id: Idec62ac298785f784485bc9065f7647034d2a607
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57605
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 601aec1..64fd2f2 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -1117,7 +1117,6 @@
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return ssl_hs_error;
}
- hs->new_session->group_id = group_id;
// Ensure the group is consistent with preferences.
if (!tls1_check_group_id(hs, group_id)) {
@@ -1126,10 +1125,9 @@
return ssl_hs_error;
}
- // Initialize ECDH and save the peer public key for later.
- hs->key_shares[0] = SSLKeyShare::Create(group_id);
- if (!hs->key_shares[0] ||
- !hs->peer_key.CopyFrom(point)) {
+ // Save the group and peer public key for later.
+ hs->new_session->group_id = group_id;
+ if (!hs->peer_key.CopyFrom(point)) {
return ssl_hs_error;
}
} else if (!(alg_k & SSL_kPSK)) {
@@ -1465,15 +1463,16 @@
return ssl_hs_error;
}
} else if (alg_k & SSL_kECDHE) {
- // Generate a keypair and serialize the public half.
CBB child;
if (!CBB_add_u8_length_prefixed(&body, &child)) {
return ssl_hs_error;
}
- // Compute the premaster.
+ // Generate the premaster secret.
+ bssl::UniquePtr<SSLKeyShare> key_share =
+ SSLKeyShare::Create(hs->new_session->group_id);
uint8_t alert = SSL_AD_DECODE_ERROR;
- if (!hs->key_shares[0]->Accept(&child, &pms, &alert, hs->peer_key)) {
+ if (!key_share || !key_share->Accept(&child, &pms, &alert, hs->peer_key)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
@@ -1481,9 +1480,7 @@
return ssl_hs_error;
}
- // The key exchange state may now be discarded.
- hs->key_shares[0].reset();
- hs->key_shares[1].reset();
+ // The peer key can now be discarded.
hs->peer_key.Reset();
} else if (alg_k & SSL_kPSK) {
// For plain PSK, other_secret is a block of 0s with the same length as
