Defer releasing early secrets to QUIC servers.

We want the QUIC/TLS interface to never release a read key without the
corresponding write key for ACKs. This is mostly done by shipping both keys
simultaneously, but 0-RTT is weird because it is ACKed by 1-RTT.

Note this means we actually release 0-RTT keys to the server *after* the 1-RTT
keys. This is kinda weird but more directly maintains our invariant.

(We may want to revisit the key configuring API in light of
https://github.com/quicwg/base-drafts/issues/3159 and
https://github.com/quicwg/base-drafts/issues/3173, but start with this more
local tweak.)

Bug: 303
Change-Id: I317fe6ae8150533738373c219f19d3034bb040ad
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38884
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Nick Harper <nharper@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 8fbf698..6211c56 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -4811,6 +4811,10 @@
         expect_read_secret = false;
       } else {
         expect_write_secret = false;
+        if (!HasSecrets(ssl_encryption_application)) {
+          ADD_FAILURE() << "early secrets installed without keys to ACK them";
+          return false;
+        }
       }
     }
 

ssl/tls13_server.cc

diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index d8115f5..c74d834 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -471,15 +471,6 @@
     return ssl_hs_error;
   }
 
-  // Note we defer releasing the early traffic secret to QUIC until after ECDHE
-  // is resolved. The early traffic secret should be derived before the key
-  // schedule incorporates ECDHE, but doing so may reject 0-RTT. To avoid
-  // confusing the caller, we split derivation and releasing the secret to QUIC.
-  if (ssl->s3->early_data_accepted &&
-      !tls13_set_early_secret_for_quic(hs)) {
-    return ssl_hs_error;
-  }
-
   ssl->method->next_message(ssl);
   hs->tls13_state = state_send_server_hello;
   return ssl_hs_ok;
@@ -737,6 +728,19 @@
   }
 
   if (ssl->s3->early_data_accepted) {
+    // 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.
+    //
+    // Second, 0-RTT data is acknowledged with 1-RTT keys. Both are derived as
+    // part of the ServerHello flight, but future TLS extensions may insert an
+    // asynchronous point in the middle of this flight. We defer releasing the
+    // 0-RTT keys to ensure the QUIC implementation never installs read keys
+    // without the write keys to send the corresponding ACKs.
+    if (!tls13_set_early_secret_for_quic(hs)) {
+      return ssl_hs_error;
+    }
+
     // If accepting 0-RTT, we send tickets half-RTT. This gets the tickets on
     // the wire sooner and also avoids triggering a write on |SSL_read| when
     // processing the client Finished. This requires computing the client