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;
 }