Implement ClientHelloOuter handshakes.
If a client offers ECH, but the server rejects it, the client completes
the handshake with ClientHelloOuter in order to authenticate retry keys.
Implement this flow. This is largely allowing the existing handshake to
proceed, but with some changes:
- Certificate verification uses the other name. This CL routes this up to
the built-in verifier and adds SSL_get0_ech_name_override for the
callback.
- We need to disable False Start to pick up server Finished in TLS 1.2.
- Client certificates, notably in TLS 1.3 where they're encrypted,
should only be revealed to the true server. Fortunately, not sending
client certs is always an option, so do that.
Channel ID has a similar issue. I've just omitted the extension in
ClientHelloOuter because it's deprecated and is unlikely to be used
with ECH at this point. ALPS may be worth some pondering but, the way
it's currently used, is not sensitive.
(Possibly we should change the draft to terminate the handshake before
even sending that flight...)
- The session is never offered in ClientHelloOuter, but our internal
book-keeping doesn't quite notice.
I had to replace ech_accept with a tri-state ech_status to correctly
handle an edge case in SSL_get0_ech_name_override: when ECH + 0-RTT +
reverify_on_resume are all enabled, the first certificate verification
is for the 0-RTT session and should be against the true name, yet we
have selected_ech_config && !ech_accept. A tri-state tracks when ECH is
actually rejected. I've maintained this on the server as well, though
the server never actually cares.
Bug: 275
Change-Id: Ie55966ca3dc4ffcc8c381479f0fe9bcacd34d0f8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48135
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index d5ccafc..ba8f4b7 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -731,13 +731,9 @@
return ssl_hs_error;
}
- // TODO(https://crbug.com/boringssl/275): If the server negotiates TLS 1.2 and
- // we offer ECH, we handshake with ClientHelloOuter instead of
- // ClientHelloInner. That path is not yet implemented. For now, terminate the
- // handshake with a distinguishable error for testing.
+ // TLS 1.2 handshakes cannot accept ECH.
if (hs->selected_ech_config) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED);
- return ssl_hs_error;
+ ssl->s3->ech_status = ssl_ech_rejected;
}
// Copy over the server random.
@@ -764,44 +760,13 @@
}
}
- if (hs->session_id_len != 0 &&
- CBS_mem_equal(&session_id, hs->session_id, hs->session_id_len)) {
- // Echoing the ClientHello session ID in TLS 1.2, whether from the session
- // or a synthetic one, indicates resumption. If there was no session, this
- // was the TLS 1.3 compatibility mode session ID. As we know this is not a
- // session the server knows about, any server resuming it is in error.
- // Reject the first connection deterministicly, rather than installing an
- // invalid session into the session cache. https://crbug.com/796910
- if (ssl->session == nullptr) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_SERVER_ECHOED_INVALID_SESSION_ID);
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
- // We never offer sessions on renegotiation.
- assert(!ssl->s3->initial_handshake_complete);
- ssl->s3->session_reused = true;
- // Note |ssl->session| may be a TLS 1.3 session, offered in a separate
- // extension altogether. In that case, the version check below will fail the
- // connection.
- } else {
- // The session wasn't resumed. Create a fresh SSL_SESSION to fill out.
- ssl_set_session(ssl, NULL);
- if (!ssl_get_new_session(hs)) {
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
- return ssl_hs_error;
- }
- // Note: session_id could be empty.
- hs->new_session->session_id_length = CBS_len(&session_id);
- OPENSSL_memcpy(hs->new_session->session_id, CBS_data(&session_id),
- CBS_len(&session_id));
- }
-
const SSL_CIPHER *cipher = SSL_get_cipher_by_value(cipher_suite);
if (cipher == NULL) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CIPHER_RETURNED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
}
+ hs->new_cipher = cipher;
// The cipher must be allowed in the selected version and enabled.
uint32_t mask_a, mask_k;
@@ -815,7 +780,20 @@
return ssl_hs_error;
}
- if (ssl->session != NULL) {
+ if (hs->session_id_len != 0 &&
+ CBS_mem_equal(&session_id, hs->session_id, hs->session_id_len)) {
+ // Echoing the ClientHello session ID in TLS 1.2, whether from the session
+ // or a synthetic one, indicates resumption. If there was no session (or if
+ // the session was only offered in ECH ClientHelloInner), this was the
+ // TLS 1.3 compatibility mode session ID. As we know this is not a session
+ // the server knows about, any server resuming it is in error. Reject the
+ // first connection deterministicly, rather than installing an invalid
+ // session into the session cache. https://crbug.com/796910
+ if (ssl->session == nullptr || ssl->s3->ech_status == ssl_ech_rejected) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_SERVER_ECHOED_INVALID_SESSION_ID);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ return ssl_hs_error;
+ }
if (ssl->session->ssl_version != ssl->version) {
OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_VERSION_NOT_RETURNED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
@@ -833,10 +811,22 @@
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
}
+ // We never offer sessions on renegotiation.
+ assert(!ssl->s3->initial_handshake_complete);
+ ssl->s3->session_reused = true;
} else {
+ // The session wasn't resumed. Create a fresh SSL_SESSION to fill out.
+ ssl_set_session(ssl, NULL);
+ if (!ssl_get_new_session(hs)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+ return ssl_hs_error;
+ }
+ // Note: session_id could be empty.
+ hs->new_session->session_id_length = CBS_len(&session_id);
+ OPENSSL_memcpy(hs->new_session->session_id, CBS_data(&session_id),
+ CBS_len(&session_id));
hs->new_session->cipher = cipher;
}
- hs->new_cipher = cipher;
// Now that the cipher is known, initialize the handshake hash and hash the
// ServerHello.
@@ -1334,8 +1324,12 @@
return ssl_hs_ok;
}
- // Call cert_cb to update the certificate.
- if (hs->config->cert->cert_cb != NULL) {
+ if (ssl->s3->ech_status == ssl_ech_rejected) {
+ // Do not send client certificates on ECH reject. We have not authenticated
+ // the server for the name that can learn the certificate.
+ SSL_certs_clear(ssl);
+ } else if (hs->config->cert->cert_cb != nullptr) {
+ // Call cert_cb to update the certificate.
int rv = hs->config->cert->cert_cb(ssl, hs->config->cert->cert_cb_arg);
if (rv == 0) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
@@ -1642,7 +1636,7 @@
}
static bool can_false_start(const SSL_HANDSHAKE *hs) {
- SSL *const ssl = hs->ssl;
+ const SSL *const ssl = hs->ssl;
// False Start bypasses the Finished check's downgrade protection. This can
// enable attacks where we send data under weaker settings than supported
@@ -1660,6 +1654,13 @@
return false;
}
+ // If ECH was rejected, disable False Start. We run the handshake to
+ // completion, including the Finished downgrade check, to authenticate the
+ // recovery flow.
+ if (ssl->s3->ech_status == ssl_ech_rejected) {
+ return false;
+ }
+
// Additionally require ALPN or NPN by default.
//
// TODO(davidben): Can this constraint be relaxed globally now that cipher
@@ -1796,6 +1797,13 @@
static enum ssl_hs_wait_t do_finish_client_handshake(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
+ if (ssl->s3->ech_status == ssl_ech_rejected) {
+ // Release the retry configs.
+ hs->ech_authenticated_reject = true;
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ECH_REQUIRED);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_ECH_REJECTED);
+ return ssl_hs_error;
+ }
ssl->method->on_handshake_complete(ssl);
