Fix original issue of QNZR by only forcing the first CHLO to be inchoate.
Also replace the connection option with QNZ2.
Manually tested with netlog at https://drive.google.com/file/d/1WDFHBZJGy9ZM6hXwriyH3GEjvgdUVy4W/view?usp=sharing
Protected by client connection option QNZ2.
PiperOrigin-RevId: 338360564
Change-Id: I1cac9589006b59ff32dbd325e97ca77d70e2b6cb
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index b5ed5c7..50fa103 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -27,7 +27,8 @@
typedef std::string ServerConfigID;
// The following tags have been deprecated and should not be reused:
-// "1CON", "BBQ4", "NCON", "RCID", "SREJ", "TBKP", "TB10", "SCLS", "SMHL"
+// "1CON", "BBQ4", "NCON", "RCID", "SREJ", "TBKP", "TB10", "SCLS", "SMHL",
+// "QNZR"
// clang-format off
const QuicTag kCHLO = TAG('C', 'H', 'L', 'O'); // Client hello
@@ -369,7 +370,8 @@
const QuicTag kXLCT = TAG('X', 'L', 'C', 'T'); // Expected leaf certificate.
const QuicTag kQLVE = TAG('Q', 'L', 'V', 'E'); // Legacy Version
// Encapsulation.
-const QuicTag kQNZR = TAG('Q', 'N', 'Z', 'R'); // Turn off QUIC crypto 0-RTT.
+
+const QuicTag kQNZ2 = TAG('Q', 'N', 'Z', '2'); // Turn off QUIC crypto 0-RTT.
const QuicTag kQNSP = TAG('Q', 'N', 'S', 'P'); // Turn off server push in
// gQUIC.
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 3335b4a..6d579f5 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -2185,6 +2185,41 @@
server_thread_->Resume();
}
+// Regression test for b/171378845
+TEST_P(EndToEndTest, ClientDisablesGQuicZeroRtt) {
+ if (version_.UsesTls()) {
+ // This feature is gQUIC only.
+ ASSERT_TRUE(Initialize());
+ return;
+ }
+ QuicTagVector options;
+ options.push_back(kQNZ2);
+ client_config_.SetClientConnectionOptions(options);
+
+ ASSERT_TRUE(Initialize());
+
+ EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
+ QuicSpdyClientSession* client_session = GetClientSession();
+ ASSERT_TRUE(client_session);
+ EXPECT_FALSE(client_session->EarlyDataAccepted());
+ EXPECT_FALSE(client_session->ReceivedInchoateReject());
+ EXPECT_FALSE(client_->client()->EarlyDataAccepted());
+ EXPECT_FALSE(client_->client()->ReceivedInchoateReject());
+
+ client_->Disconnect();
+
+ // Make sure that the request succeeds but 0-RTT was not used.
+ client_->Connect();
+ EXPECT_TRUE(client_->client()->WaitForOneRttKeysAvailable());
+ ASSERT_TRUE(client_->client()->connected());
+ EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
+
+ client_session = GetClientSession();
+ ASSERT_TRUE(client_session);
+ EXPECT_FALSE(client_session->EarlyDataAccepted());
+ EXPECT_FALSE(client_->client()->EarlyDataAccepted());
+}
+
TEST_P(EndToEndTest, MaxInitialRTT) {
// Client tries to suggest twice the server's max initial rtt and the server
// uses the max.
diff --git a/quic/core/quic_crypto_client_handshaker.cc b/quic/core/quic_crypto_client_handshaker.cc
index 784a451..f744153 100644
--- a/quic/core/quic_crypto_client_handshaker.cc
+++ b/quic/core/quic_crypto_client_handshaker.cc
@@ -317,7 +317,8 @@
early_data_reason_ = ssl_early_data_no_session_offered;
fill_inchoate_client_hello = true;
} else if (session()->config()->HasClientRequestedIndependentOption(
- kQNZR, session()->perspective())) {
+ kQNZ2, session()->perspective()) &&
+ num_client_hellos_ == 1) {
early_data_reason_ = ssl_early_data_disabled;
fill_inchoate_client_hello = true;
}
diff --git a/quic/core/quic_crypto_client_stream_test.cc b/quic/core/quic_crypto_client_stream_test.cc
index eef6b49..86fa3b8 100644
--- a/quic/core/quic_crypto_client_stream_test.cc
+++ b/quic/core/quic_crypto_client_stream_test.cc
@@ -69,7 +69,8 @@
void CompleteCryptoHandshake() {
int proof_verify_details_calls = 1;
if (stream()->handshake_protocol() != PROTOCOL_TLS1_3) {
- EXPECT_CALL(*session_, OnProofValid(testing::_));
+ EXPECT_CALL(*session_, OnProofValid(testing::_))
+ .Times(testing::AtLeast(1));
proof_verify_details_calls = 0;
}
EXPECT_CALL(*session_, OnProofVerifyDetailsAvailable(testing::_))
@@ -172,14 +173,13 @@
// Set connection option.
QuicTagVector options;
- options.push_back(kQNZR);
+ options.push_back(kQNZ2);
session_->config()->SetClientConnectionOptions(options);
- EXPECT_CALL(*session_, OnProofValid(testing::_));
- stream()->CryptoConnect();
- // Check that a client hello was sent.
- ASSERT_EQ(1u, connection_->encrypted_packets_.size());
- EXPECT_EQ(ENCRYPTION_INITIAL, connection_->encryption_level());
+ CompleteCryptoHandshake();
+ // Check that two client hellos were sent, one inchoate and one normal.
+ EXPECT_EQ(2, stream()->num_sent_client_hellos());
+ EXPECT_FALSE(stream()->EarlyDataAccepted());
EXPECT_EQ(stream()->EarlyDataReason(), ssl_early_data_disabled);
}
