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>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 9f95a06..10898a9 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4617,6 +4617,16 @@
// 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
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index afa0927..508f9bf 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -613,6 +613,10 @@
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 @@
}
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 @@
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);
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 3a8caa1..6bdd664 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -1965,6 +1965,9 @@
// 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() {
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index a0a81f9..a7ab0e2 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -583,6 +583,10 @@
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
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 3aab605..18ed78b 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -17343,6 +17343,56 @@
},
})
+ // 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{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 24b1bdd..dfe1cb4 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -354,6 +354,7 @@
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 @@
&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 @@
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,
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 89656d1..607f58d 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -118,6 +118,7 @@
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 @@
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;
