Clean up TLS 1.3 handback logic.
There's no need to treat the 1-RTT and 0-RTT handback flows differently.
This aligns the 1-RTT handback with the 0-RTT point. This consistently
installs the decryption keys in the state machine after handback rather
than while applying the handback.
Update-Note: This changes the serialization format for TLS 1.3 split
handshakes, which were only just added.
Change-Id: I0e109cb8d9ecd3c8658dfa26059cbe0139f82eed
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39988
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Matt Braithwaite <mab@google.com>
diff --git a/ssl/handoff.cc b/ssl/handoff.cc
index 3926939..a9da05e 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -258,16 +258,10 @@
type = handback_after_handshake;
break;
case state12_tls13:
- switch (hs->tls13_state) {
- case state13_send_half_rtt_ticket:
- type = handback_tls13_early_data;
- break;
- case state13_process_end_of_early_data:
- type = handback_tls13;
- break;
- default:
- return false;
+ if (hs->tls13_state != state13_send_half_rtt_ticket) {
+ return false;
}
+ type = handback_tls13;
break;
default:
return false;
@@ -303,7 +297,7 @@
// TODO(mab): make sure everything is serialized.
CBB seq, key_share;
const SSL_SESSION *session;
- if (type == handback_tls13 || type == handback_tls13_early_data) {
+ if (type == handback_tls13) {
session = hs->new_session.get();
} else {
session = s3->session_reused ? ssl->session.get() : hs->new_session.get();
@@ -348,7 +342,7 @@
!s3->hs->key_shares[0]->Serialize(&key_share)) {
return false;
}
- if (type == handback_tls13 || type == handback_tls13_early_data) {
+ if (type == handback_tls13) {
early_data_t early_data;
// Check early data invariants.
if (ssl->enable_early_data ==
@@ -389,7 +383,7 @@
!CBB_add_asn1_uint64(&seq, early_data)) {
return false;
}
- if (type == handback_tls13_early_data &&
+ if (early_data == early_data_accepted &&
!CBB_add_asn1_octet_string(&seq, hs->early_traffic_secret().data(),
hs->early_traffic_secret().size())) {
return false;
@@ -413,7 +407,7 @@
}
SSL3_STATE *const s3 = ssl->s3;
- uint64_t handback_version, negotiated_token_binding_param, cipher, type;
+ uint64_t handback_version, negotiated_token_binding_param, cipher, type_u64;
CBS seq, read_seq, write_seq, server_rand, client_rand, read_iv, write_iv,
next_proto, alpn, hostname, channel_id, transcript, key_share;
@@ -425,10 +419,12 @@
if (!CBS_get_asn1(&handback_cbs, &seq, CBS_ASN1_SEQUENCE) ||
!CBS_get_asn1_uint64(&seq, &handback_version) ||
handback_version != kHandbackVersion ||
- !CBS_get_asn1_uint64(&seq, &type)) {
+ !CBS_get_asn1_uint64(&seq, &type_u64) ||
+ type_u64 > handback_max_value) {
return false;
}
+ handback_t type = static_cast<handback_t>(type_u64);
if (!CBS_get_asn1(&seq, &read_seq, CBS_ASN1_OCTETSTRING) ||
CBS_len(&read_seq) != sizeof(s3->read_sequence) ||
!CBS_get_asn1(&seq, &write_seq, CBS_ASN1_OCTETSTRING) ||
@@ -450,8 +446,7 @@
s3->hs = ssl_handshake_new(ssl);
SSL_HANDSHAKE *const hs = s3->hs.get();
- if (!session_reused || type == handback_tls13 ||
- type == handback_tls13_early_data) {
+ if (!session_reused || type == handback_tls13) {
hs->new_session =
SSL_SESSION_parse(&seq, ssl->ctx->x509_method, ssl->ctx->pool);
session = hs->new_session.get();
@@ -487,7 +482,7 @@
}
CBS client_handshake_secret, server_handshake_secret, client_traffic_secret_0,
server_traffic_secret_0, secret, exporter_secret, early_traffic_secret;
- if (type == handback_tls13 || type == handback_tls13_early_data) {
+ if (type == handback_tls13) {
int used_hello_retry_request, accept_psk_mode;
uint64_t early_data, early_data_reason;
int64_t ticket_age_skew;
@@ -506,7 +501,8 @@
early_data > early_data_max_value) {
return false;
}
- if (type == handback_tls13_early_data &&
+ early_data_t early_data_type = static_cast<early_data_t>(early_data);
+ if (early_data_type == early_data_accepted &&
!CBS_get_asn1(&seq, &early_traffic_secret, CBS_ASN1_OCTETSTRING)) {
return false;
}
@@ -524,11 +520,6 @@
s3->skip_early_data = false;
s3->early_data_accepted = false;
hs->early_data_offered = false;
- early_data_t early_data_type = static_cast<early_data_t>(early_data);
- if ((type == handback_tls13_early_data) !=
- (early_data_type == early_data_accepted)) {
- return false;
- }
switch (early_data_type) {
case early_data_not_offered:
break;
@@ -581,10 +572,6 @@
break;
case handback_tls13:
hs->state = state12_tls13;
- hs->tls13_state = state13_read_client_certificate;
- break;
- case handback_tls13_early_data:
- hs->state = state12_tls13;
hs->tls13_state = state13_send_half_rtt_ticket;
break;
default:
@@ -622,7 +609,7 @@
!hs->transcript.Update(transcript))) {
return false;
}
- if (type == handback_tls13 || type == handback_tls13_early_data) {
+ 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) ||
@@ -635,32 +622,42 @@
}
s3->exporter_secret_len = CBS_len(&exporter_secret);
- if (type == handback_tls13_early_data &&
+ if (s3->early_data_accepted &&
!CopyExact(hs->early_traffic_secret(), &early_traffic_secret)) {
return false;
}
}
Array<uint8_t> key_block;
- if ((type == handback_after_session_resumption ||
- type == handback_after_handshake) &&
- !tls1_configure_aead(ssl, evp_aead_seal, &key_block, session->cipher,
- write_iv)) {
- return false;
- }
- if (type == handback_after_handshake &&
- !tls1_configure_aead(ssl, evp_aead_open, &key_block, session->cipher,
- read_iv)) {
- return false;
- }
- if (type == handback_tls13 || type == handback_tls13_early_data) {
- auto open_key = type == handback_tls13 ? hs->client_handshake_secret()
- : hs->early_traffic_secret();
- if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
- open_key) ||
- !tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
- hs->server_traffic_secret_0())) {
- return false;
- }
+ switch (type) {
+ case handback_after_session_resumption:
+ // The write keys are installed after server Finished, but the client
+ // keys must wait for ChangeCipherSpec.
+ if (!tls1_configure_aead(ssl, evp_aead_seal, &key_block, session->cipher,
+ write_iv)) {
+ return false;
+ }
+ break;
+ case handback_after_ecdhe:
+ // The premaster secret is not yet computed, so install no keys.
+ break;
+ case handback_after_handshake:
+ // The handshake is complete, so both keys are installed.
+ if (!tls1_configure_aead(ssl, evp_aead_seal, &key_block, session->cipher,
+ write_iv) ||
+ !tls1_configure_aead(ssl, evp_aead_open, &key_block, session->cipher,
+ read_iv)) {
+ return false;
+ }
+ break;
+ case handback_tls13:
+ // After server Finished, the application write keys are installed, but
+ // none of the read keys. The read keys are installed in the state machine
+ // immediately after processing handback.
+ if (!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
+ hs->server_traffic_secret_0())) {
+ return false;
+ }
+ break;
}
if (!CopyExact({s3->read_sequence, sizeof(s3->read_sequence)}, &read_seq) ||
!CopyExact({s3->write_sequence, sizeof(s3->write_sequence)},
diff --git a/ssl/internal.h b/ssl/internal.h
index 32303cb..4a93e15 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1499,11 +1499,11 @@
// handback_t lists the points in the state machine where a handback can occur.
// These are the different points at which key material is no longer needed.
enum handback_t {
- handback_after_session_resumption,
- handback_after_ecdhe,
- handback_after_handshake,
- handback_tls13,
- handback_tls13_early_data,
+ handback_after_session_resumption = 0,
+ handback_after_ecdhe = 1,
+ handback_after_handshake = 2,
+ handback_tls13 = 3,
+ handback_max_value = handback_tls13,
};
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index baf2a04..fda0bca 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -705,17 +705,13 @@
}
hs->tls13_state = state13_send_half_rtt_ticket;
- return ssl_hs_ok;
+ return hs->handback ? ssl_hs_handback : ssl_hs_ok;
}
static enum ssl_hs_wait_t do_send_half_rtt_ticket(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
if (ssl->s3->early_data_accepted) {
- if (hs->handback) {
- return ssl_hs_handback;
- }
-
// We defer releasing the early traffic secret to QUIC to this point. First,
// the early traffic secret is derived before ECDHE, but ECDHE may later
// reject 0-RTT. We only release the secret after 0-RTT is fully resolved.
@@ -828,9 +824,6 @@
hs->client_handshake_secret())) {
return ssl_hs_error;
}
- if (hs->handback) {
- return ssl_hs_handback;
- }
hs->tls13_state = state13_read_client_certificate;
return ssl_hs_ok;
}