Reject CHLOs without server nonce in Google QUIC (technically disable 0-RTT) by changing --gfe2_reloadable_flag_quic_require_handshake_confirmation from external default false flag to a regular feature flag and let it go through the feature flag roll out process.
Protected by FLAGS_quic_reloadable_flag_quic_require_handshake_confirmation.
PiperOrigin-RevId: 542324361
diff --git a/quiche/quic/core/crypto/crypto_server_test.cc b/quiche/quic/core/crypto/crypto_server_test.cc
index 56f7283..e987190 100644
--- a/quiche/quic/core/crypto/crypto_server_test.cc
+++ b/quiche/quic/core/crypto/crypto_server_test.cc
@@ -78,7 +78,8 @@
std::vector<TestParams> params;
// Start with all versions, remove highest on each iteration.
- ParsedQuicVersionVector supported_versions = AllSupportedVersions();
+ ParsedQuicVersionVector supported_versions =
+ AllSupportedVersionsWithQuicCrypto();
while (!supported_versions.empty()) {
params.push_back({supported_versions});
supported_versions.erase(supported_versions.begin());
@@ -566,8 +567,12 @@
CheckRejectTag();
const HandshakeFailureReason kRejectReasons1[] = {
- CLIENT_NONCE_INVALID_FAILURE};
- CheckRejectReasons(kRejectReasons1, ABSL_ARRAYSIZE(kRejectReasons1));
+ CLIENT_NONCE_INVALID_FAILURE, SERVER_NONCE_REQUIRED_FAILURE};
+ CheckRejectReasons(
+ kRejectReasons1,
+ (GetQuicReloadableFlag(quic_require_handshake_confirmation)
+ ? ABSL_ARRAYSIZE(kRejectReasons1)
+ : 1));
}
}
@@ -658,7 +663,14 @@
CheckRejectTag();
const HandshakeFailureReason kRejectReasons[] = {
SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE};
- CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+ const HandshakeFailureReason kRejectReasons1[] = {
+ SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE, SERVER_NONCE_REQUIRED_FAILURE};
+ CheckRejectReasons((GetQuicReloadableFlag(quic_require_handshake_confirmation)
+ ? kRejectReasons1
+ : kRejectReasons),
+ (GetQuicReloadableFlag(quic_require_handshake_confirmation)
+ ? ABSL_ARRAYSIZE(kRejectReasons1)
+ : ABSL_ARRAYSIZE(kRejectReasons)));
}
TEST_P(CryptoServerTest, CorruptSourceAddressTokenIsStillAccepted) {
@@ -678,6 +690,16 @@
config_.set_validate_source_address_token(false);
ShouldSucceed(msg);
+ if (GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ CheckRejectTag();
+ const HandshakeFailureReason kRejectReasons[] = {
+ SERVER_NONCE_REQUIRED_FAILURE};
+ CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+ absl::string_view server_nonce;
+ ASSERT_TRUE(out_.GetStringPiece(kSourceAddressTokenTag, &server_nonce));
+ msg.SetStringPiece(kServerNonceTag, server_nonce);
+ ShouldSucceed(msg);
+ }
EXPECT_EQ(kSHLO, out_.tag());
}
@@ -699,7 +721,15 @@
CheckRejectTag();
const HandshakeFailureReason kRejectReasons[] = {
SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE, CLIENT_NONCE_INVALID_FAILURE};
- CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+ const HandshakeFailureReason kRejectReasons1[] = {
+ SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE, CLIENT_NONCE_INVALID_FAILURE,
+ SERVER_NONCE_REQUIRED_FAILURE};
+ CheckRejectReasons((GetQuicReloadableFlag(quic_require_handshake_confirmation)
+ ? kRejectReasons1
+ : kRejectReasons),
+ (GetQuicReloadableFlag(quic_require_handshake_confirmation)
+ ? ABSL_ARRAYSIZE(kRejectReasons1)
+ : ABSL_ARRAYSIZE(kRejectReasons)));
}
TEST_P(CryptoServerTest, CorruptMultipleTags) {
@@ -727,8 +757,7 @@
}
TEST_P(CryptoServerTest, NoServerNonce) {
- // When no server nonce is present and no strike register is configured,
- // the CHLO should be rejected.
+ // When no server nonce is present the CHLO should be rejected.
CryptoHandshakeMessage msg =
crypto_test_utils::CreateCHLO({{"PDMD", "X509"},
{"AEAD", "AESG"},
@@ -744,10 +773,14 @@
ShouldSucceed(msg);
- // Even without a server nonce, this ClientHello should be accepted in
- // version 33.
- ASSERT_EQ(kSHLO, out_.tag());
- CheckServerHello(out_);
+ if (GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ CheckRejectTag();
+ } else {
+ // Even without a server nonce, this ClientHello should be accepted in
+ // version 33.
+ ASSERT_EQ(kSHLO, out_.tag());
+ CheckServerHello(out_);
+ }
}
TEST_P(CryptoServerTest, ProofForSuppliedServerConfig) {
@@ -772,7 +805,15 @@
CheckRejectTag();
const HandshakeFailureReason kRejectReasons[] = {
SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE};
- CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+ const HandshakeFailureReason kRejectReasons1[] = {
+ SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE,
+ SERVER_NONCE_REQUIRED_FAILURE};
+ CheckRejectReasons((GetQuicReloadableFlag(quic_require_handshake_confirmation)
+ ? kRejectReasons1
+ : kRejectReasons),
+ (GetQuicReloadableFlag(quic_require_handshake_confirmation)
+ ? ABSL_ARRAYSIZE(kRejectReasons1)
+ : ABSL_ARRAYSIZE(kRejectReasons)));
absl::string_view cert, proof, scfg_str;
EXPECT_TRUE(out_.GetStringPiece(kCertificateTag, &cert));
@@ -829,10 +870,23 @@
ShouldSucceed(msg);
- const HandshakeFailureReason kRejectReasons[] = {
+ if (GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ const HandshakeFailureReason kRejectReasons[] = {
+ SERVER_NONCE_REQUIRED_FAILURE};
+
+ CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+
+ absl::string_view server_nonce;
+ ASSERT_TRUE(out_.GetStringPiece(kSourceAddressTokenTag, &server_nonce));
+ msg.SetStringPiece(kServerNonceTag, server_nonce);
+
+ ShouldSucceed(msg);
+ }
+
+ const HandshakeFailureReason kRejectReasons1[] = {
INVALID_EXPECTED_LEAF_CERTIFICATE};
- CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+ CheckRejectReasons(kRejectReasons1, ABSL_ARRAYSIZE(kRejectReasons1));
}
TEST_P(CryptoServerTest, ValidXlct) {
@@ -854,6 +908,20 @@
config_.set_replay_protection(false);
ShouldSucceed(msg);
+
+ if (GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ const HandshakeFailureReason kRejectReasons[] = {
+ SERVER_NONCE_REQUIRED_FAILURE};
+
+ CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+
+ absl::string_view server_nonce;
+ ASSERT_TRUE(out_.GetStringPiece(kSourceAddressTokenTag, &server_nonce));
+ msg.SetStringPiece(kServerNonceTag, server_nonce);
+
+ ShouldSucceed(msg);
+ }
+
EXPECT_EQ(kSHLO, out_.tag());
}
@@ -876,9 +944,21 @@
config_.set_replay_protection(false);
ShouldSucceed(msg);
+ absl::string_view nonce;
+
+ if (GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ const HandshakeFailureReason kRejectReasons[] = {
+ SERVER_NONCE_REQUIRED_FAILURE};
+
+ CheckRejectReasons(kRejectReasons, ABSL_ARRAYSIZE(kRejectReasons));
+
+ ASSERT_TRUE(out_.GetStringPiece(kSourceAddressTokenTag, &nonce));
+ msg.SetStringPiece(kServerNonceTag, nonce);
+
+ ShouldSucceed(msg);
+ }
EXPECT_EQ(kSHLO, out_.tag());
- absl::string_view nonce;
EXPECT_TRUE(out_.GetStringPiece(kServerNonceTag, &nonce));
}
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index 6c9451a..f9a89d7 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -1895,6 +1895,10 @@
// Send a request and then disconnect. This prepares the client to attempt
// a 0-RTT handshake for the next request.
ASSERT_TRUE(Initialize());
+ if (!version_.UsesTls() &&
+ GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ return;
+ }
std::string body(20480, 'a');
Http2HeaderBlock headers;
@@ -1950,6 +1954,10 @@
// Regression test for b/168020146.
TEST_P(EndToEndTest, MultipleZeroRtt) {
ASSERT_TRUE(Initialize());
+ if (!version_.UsesTls() &&
+ GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ return;
+ }
EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
QuicSpdyClientSession* client_session = GetClientSession();
@@ -1991,6 +1999,10 @@
// Send a request and then disconnect. This prepares the client to attempt
// a 0-RTT handshake for the next request.
ASSERT_TRUE(Initialize());
+ if (!version_.UsesTls() &&
+ GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ return;
+ }
EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
QuicSpdyClientSession* client_session = GetClientSession();
@@ -2067,8 +2079,12 @@
client_session = GetClientSession();
ASSERT_TRUE(client_session);
- EXPECT_TRUE(client_session->EarlyDataAccepted());
- EXPECT_TRUE(client_->client()->EarlyDataAccepted());
+ EXPECT_EQ((version_.UsesTls() ||
+ !GetQuicReloadableFlag(quic_require_handshake_confirmation)),
+ client_session->EarlyDataAccepted());
+ EXPECT_EQ((version_.UsesTls() ||
+ !GetQuicReloadableFlag(quic_require_handshake_confirmation)),
+ client_->client()->EarlyDataAccepted());
client_->Disconnect();
@@ -5788,6 +5804,10 @@
TEST_P(EndToEndPacketReorderingTest, Buffer0RttRequest) {
ASSERT_TRUE(Initialize());
+ if (!version_.UsesTls() &&
+ GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ return;
+ }
// Finish one request to make sure handshake established.
client_->SendSynchronousRequest("/foo");
// Disconnect for next 0-rtt request.
diff --git a/quiche/quic/core/quic_crypto_server_stream_test.cc b/quiche/quic/core/quic_crypto_server_stream_test.cc
index 6f743d9..516a25f 100644
--- a/quiche/quic/core/quic_crypto_server_stream_test.cc
+++ b/quiche/quic/core/quic_crypto_server_stream_test.cc
@@ -210,6 +210,10 @@
InitializeFakeClient();
AdvanceHandshakeWithFakeClient();
+ if (GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ crypto_test_utils::AdvanceHandshake(client_connection_, client_stream(), 0,
+ server_connection_, server_stream(), 0);
+ }
EXPECT_TRUE(server_stream()->encryption_established());
EXPECT_TRUE(server_stream()->one_rtt_keys_available());
EXPECT_EQ(ENCRYPTION_FORWARD_SECURE,
@@ -243,7 +247,9 @@
crypto_test_utils::CommunicateHandshakeMessages(
client_connection_, client_stream(), server_connection_, server_stream());
- EXPECT_EQ(1, client_stream()->num_sent_client_hellos());
+ EXPECT_EQ(
+ (GetQuicReloadableFlag(quic_require_handshake_confirmation) ? 2 : 1),
+ client_stream()->num_sent_client_hellos());
EXPECT_TRUE(server_stream()->ResumptionAttempted());
}
@@ -301,6 +307,10 @@
InitializeServer();
InitializeFakeClient();
AdvanceHandshakeWithFakeClient();
+ if (GetQuicReloadableFlag(quic_require_handshake_confirmation)) {
+ crypto_test_utils::AdvanceHandshake(client_connection_, client_stream(), 0,
+ server_connection_, server_stream(), 0);
+ }
// Send a SCUP message and ensure that the client was able to verify it.
EXPECT_CALL(*client_connection_, CloseConnection(_, _, _)).Times(0);
diff --git a/quiche/quic/test_tools/crypto_test_utils_test.cc b/quiche/quic/test_tools/crypto_test_utils_test.cc
index 0522175..51bcc9e 100644
--- a/quiche/quic/test_tools/crypto_test_utils_test.cc
+++ b/quiche/quic/test_tools/crypto_test_utils_test.cc
@@ -54,6 +54,9 @@
return std::make_unique<ValidateClientHelloCallback>(this);
}
+ absl::string_view server_nonce() { return server_nonce_; }
+ bool chlo_accepted() const { return chlo_accepted_; }
+
private:
void ValidateClientHelloDone(
const quiche::QuicheReferenceCountedPointer<
@@ -89,9 +92,16 @@
}
void ProcessClientHelloDone(std::unique_ptr<CryptoHandshakeMessage> message) {
- // Verify output is a SHLO.
- EXPECT_EQ(message->tag(), kSHLO)
- << "Fail to pass validation. Get " << message->DebugString();
+ if (message->tag() == kSHLO) {
+ chlo_accepted_ = true;
+ } else {
+ QUIC_LOG(INFO) << "Fail to pass validation. Get "
+ << message->DebugString();
+ chlo_accepted_ = false;
+ EXPECT_EQ(1u, result_->info.reject_reasons.size());
+ EXPECT_EQ(SERVER_NONCE_REQUIRED_FAILURE, result_->info.reject_reasons[0]);
+ server_nonce_ = result_->info.server_nonce;
+ }
}
QuicCryptoServerConfig* crypto_config_;
@@ -107,6 +117,8 @@
result_;
const ParsedQuicVersion version_;
+ bool chlo_accepted_ = false;
+ absl::string_view server_nonce_;
};
class CryptoTestUtilsTest : public QuicTest {};
@@ -181,6 +193,21 @@
crypto_config.ValidateClientHello(
full_chlo, client_addr, server_addr, transport_version, &clock,
signed_config, shlo_verifier.GetValidateClientHelloCallback());
+ ASSERT_EQ(shlo_verifier.chlo_accepted(),
+ !GetQuicReloadableFlag(quic_require_handshake_confirmation));
+ if (!shlo_verifier.chlo_accepted()) {
+ ShloVerifier shlo_verifier2(
+ &crypto_config, server_addr, client_addr, &clock, signed_config,
+ &compressed_certs_cache,
+ ParsedQuicVersion(PROTOCOL_QUIC_CRYPTO, transport_version));
+ full_chlo.SetStringPiece(
+ kServerNonceTag,
+ "#" + absl::BytesToHexString(shlo_verifier.server_nonce()));
+ crypto_config.ValidateClientHello(
+ full_chlo, client_addr, server_addr, transport_version, &clock,
+ signed_config, shlo_verifier2.GetValidateClientHelloCallback());
+ EXPECT_TRUE(shlo_verifier2.chlo_accepted()) << full_chlo.DebugString();
+ }
}
} // namespace test
