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 <[email protected]>
Reviewed-by: Adam Langley <[email protected]>

Author: davidben

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;