From b1b15770f135d19c04331313aa62ceaa8f40bd44 Mon Sep 17 00:00:00 2001 From: Martin Thomson Date: Wed, 17 Mar 2021 15:03:04 +1100 Subject: [PATCH] Bug 1698419 - ECH -10 updates, r=bbeurdouche The main changes here are: * an update to HPKE -08 * a move to the single-byte configuration ID * reordering of ECHConfig The addition of the explicit configuration ID means that the API for constructing ECHConfig(List) needs to change. That means a name change, unfortunately. I took the opportunity to make further changes to the arguments. Differential Revision: https://phabricator.services.mozilla.com/D108392 --HG-- extra : rebase_source : e66ae86be746afbadc6c444d0debcc5aaabd2eb5 --- .../abi-check/expected-report-libssl3.so.txt | 11 + cmd/selfserv/selfserv.c | 22 +- gtests/ssl_gtest/libssl_internals.c | 21 -- gtests/ssl_gtest/libssl_internals.h | 1 - gtests/ssl_gtest/tls_connect.cc | 15 +- gtests/ssl_gtest/tls_connect.h | 10 +- gtests/ssl_gtest/tls_ech_unittest.cc | 319 ++++++++-------- lib/ssl/sslexp.h | 43 ++- lib/ssl/sslsock.c | 2 +- lib/ssl/sslt.h | 2 +- lib/ssl/tls13ech.c | 348 +++++++++--------- lib/ssl/tls13ech.h | 18 +- lib/ssl/tls13exthandle.c | 15 +- lib/ssl/tls13hashstate.c | 141 +++---- lib/ssl/tls13hashstate.h | 2 +- 15 files changed, 500 insertions(+), 470 deletions(-) diff --git a/automation/abi-check/expected-report-libssl3.so.txt b/automation/abi-check/expected-report-libssl3.so.txt index e69de29bb2..c547a45b39 100644 --- a/automation/abi-check/expected-report-libssl3.so.txt +++ b/automation/abi-check/expected-report-libssl3.so.txt @@ -0,0 +1,11 @@ + +1 function with some indirect sub-type change: + + [C]'function SECStatus SSL_HandshakeNegotiatedExtension(PRFileDesc*, SSLExtensionType, PRBool*)' at sslreveal.c:72:1 has some indirect sub-type changes: + parameter 2 of type 'typedef SSLExtensionType' has sub-type changes: + underlying type 'enum __anonymous_enum__' at sslt.h:519:1 changed: + type size hasn't changed + 1 enumerator change: + '__anonymous_enum__::ssl_tls13_encrypted_client_hello_xtn' from value '65033' to '65034' at sslt.h:519:1 + + diff --git a/cmd/selfserv/selfserv.c b/cmd/selfserv/selfserv.c index 6b6f53a353..00de3a6b7d 100644 --- a/cmd/selfserv/selfserv.c +++ b/cmd/selfserv/selfserv.c @@ -1894,16 +1894,23 @@ configureEchWithPublicName(PRFileDesc *model_sock, const char *public_name) SECKEYPrivateKey *privKey = NULL; SECOidData *oidData; char *echConfigBase64 = NULL; + PRUint8 configId = 0; PRUint8 configBuf[1000]; unsigned int len = 0; - unsigned int echCipherSuite = ((unsigned int)HpkeKdfHkdfSha256 << 16) | - HpkeAeadChaCha20Poly1305; + HpkeSymmetricSuite echCipherSuite = { HpkeKdfHkdfSha256, + HpkeAeadChaCha20Poly1305 }; + PK11SlotInfo *slot = PK11_GetInternalKeySlot(); if (!slot) { errWarn("PK11_GetInternalKeySlot failed"); return SECFailure; } + if (PK11_GenerateRandom(&configId, sizeof(configId)) != SECSuccess) { + errWarn("Failed to generate random configId"); + goto loser; + } + oidData = SECOID_FindOIDByTag(SEC_OID_CURVE25519); if (oidData && (2 + oidData->oid.len) < sizeof(paramBuf)) { ecParams.data[0] = SEC_ASN1_OBJECT_ID; @@ -1916,16 +1923,17 @@ configureEchWithPublicName(PRFileDesc *model_sock, const char *public_name) } privKey = PK11_GenerateKeyPair(slot, CKM_EC_KEY_PAIR_GEN, &ecParams, &pubKey, PR_FALSE, PR_FALSE, NULL); - if (!privKey || !pubKey) { errWarn("Failed to generate ECH keypair"); goto loser; } - rv = SSL_EncodeEchConfig(echParamsStr, &echCipherSuite, 1, - HpkeDhKemX25519Sha256, pubKey, 50, - configBuf, &len, sizeof(configBuf)); + + rv = SSL_EncodeEchConfigId(configId, echParamsStr, 100, + HpkeDhKemX25519Sha256, pubKey, + &echCipherSuite, 1, + configBuf, &len, sizeof(configBuf)); if (rv != SECSuccess) { - errWarn("SSL_EncodeEchConfig failed"); + errWarn("SSL_EncodeEchConfigId failed"); goto loser; } diff --git a/gtests/ssl_gtest/libssl_internals.c b/gtests/ssl_gtest/libssl_internals.c index db0c9e86bc..b6a2bd3be4 100644 --- a/gtests/ssl_gtest/libssl_internals.c +++ b/gtests/ssl_gtest/libssl_internals.c @@ -497,24 +497,3 @@ SECStatus SSLInt_SetRawEchConfigForRetry(PRFileDesc *fd, const uint8_t *buf, PORT_Memcpy(cfg->raw.data, buf, len); return SECSuccess; } - -// Zero the echConfig.config_id for all configured echConfigs. -// This mimics a collision on the 8B config ID so that we can -// test trial decryption. -SECStatus SSLInt_ZeroEchConfigIds(PRFileDesc *fd) { - if (!fd) { - return SECFailure; - } - sslSocket *ss = ssl_FindSocket(fd); - if (!ss) { - return SECFailure; - } - - for (PRCList *cur_p = PR_LIST_HEAD(&ss->echConfigs); cur_p != &ss->echConfigs; - cur_p = PR_NEXT_LINK(cur_p)) { - PORT_Memset(((sslEchConfig *)cur_p)->configId, 0, - sizeof(((sslEchConfig *)cur_p)->configId)); - } - - return SECSuccess; -} diff --git a/gtests/ssl_gtest/libssl_internals.h b/gtests/ssl_gtest/libssl_internals.h index 372f254d61..c21c4a7da7 100644 --- a/gtests/ssl_gtest/libssl_internals.h +++ b/gtests/ssl_gtest/libssl_internals.h @@ -51,5 +51,4 @@ SECStatus SSLInt_SetDCAdvertisedSigSchemes(PRFileDesc *fd, SECStatus SSLInt_RemoveServerCertificates(PRFileDesc *fd); SECStatus SSLInt_SetRawEchConfigForRetry(PRFileDesc *fd, const uint8_t *buf, size_t len); -SECStatus SSLInt_ZeroEchConfigIds(PRFileDesc *fd); #endif // ifndef libssl_internals_h_ diff --git a/gtests/ssl_gtest/tls_connect.cc b/gtests/ssl_gtest/tls_connect.cc index 6456bff563..b5438f793e 100644 --- a/gtests/ssl_gtest/tls_connect.cc +++ b/gtests/ssl_gtest/tls_connect.cc @@ -262,7 +262,7 @@ void TlsConnectTestBase::MakeEcKeyParams(SECItem* params, SSLNamedGroup group) { } void TlsConnectTestBase::GenerateEchConfig( - HpkeKemId kem_id, const std::vector& cipher_suites, + HpkeKemId kem_id, const std::vector& cipher_suites, const std::string& public_name, uint16_t max_name_len, DataBuffer& record, ScopedSECKEYPublicKey& pubKey, ScopedSECKEYPrivateKey& privKey) { bool gen_keys = !pubKey && !privKey; @@ -282,9 +282,9 @@ void TlsConnectTestBase::GenerateEchConfig( SECITEM_FreeItem(&ecParams, PR_FALSE); PRUint8 encoded[1024]; unsigned int encoded_len = 0; - SECStatus rv = SSL_EncodeEchConfig( - public_name.c_str(), cipher_suites.data(), cipher_suites.size(), kem_id, - pub, max_name_len, encoded, &encoded_len, sizeof(encoded)); + SECStatus rv = SSL_EncodeEchConfigId( + 77, public_name.c_str(), max_name_len, kem_id, pub, cipher_suites.data(), + cipher_suites.size(), encoded, &encoded_len, sizeof(encoded)); EXPECT_EQ(SECSuccess, rv); EXPECT_GT(encoded_len, 0U); @@ -305,10 +305,9 @@ void TlsConnectTestBase::SetupEch(std::shared_ptr& client, ScopedSECKEYPublicKey pub; ScopedSECKEYPrivateKey priv; DataBuffer record; - static const std::vector kDefaultSuites = { - (static_cast(HpkeKdfHkdfSha256) << 16) | - HpkeAeadChaCha20Poly1305, - (static_cast(HpkeKdfHkdfSha256) << 16) | HpkeAeadAes128Gcm}; + static const std::vector kDefaultSuites = { + {HpkeKdfHkdfSha256, HpkeAeadChaCha20Poly1305}, + {HpkeKdfHkdfSha256, HpkeAeadAes128Gcm}}; GenerateEchConfig(kem_id, kDefaultSuites, "public.name", 100, record, pub, priv); diff --git a/gtests/ssl_gtest/tls_connect.h b/gtests/ssl_gtest/tls_connect.h index 6acb809771..a44846bcf5 100644 --- a/gtests/ssl_gtest/tls_connect.h +++ b/gtests/ssl_gtest/tls_connect.h @@ -147,12 +147,10 @@ class TlsConnectTestBase : public ::testing::Test { void RestoreAlgorithmPolicy(); static void MakeEcKeyParams(SECItem* params, SSLNamedGroup group); - static void GenerateEchConfig(HpkeKemId kem_id, - const std::vector& cipher_suites, - const std::string& public_name, - uint16_t max_name_len, DataBuffer& record, - ScopedSECKEYPublicKey& pubKey, - ScopedSECKEYPrivateKey& privKey); + static void GenerateEchConfig( + HpkeKemId kem_id, const std::vector& cipher_suites, + const std::string& public_name, uint16_t max_name_len, DataBuffer& record, + ScopedSECKEYPublicKey& pubKey, ScopedSECKEYPrivateKey& privKey); void SetupEch(std::shared_ptr& client, std::shared_ptr& server, HpkeKemId kem_id = HpkeDhKemX25519Sha256, diff --git a/gtests/ssl_gtest/tls_ech_unittest.cc b/gtests/ssl_gtest/tls_ech_unittest.cc index f78bfe377a..9d082c6f59 100644 --- a/gtests/ssl_gtest/tls_ech_unittest.cc +++ b/gtests/ssl_gtest/tls_ech_unittest.cc @@ -35,18 +35,18 @@ class TlsAgentEchTest : public TlsAgentTestClient13 { static std::string kPublicName("public.name"); -static const std::vector kDefaultSuites = { - (static_cast(HpkeKdfHkdfSha256) << 16) | HpkeAeadChaCha20Poly1305, - (static_cast(HpkeKdfHkdfSha256) << 16) | HpkeAeadAes128Gcm}; -static const std::vector kSuiteChaCha = { - (static_cast(HpkeKdfHkdfSha256) << 16) | - HpkeAeadChaCha20Poly1305}; -static const std::vector kSuiteAes = { - (static_cast(HpkeKdfHkdfSha256) << 16) | HpkeAeadAes128Gcm}; -std::vector kBogusSuite = {0xfefefefe}; -static const std::vector kUnknownFirstSuite = { - 0xfefefefe, - (static_cast(HpkeKdfHkdfSha256) << 16) | HpkeAeadAes128Gcm}; +static const std::vector kDefaultSuites = { + {HpkeKdfHkdfSha256, HpkeAeadChaCha20Poly1305}, + {HpkeKdfHkdfSha256, HpkeAeadAes128Gcm}}; +static const std::vector kSuiteChaCha = { + {HpkeKdfHkdfSha256, HpkeAeadChaCha20Poly1305}}; +static const std::vector kSuiteAes = { + {HpkeKdfHkdfSha256, HpkeAeadAes128Gcm}}; +std::vector kBogusSuite = { + {static_cast(0xfefe), static_cast(0xfefe)}}; +static const std::vector kUnknownFirstSuite = { + {static_cast(0xfefe), static_cast(0xfefe)}, + {HpkeKdfHkdfSha256, HpkeAeadAes128Gcm}}; class TlsConnectStreamTls13Ech : public TlsConnectTestBase { public: @@ -149,7 +149,7 @@ class TlsConnectStreamTls13Ech : public TlsConnectTestBase { ADD_FAILURE() << "No slot"; return; } - std::vector pkcs8_r = hex_string_to_bytes(kFixedServerPubkey); + std::vector pkcs8_r = hex_string_to_bytes(kFixedServerKey); SECItem pkcs8_r_item = {siBuffer, toUcharPtr(pkcs8_r.data()), static_cast(pkcs8_r.size())}; @@ -163,18 +163,6 @@ class TlsConnectStreamTls13Ech : public TlsConnectTestBase { ASSERT_NE(nullptr, tmp_pub); } - private: - // Testing certan invalid CHInner configurations is tricky, particularly - // since the CHOuter forms AAD and isn't available in filters. Instead of - // generating these inputs on the fly, use a fixed server keypair so that - // the input can be generated once (e.g. via a debugger) and replayed in - // each invocation of the test. - std::string kFixedServerPubkey = - "3067020100301406072a8648ce3d020106092b06010401da470f01044c304a" - "02010104205a8aa0d2476b28521588e0c704b14db82cdd4970d340d293a957" - "6deaee9ec1c7a1230321008756e2580c07c1d2ffcb662f5fadc6d6ff13da85" - "abd7adfecf984aaa102c1269"; - void SetMutualEchConfigs(ScopedSECKEYPublicKey& pub, ScopedSECKEYPrivateKey& priv) { DataBuffer echconfig; @@ -188,6 +176,18 @@ class TlsConnectStreamTls13Ech : public TlsConnectTestBase { SSL_SetClientEchConfigs(client_->ssl_fd(), echconfig.data(), echconfig.len())); } + + private: + // Testing certan invalid CHInner configurations is tricky, particularly + // since the CHOuter forms AAD and isn't available in filters. Instead of + // generating these inputs on the fly, use a fixed server keypair so that + // the input can be generated once (e.g. via a debugger) and replayed in + // each invocation of the test. + std::string kFixedServerKey = + "3067020100301406072a8648ce3d020106092b06010401da470f01044c304a" + "02010104205a8aa0d2476b28521588e0c704b14db82cdd4970d340d293a957" + "6deaee9ec1c7a1230321008756e2580c07c1d2ffcb662f5fadc6d6ff13da85" + "abd7adfecf984aaa102c1269"; }; static void CheckCertVerifyPublicName(TlsAgent* agent) { @@ -225,10 +225,10 @@ TEST_P(TlsAgentEchTest, EchConfigsSupportedYesNo) { // ECHConfig 2 cipher_suites are unsupported. const std::string mixed = - "0086FE09003F000B7075626C69632E6E616D6500203BB6D46C201B820F1AE4AFD4DEC304" - "444156E4E04D1BF0FFDA7783B6B457F75600200008000100030001000100640000FE0900" - "3F000B7075626C69632E6E616D6500203BB6D46C201B820F1AE4AFD4DEC304444156E4E0" - "4D1BF0FFDA7783B6B457F756002000080001FFFFFFFF000100640000"; + "0088FE0A004000002000203BB6D46C201B820F1AE4AFD4DEC304444156E4E04D1BF0FFDA" + "7783B6B457F756000800010003000100010064000B7075626C69632E6E616D650000FE0A" + "004000002000203BB6D46C201B820F1AE4AFD4DEC304444156E4E04D1BF0FFDA7783B6B4" + "57F75600080001FFFFFFFF00010064000B7075626C69632E6E616D650000"; std::vector config = hex_string_to_bytes(mixed); DataBuffer echconfig(config.data(), config.size()); @@ -250,10 +250,10 @@ TEST_P(TlsAgentEchTest, EchConfigsSupportedNoYes) { // ECHConfig 1 cipher_suites are unsupported. const std::string mixed = - "0086FE09003F000B7075626C69632E6E616D6500203BB6D46C201B820F1AE4AFD4DEC304" - "444156E4E04D1BF0FFDA7783B6B457F756002000080001FFFFFFFF000100640000FE0900" - "3F000B7075626C69632E6E616D6500203BB6D46C201B820F1AE4AFD4DEC304444156E4E0" - "4D1BF0FFDA7783B6B457F75600200008000100030001000100640000"; + "0088FE0A004000002000203BB6D46C201B820F1AE4AFD4DEC304444156E4E04D1BF0FFDA" + "7783B6B457F75600080001FFFFFFFF00010064000B7075626C69632E6E616D650000FE0A" + "004000002000203BB6D46C201B820F1AE4AFD4DEC304444156E4E04D1BF0FFDA7783B6B4" + "57F756000800010003000100010064000B7075626C69632E6E616D650000"; std::vector config = hex_string_to_bytes(mixed); DataBuffer echconfig(config.data(), config.size()); @@ -275,10 +275,10 @@ TEST_P(TlsAgentEchTest, EchConfigsSupportedNoNo) { // ECHConfig 1 and 2 cipher_suites are unsupported. const std::string unsupported = - "0086FE09003F000B7075626C69632E6E616D6500203BB6D46C201B820F1AE4AFD4DEC304" - "444156E4E04D1BF0FFDA7783B6B457F756002000080001FFFF0001FFFF00640000FE0900" - "3F000B7075626C69632E6E616D6500203BB6D46C201B820F1AE4AFD4DEC304444156E4E0" - "4D1BF0FFDA7783B6B457F75600200008FFFF0003FFFF000100640000"; + "0088FE0A004000002000203BB6D46C201B820F1AE4AFD4DEC304444156E4E04D1BF0FFDA" + "7783B6B457F75600080001FFFF0001FFFF0064000B7075626C69632E6E616D650000FE0A" + "004000002000203BB6D46C201B820F1AE4AFD4DEC304444156E4E04D1BF0FFDA7783B6B4" + "57F7560008FFFF0003FFFF00010064000B7075626C69632E6E616D650000"; std::vector config = hex_string_to_bytes(unsupported); DataBuffer echconfig(config.data(), config.size()); @@ -354,7 +354,7 @@ TEST_P(TlsAgentEchTest, UnsupportedHpkeKem) { ScopedSECKEYPublicKey pub; ScopedSECKEYPrivateKey priv; DataBuffer echconfig; - // SSL_EncodeEchConfig encodes without validation. + // SSL_EncodeEchConfigId encodes without validation. TlsConnectTestBase::GenerateEchConfig(static_cast(0xff), kDefaultSuites, kPublicName, 100, echconfig, pub, priv); @@ -451,50 +451,50 @@ TEST_P(TlsAgentEchTest, ApiInvalidArgs) { // GetRetries EXPECT_EQ(SECFailure, SSL_GetEchRetryConfigs(agent_->ssl_fd(), nullptr)); - // EncodeEchConfig + // EncodeEchConfigId EXPECT_EQ(SECFailure, - SSL_EncodeEchConfig(nullptr, reinterpret_cast(1), 1, - static_cast(1), - reinterpret_cast(1), 1, - reinterpret_cast(1), - reinterpret_cast(1), 1)); + SSL_EncodeEchConfigId(0, nullptr, 1, static_cast(1), + reinterpret_cast(1), + reinterpret_cast(1), 1, + reinterpret_cast(1), + reinterpret_cast(1), 1)); EXPECT_EQ(SECFailure, - SSL_EncodeEchConfig("name", nullptr, 1, static_cast(1), - reinterpret_cast(1), 1, - reinterpret_cast(1), - reinterpret_cast(1), 1)); - EXPECT_EQ(SECFailure, - SSL_EncodeEchConfig("name", reinterpret_cast(1), 0, - static_cast(1), - reinterpret_cast(1), 1, - reinterpret_cast(1), - reinterpret_cast(1), 1)); - + SSL_EncodeEchConfigId(0, "name", 1, static_cast(1), + reinterpret_cast(1), + nullptr, 1, reinterpret_cast(1), + reinterpret_cast(1), 1)); EXPECT_EQ(SECFailure, - SSL_EncodeEchConfig("name", reinterpret_cast(1), 1, - static_cast(1), nullptr, 1, - reinterpret_cast(1), - reinterpret_cast(1), 1)); + SSL_EncodeEchConfigId(0, "name", 1, static_cast(1), + reinterpret_cast(1), + reinterpret_cast(1), 0, + reinterpret_cast(1), + reinterpret_cast(1), 1)); + + EXPECT_EQ(SECFailure, SSL_EncodeEchConfigId( + 0, "name", 1, static_cast(1), nullptr, + reinterpret_cast(1), 1, + reinterpret_cast(1), + reinterpret_cast(1), 1)); EXPECT_EQ(SECFailure, - SSL_EncodeEchConfig(nullptr, reinterpret_cast(1), 1, - static_cast(1), - reinterpret_cast(1), 0, - reinterpret_cast(1), - reinterpret_cast(1), 1)); + SSL_EncodeEchConfigId(0, nullptr, 0, static_cast(1), + reinterpret_cast(1), + reinterpret_cast(1), 1, + reinterpret_cast(1), + reinterpret_cast(1), 1)); + + EXPECT_EQ(SECFailure, SSL_EncodeEchConfigId( + 0, "name", 1, static_cast(1), + reinterpret_cast(1), + reinterpret_cast(1), 1, + nullptr, reinterpret_cast(1), 1)); EXPECT_EQ(SECFailure, - SSL_EncodeEchConfig("name", reinterpret_cast(1), 1, - static_cast(1), - reinterpret_cast(1), 1, - nullptr, reinterpret_cast(1), 1)); - - EXPECT_EQ(SECFailure, - SSL_EncodeEchConfig("name", reinterpret_cast(1), 1, - static_cast(1), - reinterpret_cast(1), 1, - reinterpret_cast(1), nullptr, 1)); + SSL_EncodeEchConfigId(0, "name", 1, static_cast(1), + reinterpret_cast(1), + reinterpret_cast(1), 1, + reinterpret_cast(1), nullptr, 1)); } TEST_P(TlsAgentEchTest, NoEarlyRetryConfigs) { @@ -583,21 +583,45 @@ TEST_P(TlsAgentEchTest, EchConfigDuplicateExtensions) { ASSERT_FALSE(filter->captured()); } +TEST_F(TlsConnectStreamTls13Ech, EchFixedConfig) { + ScopedSECKEYPublicKey pub; + ScopedSECKEYPrivateKey priv; + EnsureTlsSetup(); + ImportFixedEchKeypair(pub, priv); + SetMutualEchConfigs(pub, priv); + + client_->ExpectEch(); + server_->ExpectEch(); + Connect(); +} + +// The next set of tests all use a fixed server key and a pre-built ClientHello. +// This ClientHelo can be constructed using the above EchFixedConfig test, +// modifying tls13_ConstructInnerExtensionsFromOuter as indicated. For this +// small number of tests, these fixed values are easier to construct than +// constructing ClientHello in the test that can be successfully decrypted. + // Test an encoded ClientHelloInner containing an extra extensionType // in outer_extensions, for which there is no corresponding (uncompressed) // extension in ClientHelloOuter. TEST_F(TlsConnectStreamTls13Ech, EchOuterExtensionsReferencesMissing) { + // Construct this by prepending 0xabcd to ssl_tls13_outer_extensions_xtn. std::string ch = - "010001580303dfff91b5e1ba00f29d2338419b3abf125ee1051a942ae25163bbf609a1ea" - "11920000061301130313020100012900000010000e00000b7075626c69632e6e616d65ff" + "01000200030341a6813ccf3eefc2deb9c78f7627715ae343f5236e7224f454c723c93e0b" + "d875000006130113031302010001d100000010000e00000b7075626c69632e6e616d65ff" "01000100000a00140012001d00170018001901000101010201030104003300260024001d" - "0020d94c1590c261e9ea8ae55bc9581f397cc598115f8b70aec1b0236f4c8c555537002b" + "00200573a70286658ad4bc166d8f5f237f035714ba5ae4e838c077677ccb6d619813002b" "0003020304000d0018001604030503060302030804080508060401050106010201002d00" - "020101001c00024001fe09009b0001000308fde4163c5c6e8bb6002067a895efa2721c88" - "63ecfa1bea1e520ae6f6cf938e3e37802688f7a83a871a04006aa693f053f87db87cf82a" - "7caa20670d79b92ccda97893fdf99352fc766fb3dd5570948311dddb6d41214234fae585" - "e354a048c072b3fb00a0a64e8e089e4a90152ee91a2c5b947c99d3dcebfb6334453b023d" - "4d725010996a290a0552e4b238ec91c21440adc0d51a4435"; + "020101001c00024001001500aa0000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000fe0a0095000100034d00209c68779a77e4bac0e9f39c2974b1900de044ae1510bf" + "d34fb5120a2a9d039607006c76c4571099733157eb8614ef2ad6049372e9fdf740f8ad4f" + "d24723702c9104a38ecc366eea78b0285422b3f119fc057e2282433a74d8c56b2135c785" + "bd5d01f89b2dbb42aa9a609eb1c6dd89252fa04cf8fbc4097e9c85da1e3eeebc188bbe16" + "db1166f6df1a0c7c6e0dce71"; ReplayChWithMalformedInner(ch, kTlsAlertIllegalParameter, SSL_ERROR_RX_MALFORMED_ECH_EXTENSION, SSL_ERROR_ILLEGAL_PARAMETER_ALERT); @@ -605,17 +629,23 @@ TEST_F(TlsConnectStreamTls13Ech, EchOuterExtensionsReferencesMissing) { // Drop supported_versions from CHInner, make sure we don't negotiate 1.2+ECH. TEST_F(TlsConnectStreamTls13Ech, EchVersion12Inner) { + // Construct this by removing ssl_tls13_supported_versions_xtn entirely. std::string ch = - "0100015103038fbe6f75b0123116fa5c4eccf0cf26c17ab1ded5529307e419c036ac7e9c" - "e8e30000061301130313020100012200000010000e00000b7075626c69632e6e616d65ff" + "010002000303baf30ea25e5056b659a4d55233922c4ee261a04e6d84c8200713edca2f55" + "d434000006130113031302010001d100000010000e00000b7075626c69632e6e616d65ff" "01000100000a00140012001d00170018001901000101010201030104003300260024001d" - "002078d644583b4f056bec4d8ae9bddd383aed6eb7cdb3294f88b0e37a4f26a02549002b" + "002081908a3cf3ed9ebf6d6b1f7082d77bb3bf8ff309c3c1255421720c4172548762002b" "0003020304000d0018001604030503060302030804080508060401050106010201002d00" - "020101001c00024001fe0900940001000308fde4163c5c6e8bb600208958e66d1d4bbd46" - "4792f392e119dbce91ee3e65067899b45c83855dae61e67a00637df038e7b35483786707" - "dd1b25be5cd3dd07f1ca4b33a3595ddb959e5c0da3d2f0b3314417614968691700c05232" - "07c729b34f3b5de62728b3cb6b45b00e6f94b204a9504d0e7e24c66f42aacc73591c86ef" - "571e61cebd6ba671081150a2dae89e7493"; + "020101001c00024001001500b30000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000fe0a008c000100034d0020305bc263a664387b90a6975b2a" + "3aa1e4358e80a8ca0841237035d2475628582d006352a2b49912a61543dfa045c1429582" + "540c8c7019968867fde698eb37667f9aa9c23d02757492a4580fb027bbe4ba7615eea118" + "ad3bf7f02a88f8372cfa01888e7be0c55616f846e902bbdfc7edf56994d6398f5a965d9e" + "c4b1bc7afc580b28b0ac91d8"; ReplayChWithMalformedInner(ch, kTlsAlertProtocolVersion, SSL_ERROR_UNSUPPORTED_VERSION, SSL_ERROR_PROTOCOL_VERSION_ALERT); @@ -623,35 +653,48 @@ TEST_F(TlsConnectStreamTls13Ech, EchVersion12Inner) { // Use CHInner supported_versions to negotiate 1.2. TEST_F(TlsConnectStreamTls13Ech, EchVersion12InnerSupportedVersions) { + // Construct this by changing ssl_tls13_supported_versions_xtn to write + // TLS 1.2 instead of TLS 1.3. std::string ch = - "01000158030378a601a3f12229e53e0b8d92c3599bf1782e8261d2ecaec9bbe595d4c901" - "98770000061301130313020100012900000010000e00000b7075626c69632e6e616d65ff" + "0100020003036c4a7f6f6b5479a5c1f769c7b04c082ba40b514522d193df855df8bea933" + "b565000006130113031302010001d100000010000e00000b7075626c69632e6e616d65ff" "01000100000a00140012001d00170018001901000101010201030104003300260024001d" - "00201c8017d6970f3a92ac1c9919c3a26788052f84599fb0c3cb7bd381304148724e002b" + "0020ee721b8fe89260f8987d0d21b628db136c6155793fa63f4f546b244ee5357761002b" "0003020304000d0018001604030503060302030804080508060401050106010201002d00" - "020101001c00024001fe09009b0001000308fde4163c5c6e8bb60020f7347d34f125e866" - "76b1cdc43455c6c00918a3c8a961335e1b9aa864da2b5313006a21e6ad81533e90cea24e" - "c2c3656f6b53114b4c63bf89462696f1c8ad4e1193d87062a5537edbe83c9b35c41e9763" - "1d2333270854758ee02548afb7f2264f904474465415a5085024487f22b017208e250ca4" - "7902d61d98fbd1cb8afc0a14dcd70a68343cf67c258758d9"; + "020101001c00024001001500ac0000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "0000000000fe0a0093000100034d00205de27fe39481bfb370ee8441f12e28296bc5c8fe" + "b6a6198ddc6ab03b2d024638006a9e9f57c3f39c0ad1c3427549a77f301d01d718e09da4" + "5497df178c95fd598bf0c9098d68dfba80a05eeeabc84dc0bb3225cee4a74688d520c632" + "73612f98be847dea4f040a8d9b2b92bb4a44273d0cafafbfe1ee4ed69448bc243b4359c6" + "e1eb3971e125fbfb016245fa"; ReplayChWithMalformedInner(ch, kTlsAlertProtocolVersion, SSL_ERROR_UNSUPPORTED_VERSION, SSL_ERROR_PROTOCOL_VERSION_ALERT); } // Replay a CH for which CHInner lacks the required ech_is_inner extension. -TEST_F(TlsConnectStreamTls13Ech, EchInnerMissingEmptyEch) { +TEST_F(TlsConnectStreamTls13Ech, EchInnerMissing) { + // Construct by omitting ssl_tls13_ech_is_inner_xtn. std::string ch = - "010001540303033b3284790ada882445bfb38b8af3509659033c931e6ae97febbaa62b19" - "b4ac0000061301130313020100012500000010000e00000b7075626c69632e6e616d65ff" + "010002000303912d293136b843248ffeecdde6ef0d5bc5d0adb4d356b985c2fcec8fe2b0" + "8f5d000006130113031302010001d100000010000e00000b7075626c69632e6e616d65ff" "01000100000a00140012001d00170018001901000101010201030104003300260024001d" - "00209d1ed410ccb05ce9e424f52b1be3599bcc1efb0913ae14a24d9a69cbfbc39744002b" + "00209222e6b0c672fd1e564fbca5889155e9126e3b006a8ff77ff61898dd56a08429002b" "0003020304000d0018001604030503060302030804080508060401050106010201002d00" - "020101001c00024001fe0900970001000308fde4163c5c6e8bb600206321bdc543a23d47" - "7a7104ba69177cb722927c6c485117df4a077b8e82167f0b0066103d9aac7e5fc4ef990b" - "2ce38593589f7f6ba043847d7db6c9136adb811f63b956d56e6ca8cbe6864e3fc43a3bc5" - "94a332d4d63833e411c89ef14af63b5cd18c7adee99ffd1ad3112449ea18d6650bbaca66" - "528f7e4146fafbf338c27cf89b145a55022b26a3"; + "020101001c00024001001500b00000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000fe0a008f000100034d0020e1bc83c066a251621c4b055779789a2c" + "6ac4b9a3850366b2ea0d32a8e041181c0066a4e9cc6912b8bc6c1b54a2c6c40428ab01a3" + "0e4621ec65320df2beff846a606618429c108fdfe24a6fad5053f53fb36082bf7d80d6f4" + "73131a3d6c04e182595606070ac4e0be078ada56456c02d37a2fee7cb17accd273cbd810" + "30ee0dbe139e51ba1d2a471f"; ReplayChWithMalformedInner(ch, kTlsAlertIllegalParameter, SSL_ERROR_MISSING_ECH_EXTENSION, SSL_ERROR_ILLEGAL_PARAMETER_ALERT); @@ -660,17 +703,24 @@ TEST_F(TlsConnectStreamTls13Ech, EchInnerMissingEmptyEch) { // Replay a CH for which CHInner contains both an ECH and ech_is_inner // extension. TEST_F(TlsConnectStreamTls13Ech, InnerWithEchAndEchIsInner) { + // Construct by appending an empty ssl_tls13_encrypted_client_hello_xtn to + // CHInner. std::string ch = - "0100015c030383fb49c98b62bcdf04cbbae418dd684f8f9512f40fca6861ba40555269a9" - "789f0000061301130313020100012d00000010000e00000b7075626c69632e6e616d65ff" + "010002000303b690bc4090ecfd7ad167de639b1d1ea7682588ffefae1164179d370f6cd3" + "0864000006130113031302010001d100000010000e00000b7075626c69632e6e616d65ff" "01000100000a00140012001d00170018001901000101010201030104003300260024001d" - "00201e3d35a6755b7dddf7e481359429e9677baaa8dd99569c2bf0b0f7ea56e68b12002b" + "00200c3c15b0e9884d5f593634a168b70a62ae18c8d69a68f8e29c826fbabcd99b22002b" "0003020304000d0018001604030503060302030804080508060401050106010201002d00" - "020101001c00024001fe09009f0001000308fde4163c5c6e8bb6002090110b89c1ba6618" - "942ea7aae8c472c22e97f10bef7dd490bee50cc108082b48006eed016fa2b3e3419cf5ef" - "9b41ab9ecffa84a4b60e2f4cc710cf31c739d1f6f88b48207aaf7ccabdd744a25a8f2a38" - "029d1b133e9d990681cf08c07a255d9242b3a002bc0865935cbb609b2b1996fab0626cb0" - "2ece6544bbde0d3218333ffd95c383a41854b76b1a254bb346a2702b"; + "020101001c00024001001500a80000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "000000000000000000000000000000000000000000000000000000000000000000000000" + "00fe0a0097000100034d0020d46cc9042eff6efee046a5ff653d1b6a60cfd5786afef5ce" + "43300bc515ef5f09006ea6bf626854596df74b2d8f81a479a6d2fef13295a81e0571008a" + "12fc92f82170fdb899cd22ebadc33a3147c2801619f7621ffe8684c96918443e3fbe9b17" + "34fbf678ba0b2abad7ab6b018bccc1034b9537a5d399fdb9a5ccb92360bde4a94a2f2509" + "0e7313dd9254eae3603e1fee"; ReplayChWithMalformedInner(ch, kTlsAlertIllegalParameter, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, SSL_ERROR_ILLEGAL_PARAMETER_ALERT); @@ -700,13 +750,13 @@ TEST_F(TlsConnectStreamTls13Ech, EchConfigsTrialDecrypt) { ImportFixedEchKeypair(pub, priv); const std::string two_configs_str = - "007EFE09003B000B7075626C69632E6E616D650020111111111111111111111111111111" - "1111111111111111111111111111111111002000040001000100640000fe09003B000B70" - "75626C69632E6E616D6500208756E2580C07C1D2FFCB662F5FADC6D6FF13DA85ABD7ADFE" - "CF984AAA102C1269002000040001000100640000"; + "0080FE0A003C000020002011111111111111111111111111111111111111111111111111" + "111111111111110004000100010064000B7075626C69632E6E616D650000FE0A003C0000" + "2000208756E2580C07C1D2FFCB662F5FADC6D6FF13DA85ABD7ADFECF984AAA102C126900" + "04000100010064000B7075626C69632E6E616D650000"; const std::string second_config_str = - "003FFE09003B000B7075626C69632E6E616D6500208756E2580C07C1D2FFCB662F5FADC6" - "D6FF13DA85ABD7ADFECF984AAA102C1269002000040001000100640000"; + "0040FE0A003C00002000208756E2580C07C1D2FFCB662F5FADC6D6FF13DA85ABD7ADFECF" + "984AAA102C12690004000100010064000B7075626C69632E6E616D650000"; std::vector two_configs = hex_string_to_bytes(two_configs_str); std::vector second_config = hex_string_to_bytes(second_config_str); ASSERT_EQ(SECSuccess, @@ -716,38 +766,11 @@ TEST_F(TlsConnectStreamTls13Ech, EchConfigsTrialDecrypt) { SSL_SetClientEchConfigs(client_->ssl_fd(), second_config.data(), second_config.size())); - ASSERT_EQ(SECSuccess, SSLInt_ZeroEchConfigIds(client_->ssl_fd())); - ASSERT_EQ(SECSuccess, SSLInt_ZeroEchConfigIds(server_->ssl_fd())); client_->ExpectEch(); server_->ExpectEch(); Connect(); } -// An empty config_id should prompt an alert. We don't support -// Optional Configuration Identifiers. -TEST_F(TlsConnectStreamTls13, EchRejectEmptyConfigId) { - static const uint8_t junk[16] = {0}; - DataBuffer junk_buf(junk, sizeof(junk)); - DataBuffer ech_xtn; - ech_xtn.Write(ech_xtn.len(), HpkeKdfHkdfSha256, 2); - ech_xtn.Write(ech_xtn.len(), HpkeAeadAes128Gcm, 2); - ech_xtn.Write(ech_xtn.len(), 0U, 1); // empty config_id - ech_xtn.Write(ech_xtn.len(), junk_buf.len(), 2); // enc - ech_xtn.Append(junk_buf); - ech_xtn.Write(ech_xtn.len(), junk_buf.len(), 2); // payload - ech_xtn.Append(junk_buf); - - EnsureTlsSetup(); - EXPECT_EQ(SECSuccess, SSL_EnableTls13GreaseEch(client_->ssl_fd(), - PR_FALSE)); // Don't GREASE - MakeTlsFilter(client_, kTlsHandshakeClientHello, - ssl_tls13_encrypted_client_hello_xtn, - ech_xtn); - ConnectExpectAlert(server_, kTlsAlertDecodeError); - client_->CheckErrorCode(SSL_ERROR_DECODE_ERROR_ALERT); - server_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_ECH_EXTENSION); -} - TEST_F(TlsConnectStreamTls13Ech, EchAcceptBasic) { EnsureTlsSetup(); SetupEch(client_, server_); diff --git a/lib/ssl/sslexp.h b/lib/ssl/sslexp.h index 8bacc6b421..6c84a20541 100644 --- a/lib/ssl/sslexp.h +++ b/lib/ssl/sslexp.h @@ -10,6 +10,7 @@ #include "ssl.h" #include "sslerr.h" +#include "pk11hpke.h" SEC_BEGIN_PROTOS @@ -566,30 +567,33 @@ typedef SECStatus(PR_CALLBACK *SSLResumptionTokenCallback)( /* * Generate an encoded ECHConfig structure (presumably server side). * + * configId -- an identifier for the configuration. * publicName -- the public_name value to be placed in SNI. - * hpkeSuites -- the HPKE cipher suites that can be used - * hpkeSuitesCount -- the number of suites in hpkeSuites + * maxNameLen -- the maximum length of protected names * kemId -- the HKPE KEM ID value - * group -- the named group this key corresponds to * pubKey -- the public key for the key pair - * pad -- the maximum length to pad to + * hpkeSuites -- the HPKE cipher suites that can be used + * hpkeSuitesCount -- the number of suites in hpkeSuites * out/outlen/maxlen -- where to output the data */ -#define SSL_EncodeEchConfig(publicName, hpkeSuites, hpkeSuitesCount, \ - kemId, pubKey, maxNameLen, out, outlen, \ - maxlen) \ - SSL_EXPERIMENTAL_API("SSL_EncodeEchConfig", \ - (const char *_publicName, \ - const PRUint32 *_hpkeSuites, \ - unsigned int _hpkeSuitesCount, \ - HpkeKemId _kemId, \ - const SECKEYPublicKey *_pubKey, \ - PRUint16 _maxNameLen, \ - PRUint8 *_out, unsigned int *_outlen, \ - unsigned int _maxlen), \ - (publicName, hpkeSuites, hpkeSuitesCount, \ - kemId, pubKey, maxNameLen, out, outlen, \ - maxlen)) +typedef struct HpkeSymmetricSuiteStr { + HpkeKdfId kdfId; + HpkeAeadId aeadId; +} HpkeSymmetricSuite; +#define SSL_EncodeEchConfigId(configId, publicName, maxNameLen, \ + kemId, pubKey, hpkeSuites, hpkeSuiteCount, \ + out, outlen, maxlen) \ + SSL_EXPERIMENTAL_API("SSL_EncodeEchConfigId", \ + (PRUint8 _configId, const char *_publicName, \ + unsigned int _maxNameLen, HpkeKemId _kemId, \ + const SECKEYPublicKey *_pubKey, \ + const HpkeSymmetricSuite *_hpkeSuites, \ + unsigned int _hpkeSuiteCount, \ + PRUint8 *_out, unsigned int *_outlen, \ + unsigned int _maxlen), \ + (configId, publicName, maxNameLen, \ + kemId, pubKey, hpkeSuites, hpkeSuiteCount, \ + out, outlen, maxlen)) /* SSL_SetSecretCallback installs a callback that TLS calls when it installs new * traffic secrets. @@ -1035,6 +1039,7 @@ typedef struct SSLMaskingContextStr { #define SSL_EnableESNI(a, b, c, d) SSL_DEPRECATED_EXPERIMENTAL_API #define SSL_EncodeESNIKeys(a, b, c, d, e, f, g, h, i, j) SSL_DEPRECATED_EXPERIMENTAL_API #define SSL_SetESNIKeyPair(a, b, c, d) SSL_DEPRECATED_EXPERIMENTAL_API +#define SSL_EncodeEchConfig(a, b, c, d, e, f, g, h, i) SSL_DEPRECATED_EXPERIMENTAL_API SEC_END_PROTOS diff --git a/lib/ssl/sslsock.c b/lib/ssl/sslsock.c index b698d8f439..aa89f04e3f 100644 --- a/lib/ssl/sslsock.c +++ b/lib/ssl/sslsock.c @@ -4296,7 +4296,7 @@ struct { EXP(DestroyResumptionTokenInfo), EXP(EnableTls13BackendEch), EXP(EnableTls13GreaseEch), - EXP(EncodeEchConfig), + EXP(EncodeEchConfigId), EXP(GetCurrentEpoch), EXP(GetEchRetryConfigs), EXP(GetExtensionSupport), diff --git a/lib/ssl/sslt.h b/lib/ssl/sslt.h index 1d5c4d1790..2bc151c633 100644 --- a/lib/ssl/sslt.h +++ b/lib/ssl/sslt.h @@ -547,7 +547,7 @@ typedef enum { ssl_tls13_short_header_xtn = 0xff03, /* Deprecated. */ ssl_tls13_ech_is_inner_xtn = 0xda09, ssl_tls13_outer_extensions_xtn = 0xfd00, - ssl_tls13_encrypted_client_hello_xtn = 0xfe09, + ssl_tls13_encrypted_client_hello_xtn = 0xfe0a, ssl_tls13_encrypted_sni_xtn = 0xffce, /* Deprecated. */ } SSLExtensionType; diff --git a/lib/ssl/tls13ech.c b/lib/ssl/tls13ech.c index 0121f4d199..1891caf391 100644 --- a/lib/ssl/tls13ech.c +++ b/lib/ssl/tls13ech.c @@ -64,7 +64,6 @@ tls13_DestroyEchXtnState(sslEchXtnState *state) } SECITEM_FreeItem(&state->innerCh, PR_FALSE); SECITEM_FreeItem(&state->senderPubKey, PR_FALSE); - SECITEM_FreeItem(&state->configId, PR_FALSE); SECITEM_FreeItem(&state->retryConfigs, PR_FALSE); PORT_ZFree(state, sizeof(*state)); } @@ -103,12 +102,12 @@ tls13_CopyEchConfigs(PRCList *oConfigs, PRCList *configs) if (rv != SECSuccess) { goto loser; } + newConfig->contents.configId = config->contents.configId; newConfig->contents.kemId = config->contents.kemId; newConfig->contents.kdfId = config->contents.kdfId; newConfig->contents.aeadId = config->contents.aeadId; newConfig->contents.maxNameLen = config->contents.maxNameLen; newConfig->version = config->version; - PORT_Memcpy(newConfig->configId, config->configId, sizeof(newConfig->configId)); PR_APPEND_LINK(&newConfig->link, configs); } return SECSuccess; @@ -119,69 +118,34 @@ tls13_CopyEchConfigs(PRCList *oConfigs, PRCList *configs) return SECFailure; } -static SECStatus -tls13_DigestEchConfig(const sslEchConfig *cfg, PRUint8 *digest, size_t maxDigestLen) -{ - SECStatus rv; - PK11SymKey *configKey = NULL; - PK11SymKey *derived = NULL; - SECItem *derivedItem = NULL; - CK_HKDF_PARAMS params = { 0 }; - SECItem paramsi = { siBuffer, (unsigned char *)¶ms, sizeof(params) }; - PK11SlotInfo *slot = PK11_GetInternalSlot(); - - if (!slot) { - goto loser; - } - - configKey = PK11_ImportDataKey(slot, CKM_HKDF_DATA, PK11_OriginUnwrap, - CKA_DERIVE, CONST_CAST(SECItem, &cfg->raw), NULL); - if (!configKey) { - goto loser; - } - - /* We only support SHA256 KDF. */ - PORT_Assert(cfg->contents.kdfId == HpkeKdfHkdfSha256); - params.bExtract = CK_TRUE; - params.bExpand = CK_TRUE; - params.prfHashMechanism = CKM_SHA256; - params.ulSaltType = CKF_HKDF_SALT_NULL; - params.pInfo = CONST_CAST(CK_BYTE, hHkdfInfoEchConfigID); - params.ulInfoLen = strlen(hHkdfInfoEchConfigID); - derived = PK11_DeriveWithFlags(configKey, CKM_HKDF_DATA, - ¶msi, CKM_HKDF_DERIVE, CKA_DERIVE, 8, - CKF_SIGN | CKF_VERIFY); - - rv = PK11_ExtractKeyValue(derived); - if (rv != SECSuccess) { - goto loser; - } - - derivedItem = PK11_GetKeyData(derived); - if (!derivedItem) { - goto loser; - } - - if (derivedItem->len != maxDigestLen) { - PORT_SetError(SEC_ERROR_LIBRARY_FAILURE); - goto loser; - } - - PORT_Memcpy(digest, derivedItem->data, derivedItem->len); - PK11_FreeSymKey(configKey); - PK11_FreeSymKey(derived); - PK11_FreeSlot(slot); - return SECSuccess; - -loser: - PK11_FreeSymKey(configKey); - PK11_FreeSymKey(derived); - if (slot) { - PK11_FreeSlot(slot); - } - return SECFailure; -} - +/* + * struct { + * HpkeKdfId kdf_id; + * HpkeAeadId aead_id; + * } HpkeSymmetricCipherSuite; + * + * struct { + * uint8 config_id; + * HpkeKemId kem_id; + * HpkePublicKey public_key; + * HpkeSymmetricCipherSuite cipher_suites<4..2^16-4>; + * } HpkeKeyConfig; + * + * struct { + * HpkeKeyConfig key_config; + * uint16 maximum_name_length; + * opaque public_name<1..2^16-1>; + * Extension extensions<0..2^16-1>; + * } ECHConfigContents; + * + * struct { + * uint16 version; + * uint16 length; + * select (ECHConfig.version) { + * case 0xfe0a: ECHConfigContents contents; + * } + * } ECHConfig; + */ static SECStatus tls13_DecodeEchConfigContents(const sslReadBuffer *rawConfig, sslEchConfig **outConfig) @@ -199,30 +163,22 @@ tls13_DecodeEchConfigContents(const sslReadBuffer *rawConfig, sslReader extensionReader; PRBool hasValidSuite = PR_FALSE; - /* Parse the public_name. */ - rv = sslRead_ReadVariable(&configReader, 2, &tmpBuf); + /* HpkeKeyConfig key_config */ + /* uint8 config_id */ + rv = sslRead_ReadNumber(&configReader, 1, &tmpn); if (rv != SECSuccess) { goto loser; } + contents.configId = tmpn; - if (tmpBuf.len == 0) { - PORT_SetError(SSL_ERROR_RX_MALFORMED_ECH_CONFIG); - goto loser; - } - for (tmpn = 0; tmpn < tmpBuf.len; tmpn++) { - if (tmpBuf.buf[tmpn] == '\0') { - PORT_SetError(SSL_ERROR_RX_MALFORMED_ECH_CONFIG); - goto loser; - } - } - - contents.publicName = PORT_ZAlloc(tmpBuf.len + 1); - if (!contents.publicName) { + /* HpkeKemId kem_id */ + rv = sslRead_ReadNumber(&configReader, 2, &tmpn); + if (rv != SECSuccess) { goto loser; } - PORT_Memcpy(contents.publicName, (PRUint8 *)tmpBuf.buf, tmpBuf.len); + contents.kemId = tmpn; - /* Public key. */ + /* HpkePublicKey public_key */ rv = sslRead_ReadVariable(&configReader, 2, &tmpBuf); if (rv != SECSuccess) { goto loser; @@ -232,13 +188,7 @@ tls13_DecodeEchConfigContents(const sslReadBuffer *rawConfig, goto loser; } - rv = sslRead_ReadNumber(&configReader, 2, &tmpn); - if (rv != SECSuccess) { - goto loser; - } - contents.kemId = tmpn; - - /* Parse HPKE cipher suites. */ + /* HpkeSymmetricCipherSuite cipher_suites<4..2^16-4> */ rv = sslRead_ReadVariable(&configReader, 2, &tmpBuf); if (rv != SECSuccess) { goto loser; @@ -249,12 +199,12 @@ tls13_DecodeEchConfigContents(const sslReadBuffer *rawConfig, } suiteReader = (sslReader)SSL_READER(tmpBuf.buf, tmpBuf.len); while (SSL_READER_REMAINING(&suiteReader)) { - /* kdf_id */ + /* HpkeKdfId kdf_id */ rv = sslRead_ReadNumber(&suiteReader, 2, &tmpn); if (rv != SECSuccess) { goto loser; } - /* aead_id */ + /* HpkeAeadId aead_id */ rv = sslRead_ReadNumber(&suiteReader, 2, &tmpn2); if (rv != SECSuccess) { goto loser; @@ -276,13 +226,36 @@ tls13_DecodeEchConfigContents(const sslReadBuffer *rawConfig, goto loser; } - /* Read the max name length. */ + /* uint16 maximum_name_length */ rv = sslRead_ReadNumber(&configReader, 2, &tmpn); if (rv != SECSuccess) { goto loser; } contents.maxNameLen = (PRUint16)tmpn; + /* opaque public_name<1..2^16-1> */ + rv = sslRead_ReadVariable(&configReader, 2, &tmpBuf); + if (rv != SECSuccess) { + goto loser; + } + + if (tmpBuf.len == 0) { + PORT_SetError(SSL_ERROR_RX_MALFORMED_ECH_CONFIG); + goto loser; + } + for (tmpn = 0; tmpn < tmpBuf.len; tmpn++) { + if (tmpBuf.buf[tmpn] == '\0') { + PORT_SetError(SSL_ERROR_RX_MALFORMED_ECH_CONFIG); + goto loser; + } + } + + contents.publicName = PORT_ZAlloc(tmpBuf.len + 1); + if (!contents.publicName) { + goto loser; + } + PORT_Memcpy(contents.publicName, (PRUint8 *)tmpBuf.buf, tmpBuf.len); + /* Extensions. We don't support any, but must * check for any that are marked critical. */ rv = sslRead_ReadVariable(&configReader, 2, &tmpBuf); @@ -356,7 +329,7 @@ tls13_DecodeEchConfigContents(const sslReadBuffer *rawConfig, return SECFailure; } -/* Decode an ECHConfigs struct and store each ECHConfig +/* Decode an ECHConfigList struct and store each ECHConfig * into |configs|. */ SECStatus tls13_DecodeEchConfigs(const SECItem *data, PRCList *configs) @@ -390,12 +363,12 @@ tls13_DecodeEchConfigs(const SECItem *data, PRCList *configs) /* Handle each ECHConfig. */ while (SSL_READER_REMAINING(&configsReader)) { singleConfig.buf = SSL_READER_CURRENT(&configsReader); - /* Version */ + /* uint16 version */ rv = sslRead_ReadNumber(&configsReader, 2, &version); if (rv != SECSuccess) { goto loser; } - /* Length */ + /* uint16 length */ rv = sslRead_ReadNumber(&configsReader, 2, &length); if (rv != SECSuccess) { goto loser; @@ -421,11 +394,6 @@ tls13_DecodeEchConfigs(const SECItem *data, PRCList *configs) goto loser; } - rv = tls13_DigestEchConfig(decodedConfig, decodedConfig->configId, - sizeof(decodedConfig->configId)); - if (rv != SECSuccess) { - goto loser; - } PR_APPEND_LINK(&decodedConfig->link, configs); decodedConfig = NULL; } @@ -438,14 +406,14 @@ tls13_DecodeEchConfigs(const SECItem *data, PRCList *configs) return SECFailure; } -/* Encode an ECHConfigs structure. We only allow one config, and as the +/* Encode an ECHConfigList structure. We only create one config, and as the * primary use for this function is to generate test inputs, we don't * validate against what HPKE and libssl can actually support. */ SECStatus -SSLExp_EncodeEchConfig(const char *publicName, const PRUint32 *hpkeSuites, - unsigned int hpkeSuiteCount, HpkeKemId kemId, - const SECKEYPublicKey *pubKey, PRUint16 maxNameLen, - PRUint8 *out, unsigned int *outlen, unsigned int maxlen) +SSLExp_EncodeEchConfigId(PRUint8 configId, const char *publicName, unsigned int maxNameLen, + HpkeKemId kemId, const SECKEYPublicKey *pubKey, + const HpkeSymmetricSuite *hpkeSuites, unsigned int hpkeSuiteCount, + PRUint8 *out, unsigned int *outlen, unsigned int maxlen) { SECStatus rv; unsigned int savedOffset; @@ -460,11 +428,21 @@ SSLExp_EncodeEchConfig(const char *publicName, const PRUint32 *hpkeSuites, return SECFailure; } + /* ECHConfig ECHConfigList<1..2^16-1>; */ rv = sslBuffer_Skip(&b, 2, NULL); if (rv != SECSuccess) { goto loser; } + /* + * struct { + * uint16 version; + * uint16 length; + * select (ECHConfig.version) { + * case 0xfe0a: ECHConfigContents contents; + * } + * } ECHConfig; + */ rv = sslBuffer_AppendNumber(&b, TLS13_ECH_VERSION, 2); if (rv != SECSuccess) { goto loser; @@ -475,22 +453,29 @@ SSLExp_EncodeEchConfig(const char *publicName, const PRUint32 *hpkeSuites, goto loser; } - len = PORT_Strlen(publicName); - rv = sslBuffer_AppendVariable(&b, (const PRUint8 *)publicName, len, 2); + /* + * struct { + * uint8 config_id; + * HpkeKemId kem_id; + * HpkePublicKey public_key; + * HpkeSymmetricCipherSuite cipher_suites<4..2^16-4>; + * } HpkeKeyConfig; + */ + rv = sslBuffer_AppendNumber(&b, configId, 1); if (rv != SECSuccess) { goto loser; } - rv = PK11_HPKE_Serialize(pubKey, tmpBuf, &tmpLen, sizeof(tmpBuf)); + rv = sslBuffer_AppendNumber(&b, kemId, 2); if (rv != SECSuccess) { goto loser; } - rv = sslBuffer_AppendVariable(&b, tmpBuf, tmpLen, 2); + + rv = PK11_HPKE_Serialize(pubKey, tmpBuf, &tmpLen, sizeof(tmpBuf)); if (rv != SECSuccess) { goto loser; } - - rv = sslBuffer_AppendNumber(&b, kemId, 2); + rv = sslBuffer_AppendVariable(&b, tmpBuf, tmpLen, 2); if (rv != SECSuccess) { goto loser; } @@ -500,17 +485,39 @@ SSLExp_EncodeEchConfig(const char *publicName, const PRUint32 *hpkeSuites, goto loser; } for (unsigned int i = 0; i < hpkeSuiteCount; i++) { - rv = sslBuffer_AppendNumber(&b, hpkeSuites[i], 4); + rv = sslBuffer_AppendNumber(&b, hpkeSuites[i].kdfId, 2); + if (rv != SECSuccess) { + goto loser; + } + rv = sslBuffer_AppendNumber(&b, hpkeSuites[i].aeadId, 2); if (rv != SECSuccess) { goto loser; } } + /* + * struct { + * HpkeKeyConfig key_config; + * uint16 maximum_name_length; + * opaque public_name<1..2^16-1>; + * Extension extensions<0..2^16-1>; + * } ECHConfigContents; + */ rv = sslBuffer_AppendNumber(&b, maxNameLen, 2); if (rv != SECSuccess) { goto loser; } + len = PORT_Strlen(publicName); + if (len > 0xffff) { + PORT_SetError(SEC_ERROR_INVALID_ARGS); + goto loser; + } + rv = sslBuffer_AppendVariable(&b, (const PRUint8 *)publicName, len, 2); + if (rv != SECSuccess) { + goto loser; + } + /* extensions */ rv = sslBuffer_AppendNumber(&b, 0, 2); if (rv != SECSuccess) { @@ -813,16 +820,16 @@ tls13_ClientSetupEch(sslSocket *ss, sslClientHelloType type) /* * enum { - * encrypted_client_hello(0xfe09), (65535) + * encrypted_client_hello(0xfe0a), (65535) * } ExtensionType; * * struct { * HpkeKdfId kdf_id; * HpkeAeadId aead_id; - * } ECHCipherSuite; + * } HpkeSymmetricCipherSuite; * struct { - * ECHCipherSuite cipher_suite; - * opaque config_id<0..255>; + * HpkeSymmetricCipherSuite cipher_suite; + * uint8 config_id; * opaque enc<1..2^16-1>; * opaque payload<1..2^16-1>; * } ClientECH; @@ -879,18 +886,18 @@ tls13_EncryptClientHello(sslSocket *ss, sslBuffer *outerAAD, sslBuffer *chInner) goto loser; } + rv = sslBuffer_AppendNumber(chInner, cfg->contents.configId, 1); + if (rv != SECSuccess) { + goto loser; + } if (!ss->ssl3.hs.helloRetry) { - rv = sslBuffer_AppendVariable(chInner, cfg->configId, sizeof(cfg->configId), 1); - if (rv != SECSuccess) { - goto loser; - } rv = sslBuffer_AppendVariable(chInner, hpkeEnc->data, hpkeEnc->len, 2); if (rv != SECSuccess) { goto loser; } } else { - /* one byte for empty configId, two for empty Enc. */ - rv = sslBuffer_AppendNumber(chInner, 0, 3); + /* |enc| is empty. */ + rv = sslBuffer_AppendNumber(chInner, 0, 2); if (rv != SECSuccess) { goto loser; } @@ -909,21 +916,18 @@ tls13_EncryptClientHello(sslSocket *ss, sslBuffer *outerAAD, sslBuffer *chInner) SECStatus tls13_GetMatchingEchConfigs(const sslSocket *ss, HpkeKdfId kdf, HpkeAeadId aead, - const SECItem *configId, const sslEchConfig *cur, sslEchConfig **next) + const PRUint8 configId, const sslEchConfig *cur, sslEchConfig **next) { - PRINT_BUF(50, (ss, "Server GetMatchingEchConfig with digest:", - configId->data, configId->len)); + SSL_TRC(50, ("%d: TLS13[%d]: GetMatchingEchConfig %d", + SSL_GETPID(), ss->fd, configId)); /* If |cur|, resume the search at that node, else the list head. */ for (PRCList *cur_p = cur ? ((PRCList *)cur)->next : PR_LIST_HEAD(&ss->echConfigs); cur_p != &ss->echConfigs; cur_p = PR_NEXT_LINK(cur_p)) { sslEchConfig *echConfig = (sslEchConfig *)cur_p; - if (configId->len != sizeof(echConfig->configId) || - PORT_Memcmp(echConfig->configId, configId->data, sizeof(echConfig->configId))) { - continue; - } - if (echConfig->contents.aeadId == aead && + if (echConfig->contents.configId == configId && + echConfig->contents.aeadId == aead && echConfig->contents.kdfId == kdf) { *next = echConfig; return SECSuccess; @@ -1006,10 +1010,9 @@ tls13_CopyChPreamble(sslReader *reader, const SECItem *explicitSid, sslBuffer *w /* * struct { - * HpkeKdfId kdfId; // ClientECH.cipher_suite.kdf - * HpkeAeadId aeadId; // ClientECH.cipher_suite.aead - * opaque config_id<0..255>; // ClientECH.config_id - * opaque enc<1..2^16-1>; // ClientECH.enc + * HpkeSymmetricCipherSuite cipher_suite; // kdfid_, aead_id + * uint8 config_id; + * opaque enc<1..2^16-1>; * opaque outer_hello<1..2^24-1>; * } ClientHelloOuterAAD; */ @@ -1035,17 +1038,17 @@ tls13_MakeChOuterAAD(sslSocket *ss, const SECItem *outer, SECItem *outerAAD) goto loser; } + rv = sslBuffer_AppendNumber(&aad, ss->xtnData.ech->configId, 1); + if (rv != SECSuccess) { + goto loser; + } + if (!ss->ssl3.hs.helloRetry) { - rv = sslBuffer_AppendVariable(&aad, ss->xtnData.ech->configId.data, - ss->xtnData.ech->configId.len, 1); - if (rv != SECSuccess) { - goto loser; - } rv = sslBuffer_AppendVariable(&aad, ss->xtnData.ech->senderPubKey.data, ss->xtnData.ech->senderPubKey.len, 2); } else { - /* 1B config_id length, 2B enc length. */ - rv = sslBuffer_AppendNumber(&aad, 0, 3); + /* |enc| is empty for HelloRetryRequest. */ + rv = sslBuffer_AppendNumber(&aad, 0, 2); } if (rv != SECSuccess) { goto loser; @@ -1496,7 +1499,7 @@ tls13_ConstructClientHelloWithEch(sslSocket *ss, const sslSessionID *sid, PRBool * Post-encryption, we'll assert that this was correct. */ encodedChLen = 4 + 1 + 2 + 2 + encodedChInner.len + 16; if (!ss->ssl3.hs.helloRetry) { - encodedChLen += 8 + 32; /* configId || enc */ + encodedChLen += 32; /* enc */ } rv = ssl_InsertPaddingExtension(ss, chOuter->len + encodedChLen, chOuterXtnsBuf); if (rv != SECSuccess) { @@ -1513,12 +1516,12 @@ tls13_ConstructClientHelloWithEch(sslSocket *ss, const sslSessionID *sid, PRBool if (rv != SECSuccess) { goto loser; } + rv = sslBuffer_AppendNumber(&aad, cfg->contents.configId, 1); + if (rv != SECSuccess) { + goto loser; + } if (!ss->ssl3.hs.helloRetry) { - rv = sslBuffer_AppendVariable(&aad, cfg->configId, sizeof(cfg->configId), 1); - if (rv != SECSuccess) { - goto loser; - } hpkeEnc = PK11_HPKE_GetEncapPubKey(ss->ssl3.hs.echHpkeCtx); if (!hpkeEnc) { FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error); @@ -1526,8 +1529,8 @@ tls13_ConstructClientHelloWithEch(sslSocket *ss, const sslSessionID *sid, PRBool } rv = sslBuffer_AppendVariable(&aad, hpkeEnc->data, hpkeEnc->len, 2); } else { - /* 1B config_id length, 2B enc length. */ - rv = sslBuffer_AppendNumber(&aad, 0, 3); + /* 2B for empty enc length. */ + rv = sslBuffer_AppendNumber(&aad, 0, 2); } if (rv != SECSuccess) { goto loser; @@ -1711,8 +1714,8 @@ tls13_MaybeGreaseEch(sslSocket *ss, unsigned int preambleLen, sslBuffer *buf) SECItem *rawData; CK_HKDF_PARAMS params; SECItem paramsi; - /* 1B aead determinant (don't send), 8B config_id, 32B enc, payload */ - const int kNonPayloadLen = 41; + /* 1B aead determinant (don't send), 1B config_id, 32B enc, payload */ + const int kNonPayloadLen = 34; if (!ss->opt.enableTls13GreaseEch || ss->ssl3.hs.echHpkeCtx) { return SECSuccess; @@ -1777,9 +1780,8 @@ tls13_MaybeGreaseEch(sslSocket *ss, unsigned int preambleLen, sslBuffer *buf) PORT_Assert(rawData->len == kNonPayloadLen + payloadLen); /* struct { - HpkeKdfId kdf_id; - HpkeAeadId aead_id; - opaque config_id<0..255>; + HpkeSymmetricCipherSuite cipher_suite; // kdf_id, aead_id + PRUint8 config_id; opaque enc<1..2^16-1>; opaque payload<1..2^16-1>; } ClientECH; */ @@ -1797,14 +1799,14 @@ tls13_MaybeGreaseEch(sslSocket *ss, unsigned int preambleLen, sslBuffer *buf) goto loser; } - /* config_id, 8B */ - rv = sslBuffer_AppendVariable(&greaseBuf, &rawData->data[1], 8, 1); + /* config_id */ + rv = sslBuffer_AppendNumber(&greaseBuf, rawData->data[1], 1); if (rv != SECSuccess) { goto loser; } /* enc len is fixed 32B for X25519. */ - rv = sslBuffer_AppendVariable(&greaseBuf, &rawData->data[9], 32, 2); + rv = sslBuffer_AppendVariable(&greaseBuf, &rawData->data[2], 32, 2); if (rv != SECSuccess) { goto loser; } @@ -1948,14 +1950,12 @@ tls13_MaybeHandleEchSignal(sslSocket *ss, const PRUint8 *sh, PRUint32 shLen) FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_SERVER_HELLO, illegal_parameter); return SECFailure; } - if (ss->ssl3.hs.helloRetry && ss->sec.isServer) { - /* Enc and ConfigId are stored in the cookie and must not - * be included in CH2.ClientECH. */ - if (ss->xtnData.ech->senderPubKey.len || ss->xtnData.ech->configId.len) { - ssl3_ExtSendAlert(ss, alert_fatal, illegal_parameter); - PORT_SetError(SSL_ERROR_BAD_2ND_CLIENT_HELLO); - return SECFailure; - } + /* |enc| must not be included in CH2.ClientECH. */ + if (ss->ssl3.hs.helloRetry && ss->sec.isServer && + ss->xtnData.ech->senderPubKey.len) { + ssl3_ExtSendAlert(ss, alert_fatal, illegal_parameter); + PORT_SetError(SSL_ERROR_BAD_2ND_CLIENT_HELLO); + return SECFailure; } ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ssl_tls13_encrypted_client_hello_xtn; @@ -2127,12 +2127,8 @@ tls13_MaybeAcceptEch(sslSocket *ss, const SECItem *sidBytes, const PRUint8 *chOu SECStatus rv; SECItem outer = { siBuffer, CONST_CAST(PRUint8, chOuter), chOuterLen }; SECItem *decryptedChInner = NULL; - SECItem hrrCh1ConfigId = { siBuffer, NULL, 0 }; SECItem outerAAD = { siBuffer, NULL, 0 }; SECItem cookieData = { siBuffer, NULL, 0 }; - HpkeContext *ch1EchHpkeCtx = NULL; - HpkeKdfId echKdfId; - HpkeAeadId echAeadId; sslEchConfig *candidate = NULL; /* non-owning */ TLSExtension *hrrXtn; @@ -2150,7 +2146,6 @@ tls13_MaybeAcceptEch(sslSocket *ss, const SECItem *sidBytes, const PRUint8 *chOu return SECSuccess; } - PORT_Assert(!ss->xtnData.ech->configId.data); PORT_Assert(!ss->ssl3.hs.echHpkeCtx); PRUint8 *tmp = hrrXtn->data.data; @@ -2164,17 +2159,21 @@ tls13_MaybeAcceptEch(sslSocket *ss, const SECItem *sidBytes, const PRUint8 *chOu /* Extract ECH info without restoring hash state. If there's * something wrong with the cookie, continue without ECH * and let HRR code handle the problem. */ + HpkeContext *ch1EchHpkeCtx = NULL; + PRUint8 echConfigId; + HpkeKdfId echKdfId; + HpkeAeadId echAeadId; rv = tls13_HandleHrrCookie(ss, cookieData.data, cookieData.len, NULL, NULL, NULL, &echKdfId, &echAeadId, - &hrrCh1ConfigId, &ch1EchHpkeCtx, PR_FALSE); + &echConfigId, &ch1EchHpkeCtx, PR_FALSE); if (rv != SECSuccess) { return SECSuccess; } - ss->xtnData.ech->configId = hrrCh1ConfigId; ss->ssl3.hs.echHpkeCtx = ch1EchHpkeCtx; - if (echKdfId != ss->xtnData.ech->kdfId || + if (echConfigId != ss->xtnData.ech->configId || + echKdfId != ss->xtnData.ech->kdfId || echAeadId != ss->xtnData.ech->aeadId) { FATAL_ERROR(ss, SSL_ERROR_BAD_2ND_CLIENT_HELLO, illegal_parameter); @@ -2187,9 +2186,8 @@ tls13_MaybeAcceptEch(sslSocket *ss, const SECItem *sidBytes, const PRUint8 *chOu } /* Cookie data was good, proceed with ECH. */ - PORT_Assert(ss->xtnData.ech->configId.data); rv = tls13_GetMatchingEchConfigs(ss, ss->xtnData.ech->kdfId, ss->xtnData.ech->aeadId, - &ss->xtnData.ech->configId, candidate, &candidate); + ss->xtnData.ech->configId, candidate, &candidate); if (rv != SECSuccess) { FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error); return SECFailure; @@ -2207,7 +2205,7 @@ tls13_MaybeAcceptEch(sslSocket *ss, const SECItem *sidBytes, const PRUint8 *chOu if (rv != SECSuccess) { /* Get the next matching config */ rv = tls13_GetMatchingEchConfigs(ss, ss->xtnData.ech->kdfId, ss->xtnData.ech->aeadId, - &ss->xtnData.ech->configId, candidate, &candidate); + ss->xtnData.ech->configId, candidate, &candidate); if (rv != SECSuccess) { FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error); SECITEM_FreeItem(&outerAAD, PR_FALSE); diff --git a/lib/ssl/tls13ech.h b/lib/ssl/tls13ech.h index a39a0295c0..55abf76eae 100644 --- a/lib/ssl/tls13ech.h +++ b/lib/ssl/tls13ech.h @@ -21,7 +21,7 @@ * - Some of the buffering (construction/compression/decompression) could likely * be optimized, but the spec is still evolving so that work is deferred. */ -#define TLS13_ECH_VERSION 0xfe09 +#define TLS13_ECH_VERSION 0xfe0a #define TLS13_ECH_SIGNAL_LEN 8 static const char kHpkeInfoEch[] = "tls ech"; @@ -29,21 +29,21 @@ static const char hHkdfInfoEchConfigID[] = "tls ech config id"; static const char kHkdfInfoEchConfirm[] = "ech accept confirmation"; struct sslEchConfigContentsStr { - char *publicName; - SECItem publicKey; /* NULL on server. Use the keypair in sslEchConfig instead. */ + PRUint8 configId; HpkeKemId kemId; + SECItem publicKey; /* NULL on server. Use the keypair in sslEchConfig instead. */ HpkeKdfId kdfId; HpkeAeadId aeadId; SECItem suites; /* One or more HpkeCipherSuites. The selected s * suite is placed in kdfId and aeadId. */ PRUint16 maxNameLen; + char *publicName; /* No supported extensions. */ }; struct sslEchConfigStr { PRCList link; SECItem raw; - PRUint8 configId[8]; PRUint16 version; sslEchConfigContents contents; }; @@ -51,7 +51,7 @@ struct sslEchConfigStr { struct sslEchXtnStateStr { SECItem innerCh; /* Server: ClientECH.payload */ SECItem senderPubKey; /* Server: ClientECH.enc */ - SECItem configId; /* Server: ClientECH.config_id */ + PRUint8 configId; /* Server: ClientECH.config_id */ HpkeKdfId kdfId; /* Server: ClientECH.cipher_suite.kdf */ HpkeAeadId aeadId; /* Server: ClientECH.cipher_suite.aead */ SECItem retryConfigs; /* Client: ServerECH.retry_configs*/ @@ -60,10 +60,10 @@ struct sslEchXtnStateStr { * verified to the ECHConfig public name). */ }; -SECStatus SSLExp_EncodeEchConfig(const char *publicName, const PRUint32 *hpkeSuites, - unsigned int hpkeSuiteCount, HpkeKemId kemId, - const SECKEYPublicKey *pubKey, PRUint16 maxNameLen, - PRUint8 *out, unsigned int *outlen, unsigned int maxlen); +SECStatus SSLExp_EncodeEchConfigId(PRUint8 configId, const char *publicName, unsigned int maxNameLen, + HpkeKemId kemId, const SECKEYPublicKey *pubKey, + const HpkeSymmetricSuite *hpkeSuites, unsigned int hpkeSuiteCount, + PRUint8 *out, unsigned int *outlen, unsigned int maxlen); SECStatus SSLExp_GetEchRetryConfigs(PRFileDesc *fd, SECItem *retryConfigs); SECStatus SSLExp_SetClientEchConfigs(PRFileDesc *fd, const PRUint8 *echConfigs, unsigned int echConfigsLen); diff --git a/lib/ssl/tls13exthandle.c b/lib/ssl/tls13exthandle.c index 7991a12c26..942d329aa6 100644 --- a/lib/ssl/tls13exthandle.c +++ b/lib/ssl/tls13exthandle.c @@ -1460,7 +1460,7 @@ tls13_ServerHandleEchXtn(const sslSocket *ss, TLSExtensionData *xtnData, HpkeKdfId kdf; HpkeAeadId aead; PRUint32 tmp; - SECItem configId; + PRUint8 configId; SECItem senderPubKey; SECItem encryptedCh; @@ -1504,11 +1504,12 @@ tls13_ServerHandleEchXtn(const sslSocket *ss, TLSExtensionData *xtnData, aead = (HpkeAeadId)tmp; /* config_id */ - rv = ssl3_ExtConsumeHandshakeVariable(ss, &configId, 1, - &data->data, &data->len); + rv = ssl3_ExtConsumeHandshakeNumber(ss, &tmp, 1, + &data->data, &data->len); if (rv != SECSuccess) { goto alert_loser; } + configId = tmp; /* enc */ rv = ssl3_ExtConsumeHandshakeVariable(ss, &senderPubKey, 2, @@ -1530,7 +1531,7 @@ tls13_ServerHandleEchXtn(const sslSocket *ss, TLSExtensionData *xtnData, if (!ss->ssl3.hs.helloRetry) { /* In the real ECH HRR case, config_id and enc should be empty. This * is checked after acceptance, because it might be GREASE ECH. */ - if (!configId.len || !senderPubKey.len) { + if (!senderPubKey.len) { goto alert_loser; } @@ -1538,17 +1539,13 @@ tls13_ServerHandleEchXtn(const sslSocket *ss, TLSExtensionData *xtnData, if (rv == SECFailure) { return SECFailure; } - - rv = SECITEM_CopyItem(NULL, &xtnData->ech->configId, &configId); - if (rv == SECFailure) { - return SECFailure; - } } rv = SECITEM_CopyItem(NULL, &xtnData->ech->innerCh, &encryptedCh); if (rv == SECFailure) { return SECFailure; } + xtnData->ech->configId = configId; xtnData->ech->kdfId = kdf; xtnData->ech->aeadId = aead; diff --git a/lib/ssl/tls13hashstate.c b/lib/ssl/tls13hashstate.c index ada22b6e31..1c077dd9bd 100644 --- a/lib/ssl/tls13hashstate.c +++ b/lib/ssl/tls13hashstate.c @@ -24,9 +24,9 @@ * uint8 indicator = 0xff; // To disambiguate from tickets. * uint16 cipherSuite; // Selected cipher suite. * uint16 keyShare; // Requested key share group (0=none) + * PRUint8 echConfigId; // ECH config_id * HpkeKdfId kdfId; // ECH KDF (uint16) * HpkeAeadId aeadId; // ECH AEAD (uint16) - * opaque echConfigId<0..255>; // ECH config_id * opaque echHpkeCtx<0..65535>; // ECH serialized HPKE context * opaque applicationToken<0..65535>; // Application token * opaque ch_hash[rest_of_buffer]; // H(ClientHello) @@ -63,20 +63,23 @@ tls13_MakeHrrCookie(sslSocket *ss, const sslNamedGroupDef *selectedGroup, } if (ss->xtnData.ech) { - rv = sslBuffer_AppendNumber(&cookieBuf, ss->xtnData.ech->kdfId, 2); + /* Record that we received ECH. */ + rv = sslBuffer_AppendNumber(&cookieBuf, PR_TRUE, 1); if (rv != SECSuccess) { return SECFailure; } - rv = sslBuffer_AppendNumber(&cookieBuf, ss->xtnData.ech->aeadId, 2); + + rv = sslBuffer_AppendNumber(&cookieBuf, ss->xtnData.ech->configId, + 1); if (rv != SECSuccess) { return SECFailure; } - /* Received ECH config_id, regardless of acceptance or possession - * of a matching ECHConfig. */ - PORT_Assert(ss->xtnData.ech->configId.len == 8); - rv = sslBuffer_AppendVariable(&cookieBuf, ss->xtnData.ech->configId.data, - ss->xtnData.ech->configId.len, 1); + rv = sslBuffer_AppendNumber(&cookieBuf, ss->xtnData.ech->kdfId, 2); + if (rv != SECSuccess) { + return SECFailure; + } + rv = sslBuffer_AppendNumber(&cookieBuf, ss->xtnData.ech->aeadId, 2); if (rv != SECSuccess) { return SECFailure; } @@ -97,7 +100,7 @@ tls13_MakeHrrCookie(sslSocket *ss, const sslNamedGroupDef *selectedGroup, return SECFailure; } } else { - rv = sslBuffer_AppendNumber(&cookieBuf, 0, 7); + rv = sslBuffer_AppendNumber(&cookieBuf, PR_FALSE, 1); if (rv != SECSuccess) { return SECFailure; } @@ -132,7 +135,9 @@ tls13_MakeHrrCookie(sslSocket *ss, const sslNamedGroupDef *selectedGroup, /* Given a cookie and cookieLen, decrypt and parse, returning * any values that were requested via the "previous_" params. If * recoverState is true, the transcript state and application - * token are restored. */ + * token are restored. Note that previousEchKdfId, previousEchAeadId, + * previousEchConfigId, and previousEchHpkeCtx are not modified if ECH was not + * previously negotiated (i.e., previousEchOffered is PR_FALSE). */ SECStatus tls13_HandleHrrCookie(sslSocket *ss, unsigned char *cookie, unsigned int cookieLen, @@ -141,7 +146,7 @@ tls13_HandleHrrCookie(sslSocket *ss, PRBool *previousEchOffered, HpkeKdfId *previousEchKdfId, HpkeAeadId *previousEchAeadId, - SECItem *previousEchConfigId, + PRUint8 *previousEchConfigId, HpkeContext **previousEchHpkeCtx, PRBool recoverState) { @@ -150,12 +155,13 @@ tls13_HandleHrrCookie(sslSocket *ss, unsigned int plaintextLen = 0; sslBuffer messageBuf = SSL_BUFFER_EMPTY; sslReadBuffer echHpkeBuf = { 0 }; - sslReadBuffer echConfigIdBuf = { 0 }; + PRBool receivedEch; + PRUint8 echConfigId = 0; PRUint64 sentinel; PRUint64 cipherSuite; HpkeContext *hpkeContext = NULL; - HpkeKdfId echKdfId; - HpkeAeadId echAeadId; + HpkeKdfId echKdfId = 0; + HpkeAeadId echAeadId = 0; PRUint64 group; PRUint64 tmp64; const sslNamedGroupDef *selectedGroup; @@ -190,31 +196,54 @@ tls13_HandleHrrCookie(sslSocket *ss, } selectedGroup = ssl_LookupNamedGroup(group); - /* ECH Ciphersuite */ - rv = sslRead_ReadNumber(&reader, 2, &tmp64); + /* Was ECH received. */ + rv = sslRead_ReadNumber(&reader, 1, &tmp64); if (rv != SECSuccess) { FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); return SECFailure; } - echKdfId = (HpkeKdfId)tmp64; + receivedEch = tmp64 == PR_TRUE; - rv = sslRead_ReadNumber(&reader, 2, &tmp64); - if (rv != SECSuccess) { - FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); - return SECFailure; - } - echAeadId = (HpkeAeadId)tmp64; + if (receivedEch) { + /* ECH config ID */ + rv = sslRead_ReadNumber(&reader, 1, &tmp64); + if (rv != SECSuccess) { + FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); + return SECFailure; + } + echConfigId = tmp64; - /* ECH Config ID and HPKE context may be empty. */ - rv = sslRead_ReadVariable(&reader, 1, &echConfigIdBuf); - if (rv != SECSuccess) { - FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); - return SECFailure; - } - rv = sslRead_ReadVariable(&reader, 2, &echHpkeBuf); - if (rv != SECSuccess) { - FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); - return SECFailure; + /* ECH Ciphersuite */ + rv = sslRead_ReadNumber(&reader, 2, &tmp64); + if (rv != SECSuccess) { + FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); + return SECFailure; + } + echKdfId = (HpkeKdfId)tmp64; + + rv = sslRead_ReadNumber(&reader, 2, &tmp64); + if (rv != SECSuccess) { + FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); + return SECFailure; + } + echAeadId = (HpkeAeadId)tmp64; + + /* ECH HPKE context may be empty. */ + rv = sslRead_ReadVariable(&reader, 2, &echHpkeBuf); + if (rv != SECSuccess) { + FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO, illegal_parameter); + return SECFailure; + } + + if (previousEchHpkeCtx && echHpkeBuf.len) { + const SECItem hpkeItem = { siBuffer, CONST_CAST(unsigned char, echHpkeBuf.buf), + echHpkeBuf.len }; + hpkeContext = PK11_HPKE_ImportContext(&hpkeItem, NULL); + if (!hpkeContext) { + FATAL_ERROR(ss, PORT_GetError(), illegal_parameter); + return SECFailure; + } + } } /* Application token. */ @@ -276,36 +305,6 @@ tls13_HandleHrrCookie(sslSocket *ss, } } - if (previousEchHpkeCtx && echHpkeBuf.len) { - const SECItem hpkeItem = { siBuffer, CONST_CAST(unsigned char, echHpkeBuf.buf), - echHpkeBuf.len }; - hpkeContext = PK11_HPKE_ImportContext(&hpkeItem, NULL); - if (!hpkeContext) { - FATAL_ERROR(ss, PORT_GetError(), illegal_parameter); - return SECFailure; - } - } - - if (previousEchConfigId && echConfigIdBuf.len) { - SECItem tmp = { siBuffer, NULL, 0 }; - rv = SECITEM_MakeItem(NULL, &tmp, echConfigIdBuf.buf, echConfigIdBuf.len); - if (rv != SECSuccess) { - PK11_HPKE_DestroyContext(hpkeContext, PR_TRUE); - FATAL_ERROR(ss, PORT_GetError(), internal_error); - return SECFailure; - } - *previousEchConfigId = tmp; - } - - if (previousEchKdfId) { - *previousEchKdfId = echKdfId; - } - if (previousEchAeadId) { - *previousEchAeadId = echAeadId; - } - if (previousEchHpkeCtx) { - *previousEchHpkeCtx = hpkeContext; - } if (previousCipherSuite) { *previousCipherSuite = cipherSuite; } @@ -313,7 +312,21 @@ tls13_HandleHrrCookie(sslSocket *ss, *previousGroup = selectedGroup; } if (previousEchOffered) { - *previousEchOffered = echConfigIdBuf.len > 0; + *previousEchOffered = receivedEch; + } + if (receivedEch) { + if (previousEchConfigId) { + *previousEchConfigId = echConfigId; + } + if (previousEchKdfId) { + *previousEchKdfId = echKdfId; + } + if (previousEchAeadId) { + *previousEchAeadId = echAeadId; + } + if (previousEchHpkeCtx) { + *previousEchHpkeCtx = hpkeContext; + } } return SECSuccess; } diff --git a/lib/ssl/tls13hashstate.h b/lib/ssl/tls13hashstate.h index 2ea7b493b6..91e93fcf7e 100644 --- a/lib/ssl/tls13hashstate.h +++ b/lib/ssl/tls13hashstate.h @@ -24,7 +24,7 @@ SECStatus tls13_HandleHrrCookie(sslSocket *ss, PRBool *previousEchOffered, HpkeKdfId *previousEchKdfId, HpkeAeadId *previousEchAeadId, - SECItem *previousEchConfigId, + PRUint8 *previousEchConfigId, HpkeContext **previousEchHpkeCtx, PRBool recoverState); #endif