Revise QUIC encryption secret APIs.
The original API separated when keys were exported to QUIC from when
they were "active". This means the obligations on QUIC are unclear. For
instance we added SSL_quic_read_level and SSL_quic_write_level to
capture when keys were active, yet QUICHE never used them anyway. It
would be better to defer releasing keys to when they are needed.
In particular, we should align our API with the guidelines in
https://github.com/quicwg/base-drafts/issues/3173.
This means we need separate read and write callbacks, which
unfortunately means the invariants around ACKs must now be covered in
prose.
We'll likely remove SSL_quic_read_level and SSL_quic_write_level in a
later CL as QUIC has no need to know this anymore. (It should simply
feed handshake data to BoringSSL as it sees it and, if we reject it,
report a suitably error.) The notion of a "current" encryption level is
a little odd given the interaction between 0-RTT and
ServerHello..Finished ACKs.
Update-Note: This is an incompatible change to SSL_QUIC_METHOD.
BORINGSSL_API_VERSION can be used to distinguish the two revisions.
Bug: 303
Change-Id: I6c9dca1ae156d26a80c366a4ba969dcb04e65349
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40127
Reviewed-by: Nick Harper <nharper@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 9418185..7228471 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -75,20 +75,22 @@
// We do not currently implement DTLS 1.3 and, in QUIC, the caller handles
// 0-RTT data, so we can skip installing 0-RTT keys and act as if there is one
// write level. If we implement DTLS 1.3, we'll need to model this better.
- if (level == ssl_encryption_initial) {
- bssl::UniquePtr<SSLAEADContext> null_ctx =
- SSLAEADContext::CreateNullCipher(SSL_is_dtls(ssl));
- if (!null_ctx || !ssl->method->set_write_state(ssl, ssl_encryption_initial,
- std::move(null_ctx))) {
- return false;
- }
- ssl->s3->aead_write_ctx->SetVersionIfNullCipher(ssl->version);
- } else {
- 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())) {
- return false;
+ if (ssl->quic_method == nullptr) {
+ if (level == ssl_encryption_initial) {
+ bssl::UniquePtr<SSLAEADContext> null_ctx =
+ SSLAEADContext::CreateNullCipher(SSL_is_dtls(ssl));
+ if (!null_ctx || !ssl->method->set_write_state(ssl, ssl_encryption_initial,
+ std::move(null_ctx))) {
+ return false;
+ }
+ ssl->s3->aead_write_ctx->SetVersionIfNullCipher(ssl->version);
+ } else {
+ 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())) {
+ return false;
+ }
}
}
@@ -437,18 +439,15 @@
if (!tls13_advance_key_schedule(hs, dhe_secret) ||
!ssl_hash_message(hs, msg) ||
- !tls13_derive_handshake_secrets(hs) ||
- !tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
- hs->new_session.get(),
- hs->server_handshake_secret())) {
+ !tls13_derive_handshake_secrets(hs)) {
return ssl_hs_error;
}
- // If currently sending early data, we defer installing client traffic keys to
- // when the early data stream is closed. See |close_early_data|. Note if the
- // server has already rejected 0-RTT via HelloRetryRequest, |in_early_data| is
- // already false.
- if (!hs->in_early_data) {
+ // If currently sending early data over TCP, we defer installing client
+ // traffic keys to when the early data stream is closed. See
+ // |close_early_data|. Note if the server has already rejected 0-RTT via
+ // HelloRetryRequest, |in_early_data| is already false.
+ 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())) {
@@ -456,6 +455,12 @@
}
}
+ if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open,
+ hs->new_session.get(),
+ hs->server_handshake_secret())) {
+ return ssl_hs_error;
+ }
+
ssl->method->next_message(ssl);
hs->tls13_state = state_read_encrypted_extensions;
return ssl_hs_ok;
@@ -803,12 +808,12 @@
}
// Derive the final keys and enable them.
- if (!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_open,
- hs->new_session.get(),
- hs->server_traffic_secret_0()) ||
- !tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
+ if (!tls13_set_traffic_key(ssl, ssl_encryption_application, evp_aead_seal,
hs->new_session.get(),
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()) ||
!tls13_derive_resumption_secret(hs)) {
return ssl_hs_error;
}
