Skip to content

Commit

Permalink
Create ECHRetryAvailableAction
Browse files Browse the repository at this point in the history
Summary: Adds an action to the fizz client statemachine to report the reception of ech retry configs from the server's encrypted extensions.

Reviewed By: mingtaoy

Differential Revision: D56708359

fbshipit-source-id: 56ffd4bd5ec8c932f8dbc1c07a81e7dfdf8736a6
  • Loading branch information
Nick Richardson authored and facebook-github-bot committed May 20, 2024
1 parent 39bece9 commit b16b9ad
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 15 deletions.
27 changes: 26 additions & 1 deletion fizz/client/Actions.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,30 @@ struct NewCachedPsk {
CachedPsk psk;
};

/**
* New ech retry config available. This action is emitted whenever an ECH retry
* config is received from the server's encrypted extensions.
*/
struct ECHRetryAvailable {
/**
* The original SNI that was used in the `Connect` event, prior to any
* modifications due to ECH.
*
* It is present here in order to associate this `ECHRetryAvailable`
* action with a connection attempt.
*
* An empty string indicates that no SNI was sent, but the peer responded
* with a set of ECHConfigs regardless
*/
std::string sni;
/**
* A list of ECHConfigs sent by the peer. It is intended to indicate
* the set of acceptable ECHConfigs to use the next time the local
* sender intends to open a TLS connection to `sni`.
*/
std::vector<ech::ECHConfig> configs;
};

#define FIZZ_CLIENT_ACTIONS(F, ...) \
F(DeliverAppData, __VA_ARGS__) \
F(WriteToSocket, __VA_ARGS__) \
Expand All @@ -79,7 +103,8 @@ struct NewCachedPsk {
F(MutateState, __VA_ARGS__) \
F(WaitForData, __VA_ARGS__) \
F(NewCachedPsk, __VA_ARGS__) \
F(SecretAvailable, __VA_ARGS__)
F(SecretAvailable, __VA_ARGS__) \
F(ECHRetryAvailable, __VA_ARGS__)

FIZZ_DECLARE_VARIANT_TYPE(Action, FIZZ_CLIENT_ACTIONS)

Expand Down
6 changes: 6 additions & 0 deletions fizz/client/AsyncFizzClient-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,12 @@ void AsyncFizzClientT<SM>::ActionMoveVisitor::operator()(
}
}

template <typename SM>
void AsyncFizzClientT<SM>::ActionMoveVisitor::operator()(
ECHRetryAvailable& echRetry) {
client_.echRetryAvailable(echRetry);
}

