Add ECH fallback API

This commit solves
https://bugs.chromium.org/p/boringssl/issues/detail?id=714. To
summarize, there are cases where servers will advertise ECH on hostnames
that may, in practice, be unable to actually negotiate e.g. TLS 1.3. To
gracefully handle this case, this commit adds a new return value for the
select_cert_cb that signals to the server that ECH must be disabled. To
accomplish this, we slightly rewind the state machine to instead
handshake with ClientHelloOuter, and clear ech_keys on the handshake
state such that the server hello does not include any retry_configs in
EncryptedExtensions. Clients will take this as a signal that ECH is
disabled on the hostname, and that they should instead handshake without
ECH.

Bug: 42290593
Change-Id: I1806ba052ffbc3e5c46161a1596d125cc5e5a8fc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69087
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Reviewed-by: Bob Beck <bbe@google.com>

include/openssl/ssl.h

@@ -4617,6 +4617,16 @@ enum ssl_select_cert_result_t BORINGSSL_ENUM_INT {
   // ssl_select_cert_error indicates that a fatal error occured and the
   // handshake should be terminated.
   ssl_select_cert_error = -1,
+  // ssl_select_cert_disable_ech indicates that, although an encrypted
+  // ClientHelloInner was decrypted, it should be discarded. The certificate
+  // selection callback will then be called again, passing in the
+  // ClientHelloOuter instead. From there, the handshake will proceed
+  // without retry_configs, to signal to the client to disable ECH.
+  //
+  // This value may only be returned when |SSL_ech_accepted| returnes one. It
+  // may be useful if the ClientHelloInner indicated a service which does not
+  // support ECH, e.g. if it is a TLS-1.2 only service.
+  ssl_select_cert_disable_ech = -2,
 };
 
 // SSL_early_callback_ctx_extension_get searches the extensions in

ssl/handshake_server.cc

