Hand back in-progress handshakes after a session resumption.
And since there are now 3 different points in the state machine where
a handback can occur, introduce an enum to describe them.
Change-Id: I41866214c39d27d1bbd965d28eb122c0e1f9902a
Reviewed-on: https://boringssl-review.googlesource.com/28344
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/handoff.cc b/ssl/handoff.cc
index bacb6fd..c4acc21 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -100,12 +100,23 @@
}
bool SSL_serialize_handback(const SSL *ssl, CBB *out) {
- if (!ssl->server ||
- (ssl->s3->hs->state != state12_finish_server_handshake &&
- ssl->s3->hs->state != state12_read_client_certificate) ||
- ssl->method->is_dtls || ssl->version < TLS1_VERSION) {
+ if (!ssl->server || ssl->method->is_dtls || ssl->version < TLS1_VERSION) {
return false;
}
+ handback_t type;
+ switch (ssl->s3->hs->state) {
+ case state12_read_change_cipher_spec:
+ type = handback_after_session_resumption;
+ break;
+ case state12_read_client_certificate:
+ type = handback_after_ecdhe;
+ break;
+ case state12_finish_server_handshake:
+ type = handback_after_handshake;
+ break;
+ default:
+ return false;
+ }
const SSL3_STATE *const s3 = ssl->s3;
size_t hostname_len = 0;
@@ -113,19 +124,28 @@
hostname_len = strlen(s3->hostname.get());
}
- size_t iv_len = 0;
- const uint8_t *read_iv = nullptr, *write_iv = nullptr;
Span<const uint8_t> transcript;
- if (ssl->s3->hs->state == state12_finish_server_handshake) {
- if (ssl->version == TLS1_VERSION &&
- SSL_CIPHER_is_block_cipher(s3->aead_read_ctx->cipher()) &&
- (!s3->aead_read_ctx->GetIV(&read_iv, &iv_len) ||
- !s3->aead_write_ctx->GetIV(&write_iv, &iv_len))) {
- return false;
- }
- } else {
+ if (type == handback_after_ecdhe ||
+ type == handback_after_session_resumption) {
transcript = s3->hs->transcript.buffer();
}
+ size_t write_iv_len = 0;
+ const uint8_t *write_iv = nullptr;
+ if ((type == handback_after_session_resumption ||
+ type == handback_after_handshake) &&
+ ssl->version == TLS1_VERSION &&
+ SSL_CIPHER_is_block_cipher(s3->aead_write_ctx->cipher()) &&
+ !s3->aead_write_ctx->GetIV(&write_iv, &write_iv_len)) {
+ return false;
+ }
+ size_t read_iv_len = 0;
+ const uint8_t *read_iv = nullptr;
+ if (type == handback_after_handshake &&
+ ssl->version == TLS1_VERSION &&
+ SSL_CIPHER_is_block_cipher(s3->aead_read_ctx->cipher()) &&
+ !s3->aead_read_ctx->GetIV(&read_iv, &read_iv_len)) {
+ return false;
+ }
// TODO(mab): make sure everything is serialized.
CBB seq, key_share;
@@ -133,6 +153,7 @@
s3->session_reused ? ssl->session : s3->hs->new_session.get();
if (!CBB_add_asn1(out, &seq, CBS_ASN1_SEQUENCE) ||
!CBB_add_asn1_uint64(&seq, kHandbackVersion) ||
+ !CBB_add_asn1_uint64(&seq, type) ||
!CBB_add_asn1_octet_string(&seq, s3->read_sequence,
sizeof(s3->read_sequence)) ||
!CBB_add_asn1_octet_string(&seq, s3->write_sequence,
@@ -141,8 +162,8 @@
sizeof(s3->server_random)) ||
!CBB_add_asn1_octet_string(&seq, s3->client_random,
sizeof(s3->client_random)) ||
- !CBB_add_asn1_octet_string(&seq, read_iv, iv_len) ||
- !CBB_add_asn1_octet_string(&seq, write_iv, iv_len) ||
+ !CBB_add_asn1_octet_string(&seq, read_iv, read_iv_len) ||
+ !CBB_add_asn1_octet_string(&seq, write_iv, write_iv_len) ||
!CBB_add_asn1_bool(&seq, s3->session_reused) ||
!CBB_add_asn1_bool(&seq, s3->tlsext_channel_id_valid) ||
!ssl_session_serialize(session, &seq) ||
@@ -166,7 +187,7 @@
!CBB_add_asn1(&seq, &key_share, CBS_ASN1_SEQUENCE)) {
return false;
}
- if (ssl->s3->hs->state == state12_read_client_certificate &&
+ if (type == handback_after_ecdhe &&
!s3->hs->key_share->Serialize(&key_share)) {
return false;
}
@@ -180,7 +201,7 @@
}
SSL3_STATE *const s3 = ssl->s3;
- uint64_t handback_version, negotiated_token_binding_param, cipher;
+ uint64_t handback_version, negotiated_token_binding_param, cipher, type;
CBS seq, read_seq, write_seq, server_rand, client_rand, read_iv, write_iv,
next_proto, alpn, hostname, channel_id, transcript, key_share;
@@ -191,7 +212,8 @@
CBS handback_cbs(handback);
if (!CBS_get_asn1(&handback_cbs, &seq, CBS_ASN1_SEQUENCE) ||
!CBS_get_asn1_uint64(&seq, &handback_version) ||
- handback_version != kHandbackVersion) {
+ handback_version != kHandbackVersion ||
+ !CBS_get_asn1_uint64(&seq, &type)) {
return false;
}
@@ -261,13 +283,26 @@
}
ssl->do_handshake = ssl_server_handshake;
ssl->server = true;
-
- s3->hs->state = CBS_len(&transcript) == 0 ? state12_finish_server_handshake
- : state12_read_client_certificate;
- s3->session_reused = session_reused;
- if (s3->hs->state == state12_read_client_certificate && session_reused) {
- return false;
+ switch (type) {
+ case handback_after_session_resumption:
+ ssl->s3->hs->state = state12_read_change_cipher_spec;
+ if (!session_reused) {
+ return false;
+ }
+ break;
+ case handback_after_ecdhe:
+ ssl->s3->hs->state = state12_read_client_certificate;
+ if (session_reused) {
+ return false;
+ }
+ break;
+ case handback_after_handshake:
+ ssl->s3->hs->state = state12_finish_server_handshake;
+ break;
+ default:
+ return false;
}
+ s3->session_reused = session_reused;
s3->tlsext_channel_id_valid = channel_id_valid;
s3->next_proto_negotiated.CopyFrom(next_proto);
s3->alpn_selected.CopyFrom(alpn);
@@ -293,31 +328,33 @@
s3->aead_write_ctx->SetVersionIfNullCipher(ssl->version);
s3->hs->cert_request = cert_request;
- if (s3->hs->state == state12_finish_server_handshake) {
- Array<uint8_t> key_block;
- if (!tls1_configure_aead(ssl, evp_aead_open, &key_block, session->cipher,
- read_iv) ||
- !tls1_configure_aead(ssl, evp_aead_seal, &key_block, session->cipher,
- write_iv)) {
- return false;
- }
-
- if (!CBS_copy_bytes(&read_seq, s3->read_sequence,
- sizeof(s3->read_sequence)) ||
- !CBS_copy_bytes(&write_seq, s3->write_sequence,
- sizeof(s3->write_sequence))) {
- return false;
- }
- } else {
- if (!s3->hs->transcript.Init() ||
- !s3->hs->transcript.InitHash(ssl_protocol_version(ssl),
- s3->hs->new_cipher) ||
- !s3->hs->transcript.Update(transcript)) {
- return false;
- }
- if ((s3->hs->key_share = SSLKeyShare::Create(&key_share)) == nullptr) {
- 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) ||
+ !CBS_copy_bytes(&write_seq, s3->write_sequence,
+ sizeof(s3->write_sequence)))) {
+ return false;
+ }
+ if (type == handback_after_handshake &&
+ (!tls1_configure_aead(ssl, evp_aead_open, &key_block, session->cipher,
+ read_iv) ||
+ !CBS_copy_bytes(&read_seq, s3->read_sequence,
+ sizeof(s3->read_sequence)))) {
+ return false;
+ }
+ if ((type == handback_after_ecdhe ||
+ type == handback_after_session_resumption) &&
+ (!s3->hs->transcript.Init() ||
+ !s3->hs->transcript.InitHash(ssl_protocol_version(ssl),
+ s3->hs->new_cipher) ||
+ !s3->hs->transcript.Update(transcript))) {
+ return false;
+ }
+ if (type == handback_after_ecdhe &&
+ (s3->hs->key_share = SSLKeyShare::Create(&key_share)) == nullptr) {
+ return false;
}
return CBS_len(&seq) == 0;
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 3c06b33..02657f3 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -1363,6 +1363,9 @@
}
static enum ssl_hs_wait_t do_read_change_cipher_spec(SSL_HANDSHAKE *hs) {
+ if (hs->handback && hs->ssl->session != NULL) {
+ return ssl_hs_handback;
+ }
hs->state = state12_process_change_cipher_spec;
return ssl_hs_read_change_cipher_spec;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index f8214e8..9ca75b3 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1377,6 +1377,14 @@
state12_done,
};
+// 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,
+};
+
struct SSL_HANDSHAKE {
explicit SSL_HANDSHAKE(SSL *ssl);
~SSL_HANDSHAKE();