Defer early keys to QUIC clients to after certificate reverification.
On a client using SSL_CTX_set_reverify_on_resume, we currently release
the early data keys before reverification rather than afterwards. This
means the QUIC implementation needs to watch for SSL_do_handshake's
return value before using the keys we've released. It is better to be
robust, so defer releasing the keys in the first place.
To avoid oddities around TCP and QUIC differences, tweak the 0-RTT cert
reverification to not send an alert on error. Sending such an alert
under early data is somewhat questionable given the server may not be
able to read it anyway.
Bug: 303
Change-Id: I42c16f9f046322d0b03cb0b425e11471f2fbe52a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38885
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/handshake_client.cc b/ssl/handshake_client.cc
index 23f48c1..4041fe9 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -458,8 +458,7 @@
if (!tls13_init_early_key_schedule(
hs, MakeConstSpan(ssl->session->master_key,
ssl->session->master_key_length)) ||
- !tls13_derive_early_secret(hs) ||
- !tls13_set_early_secret_for_quic(hs)) {
+ !tls13_derive_early_secret(hs)) {
return ssl_hs_error;
}
if (ssl->quic_method == nullptr &&
@@ -477,17 +476,30 @@
static enum ssl_hs_wait_t do_early_reverify_server_certificate(SSL_HANDSHAKE *hs) {
if (hs->ssl->ctx->reverify_on_resume) {
- switch (ssl_reverify_peer_cert(hs)) {
- case ssl_verify_ok:
- break;
- case ssl_verify_invalid:
- return ssl_hs_error;
- case ssl_verify_retry:
- hs->state = state_early_reverify_server_certificate;
- return ssl_hs_certificate_verify;
+ // Don't send an alert on error. The alert be in early data, which the
+ // server may not accept anyway. It would also be a mismatch between QUIC
+ // and TCP because the QUIC early keys are deferred below.
+ //
+ // TODO(davidben): The client behavior should be to verify the certificate
+ // before deciding whether to offer the session and, if invalid, decline to
+ // send the session.
+ switch (ssl_reverify_peer_cert(hs, /*send_alert=*/false)) {
+ case ssl_verify_ok:
+ break;
+ case ssl_verify_invalid:
+ return ssl_hs_error;
+ case ssl_verify_retry:
+ hs->state = state_early_reverify_server_certificate;
+ return ssl_hs_certificate_verify;
}
}
+ // Defer releasing the 0-RTT key to after certificate reverification, so the
+ // QUIC implementation does not accidentally write data too early.
+ if (!tls13_set_early_secret_for_quic(hs)) {
+ return ssl_hs_error;
+ }
+
hs->in_early_data = true;
hs->can_early_write = true;
hs->state = state_read_server_hello;
@@ -907,7 +919,7 @@
static enum ssl_hs_wait_t do_reverify_server_certificate(SSL_HANDSHAKE *hs) {
assert(hs->ssl->ctx->reverify_on_resume);
- switch (ssl_reverify_peer_cert(hs)) {
+ switch (ssl_reverify_peer_cert(hs, /*send_alert=*/true)) {
case ssl_verify_ok:
break;
case ssl_verify_invalid:
