From 63e3306075f9c99fd2cd2d3ae812563ff1931381 Mon Sep 17 00:00:00 2001 From: Nick Richardson Date: Mon, 27 Nov 2023 16:15:54 -0800 Subject: [PATCH] Select ECH config from the ECHPolicy on connect Summary: Uses the ECHPolicy from the fizzContext to select an ECH config based on the given sni Reviewed By: mingtaoy Differential Revision: D51045365 fbshipit-source-id: 7ffd74c9d349747918cee3e6c23585eddc515e97 --- fizz/client/AsyncFizzClient-inl.h | 6 ++ fizz/client/test/AsyncFizzClientTest.cpp | 88 ++++++++++++++++++++++-- fizz/client/test/Mocks.h | 12 +++- 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/fizz/client/AsyncFizzClient-inl.h b/fizz/client/AsyncFizzClient-inl.h index 4000ad5aca9..b88cd0415d3 100644 --- a/fizz/client/AsyncFizzClient-inl.h +++ b/fizz/client/AsyncFizzClient-inl.h @@ -88,6 +88,12 @@ void AsyncFizzClientT::connect( if (pskIdentity) { cachedPsk = fizzContext_->getPsk(*pskIdentity); } + + auto echPolicy = fizzContext_->getECHPolicy(); + if (!echConfigs && echPolicy && sni.hasValue()) { + echConfigs = echPolicy->getConfig(sni.value()); + } + fizzClient_.connect( fizzContext_, std::move(verifier), diff --git a/fizz/client/test/AsyncFizzClientTest.cpp b/fizz/client/test/AsyncFizzClientTest.cpp index cc4f0529b26..e887ed4fd84 100644 --- a/fizz/client/test/AsyncFizzClientTest.cpp +++ b/fizz/client/test/AsyncFizzClientTest.cpp @@ -110,11 +110,7 @@ class AsyncFizzClientTest : public Test { .WillOnce(InvokeWithoutArgs([]() { return Actions(); })); const auto sni = std::string("www.example.com"); client_->connect( - &handshakeCallback_, - nullptr, - sni, - pskIdentity_, - folly::Optional>(folly::none)); + &handshakeCallback_, nullptr, sni, pskIdentity_, echConfigs_); } enum class ECHMode { NotRequested, Accepted, Rejected }; @@ -217,6 +213,7 @@ class AsyncFizzClientTest : public Test { EventBase evb_; MockReplaySafetyCallback mockReplayCallback_; folly::Optional pskIdentity_{"pskIdentity"}; + folly::Optional> echConfigs_; }; MATCHER_P(BufMatches, expected, "") { @@ -597,6 +594,87 @@ TEST_F(AsyncFizzClientTest, TestNoPskResumption) { EXPECT_FALSE(client_->pskResumed()); } +TEST_F(AsyncFizzClientTest, TestNoECHPolicy) { + auto echPolicy = std::make_shared(); + // Sanity check: ECHPolicy::getConfig should not be called if no ECH policy on + // context + EXPECT_CALL(*echPolicy, getConfig(_)).Times(0); + completeHandshake(); +} + +TEST_F(AsyncFizzClientTest, TestECHPolicyNoSNI) { + auto echPolicy = std::make_shared(); + context_->setECHPolicy(echPolicy); + // Sanity check: ECHPolicy::getConfig should not be called if no ECH policy on + // context + EXPECT_CALL(*echPolicy, getConfig(_)).Times(0); + EXPECT_CALL(*machine_, _processConnect(_, _, _, _, _, _, _)) + .WillOnce(InvokeWithoutArgs([]() { return Actions(); })); + client_->connect( + &handshakeCallback_, nullptr, folly::none, pskIdentity_, folly::none); +} + +TEST_F(AsyncFizzClientTest, TestOverrideECHPolicy) { + auto echPolicy = std::make_shared(); + context_->setECHPolicy(echPolicy); + // When an ECH config vector is passed to FizzClient::connect() the ECHPolicy + // lookup should be overridden. + echConfigs_ = std::vector{}; + EXPECT_CALL(*echPolicy, getConfig("www.example.com")).Times(0); + completeHandshake(); +} + +TEST_F(AsyncFizzClientTest, TestECHPolicyGet) { + auto echPolicy = std::make_shared(); + context_->setECHPolicy(echPolicy); + + std::vector expectedEchConfigList; + ech::ECHConfig echConfig; + ech::ECHConfigContentDraft echConfigContent; + echConfigContent.key_config.kem_id = hpke::KEMId::x25519; + echConfigContent.key_config.config_id = 1; + echConfigContent.public_name = + folly::IOBuf::copyBuffer("www.super.secret.sni.com"); + echConfigContent.maximum_name_length = 100; + echConfigContent.key_config.public_key = folly::IOBuf::copyBuffer( + "1d77eb1c522d08605b179d4214ee4a3635df7e17c336ea9006655a73fcaad63e"); + auto kdfId = hpke::KDFId::Sha256; + auto aeadId = hpke::AeadId::TLS_AES_128_GCM_SHA256; + ech::HpkeSymmetricCipherSuite suite{kdfId, aeadId}; + echConfigContent.key_config.cipher_suites.push_back(suite); + echConfig.version = ech::ECHVersion::Draft15; + echConfig.ech_config_content = encode(std::move(echConfigContent)); + expectedEchConfigList.push_back(std::move(echConfig)); + + EXPECT_CALL(*echPolicy, getConfig("www.example.com")) + .WillOnce(Return(expectedEchConfigList)); + + // processConnect() should be called with the ECH config list returned from + // ECHPolicy::getConfig() + EXPECT_CALL( + *machine_, + _processConnect( + _, + _, + _, + _, + _, + _, + Truly([&expectedEchConfigList]( + const folly::Optional>& + configList) { + return configList.hasValue() && + configList->at(0).ech_config_content->coalesce() == + expectedEchConfigList[0].ech_config_content->coalesce(); + }))) + .WillOnce(InvokeWithoutArgs([]() { + return detail::actions(ReportHandshakeSuccess(), WaitForData()); + })); + const auto sni = std::string("www.example.com"); + client_->connect( + &handshakeCallback_, nullptr, sni, pskIdentity_, folly::none); +} + TEST_F(AsyncFizzClientTest, TestECHAccepted) { connect(); EXPECT_CALL(handshakeCallback_, _fizzHandshakeSuccess()); diff --git a/fizz/client/test/Mocks.h b/fizz/client/test/Mocks.h index f9054c50cdd..8451c85969c 100644 --- a/fizz/client/test/Mocks.h +++ b/fizz/client/test/Mocks.h @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -28,7 +29,7 @@ class MockClientStateMachine : public ClientStateMachine { folly::Optional host, folly::Optional cachedPsk, const std::shared_ptr& extensions, - const folly::Optional>& echConfigs)); + folly::Optional> echConfigs)); Actions processConnect( const State& state, std::shared_ptr context, @@ -132,6 +133,15 @@ class MockPskCache : public PskCache { MOCK_METHOD(void, removePsk, (const std::string& identity)); }; +class MockECHPolicy : public fizz::client::ECHPolicy { + public: + MOCK_METHOD( + folly::Optional>, + getConfig, + (const std::string& hostname), + (const)); +}; + class MockClientExtensions : public ClientExtensions { public: MOCK_METHOD(std::vector, getClientHelloExtensions, (), (const));