Use InplaceVector for the various handshake derivations
While I'm here, remove a TODO about DTLS 1.3. DTLS 1.3 has opted to
switch to the TLS 1.3 transcript format, instead of the weird DTLS 1.2
one, so that code should work just fine.
This does mean we store each field's length separately. On the flip
side, the lengths are now single bytes and the old shared length was was
a size_t. Either way, the SSL_HANDSHAKE structure is released after the
handshake, so memory usage is less of a premium.
Change-Id: I43530d57b11f78852be478b9051d30a4657cdc4c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71967
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/ssl/handoff.cc b/ssl/handoff.cc
index e4e5d28..6f5c7e2 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -423,16 +423,16 @@
} else {
return false;
}
- if (!CBB_add_asn1_octet_string(&seq, hs->client_traffic_secret_0().data(),
- hs->client_traffic_secret_0().size()) ||
- !CBB_add_asn1_octet_string(&seq, hs->server_traffic_secret_0().data(),
- hs->server_traffic_secret_0().size()) ||
- !CBB_add_asn1_octet_string(&seq, hs->client_handshake_secret().data(),
- hs->client_handshake_secret().size()) ||
- !CBB_add_asn1_octet_string(&seq, hs->server_handshake_secret().data(),
- hs->server_handshake_secret().size()) ||
- !CBB_add_asn1_octet_string(&seq, hs->secret().data(),
- hs->secret().size()) ||
+ if (!CBB_add_asn1_octet_string(&seq, hs->client_traffic_secret_0.data(),
+ hs->client_traffic_secret_0.size()) ||
+ !CBB_add_asn1_octet_string(&seq, hs->server_traffic_secret_0.data(),
+ hs->server_traffic_secret_0.size()) ||
+ !CBB_add_asn1_octet_string(&seq, hs->client_handshake_secret.data(),
+ hs->client_handshake_secret.size()) ||
+ !CBB_add_asn1_octet_string(&seq, hs->server_handshake_secret.data(),
+ hs->server_handshake_secret.size()) ||
+ !CBB_add_asn1_octet_string(&seq, hs->secret.data(),
+ hs->secret.size()) ||
!CBB_add_asn1_octet_string(&seq, s3->exporter_secret.data(),
s3->exporter_secret.size()) ||
!CBB_add_asn1_bool(&seq, s3->used_hello_retry_request) ||
@@ -443,8 +443,8 @@
return false;
}
if (early_data == early_data_accepted &&
- !CBB_add_asn1_octet_string(&seq, hs->early_traffic_secret().data(),
- hs->early_traffic_secret().size())) {
+ !CBB_add_asn1_octet_string(&seq, hs->early_traffic_secret.data(),
+ hs->early_traffic_secret.size())) {
return false;
}
@@ -698,18 +698,17 @@
return false;
}
if (type == handback_tls13) {
- hs->ResizeSecrets(hs->transcript.DigestLen());
- if (!CopyExact(hs->client_traffic_secret_0(), &client_traffic_secret_0) ||
- !CopyExact(hs->server_traffic_secret_0(), &server_traffic_secret_0) ||
- !CopyExact(hs->client_handshake_secret(), &client_handshake_secret) ||
- !CopyExact(hs->server_handshake_secret(), &server_handshake_secret) ||
- !CopyExact(hs->secret(), &secret) ||
+ if (!hs->client_traffic_secret_0.TryCopyFrom(client_traffic_secret_0) ||
+ !hs->server_traffic_secret_0.TryCopyFrom(server_traffic_secret_0) ||
+ !hs->client_handshake_secret.TryCopyFrom(client_handshake_secret) ||
+ !hs->server_handshake_secret.TryCopyFrom(server_handshake_secret) ||
+ !hs->secret.TryCopyFrom(secret) ||
!s3->exporter_secret.TryCopyFrom(exporter_secret)) {
return false;
}
if (s3->early_data_accepted &&
- !CopyExact(hs->early_traffic_secret(), &early_traffic_secret)) {
+ !hs->early_traffic_secret.TryCopyFrom(early_traffic_secret)) {
return false;
}
}
@@ -741,7 +740,7 @@
// immediately after processing handback.
if (!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
hs->new_session.get(),
- hs->server_traffic_secret_0())) {
+ hs->server_traffic_secret_0)) {
return false;
}
break;
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index 6fe37cc..7195d66 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -164,13 +164,6 @@
ssl->ctx->x509_method->hs_flush_cached_ca_names(this);
}
-void SSL_HANDSHAKE::ResizeSecrets(size_t hash_len) {
- if (hash_len > SSL_MAX_MD_SIZE) {
- abort();
- }
- hash_len_ = hash_len;
-}
-
bool SSL_HANDSHAKE::GetClientHello(SSLMessage *out_msg,
SSL_CLIENT_HELLO *out_client_hello) {
if (!ech_client_hello_buf.empty()) {
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 3774787..44ef210 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -614,7 +614,7 @@
!tls13_derive_early_secret(hs) ||
!tls13_set_traffic_key(hs->ssl, ssl_encryption_early_data, evp_aead_seal,
hs->early_session.get(),
- hs->early_traffic_secret())) {
+ hs->early_traffic_secret)) {
return ssl_hs_error;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index 5e5d3fd..0fa35bd 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2107,18 +2107,13 @@
// |SSL_OP_NO_*| and |SSL_CTX_set_max_proto_version| APIs.
uint16_t max_version = 0;
- private:
- size_t hash_len_ = 0;
- uint8_t secret_[SSL_MAX_MD_SIZE] = {0};
- uint8_t early_traffic_secret_[SSL_MAX_MD_SIZE] = {0};
- uint8_t client_handshake_secret_[SSL_MAX_MD_SIZE] = {0};
- uint8_t server_handshake_secret_[SSL_MAX_MD_SIZE] = {0};
- uint8_t client_traffic_secret_0_[SSL_MAX_MD_SIZE] = {0};
- uint8_t server_traffic_secret_0_[SSL_MAX_MD_SIZE] = {0};
- uint8_t expected_client_finished_[SSL_MAX_MD_SIZE] = {0};
-
- public:
- void ResizeSecrets(size_t hash_len);
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> secret;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> early_traffic_secret;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> client_handshake_secret;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> server_handshake_secret;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> client_traffic_secret_0;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> server_traffic_secret_0;
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> expected_client_finished;
// GetClientHello, on the server, returns either the normal ClientHello
// message or the ClientHelloInner if it has been serialized to
@@ -2131,29 +2126,6 @@
// SSL_HANDSHAKE.
bool GetClientHello(SSLMessage *out_msg, SSL_CLIENT_HELLO *out_client_hello);
- Span<uint8_t> secret() { return MakeSpan(secret_, hash_len_); }
- Span<const uint8_t> secret() const {
- return MakeConstSpan(secret_, hash_len_);
- }
- Span<uint8_t> early_traffic_secret() {
- return MakeSpan(early_traffic_secret_, hash_len_);
- }
- Span<uint8_t> client_handshake_secret() {
- return MakeSpan(client_handshake_secret_, hash_len_);
- }
- Span<uint8_t> server_handshake_secret() {
- return MakeSpan(server_handshake_secret_, hash_len_);
- }
- Span<uint8_t> client_traffic_secret_0() {
- return MakeSpan(client_traffic_secret_0_, hash_len_);
- }
- Span<uint8_t> server_traffic_secret_0() {
- return MakeSpan(server_traffic_secret_0_, hash_len_);
- }
- Span<uint8_t> expected_client_finished() {
- return MakeSpan(expected_client_finished_, hash_len_);
- }
-
union {
// sent is a bitset where the bits correspond to elements of kExtensions
// in extensions.cc. Each bit is set if that extension was sent in a
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc
index f6d9547..4386a6f 100644
--- a/ssl/tls13_both.cc
+++ b/ssl/tls13_both.cc
@@ -367,7 +367,7 @@
Span<const uint8_t> verify_data;
if (use_saved_value) {
assert(ssl->server);
- verify_data = hs->expected_client_finished();
+ verify_data = hs->expected_client_finished;
} else {
size_t len;
if (!tls13_finished_mac(hs, verify_data_buf, &len, !ssl->server)) {
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 76f970c..c386241 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -93,7 +93,7 @@
assert(level == ssl_encryption_handshake);
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
hs->new_session.get(),
- hs->client_handshake_secret())) {
+ hs->client_handshake_secret)) {
return false;
}
}
@@ -529,14 +529,14 @@
if (!hs->in_early_data || ssl->quic_method != nullptr) {
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
hs->new_session.get(),
- hs->client_handshake_secret())) {
+ hs->client_handshake_secret)) {
return ssl_hs_error;
}
}
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
hs->new_session.get(),
- hs->server_handshake_secret())) {
+ hs->server_handshake_secret)) {
return ssl_hs_error;
}
@@ -959,10 +959,10 @@
// Derive the final keys and enable them.
if (!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
hs->new_session.get(),
- hs->client_traffic_secret_0()) ||
+ hs->client_traffic_secret_0) ||
!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_open,
hs->new_session.get(),
- hs->server_traffic_secret_0()) ||
+ hs->server_traffic_secret_0) ||
!tls13_derive_resumption_secret(hs)) {
return ssl_hs_error;
}
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc
index f601b1a..1e682f1 100644
--- a/ssl/tls13_enc.cc
+++ b/ssl/tls13_enc.cc
@@ -41,9 +41,8 @@
}
// Initialize the secret to the zero key.
- hs->ResizeSecrets(transcript->DigestLen());
- OPENSSL_memset(hs->secret().data(), 0, hs->secret().size());
-
+ hs->secret.clear();
+ hs->secret.Resize(transcript->DigestLen());
return true;
}
@@ -51,11 +50,11 @@
const SSLTranscript &transcript,
Span<const uint8_t> in) {
size_t len;
- if (!HKDF_extract(hs->secret().data(), &len, transcript.Digest(), in.data(),
- in.size(), hs->secret().data(), hs->secret().size())) {
+ if (!HKDF_extract(hs->secret.data(), &len, transcript.Digest(), in.data(),
+ in.size(), hs->secret.data(), hs->secret.size())) {
return false;
}
- assert(len == hs->secret().size());
+ assert(len == hs->secret.size());
return true;
}
@@ -147,32 +146,34 @@
unsigned derive_context_len;
return EVP_Digest(nullptr, 0, derive_context, &derive_context_len,
hs->transcript.Digest(), nullptr) &&
- hkdf_expand_label(hs->secret(), hs->transcript.Digest(), hs->secret(),
- label_to_span(kTLS13LabelDerived),
+ hkdf_expand_label(MakeSpan(hs->secret), hs->transcript.Digest(),
+ hs->secret, label_to_span(kTLS13LabelDerived),
MakeConstSpan(derive_context, derive_context_len),
SSL_is_dtls(hs->ssl)) &&
hkdf_extract_to_secret(hs, hs->transcript, in);
}
-// derive_secret_with_transcript derives a secret of length |out.size()| and
-// writes the result in |out| with the given label, the current base secret, and
-// the state of |transcript|. It returns true on success and false on error.
-static bool derive_secret_with_transcript(const SSL_HANDSHAKE *hs,
- Span<uint8_t> out,
- const SSLTranscript &transcript,
- Span<const char> label) {
+// derive_secret_with_transcript derives a secret of length
+// |transcript.DigestLen()| and writes the result in |out| with the given label,
+// the current base secret, and the state of |transcript|. It returns true on
+// success and false on error.
+static bool derive_secret_with_transcript(
+ const SSL_HANDSHAKE *hs, InplaceVector<uint8_t, SSL_MAX_MD_SIZE> *out,
+ const SSLTranscript &transcript, Span<const char> label) {
uint8_t context_hash[EVP_MAX_MD_SIZE];
size_t context_hash_len;
if (!transcript.GetHash(context_hash, &context_hash_len)) {
return false;
}
- return hkdf_expand_label(out, transcript.Digest(), hs->secret(), label,
- MakeConstSpan(context_hash, context_hash_len),
+ out->ResizeMaybeUninit(transcript.DigestLen());
+ return hkdf_expand_label(MakeSpan(*out), transcript.Digest(), hs->secret,
+ label, MakeConstSpan(context_hash, context_hash_len),
SSL_is_dtls(hs->ssl));
}
-static bool derive_secret(SSL_HANDSHAKE *hs, Span<uint8_t> out,
+static bool derive_secret(SSL_HANDSHAKE *hs,
+ InplaceVector<uint8_t, SSL_MAX_MD_SIZE> *out,
Span<const char> label) {
return derive_secret_with_transcript(hs, out, hs->transcript, label);
}
@@ -276,10 +277,10 @@
? hs->inner_transcript
: hs->transcript;
if (!derive_secret_with_transcript(
- hs, hs->early_traffic_secret(), transcript,
+ hs, &hs->early_traffic_secret, transcript,
label_to_span(kTLS13LabelClientEarlyTraffic)) ||
!ssl_log_secret(ssl, "CLIENT_EARLY_TRAFFIC_SECRET",
- hs->early_traffic_secret())) {
+ hs->early_traffic_secret)) {
return false;
}
return true;
@@ -287,14 +288,14 @@
bool tls13_derive_handshake_secrets(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- if (!derive_secret(hs, hs->client_handshake_secret(),
+ if (!derive_secret(hs, &hs->client_handshake_secret,
label_to_span(kTLS13LabelClientHandshakeTraffic)) ||
!ssl_log_secret(ssl, "CLIENT_HANDSHAKE_TRAFFIC_SECRET",
- hs->client_handshake_secret()) ||
- !derive_secret(hs, hs->server_handshake_secret(),
+ hs->client_handshake_secret) ||
+ !derive_secret(hs, &hs->server_handshake_secret,
label_to_span(kTLS13LabelServerHandshakeTraffic)) ||
!ssl_log_secret(ssl, "SERVER_HANDSHAKE_TRAFFIC_SECRET",
- hs->server_handshake_secret())) {
+ hs->server_handshake_secret)) {
return false;
}
@@ -303,18 +304,15 @@
bool tls13_derive_application_secrets(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- if (!derive_secret(hs, hs->client_traffic_secret_0(),
+ if (!derive_secret(hs, &hs->client_traffic_secret_0,
label_to_span(kTLS13LabelClientApplicationTraffic)) ||
!ssl_log_secret(ssl, "CLIENT_TRAFFIC_SECRET_0",
- hs->client_traffic_secret_0()) ||
- !derive_secret(hs, hs->server_traffic_secret_0(),
+ hs->client_traffic_secret_0) ||
+ !derive_secret(hs, &hs->server_traffic_secret_0,
label_to_span(kTLS13LabelServerApplicationTraffic)) ||
!ssl_log_secret(ssl, "SERVER_TRAFFIC_SECRET_0",
- hs->server_traffic_secret_0())) {
- return false;
- }
- ssl->s3->exporter_secret.ResizeMaybeUninit(hs->transcript.DigestLen());
- if (!derive_secret(hs, MakeSpan(ssl->s3->exporter_secret),
+ hs->server_traffic_secret_0) ||
+ !derive_secret(hs, &ssl->s3->exporter_secret,
label_to_span(kTLS13LabelExporter)) ||
!ssl_log_secret(ssl, "EXPORTER_SECRET", ssl->s3->exporter_secret)) {
return false;
@@ -342,8 +340,7 @@
static const char kTLS13LabelResumption[] = "res master";
bool tls13_derive_resumption_secret(SSL_HANDSHAKE *hs) {
- hs->new_session->secret.ResizeMaybeUninit(hs->transcript.DigestLen());
- return derive_secret(hs, MakeSpan(hs->new_session->secret),
+ return derive_secret(hs, &hs->new_session->secret,
label_to_span(kTLS13LabelResumption));
}
@@ -372,7 +369,7 @@
bool tls13_finished_mac(SSL_HANDSHAKE *hs, uint8_t *out, size_t *out_len,
bool is_server) {
Span<const uint8_t> traffic_secret =
- is_server ? hs->server_handshake_secret() : hs->client_handshake_secret();
+ is_server ? hs->server_handshake_secret : hs->client_handshake_secret;
uint8_t context_hash[EVP_MAX_MD_SIZE];
size_t context_hash_len;
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 655141f..fcee108 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -852,7 +852,7 @@
if (!tls13_derive_handshake_secrets(hs) ||
!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
hs->new_session.get(),
- hs->server_handshake_secret())) {
+ hs->server_handshake_secret)) {
return ssl_hs_error;
}
@@ -953,7 +953,7 @@
!tls13_derive_application_secrets(hs) ||
!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
hs->new_session.get(),
- hs->server_traffic_secret_0())) {
+ hs->server_traffic_secret_0)) {
return ssl_hs_error;
}
@@ -978,28 +978,27 @@
}
size_t finished_len;
- if (!tls13_finished_mac(hs, hs->expected_client_finished().data(),
+ hs->expected_client_finished.Resize(hs->transcript.DigestLen());
+ if (!tls13_finished_mac(hs, hs->expected_client_finished.data(),
&finished_len, false /* client */)) {
return ssl_hs_error;
}
- if (finished_len != hs->expected_client_finished().size()) {
+ if (finished_len != hs->expected_client_finished.size()) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return ssl_hs_error;
}
// Feed the predicted Finished into the transcript. This allows us to derive
// the resumption secret early and send half-RTT tickets.
- //
- // TODO(davidben): This will need to be updated for DTLS 1.3.
assert(!SSL_is_dtls(hs->ssl));
- assert(hs->expected_client_finished().size() <= 0xff);
+ assert(hs->expected_client_finished.size() <= 0xff);
uint8_t header[4] = {
SSL3_MT_FINISHED, 0, 0,
- static_cast<uint8_t>(hs->expected_client_finished().size())};
+ static_cast<uint8_t>(hs->expected_client_finished.size())};
bool unused_sent_tickets;
if (!hs->transcript.Update(header) ||
- !hs->transcript.Update(hs->expected_client_finished()) ||
+ !hs->transcript.Update(hs->expected_client_finished) ||
!tls13_derive_resumption_secret(hs) ||
!add_new_session_tickets(hs, &unused_sent_tickets)) {
return ssl_hs_error;
@@ -1021,7 +1020,7 @@
if (ssl->s3->early_data_accepted) {
if (!tls13_set_traffic_key(ssl, ssl_encryption_early_data, evp_aead_open,
hs->new_session.get(),
- hs->early_traffic_secret())) {
+ hs->early_traffic_secret)) {
return ssl_hs_error;
}
hs->can_early_write = true;
@@ -1034,7 +1033,7 @@
if (!uses_end_of_early_data(ssl)) {
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
hs->new_session.get(),
- hs->client_handshake_secret())) {
+ hs->client_handshake_secret)) {
return ssl_hs_error;
}
hs->tls13_state = state13_process_end_of_early_data;
@@ -1070,7 +1069,7 @@
}
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
hs->new_session.get(),
- hs->client_handshake_secret())) {
+ hs->client_handshake_secret)) {
return ssl_hs_error;
}
}
@@ -1238,7 +1237,7 @@
// evp_aead_seal keys have already been switched.
!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_open,
hs->new_session.get(),
- hs->client_traffic_secret_0())) {
+ hs->client_traffic_secret_0)) {
return ssl_hs_error;
}
