From c3b373bf4f4b2e2fba2578d1d5b5fe04e410f7cb Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 6 Jun 2021 13:04:26 -0400 Subject: [PATCH] Rename SSL_ECH_SERVER_CONFIG_LIST to SSL_ECH_KEYS. The old name was really long and a bit tedious to type out. Bug: 275 Change-Id: Ie24ef811f9288e619148a2bed36ca34b67af0a3a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48001 Reviewed-by: Adam Langley --- fuzz/ssl_ctx_api.cc | 14 ++--- include/openssl/base.h | 2 +- include/openssl/ssl.h | 63 ++++++++----------- ssl/encrypted_client_hello.cc | 36 +++++------ ssl/handoff.cc | 3 +- ssl/handshake_server.cc | 6 +- ssl/internal.h | 29 +++++---- ssl/ssl_test.cc | 115 +++++++++++++++------------------- ssl/t1_lib.cc | 9 ++- ssl/test/fuzzer.h | 17 +++-- ssl/test/test_config.cc | 11 ++-- tool/server.cc | 15 +++-- 12 files changed, 142 insertions(+), 178 deletions(-) diff --git a/fuzz/ssl_ctx_api.cc b/fuzz/ssl_ctx_api.cc index 08b3e6f60..3739e872b 100644 --- a/fuzz/ssl_ctx_api.cc +++ b/fuzz/ssl_ctx_api.cc @@ -492,9 +492,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { SSL_CTX_set1_sigalgs_list(ctx, sigalgs.c_str()); }, [](SSL_CTX *ctx, CBS *cbs) { - bssl::UniquePtr config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - if (config_list == nullptr) { + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + if (keys == nullptr) { return; } uint8_t is_retry_config; @@ -504,11 +503,10 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { !CBS_get_u16_length_prefixed(cbs, &private_key)) { return; } - SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), is_retry_config, CBS_data(&ech_config), - CBS_len(&ech_config), CBS_data(&private_key), - CBS_len(&private_key)); - SSL_CTX_set1_ech_server_config_list(ctx, config_list.get()); + SSL_ECH_KEYS_add(keys.get(), is_retry_config, CBS_data(&ech_config), + CBS_len(&ech_config), CBS_data(&private_key), + CBS_len(&private_key)); + SSL_CTX_set1_ech_keys(ctx, keys.get()); }, }; diff --git a/include/openssl/base.h b/include/openssl/base.h index d37c20244..b486f169f 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -431,7 +431,7 @@ typedef struct spake2_ctx_st SPAKE2_CTX; typedef struct srtp_protection_profile_st SRTP_PROTECTION_PROFILE; typedef struct ssl_cipher_st SSL_CIPHER; typedef struct ssl_ctx_st SSL_CTX; -typedef struct ssl_ech_server_config_list_st SSL_ECH_SERVER_CONFIG_LIST; +typedef struct ssl_ech_keys_st SSL_ECH_KEYS; typedef struct ssl_method_st SSL_METHOD; typedef struct ssl_private_key_method_st SSL_PRIVATE_KEY_METHOD; typedef struct ssl_quic_method_st SSL_QUIC_METHOD; diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 036db8be5..191cf4b2e 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -3565,43 +3565,37 @@ OPENSSL_EXPORT const char *SSL_early_data_reason_string( // as part of this connection. OPENSSL_EXPORT void SSL_set_enable_ech_grease(SSL *ssl, int enable); -// SSL_ECH_SERVER_CONFIG_LIST_new returns a newly-allocated -// |SSL_ECH_SERVER_CONFIG_LIST| or NULL on error. -OPENSSL_EXPORT SSL_ECH_SERVER_CONFIG_LIST *SSL_ECH_SERVER_CONFIG_LIST_new(void); +// SSL_ECH_KEYS_new returns a newly-allocated |SSL_ECH_KEYS| or NULL on error. +OPENSSL_EXPORT SSL_ECH_KEYS *SSL_ECH_KEYS_new(void); -// SSL_ECH_SERVER_CONFIG_LIST_up_ref increments the reference count of |list|. -OPENSSL_EXPORT void SSL_ECH_SERVER_CONFIG_LIST_up_ref( - SSL_ECH_SERVER_CONFIG_LIST *list); +// SSL_ECH_KEYS_up_ref increments the reference count of |keys|. +OPENSSL_EXPORT void SSL_ECH_KEYS_up_ref(SSL_ECH_KEYS *keys); -// SSL_ECH_SERVER_CONFIG_LIST_free releases memory associated with |list|. -OPENSSL_EXPORT void SSL_ECH_SERVER_CONFIG_LIST_free( - SSL_ECH_SERVER_CONFIG_LIST *list); +// SSL_ECH_KEYS_free releases memory associated with |keys|. +OPENSSL_EXPORT void SSL_ECH_KEYS_free(SSL_ECH_KEYS *keys); -// SSL_ECH_SERVER_CONFIG_LIST_add appends an ECHConfig in |ech_config| and its -// corresponding private key in |private_key| to |list|. When |is_retry_config| +// SSL_ECH_KEYS_add appends an ECHConfig in |ech_config| and its +// corresponding private key in |private_key| to |keys|. When |is_retry_config| // is non-zero, this config will be returned to the client on configuration // mismatch. It returns one on success and zero on error. See also -// |SSL_CTX_set1_ech_server_config_list|. +// |SSL_CTX_set1_ech_keys|. // // This function should be called successively to register each ECHConfig in // decreasing order of preference. This configuration must be completed before -// setting |list| on an |SSL_CTX| with |SSL_CTX_set1_ech_server_config_list|. -// After that point, |list| is immutable; no more ECHConfig values may be added. -OPENSSL_EXPORT int SSL_ECH_SERVER_CONFIG_LIST_add( - SSL_ECH_SERVER_CONFIG_LIST *list, int is_retry_config, - const uint8_t *ech_config, size_t ech_config_len, - const uint8_t *private_key, size_t private_key_len); - -// SSL_CTX_set1_ech_server_config_list atomically sets the refcounted |list| -// onto |ctx|, releasing the old list. |SSL| objects associated with |ctx|, as -// servers, will use |list| to decrypt incoming encrypted ClientHello messages. -// It returns one on success, and zero on failure. -// -// If |list| does not contain any retry configs, this function will fail. Retry -// configs are marked as such when they are added to |list| with -// |SSL_ECH_SERVER_CONFIG_LIST_add|. -// -// Once |list| has been passed to this function, it is immutable. Unlike most +// setting |keys| on an |SSL_CTX| with |SSL_CTX_set1_ech_keys|. After that +// point, |keys| is immutable; no more ECHConfig values may be added. +OPENSSL_EXPORT int SSL_ECH_KEYS_add(SSL_ECH_KEYS *keys, int is_retry_config, + const uint8_t *ech_config, + size_t ech_config_len, + const uint8_t *private_key, + size_t private_key_len); + +// SSL_CTX_set1_ech_keys configures |ctx| to use |keys| to decrypt encrypted +// ClientHellos. It returns one on success, and zero on failure. If |keys| does +// not contain any retry configs, this function will fail. Retry configs are +// marked as such when they are added to |keys| with |SSL_ECH_KEYS_add|. +// +// Once |keys| has been passed to this function, it is immutable. Unlike most // |SSL_CTX| configuration functions, this function may be called even if |ctx| // already has associated connections on multiple threads. This may be used to // rotate keys in a long-lived server process. @@ -3612,7 +3606,7 @@ OPENSSL_EXPORT int SSL_ECH_SERVER_CONFIG_LIST_add( // the ECHConfig and corresponding private key. // // Only the most recent fully-deployed ECHConfigs should be advertised in DNS. -// |list| may contain a newer set if those ECHConfigs are mid-deployment. It +// |keys| may contain a newer set if those ECHConfigs are mid-deployment. It // should also contain older sets, until the DNS change has rolled out and the // old records have expired from caches. // @@ -3628,8 +3622,7 @@ OPENSSL_EXPORT int SSL_ECH_SERVER_CONFIG_LIST_add( // reported |SSL_CLIENT_HELLO| structure and |SSL_get_servername| function will // transparently reflect the inner ClientHello. Callers should select parameters // based on these values to correctly handle ECH as well as the recovery flow. -OPENSSL_EXPORT int SSL_CTX_set1_ech_server_config_list( - SSL_CTX *ctx, SSL_ECH_SERVER_CONFIG_LIST *list); +OPENSSL_EXPORT int SSL_CTX_set1_ech_keys(SSL_CTX *ctx, SSL_ECH_KEYS *keys); // SSL_ech_accepted returns one if |ssl| negotiated ECH and zero otherwise. OPENSSL_EXPORT int SSL_ech_accepted(const SSL *ssl); @@ -5101,10 +5094,8 @@ BSSL_NAMESPACE_BEGIN BORINGSSL_MAKE_DELETER(SSL, SSL_free) BORINGSSL_MAKE_DELETER(SSL_CTX, SSL_CTX_free) BORINGSSL_MAKE_UP_REF(SSL_CTX, SSL_CTX_up_ref) -BORINGSSL_MAKE_DELETER(SSL_ECH_SERVER_CONFIG_LIST, - SSL_ECH_SERVER_CONFIG_LIST_free) -BORINGSSL_MAKE_UP_REF(SSL_ECH_SERVER_CONFIG_LIST, - SSL_ECH_SERVER_CONFIG_LIST_up_ref) +BORINGSSL_MAKE_DELETER(SSL_ECH_KEYS, SSL_ECH_KEYS_free) +BORINGSSL_MAKE_UP_REF(SSL_ECH_KEYS, SSL_ECH_KEYS_up_ref) BORINGSSL_MAKE_DELETER(SSL_SESSION, SSL_SESSION_free) BORINGSSL_MAKE_UP_REF(SSL_SESSION, SSL_SESSION_up_ref) diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc index 48eb70743..46590bd7d 100644 --- a/ssl/encrypted_client_hello.cc +++ b/ssl/encrypted_client_hello.cc @@ -499,30 +499,25 @@ void SSL_set_enable_ech_grease(SSL *ssl, int enable) { ssl->config->ech_grease_enabled = !!enable; } -SSL_ECH_SERVER_CONFIG_LIST *SSL_ECH_SERVER_CONFIG_LIST_new() { - return New(); -} +SSL_ECH_KEYS *SSL_ECH_KEYS_new() { return New(); } -void SSL_ECH_SERVER_CONFIG_LIST_up_ref(SSL_ECH_SERVER_CONFIG_LIST *configs) { - CRYPTO_refcount_inc(&configs->references); +void SSL_ECH_KEYS_up_ref(SSL_ECH_KEYS *keys) { + CRYPTO_refcount_inc(&keys->references); } -void SSL_ECH_SERVER_CONFIG_LIST_free(SSL_ECH_SERVER_CONFIG_LIST *configs) { - if (configs == nullptr || - !CRYPTO_refcount_dec_and_test_zero(&configs->references)) { +void SSL_ECH_KEYS_free(SSL_ECH_KEYS *keys) { + if (keys == nullptr || + !CRYPTO_refcount_dec_and_test_zero(&keys->references)) { return; } - configs->~ssl_ech_server_config_list_st(); - OPENSSL_free(configs); + keys->~ssl_ech_keys_st(); + OPENSSL_free(keys); } -int SSL_ECH_SERVER_CONFIG_LIST_add(SSL_ECH_SERVER_CONFIG_LIST *configs, - int is_retry_config, - const uint8_t *ech_config, - size_t ech_config_len, - const uint8_t *private_key, - size_t private_key_len) { +int SSL_ECH_KEYS_add(SSL_ECH_KEYS *configs, int is_retry_config, + const uint8_t *ech_config, size_t ech_config_len, + const uint8_t *private_key, size_t private_key_len) { UniquePtr parsed_config = MakeUnique(); if (!parsed_config) { return 0; @@ -540,10 +535,9 @@ int SSL_ECH_SERVER_CONFIG_LIST_add(SSL_ECH_SERVER_CONFIG_LIST *configs, return 1; } -int SSL_CTX_set1_ech_server_config_list(SSL_CTX *ctx, - SSL_ECH_SERVER_CONFIG_LIST *list) { +int SSL_CTX_set1_ech_keys(SSL_CTX *ctx, SSL_ECH_KEYS *keys) { bool has_retry_config = false; - for (const auto &config : list->configs) { + for (const auto &config : keys->configs) { if (config->is_retry_config()) { has_retry_config = true; break; @@ -553,9 +547,9 @@ int SSL_CTX_set1_ech_server_config_list(SSL_CTX *ctx, OPENSSL_PUT_ERROR(SSL, SSL_R_ECH_SERVER_WOULD_HAVE_NO_RETRY_CONFIGS); return 0; } - UniquePtr owned_list = UpRef(list); + UniquePtr owned_keys = UpRef(keys); MutexWriteLock lock(&ctx->lock); - ctx->ech_server_config_list.swap(owned_list); + ctx->ech_keys.swap(owned_keys); return 1; } diff --git a/ssl/handoff.cc b/ssl/handoff.cc index 4324c7544..883f832bf 100644 --- a/ssl/handoff.cc +++ b/ssl/handoff.cc @@ -232,8 +232,7 @@ static bool apply_remote_features(SSL *ssl, CBS *in) { // disqualifies it for split handshakes. static bool uses_disallowed_feature(const SSL *ssl) { return ssl->method->is_dtls || (ssl->config->cert && ssl->config->cert->dc) || - ssl->config->quic_transport_params.size() > 0 || - ssl->ctx->ech_server_config_list; + ssl->config->quic_transport_params.size() > 0 || ssl->ctx->ech_keys; } bool SSL_apply_handoff(SSL *ssl, Span handoff) { diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index c9a2048c3..c7d45f3cd 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc @@ -614,11 +614,11 @@ static enum ssl_hs_wait_t do_read_client_hello(SSL_HANDSHAKE *hs) { { MutexReadLock lock(&ssl->ctx->lock); - hs->ech_server_config_list = UpRef(ssl->ctx->ech_server_config_list); + hs->ech_keys = UpRef(ssl->ctx->ech_keys); } - if (hs->ech_server_config_list) { - for (const auto &ech_config : hs->ech_server_config_list->configs) { + if (hs->ech_keys) { + for (const auto &ech_config : hs->ech_keys->configs) { hs->ech_hpke_ctx.Reset(); if (config_id != ech_config->config_id() || !ech_config->SetupContext(hs->ech_hpke_ctx.get(), kdf_id, aead_id, diff --git a/ssl/internal.h b/ssl/internal.h index 6a1a65067..8aceac703 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1838,10 +1838,10 @@ struct SSL_HANDSHAKE { // the client if |in_early_data| is true. UniquePtr early_session; - // ech_server_config_list, for servers, is the list of ECHConfig values that - // were valid when the server received the first ClientHello. Its value will - // not change when the config list on |SSL_CTX| is updated. - UniquePtr ech_server_config_list; + // ssl_ech_keys, for servers, is the set of ECH keys to use with this + // handshake. This is copied from |SSL_CTX| to ensure consistent behavior as + // |SSL_CTX| rotates keys. + UniquePtr ech_keys; // new_cipher is the cipher being negotiated in this handshake. const SSL_CIPHER *new_cipher = nullptr; @@ -3465,10 +3465,10 @@ struct ssl_ctx_st { // Channel ID should not be offered on this connection. bssl::UniquePtr channel_id_private; - // ech_server_config_list contains the server's list of ECHConfig values and - // associated private keys. This list may be swapped out at any time, so all - // access must be synchronized through |lock|. - bssl::UniquePtr ech_server_config_list; + // ech_keys contains the server's list of ECHConfig values and associated + // private keys. This list may be swapped out at any time, so all access must + // be synchronized through |lock|. + bssl::UniquePtr ech_keys; // keylog_callback, if not NULL, is the key logging callback. See // |SSL_CTX_set_keylog_callback|. @@ -3783,18 +3783,17 @@ struct ssl_session_st { friend void SSL_SESSION_free(SSL_SESSION *); }; -struct ssl_ech_server_config_list_st { - ssl_ech_server_config_list_st() = default; - ssl_ech_server_config_list_st(const ssl_ech_server_config_list_st &) = delete; - ssl_ech_server_config_list_st &operator=( - const ssl_ech_server_config_list_st &) = delete; +struct ssl_ech_keys_st { + ssl_ech_keys_st() = default; + ssl_ech_keys_st(const ssl_ech_keys_st &) = delete; + ssl_ech_keys_st &operator=(const ssl_ech_keys_st &) = delete; bssl::GrowableArray> configs; CRYPTO_refcount_t references = 1; private: - ~ssl_ech_server_config_list_st() = default; - friend void SSL_ECH_SERVER_CONFIG_LIST_free(SSL_ECH_SERVER_CONFIG_LIST *); + ~ssl_ech_keys_st() = default; + friend void SSL_ECH_KEYS_free(SSL_ECH_KEYS *); }; #endif // OPENSSL_HEADER_SSL_INTERNAL_H diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 6d5b1f155..f939bfe0a 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -1487,15 +1487,14 @@ static const uint8_t kECHConfig[] = { // contents.extensions 0x00, 0x00}; -// kECHConfigPublicKey is the public key encoded in |kECHConfig|. -static const uint8_t kECHConfigPublicKey[X25519_PUBLIC_VALUE_LEN] = { +// kECHPublicKey is the public key encoded in |kECHConfig|. +static const uint8_t kECHPublicKey[X25519_PUBLIC_VALUE_LEN] = { 0xa6, 0x9a, 0x41, 0x48, 0x5d, 0x32, 0x96, 0xa4, 0xe0, 0xc3, 0x6a, 0xee, 0xf6, 0x63, 0x0f, 0x59, 0x32, 0x6f, 0xdc, 0xff, 0x81, 0x29, 0x59, 0xa5, 0x85, 0xd3, 0x9b, 0x3b, 0xde, 0x98, 0x55, 0x5c}; -// kECHConfigPrivateKey is the X25519 private key corresponding to -// |kECHConfigPublicKey|. -static const uint8_t kECHConfigPrivateKey[X25519_PRIVATE_KEY_LEN] = { +// kECHPrivateKey is the X25519 private key corresponding to |kECHPublicKey|. +static const uint8_t kECHPrivateKey[X25519_PRIVATE_KEY_LEN] = { 0xbc, 0xb5, 0x51, 0x29, 0x31, 0x10, 0x30, 0xc9, 0xed, 0x26, 0xde, 0xd4, 0xb3, 0xdf, 0x3a, 0xce, 0x06, 0x8a, 0xee, 0x17, 0xab, 0xce, 0xd7, 0xdb, 0xf3, 0x11, 0xe5, 0xa8, 0xf3, 0xb1, 0x8e, 0x24}; @@ -1539,7 +1538,7 @@ bool MakeECHConfig(std::vector *out, uint8_t config_id, return true; } -TEST(SSLTest, ECHServerConfigList) { +TEST(SSLTest, ECHKeys) { // kWrongPrivateKey is an unrelated, but valid X25519 private key. const uint8_t kWrongPrivateKey[X25519_PRIVATE_KEY_LEN] = { 0xbb, 0xfe, 0x08, 0xf7, 0x31, 0xde, 0x9c, 0x8a, 0xf2, 0x06, 0x4a, @@ -1549,14 +1548,13 @@ TEST(SSLTest, ECHServerConfigList) { bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); ASSERT_TRUE(ctx); - bssl::UniquePtr config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - ASSERT_TRUE(config_list); + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + ASSERT_TRUE(keys); // Adding an ECHConfig with the wrong private key is an error. - ASSERT_FALSE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, kECHConfig, sizeof(kECHConfig), - kWrongPrivateKey, sizeof(kWrongPrivateKey))); + ASSERT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, kECHConfig, + sizeof(kECHConfig), kWrongPrivateKey, + sizeof(kWrongPrivateKey))); uint32_t err = ERR_get_error(); EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(err)); EXPECT_EQ(SSL_R_ECH_SERVER_CONFIG_AND_PRIVATE_KEY_MISMATCH, @@ -1564,40 +1562,36 @@ TEST(SSLTest, ECHServerConfigList) { ERR_clear_error(); // Adding an ECHConfig with the matching private key succeeds. - ASSERT_TRUE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, kECHConfig, sizeof(kECHConfig), - kECHConfigPrivateKey, sizeof(kECHConfigPrivateKey))); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, kECHConfig, + sizeof(kECHConfig), kECHPrivateKey, + sizeof(kECHPrivateKey))); - ASSERT_TRUE( - SSL_CTX_set1_ech_server_config_list(ctx.get(), config_list.get())); + ASSERT_TRUE(SSL_CTX_set1_ech_keys(ctx.get(), keys.get())); // Build a new config list and replace the old one on |ctx|. - bssl::UniquePtr next_config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - ASSERT_TRUE(SSL_ECH_SERVER_CONFIG_LIST_add( - next_config_list.get(), /*is_retry_config=*/1, kECHConfig, - sizeof(kECHConfig), kECHConfigPrivateKey, sizeof(kECHConfigPrivateKey))); - ASSERT_TRUE( - SSL_CTX_set1_ech_server_config_list(ctx.get(), next_config_list.get())); + bssl::UniquePtr next_keys(SSL_ECH_KEYS_new()); + ASSERT_TRUE(SSL_ECH_KEYS_add(next_keys.get(), /*is_retry_config=*/1, + kECHConfig, sizeof(kECHConfig), kECHPrivateKey, + sizeof(kECHPrivateKey))); + ASSERT_TRUE(SSL_CTX_set1_ech_keys(ctx.get(), next_keys.get())); } TEST(SSLTest, ECHServerConfigListTruncatedPublicKey) { std::vector ech_config; ASSERT_TRUE(MakeECHConfig( &ech_config, 0x42, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, - MakeConstSpan(kECHConfigPublicKey, sizeof(kECHConfigPublicKey) - 1), + MakeConstSpan(kECHPublicKey, sizeof(kECHPublicKey) - 1), std::vector{EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_128_GCM}, /*extensions=*/{})); bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); ASSERT_TRUE(ctx); - bssl::UniquePtr config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - ASSERT_TRUE(config_list); - ASSERT_FALSE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, ech_config.data(), - ech_config.size(), kECHConfigPrivateKey, sizeof(kECHConfigPrivateKey))); + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + ASSERT_TRUE(keys); + ASSERT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), + kECHPrivateKey, sizeof(kECHPrivateKey))); uint32_t err = ERR_peek_error(); EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(err)); @@ -1606,23 +1600,21 @@ TEST(SSLTest, ECHServerConfigListTruncatedPublicKey) { ERR_clear_error(); } -// Test that |SSL_CTX_set1_ech_server_config_list| fails when the config list +// Test that |SSL_CTX_set1_ech_keys| fails when the config list // has no retry configs. TEST(SSLTest, ECHServerConfigsWithoutRetryConfigs) { bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); ASSERT_TRUE(ctx); - bssl::UniquePtr config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - ASSERT_TRUE(config_list); + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + ASSERT_TRUE(keys); // Adding an ECHConfig with the matching private key succeeds. - ASSERT_TRUE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/0, kECHConfig, sizeof(kECHConfig), - kECHConfigPrivateKey, sizeof(kECHConfigPrivateKey))); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/0, kECHConfig, + sizeof(kECHConfig), kECHPrivateKey, + sizeof(kECHPrivateKey))); - ASSERT_FALSE( - SSL_CTX_set1_ech_server_config_list(ctx.get(), config_list.get())); + ASSERT_FALSE(SSL_CTX_set1_ech_keys(ctx.get(), keys.get())); uint32_t err = ERR_peek_error(); EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(err)); EXPECT_EQ(SSL_R_ECH_SERVER_WOULD_HAVE_NO_RETRY_CONFIGS, ERR_GET_REASON(err)); @@ -1630,35 +1622,32 @@ TEST(SSLTest, ECHServerConfigsWithoutRetryConfigs) { // Add the same ECHConfig to the list, but this time mark it as a retry // config. - ASSERT_TRUE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, kECHConfig, sizeof(kECHConfig), - kECHConfigPrivateKey, sizeof(kECHConfigPrivateKey))); - ASSERT_TRUE( - SSL_CTX_set1_ech_server_config_list(ctx.get(), config_list.get())); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, kECHConfig, + sizeof(kECHConfig), kECHPrivateKey, + sizeof(kECHPrivateKey))); + ASSERT_TRUE(SSL_CTX_set1_ech_keys(ctx.get(), keys.get())); } // Test that the server APIs reject ECHConfigs with unsupported features. TEST(SSLTest, UnsupportedECHConfig) { - bssl::UniquePtr config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - ASSERT_TRUE(config_list); + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + ASSERT_TRUE(keys); // Unsupported versions are rejected. static const uint8_t kUnsupportedVersion[] = {0xff, 0xff, 0x00, 0x00}; - EXPECT_FALSE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, kUnsupportedVersion, - sizeof(kUnsupportedVersion), kECHConfigPrivateKey, - sizeof(kECHConfigPrivateKey))); + EXPECT_FALSE(SSL_ECH_KEYS_add( + keys.get(), /*is_retry_config=*/1, kUnsupportedVersion, + sizeof(kUnsupportedVersion), kECHPrivateKey, sizeof(kECHPrivateKey))); // Unsupported cipher suites are rejected. (We only support HKDF-SHA256.) std::vector ech_config; ASSERT_TRUE(MakeECHConfig( - &ech_config, 0x42, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, kECHConfigPublicKey, + &ech_config, 0x42, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, kECHPublicKey, std::vector{0x002 /* HKDF-SHA384 */, EVP_HPKE_AES_128_GCM}, /*extensions=*/{})); - EXPECT_FALSE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, ech_config.data(), - ech_config.size(), kECHConfigPrivateKey, sizeof(kECHConfigPrivateKey))); + EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), + kECHPrivateKey, sizeof(kECHPrivateKey))); // Unsupported KEMs are rejected. static const uint8_t kP256PublicKey[] = { @@ -1676,19 +1665,19 @@ TEST(SSLTest, UnsupportedECHConfig) { &ech_config, 0x42, 0x0010 /* DHKEM(P-256, HKDF-SHA256) */, kP256PublicKey, std::vector{EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_128_GCM}, /*extensions=*/{})); - EXPECT_FALSE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, ech_config.data(), - ech_config.size(), kP256PrivateKey, sizeof(kP256PrivateKey))); + EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), + kP256PrivateKey, sizeof(kP256PrivateKey))); // Unsupported extensions are rejected. static const uint8_t kExtensions[] = {0x00, 0x01, 0x00, 0x00}; ASSERT_TRUE(MakeECHConfig( - &ech_config, 0x42, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, kECHConfigPublicKey, + &ech_config, 0x42, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, kECHPublicKey, std::vector{EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_128_GCM}, kExtensions)); - EXPECT_FALSE(SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/1, ech_config.data(), - ech_config.size(), kECHConfigPrivateKey, sizeof(kECHConfigPrivateKey))); + EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), + kECHPrivateKey, sizeof(kECHPrivateKey))); } static void AppendSession(SSL_SESSION *session, void *arg) { diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index 9cd9f490f..106c99a1e 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc @@ -733,20 +733,19 @@ 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 || // - hs->ech_server_config_list == nullptr) { + hs->ech_keys == nullptr) { return true; } - // Write the list of retry configs to |out|. Note - // |SSL_CTX_set1_ech_server_config_list| ensures |ech_server_config_list| - // contains at least one retry config. + // Write the list of retry configs to |out|. Note |SSL_CTX_set1_ech_keys| + // ensures |ech_keys| contains at least one retry config. CBB body, retry_configs; if (!CBB_add_u16(out, TLSEXT_TYPE_encrypted_client_hello) || !CBB_add_u16_length_prefixed(out, &body) || !CBB_add_u16_length_prefixed(&body, &retry_configs)) { return false; } - for (const auto &config : hs->ech_server_config_list->configs) { + for (const auto &config : hs->ech_keys->configs) { if (!config->is_retry_config()) { continue; } diff --git a/ssl/test/fuzzer.h b/ssl/test/fuzzer.h index 839560efa..07c2d6d12 100644 --- a/ssl/test/fuzzer.h +++ b/ssl/test/fuzzer.h @@ -230,7 +230,7 @@ const uint8_t kALPNProtocols[] = { 0x01, 'a', 0x02, 'a', 'a', 0x03, 'a', 'a', 'a', }; -const uint8_t kECHServerConfig[] = { +const uint8_t kECHConfig[] = { 0xfe, 0x0a, 0x00, 0x47, 0x2a, 0x00, 0x20, 0x00, 0x20, 0x6c, 0x55, 0x96, 0x41, 0x3d, 0x12, 0x4e, 0x63, 0x3d, 0x39, 0x7a, 0xe9, 0xbc, 0xec, 0xb2, 0x55, 0xd0, 0xe6, 0xaa, 0xbd, 0xa9, 0x79, 0xb8, 0x86, @@ -240,7 +240,7 @@ const uint8_t kECHServerConfig[] = { 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x00, 0x00, }; -const uint8_t kECHServerConfigPrivateKey[] = { +const uint8_t kECHKey[] = { 0x35, 0x6d, 0x45, 0x06, 0xb3, 0x88, 0x89, 0x2e, 0xd6, 0x87, 0x84, 0xd2, 0x2d, 0x6f, 0x83, 0x48, 0xad, 0xf2, 0xfd, 0x08, 0x51, 0x73, 0x10, 0xa0, 0xb8, 0xdd, 0xe9, 0x96, 0x6a, 0xde, 0xbc, 0x82, @@ -455,18 +455,15 @@ class TLSFuzzer { SSL_CTX_set_tls_channel_id_enabled(ctx_.get(), 1); if (role_ == kServer) { - bssl::UniquePtr config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - if (!config_list) { + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + if (!keys) { return false; } - if (!SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), /*is_retry_config=*/true, kECHServerConfig, - sizeof(kECHServerConfig), kECHServerConfigPrivateKey, - sizeof(kECHServerConfigPrivateKey))) { + if (!SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/true, kECHConfig, + sizeof(kECHConfig), kECHKey, sizeof(kECHKey))) { return false; } - if (!SSL_CTX_set1_ech_server_config_list(ctx_.get(), config_list.get())) { + if (!SSL_CTX_set1_ech_keys(ctx_.get(), keys.get())) { return false; } } diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index e24f79b77..9e682295d 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -1729,17 +1729,16 @@ bssl::UniquePtr TestConfig::NewSSL( return nullptr; } if (!ech_server_configs.empty()) { - bssl::UniquePtr config_list( - SSL_ECH_SERVER_CONFIG_LIST_new()); - if (!config_list) { + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + if (!keys) { return nullptr; } for (size_t i = 0; i < ech_server_configs.size(); i++) { const std::string &ech_config = ech_server_configs[i]; const std::string &ech_private_key = ech_server_keys[i]; const int is_retry_config = ech_is_retry_config[i]; - if (!SSL_ECH_SERVER_CONFIG_LIST_add( - config_list.get(), is_retry_config, + if (!SSL_ECH_KEYS_add( + keys.get(), is_retry_config, reinterpret_cast(ech_config.data()), ech_config.size(), reinterpret_cast(ech_private_key.data()), @@ -1747,7 +1746,7 @@ bssl::UniquePtr TestConfig::NewSSL( return nullptr; } } - if (!SSL_CTX_set1_ech_server_config_list(ssl_ctx, config_list.get())) { + if (!SSL_CTX_set1_ech_keys(ssl_ctx, keys.get())) { return nullptr; } } diff --git a/tool/server.cc b/tool/server.cc index 6993e0971..858e8a1e8 100644 --- a/tool/server.cc +++ b/tool/server.cc @@ -299,14 +299,13 @@ bool Server(const std::vector &args) { return false; } - bssl::UniquePtr configs( - SSL_ECH_SERVER_CONFIG_LIST_new()); - if (!configs || - !SSL_ECH_SERVER_CONFIG_LIST_add(configs.get(), - /*is_retry_config=*/1, echconfig.data(), - echconfig.size(), echconfig_key.data(), - echconfig_key.size()) || - !SSL_CTX_set1_ech_server_config_list(ctx.get(), configs.get())) { + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + if (!keys || + !SSL_ECH_KEYS_add(keys.get(), + /*is_retry_config=*/1, echconfig.data(), + echconfig.size(), echconfig_key.data(), + echconfig_key.size()) || + !SSL_CTX_set1_ech_keys(ctx.get(), keys.get())) { fprintf(stderr, "Error setting server's ECHConfig and private key\n"); return false; }