Implement ClientHelloOuter handshakes.
If a client offers ECH, but the server rejects it, the client completes
the handshake with ClientHelloOuter in order to authenticate retry keys.
Implement this flow. This is largely allowing the existing handshake to
proceed, but with some changes:
- Certificate verification uses the other name. This CL routes this up to
the built-in verifier and adds SSL_get0_ech_name_override for the
callback.
- We need to disable False Start to pick up server Finished in TLS 1.2.
- Client certificates, notably in TLS 1.3 where they're encrypted,
should only be revealed to the true server. Fortunately, not sending
client certs is always an option, so do that.
Channel ID has a similar issue. I've just omitted the extension in
ClientHelloOuter because it's deprecated and is unlikely to be used
with ECH at this point. ALPS may be worth some pondering but, the way
it's currently used, is not sensitive.
(Possibly we should change the draft to terminate the handshake before
even sending that flight...)
- The session is never offered in ClientHelloOuter, but our internal
book-keeping doesn't quite notice.
I had to replace ech_accept with a tri-state ech_status to correctly
handle an edge case in SSL_get0_ech_name_override: when ECH + 0-RTT +
reverify_on_resume are all enabled, the first certificate verification
is for the 0-RTT session and should be against the true name, yet we
have selected_ech_config && !ech_accept. A tri-state tracks when ECH is
actually rejected. I've maintained this on the server as well, though
the server never actually cares.
Bug: 275
Change-Id: Ie55966ca3dc4ffcc8c381479f0fe9bcacd34d0f8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48135
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 4bb9c32..76f88c7 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -1691,14 +1691,13 @@
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 @@
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.