@@ -613,6 +613,10 @@ static bool extract_sni(SSL_HANDSHAKE *hs, uint8_t *out_alert,
   if (!ssl_client_hello_get_extension(client_hello, &sni,
                                       TLSEXT_TYPE_server_name)) {
     // No SNI extension to parse.
+    //
+    // Clear state in case we previously extracted SNI from ClientHelloOuter.
+    ssl->s3->hostname.reset();
+    hs->should_ack_sni = false;
     return true;
   }
 
@@ -686,7 +690,10 @@ static enum ssl_hs_wait_t do_read_client_hello(SSL_HANDSHAKE *hs) {
   }
 
   uint8_t alert = SSL_AD_DECODE_ERROR;
-  if (!decrypt_ech(hs, &alert, &client_hello)) {
+  // We check for rejection status in case we've rewound the state machine after
+  // determining `ClientHelloInner` is invalid.
+  if (ssl->s3->ech_status != ssl_ech_rejected &&
+      !decrypt_ech(hs, &alert, &client_hello)) {
     ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
     return ssl_hs_error;
   }
@@ -722,6 +729,13 @@ static enum ssl_hs_wait_t do_read_client_hello_after_ech(SSL_HANDSHAKE *hs) {
       case ssl_select_cert_retry:
         return ssl_hs_certificate_selection_pending;
 
+      case ssl_select_cert_disable_ech:
+        hs->ech_client_hello_buf.Reset();
+        hs->ech_keys = nullptr;
+        hs->state = state12_read_client_hello;
+        ssl->s3->ech_status = ssl_ech_rejected;
+        return ssl_hs_ok;
+
       case ssl_select_cert_error:
         // Connection rejected.
         OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED);

ssl/test/runner/common.go

@@ -1965,6 +1965,9 @@ type ProtocolBugs struct {
 	// EncryptSessionTicketKey, if non-nil, is the ticket key to use when
 	// encrypting tickets.
 	EncryptSessionTicketKey *[32]byte
+
+	// OmitPublicName omits the server name extension from ClientHelloOuter.
+	OmitPublicName bool
 }
 
 func (c *Config) serverInit() {

ssl/test/runner/handshake_client.go

@@ -583,6 +583,10 @@ func (hs *clientHandshakeState) createClientHello(innerHello *clientHelloMsg, ec
 		hello.serverName = c.config.ServerName
 	}
 
+	if !isInner && c.config.Bugs.OmitPublicName {
+		hello.serverName = ""
+	}
+
 	disableEMS := c.config.Bugs.NoExtendedMasterSecret
 	if c.cipherSuite != nil {
 		disableEMS = c.config.Bugs.NoExtendedMasterSecretOnRenegotiation

ssl/test/runner/runner.go

@@ -17343,6 +17343,56 @@ write hs 4
 			},
 		})
 
+		// Test that we successfully rewind the TLS state machine and disable ECH in the
+		// case that the select_cert_cb signals that ECH is not possible for the SNI in
+		// ClientHelloInner.
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Server-FailCallbackNeedRewind",
+			config: Config{
+				ServerName:      "secret.example",
+				ClientECHConfig: echConfig.ECHConfig,
+			},
+			flags: []string{
+				"-async",
+				"-fail-early-callback-ech-rewind",
+				"-ech-server-config", base64FlagValue(echConfig.ECHConfig.Raw),
+				"-ech-server-key", base64FlagValue(echConfig.Key),
+				"-ech-is-retry-config", "1",
+				"-expect-server-name", "public.example",
+			},
+			expectations: connectionExpectations{
+				echAccepted: false,
+			},
+		})
+
+		// Test that we correctly handle falling back to a ClientHelloOuter with
+		// no SNI (public name).
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			protocol: protocol,
+			name:     prefix + "ECH-Server-RewindWithNoPublicName",
+			config: Config{
+				ServerName:      "secret.example",
+				ClientECHConfig: echConfig.ECHConfig,
+				Bugs: ProtocolBugs{
+					OmitPublicName: true,
+				},
+			},
+			flags: []string{
+				"-async",
+				"-fail-early-callback-ech-rewind",
+				"-ech-server-config", base64FlagValue(echConfig.ECHConfig.Raw),
+				"-ech-server-key", base64FlagValue(echConfig.Key),
+				"-ech-is-retry-config", "1",
+				"-expect-no-server-name",
+			},
+			expectations: connectionExpectations{
+				echAccepted: false,
+			},
+		})
+
 		// Test ECH-enabled server with two ECHConfigs can decrypt client's ECH when
 		// it uses the second ECHConfig.
 		testCases = append(testCases, testCase{

ssl/test/test_config.cc

@@ -354,6 +354,7 @@ const Flag<TestConfig> *FindFlag(const char *name) {
         BoolFlag("-implicit-handshake", &TestConfig::implicit_handshake),
         BoolFlag("-use-early-callback", &TestConfig::use_early_callback),
         BoolFlag("-fail-early-callback", &TestConfig::fail_early_callback),
+        BoolFlag("-fail-early-callback-ech-rewind", &TestConfig::fail_early_callback_ech_rewind),
         BoolFlag("-install-ddos-callback", &TestConfig::install_ddos_callback),
         BoolFlag("-fail-ddos-callback", &TestConfig::fail_ddos_callback),
         BoolFlag("-fail-cert-callback", &TestConfig::fail_cert_callback),
@@ -374,6 +375,8 @@ const Flag<TestConfig> *FindFlag(const char *name) {
                  &TestConfig::expect_reject_early_data),
         BoolFlag("-expect-no-offer-early-data",
                  &TestConfig::expect_no_offer_early_data),
+        BoolFlag("-expect-no-server-name",
+                 &TestConfig::expect_no_server_name),
         BoolFlag("-use-ticket-callback", &TestConfig::use_ticket_callback),
         BoolFlag("-renew-ticket", &TestConfig::renew_ticket),
         BoolFlag("-enable-early-data", &TestConfig::enable_early_data),
@@ -1581,9 +1584,23 @@ static enum ssl_select_cert_result_t SelectCertificateCallback(
   TestState *test_state = GetTestState(ssl);
   test_state->early_callback_called = true;
 
+  // Invoke the rewind before we sanity check SNI because we will
+  // end up calling the select_cert_cb twice with two different SNIs.
+  if (SSL_ech_accepted(ssl) && config->fail_early_callback_ech_rewind) {
+      return ssl_select_cert_disable_ech;
+  }
+
+  const char *server_name =
+      SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
+
+  if (config->expect_no_server_name && server_name != nullptr) {
+    fprintf(stderr,
+            "Expected no server name but got %s.\n",
+            server_name);
+    return ssl_select_cert_error;
+  }
+
   if (!config->expect_server_name.empty()) {
-    const char *server_name =
-        SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
     if (server_name == nullptr ||
         std::string(server_name) != config->expect_server_name) {
       fprintf(stderr,

ssl/test/test_config.h

@@ -118,6 +118,7 @@ struct TestConfig {
   bool implicit_handshake = false;
   bool use_early_callback = false;
   bool fail_early_callback = false;
+  bool fail_early_callback_ech_rewind = false;
   bool install_ddos_callback = false;
   bool fail_ddos_callback = false;
   bool fail_cert_callback = false;
@@ -134,6 +135,7 @@ struct TestConfig {
   bool expect_accept_early_data = false;
   bool expect_reject_early_data = false;
   bool expect_no_offer_early_data = false;
+  bool expect_no_server_name = false;
   bool use_ticket_callback = false;
   bool renew_ticket = false;
   bool enable_early_data = false;