template <typename SM>
void AsyncFizzClientT<SM>::ActionMoveVisitor::operator()(
SecretAvailable& secret) {
Expand Down
30 changes: 30 additions & 0 deletions fizz/client/AsyncFizzClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,23 @@
namespace fizz {
namespace client {

/**
* ECHRetryCallback is used to convey ECHRetryConfigs that are
* received by the client.
*/
class ECHRetryCallback {
public:
virtual ~ECHRetryCallback() = default;

/**
* retryAvailable may be invoked whenever the client receives a list of
* ECHRetryConfigs from the server.
*
* There is no guarantee that this callback will be invoked on a connection.
*/
virtual void retryAvailable(ECHRetryAvailable retry) = 0;
};

template <typename SM>
class AsyncFizzClientT : public AsyncFizzBase,
private folly::AsyncSocket::ConnectCallback {
Expand Down Expand Up @@ -172,6 +189,16 @@ class AsyncFizzClientT : public AsyncFizzBase,
*/
folly::Optional<std::vector<ech::ECHConfig>> getEchRetryConfigs() const;

void echRetryAvailable(const ECHRetryAvailable& retry) noexcept {
if (echRetryCallback_) {
echRetryCallback_->retryAvailable(retry);
}
}

void setECHRetryCallback(ECHRetryCallback* cb) {
echRetryCallback_ = cb;
}

protected:
~AsyncFizzClientT() override = default;
void writeAppData(
Expand Down Expand Up @@ -215,6 +242,7 @@ class AsyncFizzClientT : public AsyncFizzBase,
void operator()(WaitForData&);
void operator()(MutateState&);
void operator()(NewCachedPsk&);
void operator()(ECHRetryAvailable&);
void operator()(SecretAvailable&);
void operator()(EndOfData&);

Expand Down Expand Up @@ -299,6 +327,8 @@ class AsyncFizzClientT : public AsyncFizzBase,

folly::Optional<AsyncClientCallbackPtr> callback_;

ECHRetryCallback* echRetryCallback_{nullptr};

std::shared_ptr<const FizzClientContext> fizzContext_;

std::shared_ptr<ClientExtensions> extensions_;
Expand Down
26 changes: 16 additions & 10 deletions fizz/client/ClientProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1779,19 +1779,24 @@ Actions EventHandler<
}

folly::Optional<std::vector<ech::ECHConfig>> retryConfigs;
folly::Optional<ECHRetryAvailable> echRetryAvailable;
if (state.echState().has_value()) {
// Check if we were sent retry configs
auto serverECH = getExtension<ech::ECHEncryptedExtensions>(ee.extensions);
if (serverECH.has_value()) {
retryConfigs = std::move(serverECH->retry_configs);
echRetryAvailable = ECHRetryAvailable{};
echRetryAvailable->sni = state.echState()->sni.value_or("");
echRetryAvailable->configs = retryConfigs.value();
}
}

if (state.extensions()) {
state.extensions()->onEncryptedExtensions(ee.extensions);
}

MutateState mutateState(
Actions ret;
ret.emplace_back(MutateState(
[appProto = std::move(appProto),
earlyDataType,
retryConfigs = std::move(retryConfigs)](State& newState) mutable {
Expand All @@ -1801,17 +1806,18 @@ Actions EventHandler<
if (retryConfigs.has_value()) {
newState.echState()->retryConfigs = std::move(retryConfigs);
}
});
}));

if (isPskAccepted(*state.pskType())) {
return actions(
std::move(mutateState),
MutateState(&Transition<StateEnum::ExpectingFinished>));
} else {
return actions(
std::move(mutateState),
MutateState(&Transition<StateEnum::ExpectingCertificate>));
if (echRetryAvailable) {
ret.emplace_back(std::move(*echRetryAvailable));
}

auto nextState = (isPskAccepted(*state.pskType()))
? &Transition<StateEnum::ExpectingFinished>
: &Transition<StateEnum::ExpectingCertificate>;
ret.emplace_back(MutateState(nextState));

return ret;
}

static folly::Optional<
Expand Down
3 changes: 3 additions & 0 deletions fizz/client/FizzClient-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ void FizzClient<ActionMoveVisitor, SM>::visitActions(
case Action::Type::SecretAvailable_E:
this->visitor_(*action.asSecretAvailable());
break;
case Action::Type::ECHRetryAvailable_E:
this->visitor_(*action.asECHRetryAvailable());
break;
}
}
}
Expand Down
46 changes: 43 additions & 3 deletions fizz/client/test/AsyncFizzClientTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <fizz/client/AsyncFizzClient.h>

#include <fizz/client/test/Mocks.h>
#include <fizz/protocol/ech/test/TestUtil.h>
#include <fizz/protocol/test/Mocks.h>
#include <folly/io/SocketOptionMap.h>
#include <folly/io/async/test/AsyncSocketTest.h>
Expand Down Expand Up @@ -121,7 +122,8 @@ class AsyncFizzClientTest : public Test {
std::shared_ptr<const Cert> clientCert = nullptr,
std::shared_ptr<const Cert> serverCert = nullptr,
bool pskResumed = false,
ECHMode echMode = ECHMode::NotRequested) {
ECHMode echMode = ECHMode::NotRequested,
folly::Optional<ECHRetryAvailable> expectedECHRetry = {}) {
EXPECT_CALL(*machine_, _processSocketData(_, _, _))
.WillOnce(InvokeWithoutArgs([=]() {
MutateState addToState([=](State& newState) {
Expand Down Expand Up @@ -161,8 +163,18 @@ class AsyncFizzClientTest : public Test {
});
ReportHandshakeSuccess reportSuccess;
reportSuccess.earlyDataAccepted = acceptEarlyData;
return detail::actions(
std::move(addToState), std::move(reportSuccess), WaitForData());
ECHRetryAvailable echRetryAvailable;
if (expectedECHRetry.has_value()) {
echRetryAvailable = expectedECHRetry.value();
return detail::actions(
std::move(addToState),
std::move(echRetryAvailable),
std::move(reportSuccess),
WaitForData());
} else {
return detail::actions(
std::move(addToState), std::move(reportSuccess), WaitForData());
}
}));
socketReadCallback_->readBufferAvailable(IOBuf::copyBuffer("ServerData"));
}
Expand Down Expand Up @@ -1217,6 +1229,34 @@ TEST_F(AsyncFizzClientTest, TestTransportEof) {
EXPECT_FALSE(client_->good());
}

TEST_F(AsyncFizzClientTest, TestECHRetryAvailableAction) {
connect();
EXPECT_CALL(handshakeCallback_, _fizzHandshakeSuccess());
auto callback = MockECHRetryCallback();
EXPECT_CALL(callback, retryAvailable(_))
.WillOnce(Invoke([](ECHRetryAvailable gotRetryConfig) {
EXPECT_EQ(
gotRetryConfig.configs.at(0).version, ech::ECHVersion::Draft15);
EXPECT_EQ(
gotRetryConfig.configs.at(0).ech_config_content->moveToFbString(),
"retryConfig");
}));
client_->setECHRetryCallback(&callback);
ech::ECHConfig retryConfig;
retryConfig.version = ech::ECHVersion::Draft15;
retryConfig.ech_config_content = folly::IOBuf::copyBuffer("retryConfig");
ECHRetryAvailable expectedECHRetries;
expectedECHRetries.configs.push_back(retryConfig);
fullHandshakeSuccess(
false,
"h2",
nullptr,
nullptr,
false,
ECHMode::Rejected,
expectedECHRetries);
}

TEST_F(AsyncFizzClientTest, TestNewCachedPskActions) {
completeHandshake();
client_->setReadCB(&readCallback_);
Expand Down
1 change: 1 addition & 0 deletions fizz/client/test/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ cpp_unittest(
deps = [
":mocks",
"//fizz/client:async_fizz_client",
"//fizz/protocol/ech/test:test_util",
"//fizz/protocol/test:mocks",
"//folly/io:socket_option_map",
"//folly/io/async/test:async_socket_test_lib",
Expand Down
11 changes: 10 additions & 1 deletion fizz/client/test/ClientProtocolTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3712,8 +3712,10 @@ TEST_F(ClientProtocolTest, TestEncryptedExtensionsECHRetryConfigs) {
setupExpectingEncryptedExtensions();
state_.requestedExtensions()->push_back(
ExtensionType::encrypted_client_hello);
state_.sni() = "www.facebook.com";
state_.echState().emplace();
state_.echState()->status = ECHStatus::Rejected;
state_.echState()->sni = "private.facebook.com";
EXPECT_CALL(
*mockHandshakeContext_, appendToTranscript(BufMatches("eeencoding")));

Expand All @@ -3726,13 +3728,20 @@ TEST_F(ClientProtocolTest, TestEncryptedExtensionsECHRetryConfigs) {
ee.extensions.push_back(encodeExtension(std::move(serverECH)));

auto actions = detail::processEvent(state_, std::move(ee));
expectActions<MutateState>(actions);
expectActions<MutateState, ECHRetryAvailable>(actions);
processStateMutations(actions);
EXPECT_EQ(*state_.alpn(), "h2");
EXPECT_TRUE(folly::IOBufEqualTo()(
state_.echState()->retryConfigs.value()[0].ech_config_content,
folly::IOBuf::copyBuffer("retryconfig")));
EXPECT_EQ(state_.state(), StateEnum::ExpectingCertificate);
auto retry = expectAction<ECHRetryAvailable>(actions);
EXPECT_EQ(retry.sni, "private.facebook.com");
ASSERT_EQ(retry.configs.size(), 1);
EXPECT_EQ(retry.configs.at(0).version, ech::ECHVersion::Draft15);
EXPECT_TRUE(folly::IOBufEqualTo()(
retry.configs.at(0).ech_config_content,
folly::IOBuf::copyBuffer("retryconfig")));
}

TEST_F(ClientProtocolTest, TestCertificateFlow) {
Expand Down
5 changes: 5 additions & 0 deletions fizz/client/test/Mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ class MockPskCache : public PskCache {
MOCK_METHOD(void, removePsk, (const std::string& identity));
};

class MockECHRetryCallback : public ECHRetryCallback {
public:
MOCK_METHOD(void, retryAvailable, (ECHRetryAvailable retry));
};

class MockECHPolicy : public fizz::client::ECHPolicy {
public:
MOCK_METHOD(
Expand Down

0 comments on commit b16b9ad

Please sign in to comment.