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>

crypto/err/ssl.errordata

@@ -59,6 +59,7 @@ SSL,264,DUPLICATE_KEY_SHARE
 SSL,296,DUPLICATE_SIGNATURE_ALGORITHM
 SSL,283,EARLY_DATA_NOT_IN_USE
 SSL,144,ECC_CERT_NOT_FOR_SIGNING
+SSL,319,ECH_REJECTED
 SSL,310,ECH_SERVER_CONFIG_AND_PRIVATE_KEY_MISMATCH
 SSL,311,ECH_SERVER_CONFIG_UNSUPPORTED_EXTENSION
 SSL,313,ECH_SERVER_WOULD_HAVE_NO_RETRY_CONFIGS

include/openssl/ssl.h

@@ -3557,6 +3557,11 @@ OPENSSL_EXPORT const char *SSL_early_data_reason_string(
 // This can prevent observers from seeing cleartext information about the
 // connection, such as the server_name extension.
 //
+// By default, BoringSSL will treat the server name, session ticket, and client
+// certificate as secret, but most other parameters, such as the ALPN protocol
+// list will be treated as public and sent in the cleartext ClientHello. Other
+// APIs may be added for applications with different secrecy requirements.
+//
 // ECH support in BoringSSL is still experimental and under development.
 //
 // See https://tools.ietf.org/html/draft-ietf-tls-esni-10.
@@ -3573,16 +3578,57 @@ OPENSSL_EXPORT void SSL_set_enable_ech_grease(SSL *ssl, int enable);
 // valid but none of the ECHConfigs implement supported parameters, it will
 // return success and proceed without ECH.
 //
-// WARNING: Client ECH support is still incomplete and does not yet implement
-// the recovery flow. It currently treats ECH rejection as a fatal error. Do not
-// use this API yet.
-//
-// TODO(https://crbug.com/boringssl/275): When the recovery flow is implemented,
-// fill in the remaining docs.
+// If a supported ECHConfig is found, |ssl| will encrypt the true ClientHello
+// parameters. If the server cannot decrypt it, e.g. due to a key mismatch, ECH
+// has a recovery flow. |ssl| will handshake using the cleartext parameters,
+// including a public name in the ECHConfig. If using
+// |SSL_CTX_set_custom_verify|, callers should use |SSL_get0_ech_name_override|
+// to verify the certificate with the public name. If using the built-in
+// verifier, the |X509_STORE_CTX| will be configured automatically.
+//
+// If no other errors are found in this handshake, it will fail with
+// |SSL_R_ECH_REJECTED|. Since it didn't use the true parameters, the connection
+// cannot be used for application data. Instead, callers should handle this
+// error by calling |SSL_get0_ech_retry_configs| and retrying the connection
+// with updated ECH parameters. If the retry also fails with
+// |SSL_R_ECH_REJECTED|, the caller should report a connection failure.
 OPENSSL_EXPORT int SSL_set1_ech_config_list(SSL *ssl,
                                             const uint8_t *ech_config_list,
                                             size_t ech_config_list_len);
 
+// SSL_get0_ech_name_override sets |*out_name| and |*out_name_len| to point to a
+// buffer containing the ECH public name, if the server rejected ECH, or the
+// empty string otherwise.
+//
+// This function should be called during the certificate verification callback
+// (see |SSL_CTX_set_custom_verify|) if |ssl| is a client offering ECH. If
+// |*out_name_len| is non-zero, the caller should verify the certificate against
+// the result, interpreted as a DNS name, rather than the true server name. In
+// this case, the handshake will never succeed and is only used to authenticate
+// retry configs. See also |SSL_get0_ech_retry_configs|.
+OPENSSL_EXPORT void SSL_get0_ech_name_override(const SSL *ssl,
+                                               const char **out_name,
+                                               size_t *out_name_len);
+
+// SSL_get0_ech_retry_configs sets |*out_retry_configs| and
+// |*out_retry_configs_len| to a buffer containing a serialized ECHConfigList.
+// If the server did not provide an ECHConfigList, |*out_retry_configs_len| will
+// be zero.
+//
+// When handling an |SSL_R_ECH_REJECTED| error code as a client, callers should
+// use this function to recover from potential key mismatches. If the result is
+// non-empty, the caller should retry the connection, passing this buffer to
+// |SSL_set1_ech_config_list|. If the result is empty, the server has rolled
+// back ECH support, and the caller should retry without ECH.
+//
+// This function must only be called in response to an |SSL_R_ECH_REJECTED|
+// error code. Calling this function on |ssl|s that have not authenticated the
+// rejection handshake will assert in debug builds and otherwise return an
+// unparsable list.
+OPENSSL_EXPORT void SSL_get0_ech_retry_configs(
+    const SSL *ssl, const uint8_t **out_retry_configs,
+    size_t *out_retry_configs_len);
+
 // SSL_marshal_ech_config constructs a new serialized ECHConfig. On success, it
 // sets |*out| to a newly-allocated buffer containing the result and |*out_len|
 // to the size of the buffer. The caller must call |OPENSSL_free| on |*out| to
@@ -5502,6 +5548,7 @@ BSSL_NAMESPACE_END
 #define SSL_R_COULD_NOT_PARSE_HINTS 316
 #define SSL_R_INVALID_ECH_PUBLIC_NAME 317
 #define SSL_R_INVALID_ECH_CONFIG_LIST 318
+#define SSL_R_ECH_REJECTED 319
 #define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
 #define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
 #define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020

include/openssl/x509.h

@@ -1820,6 +1820,7 @@ BORINGSSL_MAKE_DELETER(X509_REQ, X509_REQ_free)
 BORINGSSL_MAKE_DELETER(X509_REVOKED, X509_REVOKED_free)
 BORINGSSL_MAKE_DELETER(X509_SIG, X509_SIG_free)
 BORINGSSL_MAKE_DELETER(X509_STORE, X509_STORE_free)
+BORINGSSL_MAKE_UP_REF(X509_STORE, X509_STORE_up_ref)
 BORINGSSL_MAKE_DELETER(X509_STORE_CTX, X509_STORE_CTX_free)
 BORINGSSL_MAKE_DELETER(X509_VERIFY_PARAM, X509_VERIFY_PARAM_free)
 

ssl/encrypted_client_hello.cc

@@ -40,7 +40,8 @@
 BSSL_NAMESPACE_BEGIN
 
 // ECH reuses the extension code point for the version number.
-static const uint16_t kECHConfigVersion = TLSEXT_TYPE_encrypted_client_hello;
+static constexpr uint16_t kECHConfigVersion =
+    TLSEXT_TYPE_encrypted_client_hello;
 
 static const decltype(&EVP_hpke_aes_128_gcm) kSupportedAEADs[] = {
     &EVP_hpke_aes_128_gcm,
@@ -993,6 +994,47 @@ int SSL_set1_ech_config_list(SSL *ssl, const uint8_t *ech_config_list,
   return ssl->config->client_ech_config_list.CopyFrom(span);
 }
 
+void SSL_get0_ech_name_override(const SSL *ssl, const char **out_name,
+                                size_t *out_name_len) {
+  // When ECH is rejected, we use the public name. Note that, if
+  // |SSL_CTX_set_reverify_on_resume| is enabled, we reverify the certificate
+  // before the 0-RTT point. If also offering ECH, we verify as if
+  // ClientHelloInner was accepted and do not override. This works because, at
+  // this point, |ech_status| will be |ssl_ech_none|. See the
+  // ECH-Client-Reject-EarlyDataReject-OverrideNameOnRetry tests in runner.go.
+  const SSL_HANDSHAKE *hs = ssl->s3->hs.get();
+  if (hs && ssl->s3->ech_status == ssl_ech_rejected) {
+    *out_name = reinterpret_cast<const char *>(
+        hs->selected_ech_config->public_name.data());
+    *out_name_len = hs->selected_ech_config->public_name.size();
+  } else {
+    *out_name = nullptr;
+    *out_name_len = 0;
+  }
+}
+
+void SSL_get0_ech_retry_configs(
+    const SSL *ssl, const uint8_t **out_retry_configs,
+    size_t *out_retry_configs_len) {
+  const SSL_HANDSHAKE *hs = ssl->s3->hs.get();
+  if (!hs || !hs->ech_authenticated_reject) {
+    // It is an error to call this function except in response to
+    // |SSL_R_ECH_REJECTED|. Returning an empty string risks the caller
+    // mistakenly believing the server has disabled ECH. Instead, return a
+    // non-empty ECHConfigList with a syntax error, so the subsequent
+    // |SSL_set1_ech_config_list| call will fail.
+    assert(0);
+    static const uint8_t kPlaceholder[] = {
+        kECHConfigVersion >> 8, kECHConfigVersion & 0xff, 0xff, 0xff, 0xff};
+    *out_retry_configs = kPlaceholder;
+    *out_retry_configs_len = sizeof(kPlaceholder);
+    return;
+  }
+
+  *out_retry_configs = hs->ech_retry_configs.data();
+  *out_retry_configs_len = hs->ech_retry_configs.size();
+}
+
 int SSL_marshal_ech_config(uint8_t **out, size_t *out_len, uint8_t config_id,
                            const EVP_HPKE_KEY *key, const char *public_name,
                            size_t max_name_len) {
@@ -1129,5 +1171,5 @@ int SSL_ech_accepted(const SSL *ssl) {
     return ssl->s3->hs->selected_ech_config != nullptr;
   }
 
-  return ssl->s3->ech_accept;
+  return ssl->s3->ech_status == ssl_ech_accepted;
 }

ssl/extensions.cc

@@ -654,6 +654,11 @@ static bool ext_ech_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
     return false;
   }
 
+  if (!ssl_is_valid_ech_config_list(*contents)) {
+    *out_alert = SSL_AD_DECODE_ERROR;
+    return false;
+  }
+
   // The server may only send retry configs in response to ClientHelloOuter (or
   // ECH GREASE), not ClientHelloInner. The unsolicited extension rule checks
   // this implicitly because the ClientHelloInner has no encrypted_client_hello
@@ -663,14 +668,13 @@ static bool ext_ech_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
   // https://github.com/tlswg/draft-ietf-tls-esni/pull/422 is merged, a later
   // draft will fold encrypted_client_hello and ech_is_inner together. Then this
   // assert should become a runtime check.
-  assert(!ssl->s3->ech_accept);
-
-  // TODO(https://crbug.com/boringssl/275): When the implementing the
-  // ClientHelloOuter flow, save the retry configs.
-  if (!ssl_is_valid_ech_config_list(*contents)) {
-    *out_alert = SSL_AD_DECODE_ERROR;
+  assert(ssl->s3->ech_status != ssl_ech_accepted);
+  if (hs->selected_ech_config &&
+      !hs->ech_retry_configs.CopyFrom(*contents)) {
+    *out_alert = SSL_AD_INTERNAL_ERROR;
     return false;
   }
+
   return true;
 }
 
@@ -685,8 +689,8 @@ static bool ext_ech_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
 
 static bool ext_ech_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {
   SSL *const ssl = hs->ssl;
-  if (ssl_protocol_version(ssl) < TLS1_3_VERSION ||  //
-      ssl->s3->ech_accept ||                         //
+  if (ssl_protocol_version(ssl) < TLS1_3_VERSION ||
+      ssl->s3->ech_status == ssl_ech_accepted ||  //
       hs->ech_keys == nullptr) {
     return true;
   }
@@ -1634,12 +1638,21 @@ static bool ext_channel_id_add_clienthello(const SSL_HANDSHAKE *hs, CBB *out,
                                            CBB *out_compressible,
                                            ssl_client_hello_type_t type) {
   const SSL *const ssl = hs->ssl;
-  if (!hs->config->channel_id_private || SSL_is_dtls(ssl)) {
+  if (!hs->config->channel_id_private || SSL_is_dtls(ssl) ||
+      // Don't offer Channel ID in ClientHelloOuter. ClientHelloOuter handshakes
+      // are not authenticated for the name that can learn the Channel ID.
+      //
+      // We could alternatively offer the extension but sign with a random key.
+      // For other extensions, we try to align |ssl_client_hello_outer| and
+      // |ssl_client_hello_unencrypted|, to improve the effectiveness of ECH
+      // GREASE. However, Channel ID is deprecated and unlikely to be used with
+      // ECH, so do the simplest thing.
+      type == ssl_client_hello_outer) {
     return true;
   }
 
-  if (!CBB_add_u16(out_compressible, TLSEXT_TYPE_channel_id) ||
-      !CBB_add_u16(out_compressible, 0 /* length */)) {
+  if (!CBB_add_u16(out, TLSEXT_TYPE_channel_id) ||
+      !CBB_add_u16(out, 0 /* length */)) {
     return false;
   }
 

ssl/handshake.cc

@@ -128,6 +128,7 @@ SSL_HANDSHAKE::SSL_HANDSHAKE(SSL *ssl_arg)
     : ssl(ssl_arg),
       ech_present(false),
       ech_is_inner_present(false),
+      ech_authenticated_reject(false),
       scts_requested(false),
       handshake_finalized(false),
       accept_psk_mode(false),
@@ -715,6 +716,10 @@ int ssl_run_handshake(SSL_HANDSHAKE *hs, bool *out_early_return) {
         return -1;
 
       case ssl_hs_early_return:
+        if (!ssl->server) {
+          // On ECH reject, the handshake should never complete.
+          assert(ssl->s3->ech_status != ssl_ech_rejected);
+        }
         *out_early_return = true;
         hs->wait = ssl_hs_ok;
         return 1;
@@ -734,6 +739,10 @@ int ssl_run_handshake(SSL_HANDSHAKE *hs, bool *out_early_return) {
       return -1;
     }
     if (hs->wait == ssl_hs_ok) {
+      if (!ssl->server) {
+        // On ECH reject, the handshake should never complete.
+        assert(ssl->s3->ech_status != ssl_ech_rejected);
+      }
       // The handshake has completed.
       *out_early_return = false;
       return 1;

ssl/handshake_client.cc

@@ -731,13 +731,9 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
     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 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
     }
   }
 
-  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 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
     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 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
       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 @@ static enum ssl_hs_wait_t do_send_client_certificate(SSL_HANDSHAKE *hs) {
     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 enum ssl_hs_wait_t do_send_client_finished(SSL_HANDSHAKE *hs) {
 }
 
 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 @@ static bool can_false_start(const SSL_HANDSHAKE *hs) {
     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_read_server_finished(SSL_HANDSHAKE *hs) {
 
 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);
 

ssl/handshake_server.cc

@@ -661,14 +661,17 @@ static enum ssl_hs_wait_t do_read_client_hello(SSL_HANDSHAKE *hs) {
         }
 
         hs->ech_config_id = config_id;
-        ssl->s3->ech_accept = true;
+        ssl->s3->ech_status = ssl_ech_accepted;
         break;
       }
     }
 
-    // If we did not accept ECH, we will send the current ECHConfigs as
-    // retry_configs in the ServerHello's encrypted extensions. Proceed with the
-    // ClientHelloOuter.
+    // If we did not accept ECH, proceed with the ClientHelloOuter. Note this
+    // could be key mismatch or ECH GREASE, so we most complete the handshake
+    // as usual, except EncryptedExtensions will contain retry configs.
+    if (ssl->s3->ech_status != ssl_ech_accepted) {
+      ssl->s3->ech_status = ssl_ech_rejected;
+    }
   }
 
   uint8_t alert = SSL_AD_DECODE_ERROR;
@@ -803,7 +806,7 @@ static enum ssl_hs_wait_t do_select_certificate(SSL_HANDSHAKE *hs) {
   // It should not be possible to negotiate TLS 1.2 with ECH. The
   // ClientHelloInner decoding function rejects ClientHellos which offer TLS 1.2
   // or below.
-  assert(!ssl->s3->ech_accept);
+  assert(ssl->s3->ech_status != ssl_ech_accepted);
 
   // TODO(davidben): Also compute hints for TLS 1.2. When doing so, update the
   // check in bssl_shim.cc to test this.

ssl/internal.h

@@ -1833,6 +1833,10 @@ struct SSL_HANDSHAKE {
   // ech_client_bytes contains the ECH extension to send in the ClientHello.
   Array<uint8_t> ech_client_bytes;
 
+  // ech_retry_configs, on the client, contains the retry configs from the
+  // server as a serialized ECHConfigList.
+  Array<uint8_t> ech_retry_configs;
+
   // ech_client_hello_buf, on the server, contains the bytes of the
   // reconstructed ClientHelloInner message.
   Array<uint8_t> ech_client_hello_buf;
@@ -1872,8 +1876,8 @@ struct SSL_HANDSHAKE {
   uint16_t cert_compression_alg_id;
 
   // ech_hpke_ctx is the HPKE context used in ECH. On the server, it is
-  // initialized if |ech_accept| is true. On the client, it is initialized if
-  // |selected_ech_config| is not nullptr.
+  // initialized if |ech_status| is |ssl_ech_accepted|. On the client, it is
+  // initialized if |selected_ech_config| is not nullptr.
   ScopedEVP_HPKE_CTX ech_hpke_ctx;
 
   // server_params, in a TLS 1.2 server, stores the ServerKeyExchange
@@ -1943,6 +1947,10 @@ struct SSL_HANDSHAKE {
   // contained an ech_is_inner extension.
   bool ech_is_inner_present : 1;
 
+  // ech_authenticated_reject, on the client, indicates whether an ECH rejection
+  // handshake has been authenticated.
+  bool ech_authenticated_reject : 1;
+
   // scts_requested is true if the SCT extension is in the ClientHello.
   bool scts_requested : 1;
 
@@ -2574,6 +2582,16 @@ enum ssl_shutdown_t {
   ssl_shutdown_error = 2,
 };
 
+enum ssl_ech_status_t {
+  // ssl_ech_none indicates ECH was not offered, or we have not gotten far
+  // enough in the handshake to determine the status.
+  ssl_ech_none,
+  // ssl_ech_accepted indicates the server accepted ECH.
+  ssl_ech_accepted,
+  // ssl_ech_rejected indicates the server was offered ECH but rejected it.
+  ssl_ech_rejected,
+};
+
 struct SSL3_STATE {
   static constexpr bool kAllowUniquePtr = true;
 
@@ -2636,8 +2654,8 @@ struct SSL3_STATE {
   // key_update_count is the number of consecutive KeyUpdates received.
   uint8_t key_update_count = 0;
 
-  // ech_accept indicates whether ECH was accepted by the server.
-  bool ech_accept : 1;
+  // ech_status indicates whether ECH was accepted by the server.
+  ssl_ech_status_t ech_status = ssl_ech_none;
 
   // skip_early_data instructs the record layer to skip unexpected early data
   // messages when 0RTT is rejected.

ssl/s3_lib.cc

@@ -164,8 +164,7 @@
 BSSL_NAMESPACE_BEGIN
 
 SSL3_STATE::SSL3_STATE()
-    : ech_accept(false),
-      skip_early_data(false),
+    : skip_early_data(false),
       have_version(false),
       v2_hello_done(false),
       is_v2_hello(false),

ssl/ssl_test.cc

@@ -1691,14 +1691,13 @@ bool MakeECHConfig(std::vector<uint8_t> *out,
   return true;
 }
 
-static bssl::UniquePtr<SSL_ECH_KEYS> MakeTestECHKeys() {
+static bssl::UniquePtr<SSL_ECH_KEYS> MakeTestECHKeys(uint8_t config_id = 1) {
   bssl::ScopedEVP_HPKE_KEY key;
   uint8_t *ech_config;
   size_t ech_config_len;
   if (!EVP_HPKE_KEY_generate(key.get(), EVP_hpke_x25519_hkdf_sha256()) ||
-      !SSL_marshal_ech_config(&ech_config, &ech_config_len,
-                              /*config_id=*/1, key.get(), "public.example",
-                              16)) {
+      !SSL_marshal_ech_config(&ech_config, &ech_config_len, config_id,
+                              key.get(), "public.example", 16)) {
     return nullptr;
   }
   bssl::UniquePtr<uint8_t> free_ech_config(ech_config);
@@ -2154,6 +2153,147 @@ TEST(SSLTest, ECHPublicName) {
   EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("4294967296")));
 }
 
+// When using the built-in verifier, test that |SSL_get0_ech_name_override| is
+// applied automatically.
+TEST(SSLTest, ECHBuiltinVerifier) {
+  // These test certificates generated with the following Go program.
+  /* clang-format off
+func main() {
+  notBefore := time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC)
+  notAfter := time.Date(2099, time.January, 1, 0, 0, 0, 0, time.UTC)
+  rootKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+  rootTemplate := &x509.Certificate{
+    SerialNumber:          big.NewInt(1),
+    Subject:               pkix.Name{CommonName: "Test CA"},
+    NotBefore:             notBefore,
+    NotAfter:              notAfter,
+    BasicConstraintsValid: true,
+    IsCA:                  true,
+  }
+  rootDER, _ := x509.CreateCertificate(rand.Reader, rootTemplate, rootTemplate, &rootKey.PublicKey, rootKey)
+  root, _ := x509.ParseCertificate(rootDER)
+  pem.Encode(os.Stdout, &pem.Block{Type: "CERTIFICATE", Bytes: rootDER})
+  leafKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+  leafKeyDER, _ := x509.MarshalPKCS8PrivateKey(leafKey)
+  pem.Encode(os.Stdout, &pem.Block{Type: "PRIVATE KEY", Bytes: leafKeyDER})
+  for i, name := range []string{"public.example", "secret.example"} {
+    leafTemplate := &x509.Certificate{
+      SerialNumber:          big.NewInt(int64(i) + 2),
+      Subject:               pkix.Name{CommonName: name},
+      NotBefore:             notBefore,
+      NotAfter:              notAfter,
+      BasicConstraintsValid: true,
+      DNSNames:              []string{name},
+    }
+    leafDER, _ := x509.CreateCertificate(rand.Reader, leafTemplate, root, &leafKey.PublicKey, rootKey)
+    pem.Encode(os.Stdout, &pem.Block{Type: "CERTIFICATE", Bytes: leafDER})
+  }
+}
+clang-format on */
+  bssl::UniquePtr<X509> root = CertFromPEM(R"(
+-----BEGIN CERTIFICATE-----
+MIIBRzCB7aADAgECAgEBMAoGCCqGSM49BAMCMBIxEDAOBgNVBAMTB1Rlc3QgQ0Ew
+IBcNMDAwMTAxMDAwMDAwWhgPMjA5OTAxMDEwMDAwMDBaMBIxEDAOBgNVBAMTB1Rl
+c3QgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAT5JUjrI1DAxSpEl88UkmJw
+tAJqxo/YrSFo9V3MkcNkfTixi5p6MUtO8DazhEgekBcd2+tBAWtl7dy0qpvTqx92
+ozIwMDAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTw6ftkexAI6o4r5FntJIfL
+GU5F4zAKBggqhkjOPQQDAgNJADBGAiEAiiNowddQeHZaZFIygwe6RW5/WG4sUXWC
+dkyl9CQzRaYCIQCFS1EvwZbZtMny27fYm1eeYciY0TkJTEi34H1KwyzzIA==
+-----END CERTIFICATE-----
+)");
+  ASSERT_TRUE(root);
+  bssl::UniquePtr<EVP_PKEY> leaf_key = KeyFromPEM(R"(
+-----BEGIN PRIVATE KEY-----
+MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgj5WKHwHnziiyPauf
+7QukxTwtTyGZkk8qNdms4puJfxqhRANCAARNrkhxabALDlJrHtvkuDwvCWUF/oVC
+hr6PDITHi1lDlJzvVT4aXBH87sH2n2UV5zpx13NHkq1bIC8eRT8eOIe0
+-----END PRIVATE KEY-----
+)");
+  ASSERT_TRUE(leaf_key);
+  bssl::UniquePtr<X509> leaf_public = CertFromPEM(R"(
+-----BEGIN CERTIFICATE-----
+MIIBaDCCAQ6gAwIBAgIBAjAKBggqhkjOPQQDAjASMRAwDgYDVQQDEwdUZXN0IENB
+MCAXDTAwMDEwMTAwMDAwMFoYDzIwOTkwMTAxMDAwMDAwWjAZMRcwFQYDVQQDEw5w
+dWJsaWMuZXhhbXBsZTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABE2uSHFpsAsO
+Umse2+S4PC8JZQX+hUKGvo8MhMeLWUOUnO9VPhpcEfzuwfafZRXnOnHXc0eSrVsg
+Lx5FPx44h7SjTDBKMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAU8On7ZHsQCOqO
+K+RZ7SSHyxlOReMwGQYDVR0RBBIwEIIOcHVibGljLmV4YW1wbGUwCgYIKoZIzj0E
+AwIDSAAwRQIhANqZRhDR/+QL05hsWXMYEwaiHifd9iakKoFEhKFchcF3AiBRAeXw
+wRGGT6+iPmTYM6N5/IDyAb5B9Ke38O6lLEsUwA==
+-----END CERTIFICATE-----
+)");
+  ASSERT_TRUE(leaf_public);
+  bssl::UniquePtr<X509> leaf_secret = CertFromPEM(R"(
+-----BEGIN CERTIFICATE-----
+MIIBaTCCAQ6gAwIBAgIBAzAKBggqhkjOPQQDAjASMRAwDgYDVQQDEwdUZXN0IENB
+MCAXDTAwMDEwMTAwMDAwMFoYDzIwOTkwMTAxMDAwMDAwWjAZMRcwFQYDVQQDEw5z
+ZWNyZXQuZXhhbXBsZTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABE2uSHFpsAsO
+Umse2+S4PC8JZQX+hUKGvo8MhMeLWUOUnO9VPhpcEfzuwfafZRXnOnHXc0eSrVsg
+Lx5FPx44h7SjTDBKMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAU8On7ZHsQCOqO
+K+RZ7SSHyxlOReMwGQYDVR0RBBIwEIIOc2VjcmV0LmV4YW1wbGUwCgYIKoZIzj0E
+AwIDSQAwRgIhAPQdIz1xCFkc9WuSkxOxJDpywZiEp9SnKcxJ9nwrlRp3AiEA+O3+
+XRqE7XFhHL+7TNC2a9OOAjQsEF137YPWo+rhgko=
+-----END CERTIFICATE-----
+)");
+  ASSERT_TRUE(leaf_secret);
+
+  // Use different config IDs so that fuzzer mode, which breaks trial
+  // decryption, will observe the key mismatch.
+  bssl::UniquePtr<SSL_ECH_KEYS> keys = MakeTestECHKeys(/*config_id=*/1);
+  ASSERT_TRUE(keys);
+  bssl::UniquePtr<SSL_ECH_KEYS> wrong_keys = MakeTestECHKeys(/*config_id=*/2);
+  ASSERT_TRUE(wrong_keys);
+  bssl::UniquePtr<SSL_CTX> server_ctx =
+      CreateContextWithTestCertificate(TLS_method());
+  ASSERT_TRUE(server_ctx);
+  bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
+  ASSERT_TRUE(client_ctx);
+
+  // Configure the client to verify certificates and expect the secret name.
+  // This is the name the client is trying to connect to. If ECH is rejected,
+  // BoringSSL will internally override this setting with the public name.
+  bssl::UniquePtr<X509_STORE> store(X509_STORE_new());
+  ASSERT_TRUE(store);
+  ASSERT_TRUE(X509_STORE_add_cert(store.get(), root.get()));
+  SSL_CTX_set_cert_store(client_ctx.get(), store.release());
+  SSL_CTX_set_verify(client_ctx.get(), SSL_VERIFY_PEER, nullptr);
+  static const char kSecretName[] = "secret.example";
+  ASSERT_TRUE(X509_VERIFY_PARAM_set1_host(SSL_CTX_get0_param(client_ctx.get()),
+                                          kSecretName, strlen(kSecretName)));
+
+  // For simplicity, we only run through a pair of representative scenarios here
+  // and rely on runner.go to verify that |SSL_get0_ech_name_override| behaves
+  // correctly.
+  for (bool accept_ech : {false, true}) {
+    SCOPED_TRACE(accept_ech);
+    for (bool use_leaf_secret : {false, true}) {
+      SCOPED_TRACE(use_leaf_secret);
+
+      // The server will reject ECH when configured with the wrong keys.
+      ASSERT_TRUE(SSL_CTX_set1_ech_keys(
+          server_ctx.get(), accept_ech ? keys.get() : wrong_keys.get()));
+
+      bssl::UniquePtr<SSL> client, server;
+      ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(),
+                                        server_ctx.get()));
+      ASSERT_TRUE(InstallECHConfigList(client.get(), keys.get()));
+
+      // Configure the server with the selected certificate.
+      ASSERT_TRUE(SSL_use_certificate(server.get(), use_leaf_secret
+                                                        ? leaf_secret.get()
+                                                        : leaf_public.get()));
+      ASSERT_TRUE(SSL_use_PrivateKey(server.get(), leaf_key.get()));
+
+      // The handshake may fail due to name mismatch or ECH reject. We check
+      // |SSL_get_verify_result| to confirm the handshake got far enough.
+      CompleteHandshakes(client.get(), server.get());
+      EXPECT_EQ(accept_ech == use_leaf_secret ? X509_V_OK
+                                              : X509_V_ERR_HOSTNAME_MISMATCH,
+                SSL_get_verify_result(client.get()));
+    }
+  }
+}
+
 #if defined(OPENSSL_THREADS)
 // Test that the server ECH config can be swapped out while the |SSL_CTX| is
 // in use on other threads. This test is intended to be run with TSan.

ssl/ssl_x509.cc

@@ -368,25 +368,33 @@ static bool ssl_crypto_x509_session_verify_cert_chain(SSL_SESSION *session,
     return false;
   }
 
-  SSL_CTX *ssl_ctx = hs->ssl->ctx.get();
+  SSL *const ssl = hs->ssl;
+  SSL_CTX *ssl_ctx = ssl->ctx.get();
   X509_STORE *verify_store = ssl_ctx->cert_store;
   if (hs->config->cert->verify_store != nullptr) {
     verify_store = hs->config->cert->verify_store;
   }
 
   X509 *leaf = sk_X509_value(cert_chain, 0);
+  const char *name;
+  size_t name_len;
+  SSL_get0_ech_name_override(ssl, &name, &name_len);
   ScopedX509_STORE_CTX ctx;
   if (!X509_STORE_CTX_init(ctx.get(), verify_store, leaf, cert_chain) ||
-      !X509_STORE_CTX_set_ex_data(
-          ctx.get(), SSL_get_ex_data_X509_STORE_CTX_idx(), hs->ssl) ||
+      !X509_STORE_CTX_set_ex_data(ctx.get(),
+                                  SSL_get_ex_data_X509_STORE_CTX_idx(), ssl) ||
       // We need to inherit the verify parameters. These can be determined by
       // the context: if its a server it will verify SSL client certificates or
       // vice versa.
-      !X509_STORE_CTX_set_default(
-          ctx.get(), hs->ssl->server ? "ssl_client" : "ssl_server") ||
+      !X509_STORE_CTX_set_default(ctx.get(),
+                                  ssl->server ? "ssl_client" : "ssl_server") ||
       // Anything non-default in "param" should overwrite anything in the ctx.
       !X509_VERIFY_PARAM_set1(X509_STORE_CTX_get0_param(ctx.get()),
-                              hs->config->param)) {
+                              hs->config->param) ||
+      // ClientHelloOuter connections use a different name.
+      (name_len != 0 &&
+       !X509_VERIFY_PARAM_set1_host(X509_STORE_CTX_get0_param(ctx.get()), name,
+                                    name_len))) {
     OPENSSL_PUT_ERROR(SSL, ERR_R_X509_LIB);
     return false;
   }

ssl/test/bssl_shim.cc

@@ -809,9 +809,44 @@ static bool DoConnection(bssl::UniquePtr<SSL_SESSION> *out_session,
     }
 
     assert(!config->handoff);
+    config = retry_config;
     ret = DoExchange(out_session, &ssl, retry_config, is_resume, true, writer);
   }
 
+  // An ECH rejection appears as a failed connection. Note |ssl| may use a
+  // different config on ECH rejection.
+  if (config->expect_no_ech_retry_configs ||
+      !config->expect_ech_retry_configs.empty()) {
+    bssl::Span<const uint8_t> expected =
+        config->expect_no_ech_retry_configs
+            ? bssl::Span<const uint8_t>()
+            : bssl::MakeConstSpan(reinterpret_cast<const uint8_t *>(
+                                      config->expect_ech_retry_configs.data()),
+                                  config->expect_ech_retry_configs.size());
+    if (ret) {
+      fprintf(stderr, "Expected ECH rejection, but connection succeeded.\n");
+      return false;
+    }
+    uint32_t err = ERR_peek_error();
+    if (SSL_get_error(ssl.get(), -1) != SSL_ERROR_SSL ||
+        ERR_GET_LIB(err) != ERR_LIB_SSL ||
+        ERR_GET_REASON(err) != SSL_R_ECH_REJECTED) {
+      fprintf(stderr, "Expected ECH rejection, but connection succeeded.\n");
+      return false;
+    }
+    const uint8_t *retry_configs;
+    size_t retry_configs_len;
+    SSL_get0_ech_retry_configs(ssl.get(), &retry_configs, &retry_configs_len);
+    if (bssl::MakeConstSpan(retry_configs, retry_configs_len) != expected) {
+      fprintf(stderr, "ECH retry configs did not match expectations.\n");
+      // Clear the error queue. Otherwise |SSL_R_ECH_REJECTED| will be printed
+      // to stderr and the test framework will think the test had the expected
+      // expectations.
+      ERR_clear_error();
+      return false;
+    }
+  }
+
   if (!ret) {
     // Print the |SSL_get_error| code. Otherwise, some failures are silent and
     // hard to debug.

ssl/test/runner/alert.go

@@ -45,6 +45,7 @@ const (
 	alertUnknownPSKIdentity           alert = 115
 	alertCertificateRequired          alert = 116
 	alertNoApplicationProtocol        alert = 120
+	alertECHRequired                  alert = 121
 )
 
 var alertText = map[alert]string{
@@ -78,6 +79,7 @@ var alertText = map[alert]string{
 	alertUnknownPSKIdentity:           "unknown PSK identity",
 	alertCertificateRequired:          "certificate required",
 	alertNoApplicationProtocol:        "no application protocol",
+	alertECHRequired:                  "ECH required",
 }
 
 func (e alert) String() string {

ssl/test/runner/common.go

@@ -1677,6 +1677,10 @@ type ProtocolBugs struct {
 	// invalid Channel ID signature.
 	InvalidChannelIDSignature bool
 
+	// AlwaysNegotiateChannelID, if true, causes the server to negotiate Channel
+	// ID, even whenn the client does not offer it.
+	AlwaysNegotiateChannelID bool
+
 	// ExpectGREASE, if true, causes messages without GREASE values to be
 	// rejected. See RFC 8701.
 	ExpectGREASE bool

ssl/test/runner/handshake_server.go

@@ -1650,7 +1650,7 @@ func (hs *serverHandshakeState) processClientExtensions(serverExtensions *server
 		serverExtensions.extendedMasterSecret = hs.clientHello.extendedMasterSecret && !disableEMS
 	}
 
-	if hs.clientHello.channelIDSupported && config.RequestChannelID {
+	if config.Bugs.AlwaysNegotiateChannelID || (hs.clientHello.channelIDSupported && config.RequestChannelID) {
 		serverExtensions.channelIDRequested = true
 	}
 

ssl/test/runner/runner.go

@@ -17708,28 +17708,30 @@ func addEncryptedClientHelloTests() {
 		})
 
 		// Test the client can recognize when ECH is rejected.
-		// TODO(https://crbug.com/boringssl/275): Once implemented, this
-		// handshake should complete.
 		testCases = append(testCases, testCase{
 			testType: clientTest,
 			protocol: protocol,
 			name:     prefix + "ECH-Client-Reject",
 			config: Config{
+				ServerECHConfigs: []ServerECHConfig{echConfig2, echConfig3},
 				Bugs: ProtocolBugs{
 					ExpectServerName: "public.example",
 				},
 			},
 			flags: []string{
 				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-ech-retry-configs", base64FlagValue(CreateECHConfigList(echConfig2.ECHConfig.Raw, echConfig3.ECHConfig.Raw)),
 			},
-			shouldFail:    true,
-			expectedError: ":CONNECTION_REJECTED:",
+			shouldFail:         true,
+			expectedLocalError: "remote error: ECH required",
+			expectedError:      ":ECH_REJECTED:",
 		})
 		testCases = append(testCases, testCase{
 			testType: clientTest,
 			protocol: protocol,
 			name:     prefix + "ECH-Client-Reject-HelloRetryRequest",
 			config: Config{
+				ServerECHConfigs: []ServerECHConfig{echConfig2, echConfig3},
 				CurvePreferences: []CurveID{CurveP384},
 				Bugs: ProtocolBugs{
 					ExpectServerName:      "public.example",
@@ -17738,10 +17740,29 @@ func addEncryptedClientHelloTests() {
 			},
 			flags: []string{
 				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-ech-retry-configs", base64FlagValue(CreateECHConfigList(echConfig2.ECHConfig.Raw, echConfig3.ECHConfig.Raw)),
 				"-expect-hrr", // Check we triggered HRR.
 			},
-			shouldFail:    true,
-			expectedError: ":CONNECTION_REJECTED:",
+			shouldFail:         true,
+			expectedLocalError: "remote error: ECH required",
+			expectedError:      ":ECH_REJECTED:",
+		})
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-Reject-NoRetryConfigs",
+			config: Config{
+				Bugs: ProtocolBugs{
+					ExpectServerName: "public.example",
+				},
+			},
+			flags: []string{
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-no-ech-retry-configs",
+			},
+			shouldFail:         true,
+			expectedLocalError: "remote error: ECH required",
+			expectedError:      ":ECH_REJECTED:",
 		})
 		if protocol != quic {
 			testCases = append(testCases, testCase{
@@ -17751,19 +17772,79 @@ func addEncryptedClientHelloTests() {
 				config: Config{
 					MaxVersion: VersionTLS12,
 					Bugs: ProtocolBugs{
-						ExpectServerName:      "public.example",
-						ExpectMissingKeyShare: true, // Check we triggered HRR.
+						ExpectServerName: "public.example",
 					},
 				},
 				flags: []string{
 					"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
-					"-expect-hrr", // Check we triggered HRR.
+					// TLS 1.2 cannot provide retry configs.
+					"-expect-no-ech-retry-configs",
 				},
-				shouldFail:    true,
-				expectedError: ":CONNECTION_REJECTED:",
+				shouldFail:         true,
+				expectedLocalError: "remote error: ECH required",
+				expectedError:      ":ECH_REJECTED:",
+			})
+
+			// Test that the client disables False Start when ECH is rejected.
+			testCases = append(testCases, testCase{
+				name: prefix + "ECH-Client-Reject-TLS12-NoFalseStart",
+				config: Config{
+					MaxVersion:   VersionTLS12,
+					CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
+					NextProtos:   []string{"foo"},
+					Bugs: ProtocolBugs{
+						// The options below cause the server to, immediately
+						// after client Finished, send an alert and try to read
+						// application data without sending server Finished.
+						ExpectFalseStart:          true,
+						AlertBeforeFalseStartTest: alertAccessDenied,
+					},
+				},
+				flags: []string{
+					"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+					"-false-start",
+					"-advertise-alpn", "\x03foo",
+					"-expect-alpn", "foo",
+				},
+				shimWritesFirst: true,
+				shouldFail:      true,
+				// Ensure the client does not send application data at the False
+				// Start point. EOF comes from the client closing the connection
+				// in response ot the alert.
+				expectedLocalError: "tls: peer did not false start: EOF",
+				// Ensures the client picks up the alert before reporting an
+				// authenticated |SSL_R_ECH_REJECTED|.
+				expectedError: ":TLSV1_ALERT_ACCESS_DENIED:",
 			})
 		}
 
+		// Test that unsupported retry configs in a valid ECHConfigList are
+		// allowed. They will be skipped when configured in the retry.
+		retryConfigs := CreateECHConfigList(
+			unsupportedVersion,
+			unsupportedKEM.Raw,
+			unsupportedCipherSuites.Raw,
+			unsupportedMandatoryExtension.Raw,
+			echConfig2.ECHConfig.Raw)
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-Reject-UnsupportedRetryConfigs",
+			config: Config{
+				Bugs: ProtocolBugs{
+					SendECHRetryConfigs: retryConfigs,
+					ExpectServerName:    "public.example",
+				},
+			},
+			flags: []string{
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-ech-retry-configs", base64FlagValue(retryConfigs),
+			},
+			shouldFail:         true,
+			expectedLocalError: "remote error: ECH required",
+			expectedError:      ":ECH_REJECTED:",
+		})
+
 		// Test that the client rejects ClientHelloOuter handshakes that attempt
 		// to resume the ClientHelloInner's ticket. In draft-ietf-tls-esni-10,
 		// the confirmation signal is computed in an odd order, so this requires
@@ -17771,7 +17852,7 @@ func addEncryptedClientHelloTests() {
 		testCases = append(testCases, testCase{
 			testType: clientTest,
 			protocol: protocol,
-			name:     prefix + "ECH-Client-Reject-ResumeInnerSession",
+			name:     prefix + "ECH-Client-Reject-ResumeInnerSession-TLS13",
 			config: Config{
 				ServerECHConfigs: []ServerECHConfig{echConfig},
 				Bugs: ProtocolBugs{
@@ -17779,6 +17860,7 @@ func addEncryptedClientHelloTests() {
 				},
 			},
 			resumeConfig: &Config{
+				MaxVersion:       VersionTLS13,
 				ServerECHConfigs: []ServerECHConfig{echConfig},
 				Bugs: ProtocolBugs{
 					ExpectServerName:                    "public.example",
@@ -17797,11 +17879,54 @@ func addEncryptedClientHelloTests() {
 			resumeExpectations: &connectionExpectations{echAccepted: false},
 		})
 
+		// Test the above, but the server now attempts to resume the
+		// ClientHelloInner's ticket at TLS 1.2.
+		if protocol != quic {
+			testCases = append(testCases, testCase{
+				testType: clientTest,
+				protocol: protocol,
+				name:     prefix + "ECH-Client-Reject-ResumeInnerSession-TLS12",
+				config: Config{
+					ServerECHConfigs: []ServerECHConfig{echConfig},
+					Bugs: ProtocolBugs{
+						ExpectServerName: "secret.example",
+					},
+				},
+				resumeConfig: &Config{
+					MinVersion:       VersionTLS12,
+					MaxVersion:       VersionTLS12,
+					ServerECHConfigs: []ServerECHConfig{echConfig},
+					Bugs: ProtocolBugs{
+						ExpectServerName:                    "public.example",
+						UseInnerSessionWithClientHelloOuter: true,
+						// The client only ever offers TLS 1.3 sessions in
+						// ClientHelloInner. AcceptAnySession allows them to be
+						// resumed at TLS 1.2.
+						AcceptAnySession: true,
+					},
+				},
+				resumeSession: true,
+				flags: []string{
+					"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+					"-host-name", "secret.example",
+					"-on-initial-expect-ech-accept",
+				},
+				// From the client's perspective, the server echoed a session ID to
+				// signal resumption, but the selected ClientHello had nothing to
+				// resume.
+				shouldFail:         true,
+				expectedError:      ":SERVER_ECHOED_INVALID_SESSION_ID:",
+				expectedLocalError: "remote error: illegal parameter",
+				expectations:       connectionExpectations{echAccepted: true},
+				resumeExpectations: &connectionExpectations{echAccepted: false},
+			})
+		}
+
 		// Test that the client can process ECH rejects after an early data reject.
 		testCases = append(testCases, testCase{
 			testType: clientTest,
 			protocol: protocol,
-			name:     prefix + "ECH-Client-Reject-EarlyDataReject",
+			name:     prefix + "ECH-Client-Reject-EarlyDataRejected",
 			config: Config{
 				ServerECHConfigs: []ServerECHConfig{echConfig},
 				Bugs: ProtocolBugs{
@@ -17809,6 +17934,7 @@ func addEncryptedClientHelloTests() {
 				},
 			},
 			resumeConfig: &Config{
+				ServerECHConfigs: []ServerECHConfig{echConfig2},
 				Bugs: ProtocolBugs{
 					ExpectServerName: "public.example",
 				},
@@ -17819,6 +17945,10 @@ func addEncryptedClientHelloTests() {
 				// Although the resumption connection does not accept ECH, the
 				// API will report ECH was accepted at the 0-RTT point.
 				"-expect-ech-accept",
+				// -on-retry refers to the retried handshake after 0-RTT reject,
+				// while ech-retry-configs refers to the ECHConfigs to use in
+				// the next connection attempt.
+				"-on-retry-expect-ech-retry-configs", base64FlagValue(CreateECHConfigList(echConfig2.ECHConfig.Raw)),
 			},
 			resumeSession:           true,
 			expectResumeRejected:    true,
@@ -17826,16 +17956,15 @@ func addEncryptedClientHelloTests() {
 			expectEarlyDataRejected: true,
 			expectations:            connectionExpectations{echAccepted: true},
 			resumeExpectations:      &connectionExpectations{echAccepted: false},
-			// TODO(https://crbug.com/boringssl/275): Once implemented, this
-			// should complete the handshake.
-			shouldFail:    true,
-			expectedError: ":CONNECTION_REJECTED:",
+			shouldFail:              true,
+			expectedLocalError:      "remote error: ECH required",
+			expectedError:           ":ECH_REJECTED:",
 		})
 		if protocol != quic {
 			testCases = append(testCases, testCase{
 				testType: clientTest,
 				protocol: protocol,
-				name:     prefix + "ECH-Client-Reject-EarlyDataReject-TLS12",
+				name:     prefix + "ECH-Client-Reject-EarlyDataRejected-TLS12",
 				config: Config{
 					ServerECHConfigs: []ServerECHConfig{echConfig},
 					Bugs: ProtocolBugs{
@@ -17911,6 +18040,266 @@ func addEncryptedClientHelloTests() {
 			},
 			expectations: connectionExpectations{echAccepted: true},
 		})
+
+		// Test both sync and async mode, to test both with and without the
+		// client certificate callback.
+		for _, async := range []bool{false, true} {
+			var flags []string
+			var suffix string
+			if async {
+				flags = []string{"-async"}
+				suffix = "-Async"
+			}
+
+			// Test that ECH and client certificates can be used together.
+			testCases = append(testCases, testCase{
+				testType: clientTest,
+				protocol: protocol,
+				name:     prefix + "ECH-Client-ClientCertificate" + suffix,
+				config: Config{
+					ServerECHConfigs: []ServerECHConfig{echConfig},
+					ClientAuth:       RequireAnyClientCert,
+				},
+				flags: append([]string{
+					"-cert-file", path.Join(*resourceDir, rsaCertificateFile),
+					"-key-file", path.Join(*resourceDir, rsaKeyFile),
+					"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+					"-expect-ech-accept",
+				}, flags...),
+				expectations: connectionExpectations{echAccepted: true},
+			})
+
+			// Test that, when ECH is rejected, the client does not send a client
+			// certificate.
+			testCases = append(testCases, testCase{
+				testType: clientTest,
+				protocol: protocol,
+				name:     prefix + "ECH-Client-Reject-NoClientCertificate-TLS13" + suffix,
+				config: Config{
+					MinVersion: VersionTLS13,
+					MaxVersion: VersionTLS13,
+					ClientAuth: RequireAnyClientCert,
+				},
+				flags: append([]string{
+					"-cert-file", path.Join(*resourceDir, rsaCertificateFile),
+					"-key-file", path.Join(*resourceDir, rsaKeyFile),
+					"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				}, flags...),
+				shouldFail:         true,
+				expectedLocalError: "tls: client didn't provide a certificate",
+			})
+			if protocol != quic {
+				testCases = append(testCases, testCase{
+					testType: clientTest,
+					protocol: protocol,
+					name:     prefix + "ECH-Client-Reject-NoClientCertificate-TLS12" + suffix,
+					config: Config{
+						MinVersion: VersionTLS12,
+						MaxVersion: VersionTLS12,
+						ClientAuth: RequireAnyClientCert,
+					},
+					flags: append([]string{
+						"-cert-file", path.Join(*resourceDir, rsaCertificateFile),
+						"-key-file", path.Join(*resourceDir, rsaKeyFile),
+						"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+					}, flags...),
+					shouldFail:         true,
+					expectedLocalError: "tls: client didn't provide a certificate",
+				})
+			}
+		}
+
+		// Test that ECH and Channel ID can be used together.
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-ChannelID",
+			config: Config{
+				ServerECHConfigs: []ServerECHConfig{echConfig},
+				RequestChannelID: true,
+			},
+			flags: []string{
+				"-send-channel-id", path.Join(*resourceDir, channelIDKeyFile),
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-ech-accept",
+			},
+			resumeSession: true,
+			expectations: connectionExpectations{
+				channelID:   true,
+				echAccepted: true,
+			},
+		})
+
+		// Handshakes where ECH is rejected do not offer or accept Channel ID.
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-Reject-NoChannelID-TLS13",
+			config: Config{
+				MinVersion: VersionTLS13,
+				MaxVersion: VersionTLS13,
+				Bugs: ProtocolBugs{
+					AlwaysNegotiateChannelID: true,
+				},
+			},
+			flags: []string{
+				"-send-channel-id", path.Join(*resourceDir, channelIDKeyFile),
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+			},
+			shouldFail:         true,
+			expectedLocalError: "remote error: unsupported extension",
+			expectedError:      ":UNEXPECTED_EXTENSION:",
+		})
+		if protocol != quic {
+			testCases = append(testCases, testCase{
+				testType: clientTest,
+				protocol: protocol,
+				name:     prefix + "ECH-Client-Reject-NoChannelID-TLS12",
+				config: Config{
+					MinVersion: VersionTLS12,
+					MaxVersion: VersionTLS12,
+					Bugs: ProtocolBugs{
+						AlwaysNegotiateChannelID: true,
+					},
+				},
+				flags: []string{
+					"-send-channel-id", path.Join(*resourceDir, channelIDKeyFile),
+					"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				},
+				shouldFail:         true,
+				expectedLocalError: "remote error: unsupported extension",
+				expectedError:      ":UNEXPECTED_EXTENSION:",
+			})
+		}
+
+		// Test that ECH correctly overrides the host name for certificate
+		// verification.
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-NotOffered-NoOverrideName",
+			flags: []string{
+				"-verify-peer",
+				"-use-custom-verify-callback",
+				// When not offering ECH, verify the usual name in both full
+				// and resumption handshakes.
+				"-reverify-on-resume",
+				"-expect-no-ech-name-override",
+			},
+			resumeSession: true,
+		})
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-GREASE-NoOverrideName",
+			flags: []string{
+				"-verify-peer",
+				"-use-custom-verify-callback",
+				"-enable-ech-grease",
+				// When offering ECH GREASE, verify the usual name in both full
+				// and resumption handshakes.
+				"-reverify-on-resume",
+				"-expect-no-ech-name-override",
+			},
+			resumeSession: true,
+		})
+		if protocol != quic {
+			testCases = append(testCases, testCase{
+				testType: clientTest,
+				protocol: protocol,
+				name:     prefix + "ECH-Client-Rejected-OverrideName-TLS12",
+				config: Config{
+					MinVersion: VersionTLS12,
+					MaxVersion: VersionTLS12,
+				},
+				flags: []string{
+					"-verify-peer",
+					"-use-custom-verify-callback",
+					"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+					// When ECH is rejected, verify the public name. This can
+					// only happen in full handshakes.
+					"-expect-ech-name-override", "public.example",
+				},
+				shouldFail:         true,
+				expectedError:      ":ECH_REJECTED:",
+				expectedLocalError: "remote error: ECH required",
+			})
+		}
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-Reject-OverrideName-TLS13",
+			config: Config{
+				MinVersion: VersionTLS13,
+				MaxVersion: VersionTLS13,
+			},
+			flags: []string{
+				"-verify-peer",
+				"-use-custom-verify-callback",
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				// When ECH is rejected, verify the public name. This can
+				// only happen in full handshakes.
+				"-expect-ech-name-override", "public.example",
+			},
+			shouldFail:         true,
+			expectedError:      ":ECH_REJECTED:",
+			expectedLocalError: "remote error: ECH required",
+		})
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-Accept-NoOverrideName",
+			config: Config{
+				ServerECHConfigs: []ServerECHConfig{echConfig},
+			},
+			flags: []string{
+				"-verify-peer",
+				"-use-custom-verify-callback",
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				"-expect-ech-accept",
+				// When ECH is accepted, verify the usual name in both full and
+				// resumption handshakes.
+				"-reverify-on-resume",
+				"-expect-no-ech-name-override",
+			},
+			resumeSession: true,
+			expectations:  connectionExpectations{echAccepted: true},
+		})
+		testCases = append(testCases, testCase{
+			testType: clientTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Client-Reject-EarlyDataRejected-OverrideNameOnRetry",
+			config: Config{
+				ServerECHConfigs: []ServerECHConfig{echConfig},
+			},
+			resumeConfig: &Config{},
+			flags: []string{
+				"-verify-peer",
+				"-use-custom-verify-callback",
+				"-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+				// Although the resumption connection does not accept ECH, the
+				// API will report ECH was accepted at the 0-RTT point.
+				"-expect-ech-accept",
+				// The resumption connection verifies certificates twice. First,
+				// if reverification is enabled, we verify the 0-RTT certificate
+				// as if ECH as accepted. There should be no name override.
+				// Next, on the post-0-RTT-rejection retry, we verify the new
+				// server certificate. This picks up the ECH reject, so it
+				// should use public.example.
+				"-reverify-on-resume",
+				"-on-resume-expect-no-ech-name-override",
+				"-on-retry-expect-ech-name-override", "public.example",
+			},
+			resumeSession:           true,
+			expectResumeRejected:    true,
+			earlyData:               true,
+			expectEarlyDataRejected: true,
+			expectations:            connectionExpectations{echAccepted: true},
+			resumeExpectations:      &connectionExpectations{echAccepted: false},
+			shouldFail:              true,
+			expectedError:           ":ECH_REJECTED:",
+			expectedLocalError:      "remote error: ECH required",
+		})
 	}
 }
 

ssl/test/test_config.cc

@@ -60,6 +60,8 @@ const Flag<bool> kBoolFlags[] = {
     {"-fallback-scsv", &TestConfig::fallback_scsv},
     {"-enable-ech-grease", &TestConfig::enable_ech_grease},
     {"-expect-ech-accept", &TestConfig::expect_ech_accept},
+    {"-expect-no-ech-name-override", &TestConfig::expect_no_ech_name_override},
+    {"-expect-no-ech-retry-configs", &TestConfig::expect_no_ech_retry_configs},
     {"-require-any-client-certificate",
      &TestConfig::require_any_client_certificate},
     {"-false-start", &TestConfig::false_start},
@@ -167,6 +169,7 @@ const Flag<std::string> kStringFlags[] = {
     {"-key-file", &TestConfig::key_file},
     {"-cert-file", &TestConfig::cert_file},
     {"-expect-server-name", &TestConfig::expect_server_name},
+    {"-expect-ech-name-override", &TestConfig::expect_ech_name_override},
     {"-advertise-npn", &TestConfig::advertise_npn},
     {"-expect-next-proto", &TestConfig::expect_next_proto},
     {"-select-next-proto", &TestConfig::select_next_proto},
@@ -201,6 +204,7 @@ const Flag<std::unique_ptr<std::string>> kOptionalStringFlags[] = {
 };
 
 const Flag<std::string> kBase64Flags[] = {
+    {"-expect-ech-retry-configs", &TestConfig::expect_ech_retry_configs},
     {"-ech-config-list", &TestConfig::ech_config_list},
     {"-expect-certificate-types", &TestConfig::expect_certificate_types},
     {"-expect-channel-id", &TestConfig::expect_channel_id},
@@ -774,6 +778,20 @@ static bool CheckVerifyCallback(SSL *ssl) {
     }
   }
 
+  const char *name_override;
+  size_t name_override_len;
+  SSL_get0_ech_name_override(ssl, &name_override, &name_override_len);
+  if (config->expect_no_ech_name_override && name_override_len != 0) {
+    fprintf(stderr, "Unexpected ECH name override.\n");
+    return false;
+  }
+  if (!config->expect_ech_name_override.empty() &&
+      config->expect_ech_name_override !=
+          std::string(name_override, name_override_len)) {
+    fprintf(stderr, "ECH name did not match expected value.\n");
+    return false;
+  }
+
   if (GetTestState(ssl)->cert_verified) {
     fprintf(stderr, "Certificate verified twice.\n");
     return false;

ssl/test/test_config.h

@@ -44,6 +44,10 @@ struct TestConfig {
   std::vector<std::string> ech_server_keys;
   std::vector<int> ech_is_retry_config;
   bool expect_ech_accept = false;
+  std::string expect_ech_name_override;
+  bool expect_no_ech_name_override = false;
+  std::string expect_ech_retry_configs;
+  bool expect_no_ech_retry_configs = false;
   std::string ech_config_list;
   std::string expect_certificate_types;
   bool require_any_client_certificate = false;

ssl/tls13_client.cc

@@ -476,7 +476,7 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
                       ssl->s3->server_random + sizeof(ssl->s3->server_random) -
                           sizeof(ech_confirmation),
                       sizeof(ech_confirmation)) == 0) {
-      ssl->s3->ech_accept = true;
+      ssl->s3->ech_status = ssl_ech_accepted;
       hs->transcript = std::move(hs->inner_transcript);
       hs->extensions.sent = hs->inner_extensions_sent;
       // Report the inner random value through |SSL_get_client_random|.
@@ -489,13 +489,7 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
         ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
         return ssl_hs_error;
       }
-
-      // TODO(https://crbug.com/boringssl/275): If the server declines ECH, we
-      // handshake with ClientHelloOuter instead of ClientHelloInner. That path
-      // is not yet implemented. For now, terminate the handshake with a
-      // distiguisable error for testing.
-      OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED);
-      return ssl_hs_error;
+      ssl->s3->ech_status = ssl_ech_rejected;
     }
   }
 
@@ -555,7 +549,7 @@ static enum ssl_hs_wait_t do_read_encrypted_extensions(SSL_HANDSHAKE *hs) {
     // If offering ECH, the server may not accept early data with
     // ClientHelloOuter. We do not offer sessions with ClientHelloOuter, so this
     // this should be implied by checking |session_reused|.
-    assert(hs->selected_ech_config == nullptr || ssl->s3->ech_accept);
+    assert(ssl->s3->ech_status != ssl_ech_rejected);
 
     if (hs->early_session->cipher != hs->new_session->cipher) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_CIPHER_MISMATCH_ON_EARLY_DATA);
@@ -835,8 +829,12 @@ static enum ssl_hs_wait_t do_send_client_certificate(SSL_HANDSHAKE *hs) {
     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);

ssl/tls13_server.cc

@@ -601,7 +601,7 @@ static enum ssl_hs_wait_t do_read_second_client_hello(SSL_HANDSHAKE *hs) {
     return ssl_hs_error;
   }
 
-  if (ssl->s3->ech_accept) {
+  if (ssl->s3->ech_status == ssl_ech_accepted) {
     // If we previously accepted the ClientHelloInner, check that the second
     // ClientHello contains an encrypted_client_hello extension.
     CBS ech_body;
@@ -761,7 +761,7 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) {
     return ssl_hs_error;
   }
 
-  assert(!ssl->s3->ech_accept || hs->ech_is_inner_present);
+  assert(ssl->s3->ech_status != ssl_ech_accepted || hs->ech_is_inner_present);
   if (hs->ech_is_inner_present) {
     // Fill in the ECH confirmation signal.
     Span<uint8_t> random_suffix =