From 24545c541c424b4f9bd8f42edc06d84d6542e764 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 7 Jun 2021 16:05:07 -0400 Subject: [PATCH] Add a basic API to make ECHConfigs. We'll probably need to make this more complex later, but this should be a start. I had hoped this would also simplify tests, MakeECHConfig() was still needed to generate weird inputs for tests. I've instead tidied that up a bit with a params structure. Now the only hard-coded ECHConfig in tests is to check the output of the new API. Bug: 275 Change-Id: I640a224fb4b7a7d20e8a2cd7a1e75d1e3fe69936 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48003 Commit-Queue: David Benjamin Reviewed-by: Adam Langley --- crypto/err/ssl.errordata | 1 + include/openssl/ssl.h | 38 ++++ ssl/encrypted_client_hello.cc | 83 +++++++- ssl/internal.h | 8 +- ssl/ssl_test.cc | 370 +++++++++++++++++++++------------- 5 files changed, 354 insertions(+), 146 deletions(-) diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index 661e52b03..1e5b1c08e 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata @@ -85,6 +85,7 @@ SSL,314,INVALID_CLIENT_HELLO_INNER SSL,158,INVALID_COMMAND SSL,256,INVALID_COMPRESSION_LIST SSL,301,INVALID_DELEGATED_CREDENTIAL +SSL,317,INVALID_ECH_PUBLIC_NAME SSL,159,INVALID_MESSAGE SSL,251,INVALID_OUTER_RECORD_TYPE SSL,269,INVALID_SCT_LIST diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 5b07ebffd..b487a1225 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -3565,6 +3565,26 @@ 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_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 +// release the memory. On failure, it returns zero. +// +// The |config_id| field is a single byte identifer for the ECHConfig. Reusing +// config IDs is allowed, but if multiple ECHConfigs with the same config ID are +// active at a time, server load may increase. See +// |SSL_ECH_KEYS_has_duplicate_config_id|. +// +// The public key and KEM algorithm are taken from |key|. |public_name| is the +// DNS name used to authenticate the recovery flow. |max_name_len| should be the +// length of the longest name in the ECHConfig's anonymity set and influences +// client padding decisions. +OPENSSL_EXPORT 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); + // 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); @@ -3590,6 +3610,23 @@ OPENSSL_EXPORT int SSL_ECH_KEYS_add(SSL_ECH_KEYS *keys, int is_retry_config, size_t ech_config_len, const EVP_HPKE_KEY *key); +// SSL_ECH_KEYS_has_duplicate_config_id returns one if |keys| has duplicate +// config IDs or zero otherwise. Duplicate config IDs still work, but may +// increase server load due to trial decryption. +OPENSSL_EXPORT int SSL_ECH_KEYS_has_duplicate_config_id( + const SSL_ECH_KEYS *keys); + +// SSL_ECH_KEYS_marshal_retry_configs serializes the retry configs in |keys| as +// an ECHConfigList. 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 release the memory. On failure, it +// returns zero. +// +// This output may be advertised to clients in DNS. +OPENSSL_EXPORT int SSL_ECH_KEYS_marshal_retry_configs(const SSL_ECH_KEYS *keys, + uint8_t **out, + size_t *out_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 @@ -5437,6 +5474,7 @@ BSSL_NAMESPACE_END #define SSL_R_INVALID_CLIENT_HELLO_INNER 314 #define SSL_R_INVALID_ALPN_PROTOCOL_LIST 315 #define SSL_R_COULD_NOT_PARSE_HINTS 316 +#define SSL_R_INVALID_ECH_PUBLIC_NAME 317 #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 diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc index d1adc684b..8b606f09c 100644 --- a/ssl/encrypted_client_hello.cc +++ b/ssl/encrypted_client_hello.cc @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -33,6 +34,9 @@ BSSL_NAMESPACE_BEGIN +// ECH reuses the extension code point for the version number. +static const uint16_t kECHConfigVersion = TLSEXT_TYPE_encrypted_client_hello; + static const decltype(&EVP_hpke_aes_128_gcm) kSupportedAEADs[] = { &EVP_hpke_aes_128_gcm, &EVP_hpke_aes_256_gcm, @@ -373,7 +377,7 @@ bool ECHServerConfig::Init(Span ech_config, // configured in both the server and DNS. If the caller configures an // unsupported parameter, this is a deployment error. To catch these errors, // we fail early. - if (version != TLSEXT_TYPE_encrypted_client_hello) { + if (version != kECHConfigVersion) { OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_ECH_SERVER_CONFIG); return false; } @@ -493,6 +497,52 @@ void SSL_set_enable_ech_grease(SSL *ssl, int enable) { ssl->config->ech_grease_enabled = !!enable; } +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) { + size_t public_name_len = strlen(public_name); + if (public_name_len == 0) { + // TODO(https://crbug.com/boringssl/275): Check |public_name| is a valid DNS + // name. + OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_ECH_PUBLIC_NAME); + return 0; + } + + // See draft-ietf-tls-esni-10, section 4. + ScopedCBB cbb; + CBB contents, child; + uint8_t *public_key; + size_t public_key_len; + if (!CBB_init(cbb.get(), 128) || // + !CBB_add_u16(cbb.get(), kECHConfigVersion) || + !CBB_add_u16_length_prefixed(cbb.get(), &contents) || + !CBB_add_u8(&contents, config_id) || + !CBB_add_u16(&contents, EVP_HPKE_KEM_id(EVP_HPKE_KEY_kem(key))) || + !CBB_add_u16_length_prefixed(&contents, &child) || + !CBB_reserve(&child, &public_key, EVP_HPKE_MAX_PUBLIC_KEY_LENGTH) || + !EVP_HPKE_KEY_public_key(key, public_key, &public_key_len, + EVP_HPKE_MAX_PUBLIC_KEY_LENGTH) || + !CBB_did_write(&child, public_key_len) || + !CBB_add_u16_length_prefixed(&contents, &child) || + // Write a default cipher suite configuration. + !CBB_add_u16(&child, EVP_HPKE_HKDF_SHA256) || + !CBB_add_u16(&child, EVP_HPKE_AES_128_GCM) || + !CBB_add_u16(&child, EVP_HPKE_HKDF_SHA256) || + !CBB_add_u16(&child, EVP_HPKE_CHACHA20_POLY1305) || + !CBB_add_u16(&contents, max_name_len) || + !CBB_add_u16_length_prefixed(&contents, &child) || + !CBB_add_bytes(&child, reinterpret_cast(public_name), + public_name_len) || + // TODO(https://crbug.com/boringssl/275): Reserve some GREASE extensions + // and include some. + !CBB_add_u16(&contents, 0 /* no extensions */) || + !CBB_finish(cbb.get(), out, out_len)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); + return 0; + } + return 1; +} + SSL_ECH_KEYS *SSL_ECH_KEYS_new() { return New(); } void SSL_ECH_KEYS_up_ref(SSL_ECH_KEYS *keys) { @@ -528,6 +578,37 @@ int SSL_ECH_KEYS_add(SSL_ECH_KEYS *configs, int is_retry_config, return 1; } +int SSL_ECH_KEYS_has_duplicate_config_id(const SSL_ECH_KEYS *keys) { + bool seen[256] = {false}; + for (const auto &config : keys->configs) { + if (seen[config->config_id()]) { + return 1; + } + seen[config->config_id()] = true; + } + return 0; +} + +int SSL_ECH_KEYS_marshal_retry_configs(const SSL_ECH_KEYS *keys, uint8_t **out, + size_t *out_len) { + ScopedCBB cbb; + CBB child; + if (!CBB_init(cbb.get(), 128) || + !CBB_add_u16_length_prefixed(cbb.get(), &child)) { + OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + return false; + } + for (const auto &config : keys->configs) { + if (config->is_retry_config() && + !CBB_add_bytes(&child, config->ech_config().data(), + config->ech_config().size())) { + OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + return false; + } + } + return CBB_finish(cbb.get(), out, out_len); +} + int SSL_CTX_set1_ech_keys(SSL_CTX *ctx, SSL_ECH_KEYS *keys) { bool has_retry_config = false; for (const auto &config : keys->configs) { diff --git a/ssl/internal.h b/ssl/internal.h index 62a9a067d..5b8ff68ff 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -278,9 +278,9 @@ class Array { T &operator[](size_t i) { return data_[i]; } T *begin() { return data_; } - const T *cbegin() const { return data_; } + const T *begin() const { return data_; } T *end() { return data_ + size_; } - const T *cend() const { return data_ + size_; } + const T *end() const { return data_ + size_; } void Reset() { Reset(nullptr, 0); } @@ -389,9 +389,9 @@ class GrowableArray { T &operator[](size_t i) { return array_[i]; } T *begin() { return array_.data(); } - const T *cbegin() const { return array_.data(); } + const T *begin() const { return array_.data(); } T *end() { return array_.data() + size_; } - const T *cend() const { return array_.data() + size_; } + const T *end() const { return array_.data() + size_; } void clear() { size_ = 0; diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index a8e6a0c2a..cdd2e97ed 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -1464,73 +1464,66 @@ TEST(SSLTest, AddClientCA) { EXPECT_EQ(0, X509_NAME_cmp(sk_X509_NAME_value(list, 2), name1)); } -// kECHConfig contains a serialized ECHConfig value. -static const uint8_t kECHConfig[] = { - // version - 0xfe, 0x0a, - // length - 0x00, 0x43, - // contents.config_id - 0x42, - // contents.kem_id - 0x00, 0x20, - // contents.public_key - 0x00, 0x20, 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, - // contents.cipher_suites - 0x00, 0x08, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x03, - // contents.maximum_name_length - 0x00, 0x10, - // contents.public_name - 0x00, 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61, - 0x6d, 0x70, 0x6c, 0x65, - // contents.extensions - 0x00, 0x00}; - -// 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}; - -// 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}; - -// MakeECHConfig serializes an ECHConfig and writes it to |*out| with the -// specified parameters. |cipher_suites| is a list of code points which should -// contain pairs of KDF and AEAD IDs. -bool MakeECHConfig(std::vector *out, uint8_t config_id, - uint16_t kem_id, Span public_key, - Span cipher_suites, - Span extensions) { +struct ECHConfigParams { + uint16_t version = TLSEXT_TYPE_encrypted_client_hello; + uint16_t config_id = 1; + std::string public_name = "example.com"; + const EVP_HPKE_KEY *key = nullptr; + // kem_id, if zero, takes its value from |key|. + uint16_t kem_id = 0; + // public_key, if empty takes its value from |key|. + std::vector public_key; + size_t max_name_len = 16; + // cipher_suites is a list of code points which should contain pairs of KDF + // and AEAD IDs. + std::vector cipher_suites = {EVP_HPKE_HKDF_SHA256, + EVP_HPKE_AES_128_GCM}; + std::vector extensions; +}; + +// MakeECHConfig serializes an ECHConfig from |params| and writes it to +// |*out|. +bool MakeECHConfig(std::vector *out, + const ECHConfigParams ¶ms) { + uint16_t kem_id = params.kem_id == 0 + ? EVP_HPKE_KEM_id(EVP_HPKE_KEY_kem(params.key)) + : params.kem_id; + std::vector public_key = params.public_key; + if (public_key.empty()) { + public_key.resize(EVP_HPKE_MAX_PUBLIC_KEY_LENGTH); + size_t len; + if (!EVP_HPKE_KEY_public_key(params.key, public_key.data(), &len, + public_key.size())) { + return false; + } + public_key.resize(len); + } + bssl::ScopedCBB cbb; CBB contents, child; - static const char kPublicName[] = "example.com"; if (!CBB_init(cbb.get(), 64) || - !CBB_add_u16(cbb.get(), TLSEXT_TYPE_encrypted_client_hello) || + !CBB_add_u16(cbb.get(), params.version) || !CBB_add_u16_length_prefixed(cbb.get(), &contents) || - !CBB_add_u8(&contents, config_id) || + !CBB_add_u8(&contents, params.config_id) || !CBB_add_u16(&contents, kem_id) || !CBB_add_u16_length_prefixed(&contents, &child) || !CBB_add_bytes(&child, public_key.data(), public_key.size()) || !CBB_add_u16_length_prefixed(&contents, &child)) { return false; } - for (uint16_t cipher_suite : cipher_suites) { + for (uint16_t cipher_suite : params.cipher_suites) { if (!CBB_add_u16(&child, cipher_suite)) { return false; } } - if (!CBB_add_u16(&contents, strlen(kPublicName)) || // maximum_name_length + if (!CBB_add_u16(&contents, params.max_name_len) || !CBB_add_u16_length_prefixed(&contents, &child) || - !CBB_add_bytes(&child, reinterpret_cast(kPublicName), - strlen(kPublicName)) || + !CBB_add_bytes( + &child, reinterpret_cast(params.public_name.data()), + params.public_name.size()) || !CBB_add_u16_length_prefixed(&contents, &child) || - !CBB_add_bytes(&child, extensions.data(), extensions.size()) || + !CBB_add_bytes(&child, params.extensions.data(), + params.extensions.size()) || !CBB_flush(cbb.get())) { return false; } @@ -1539,105 +1532,190 @@ bool MakeECHConfig(std::vector *out, uint8_t config_id, return true; } -TEST(SSLTest, ECHKeys) { - bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); - ASSERT_TRUE(ctx); +// Test that |SSL_marshal_ech_config| and |SSL_ECH_KEYS_marshal_retry_configs| +// output values as expected. +TEST(SSLTest, MarshalECHConfig) { + static const uint8_t kPrivateKey[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}; + bssl::ScopedEVP_HPKE_KEY key; + ASSERT_TRUE(EVP_HPKE_KEY_init(key.get(), EVP_hpke_x25519_hkdf_sha256(), + kPrivateKey, sizeof(kPrivateKey))); + + static const uint8_t kECHConfig[] = { + // version + 0xfe, 0x0a, + // length + 0x00, 0x43, + // contents.config_id + 0x01, + // contents.kem_id + 0x00, 0x20, + // contents.public_key + 0x00, 0x20, 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, + // contents.cipher_suites + 0x00, 0x08, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x03, + // contents.maximum_name_length + 0x00, 0x10, + // contents.public_name + 0x00, 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61, + 0x6d, 0x70, 0x6c, 0x65, + // contents.extensions + 0x00, 0x00}; + uint8_t *ech_config; + size_t ech_config_len; + ASSERT_TRUE(SSL_marshal_ech_config(&ech_config, &ech_config_len, + /*config_id=*/1, key.get(), + "public.example", 16)); + bssl::UniquePtr free_ech_config(ech_config); + EXPECT_EQ(Bytes(kECHConfig), Bytes(ech_config, ech_config_len)); + + // Generate a second ECHConfig. + bssl::ScopedEVP_HPKE_KEY key2; + ASSERT_TRUE(EVP_HPKE_KEY_generate(key2.get(), EVP_hpke_x25519_hkdf_sha256())); + uint8_t *ech_config2; + size_t ech_config2_len; + ASSERT_TRUE(SSL_marshal_ech_config(&ech_config2, &ech_config2_len, + /*config_id=*/2, key2.get(), + "public.example", 16)); + bssl::UniquePtr free_ech_config2(ech_config2); + + // Install both ECHConfigs in an |SSL_ECH_KEYS|. + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + ASSERT_TRUE(keys); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, ech_config, + ech_config_len, key.get())); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, ech_config2, + ech_config2_len, key2.get())); + + // The ECHConfigList should be correctly serialized. + uint8_t *ech_config_list; + size_t ech_config_list_len; + ASSERT_TRUE(SSL_ECH_KEYS_marshal_retry_configs(keys.get(), &ech_config_list, + &ech_config_list_len)); + bssl::UniquePtr free_ech_config_list(ech_config_list); + + // ECHConfigList is just the concatenation with a length prefix. + size_t len = ech_config_len + ech_config2_len; + std::vector expected = {uint8_t(len >> 8), uint8_t(len)}; + expected.insert(expected.end(), ech_config, ech_config + ech_config_len); + expected.insert(expected.end(), ech_config2, ech_config2 + ech_config2_len); + EXPECT_EQ(Bytes(expected), Bytes(ech_config_list, ech_config_list_len)); + + // TODO(https://crbug.com/boringssl/275): When the client is implemented, test + // that we are able to use our own ECHConfigs. +} + +TEST(SSLTest, ECHHasDuplicateConfigID) { + const struct { + std::vector ids; + bool has_duplicate; + } kTests[] = { + {{}, false}, + {{1}, false}, + {{1, 2, 3, 255}, false}, + {{1, 2, 3, 1}, true}, + }; + for (const auto &test : kTests) { + bssl::UniquePtr keys(SSL_ECH_KEYS_new()); + ASSERT_TRUE(keys); + for (const uint8_t id : test.ids) { + bssl::ScopedEVP_HPKE_KEY key; + ASSERT_TRUE( + EVP_HPKE_KEY_generate(key.get(), EVP_hpke_x25519_hkdf_sha256())); + uint8_t *ech_config; + size_t ech_config_len; + ASSERT_TRUE(SSL_marshal_ech_config(&ech_config, &ech_config_len, id, + key.get(), "public.example", 16)); + bssl::UniquePtr free_ech_config(ech_config); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config, ech_config_len, key.get())); + } + + EXPECT_EQ(test.has_duplicate ? 1 : 0, + SSL_ECH_KEYS_has_duplicate_config_id(keys.get())); + } +} +// Test that |SSL_ECH_KEYS_add| checks consistency between the public and +// private key. +TEST(SSLTest, ECHKeyConsistency) { bssl::UniquePtr keys(SSL_ECH_KEYS_new()); ASSERT_TRUE(keys); + bssl::ScopedEVP_HPKE_KEY key; + ASSERT_TRUE(EVP_HPKE_KEY_generate(key.get(), EVP_hpke_x25519_hkdf_sha256())); + uint8_t public_key[EVP_HPKE_MAX_PUBLIC_KEY_LENGTH]; + size_t public_key_len; + ASSERT_TRUE(EVP_HPKE_KEY_public_key(key.get(), public_key, &public_key_len, + sizeof(public_key))); + + // Adding an ECHConfig with the matching public key succeeds. + ECHConfigParams params; + params.key = key.get(); + std::vector ech_config; + ASSERT_TRUE(MakeECHConfig(&ech_config, params)); + EXPECT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), + key.get())); // Adding an ECHConfig with the wrong public key is an error. bssl::ScopedEVP_HPKE_KEY wrong_key; ASSERT_TRUE( EVP_HPKE_KEY_generate(wrong_key.get(), EVP_hpke_x25519_hkdf_sha256())); - ASSERT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, kECHConfig, - sizeof(kECHConfig), wrong_key.get())); - 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, - ERR_GET_REASON(err)); - ERR_clear_error(); - - // Adding an ECHConfig with the right public key, but wrong KEM ID, is an - // error. - bssl::ScopedEVP_HPKE_KEY key; - ASSERT_TRUE(EVP_HPKE_KEY_init(key.get(), EVP_hpke_x25519_hkdf_sha256(), - kECHPrivateKey, sizeof(kECHPrivateKey))); - std::vector ech_config; - ASSERT_TRUE(MakeECHConfig( - &ech_config, 0x42, 0x0010 /* DHKEM(P-256, HKDF-SHA256) */, kECHPublicKey, - std::vector{EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_128_GCM}, - /*extensions=*/{})); EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, ech_config.data(), ech_config.size(), - key.get())); - - // Adding an ECHConfig with the matching public key succeeds. - ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, kECHConfig, - sizeof(kECHConfig), key.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_keys(SSL_ECH_KEYS_new()); - ASSERT_TRUE(SSL_ECH_KEYS_add(next_keys.get(), /*is_retry_config=*/1, - kECHConfig, sizeof(kECHConfig),key.get())); - 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(kECHPublicKey, sizeof(kECHPublicKey) - 1), - std::vector{EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_128_GCM}, - /*extensions=*/{})); + wrong_key.get())); - bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); - ASSERT_TRUE(ctx); + // Adding an ECHConfig with a truncated public key is an error. + ECHConfigParams truncated; + truncated.key = key.get(); + truncated.public_key.assign(public_key, public_key + public_key_len - 1); + ASSERT_TRUE(MakeECHConfig(&ech_config, truncated)); + EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), key.get())); - bssl::ScopedEVP_HPKE_KEY key; - ASSERT_TRUE(EVP_HPKE_KEY_init(key.get(), EVP_hpke_x25519_hkdf_sha256(), - kECHPrivateKey, sizeof(kECHPrivateKey))); - bssl::UniquePtr keys(SSL_ECH_KEYS_new()); - ASSERT_TRUE(keys); - ASSERT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + // Adding an ECHConfig with the right public key, but wrong KEM ID, is an + // error. + ECHConfigParams wrong_kem; + wrong_kem.key = key.get(); + wrong_kem.kem_id = 0x0010; // DHKEM(P-256, HKDF-SHA256) + ASSERT_TRUE(MakeECHConfig(&ech_config, wrong_kem)); + EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, ech_config.data(), ech_config.size(), key.get())); - - uint32_t err = ERR_peek_error(); - EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(err)); - EXPECT_EQ(SSL_R_ECH_SERVER_CONFIG_AND_PRIVATE_KEY_MISMATCH, - ERR_GET_REASON(err)); - ERR_clear_error(); } // 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::ScopedEVP_HPKE_KEY key; + ASSERT_TRUE(EVP_HPKE_KEY_generate(key.get(), EVP_hpke_x25519_hkdf_sha256())); + uint8_t *ech_config; + size_t ech_config_len; + ASSERT_TRUE(SSL_marshal_ech_config(&ech_config, &ech_config_len, + /*config_id=*/1, key.get(), + "public.example", 16)); + bssl::UniquePtr free_ech_config(ech_config); + + // Install a non-retry config. bssl::UniquePtr keys(SSL_ECH_KEYS_new()); ASSERT_TRUE(keys); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/0, ech_config, + ech_config_len, key.get())); - bssl::ScopedEVP_HPKE_KEY key; - ASSERT_TRUE(EVP_HPKE_KEY_init(key.get(), EVP_hpke_x25519_hkdf_sha256(), - kECHPrivateKey, sizeof(kECHPrivateKey))); - ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/0, kECHConfig, - sizeof(kECHConfig), key.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)); - ERR_clear_error(); + // |keys| has no retry configs. + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + EXPECT_FALSE(SSL_CTX_set1_ech_keys(ctx.get(), keys.get())); // Add the same ECHConfig to the list, but this time mark it as a retry // config. - ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, kECHConfig, - sizeof(kECHConfig), key.get())); - ASSERT_TRUE(SSL_CTX_set1_ech_keys(ctx.get(), keys.get())); + ASSERT_TRUE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, ech_config, + ech_config_len, key.get())); + EXPECT_TRUE(SSL_CTX_set1_ech_keys(ctx.get(), keys.get())); } // Test that the server APIs reject ECHConfigs with unsupported features. @@ -1645,31 +1723,41 @@ TEST(SSLTest, UnsupportedECHConfig) { bssl::UniquePtr keys(SSL_ECH_KEYS_new()); ASSERT_TRUE(keys); bssl::ScopedEVP_HPKE_KEY key; - ASSERT_TRUE(EVP_HPKE_KEY_init(key.get(), EVP_hpke_x25519_hkdf_sha256(), - kECHPrivateKey, sizeof(kECHPrivateKey))); + ASSERT_TRUE(EVP_HPKE_KEY_generate(key.get(), EVP_hpke_x25519_hkdf_sha256())); // Unsupported versions are rejected. - static const uint8_t kUnsupportedVersion[] = {0xff, 0xff, 0x00, 0x00}; - EXPECT_FALSE(SSL_ECH_KEYS_add( - keys.get(), /*is_retry_config=*/1, kUnsupportedVersion, - sizeof(kUnsupportedVersion), key.get())); + ECHConfigParams unsupported_version; + unsupported_version.version = 0xffff; + unsupported_version.key = key.get(); + std::vector ech_config; + ASSERT_TRUE(MakeECHConfig(&ech_config, unsupported_version)); + EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), + key.get())); // 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, kECHPublicKey, - std::vector{0x002 /* HKDF-SHA384 */, EVP_HPKE_AES_128_GCM}, - /*extensions=*/{})); + ECHConfigParams unsupported_kdf; + unsupported_kdf.key = key.get(); + unsupported_kdf.cipher_suites = {0x002 /* HKDF-SHA384 */, + EVP_HPKE_AES_128_GCM}; + ASSERT_TRUE(MakeECHConfig(&ech_config, unsupported_kdf)); EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, ech_config.data(), ech_config.size(), key.get())); + ECHConfigParams unsupported_aead; + unsupported_aead.key = key.get(); + unsupported_aead.cipher_suites = {EVP_HPKE_HKDF_SHA256, 0xffff}; + ASSERT_TRUE(MakeECHConfig(&ech_config, unsupported_aead)); + EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, + ech_config.data(), ech_config.size(), + key.get())); + // 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, kECHPublicKey, - std::vector{EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_128_GCM}, - kExtensions)); + ECHConfigParams extensions; + extensions.key = key.get(); + extensions.extensions = {0x00, 0x01, 0x00, 0x00}; + ASSERT_TRUE(MakeECHConfig(&ech_config, extensions)); EXPECT_FALSE(SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/1, ech_config.data(), ech_config.size(), key.get()));