Use more detailed error logging when 0-RTT SETTINGS are mismatched.
This CL also fixed a bug on SETTINGS_MAX_HEADER_LIST_SIZE, where we check on increase of the setting but instead should check on decrease.
Protected by disabled quic_enable_zero_rtt_for_tls
PiperOrigin-RevId: 317401398
Change-Id: Id464d788ff01f3cd76733717bc5a20cf15d2016a
diff --git a/quic/core/http/quic_spdy_client_session_base.cc b/quic/core/http/quic_spdy_client_session_base.cc
index eaa022e..b433c56 100644
--- a/quic/core/http/quic_spdy_client_session_base.cc
+++ b/quic/core/http/quic_spdy_client_session_base.cc
@@ -239,7 +239,7 @@
frame.values.find(SETTINGS_MAX_HEADER_LIST_SIZE) ==
frame.values.end()) {
CloseConnectionWithDetails(
- QUIC_HTTP_ZERO_RTT_SETTINGS_MISMATCH,
+ QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH,
"Server accepted 0-RTT but omitted non-default "
"SETTINGS_MAX_HEADER_LIST_SIZE");
return false;
@@ -249,7 +249,7 @@
frame.values.find(SETTINGS_QPACK_BLOCKED_STREAMS) ==
frame.values.end()) {
CloseConnectionWithDetails(
- QUIC_HTTP_ZERO_RTT_SETTINGS_MISMATCH,
+ QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH,
"Server accepted 0-RTT but omitted non-default "
"SETTINGS_QPACK_BLOCKED_STREAMS");
return false;
@@ -259,7 +259,7 @@
frame.values.find(SETTINGS_QPACK_MAX_TABLE_CAPACITY) ==
frame.values.end()) {
CloseConnectionWithDetails(
- QUIC_HTTP_ZERO_RTT_SETTINGS_MISMATCH,
+ QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH,
"Server accepted 0-RTT but omitted non-default "
"SETTINGS_QPACK_MAX_TABLE_CAPACITY");
return false;
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc
index d5481b5..3464a7e 100644
--- a/quic/core/http/quic_spdy_client_session_test.cc
+++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -1369,7 +1369,7 @@
EXPECT_TRUE(session_->GetCryptoStream()->IsResumption());
}
-TEST_P(QuicSpdyClientSessionTest, BadSettingsInZeroRtt) {
+TEST_P(QuicSpdyClientSessionTest, BadSettingsInZeroRttResumption) {
if (!session_->version().UsesHttp3()) {
return;
}
@@ -1380,7 +1380,9 @@
CompleteCryptoHandshake();
EXPECT_TRUE(session_->GetCryptoStream()->EarlyDataAccepted());
- EXPECT_CALL(*connection_, CloseConnection(QUIC_INTERNAL_ERROR, _, _))
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH, _, _))
.WillOnce(testing::Invoke(connection_,
&MockQuicConnection::ReallyCloseConnection));
// Let the session receive a different SETTINGS frame.
@@ -1391,6 +1393,37 @@
session_->OnSettingsFrame(settings);
}
+TEST_P(QuicSpdyClientSessionTest, BadSettingsInZeroRttRejection) {
+ if (!session_->version().UsesHttp3()) {
+ return;
+ }
+
+ CompleteFirstConnection();
+
+ CreateConnection();
+ SSL_CTX_set_early_data_enabled(server_crypto_config_->ssl_ctx(), false);
+ session_->CryptoConnect();
+ EXPECT_TRUE(session_->IsEncryptionEstablished());
+ QuicConfig config = DefaultQuicConfig();
+ crypto_test_utils::HandshakeWithFakeServer(
+ &config, server_crypto_config_.get(), &helper_, &alarm_factory_,
+ connection_, crypto_stream_, AlpnForVersion(connection_->version()));
+ EXPECT_FALSE(session_->GetCryptoStream()->EarlyDataAccepted());
+
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH, _, _))
+ .WillOnce(testing::Invoke(connection_,
+ &MockQuicConnection::ReallyCloseConnection));
+ // Let the session receive a different SETTINGS frame.
+ SettingsFrame settings;
+ settings.values[SETTINGS_QPACK_MAX_TABLE_CAPACITY] = 2;
+ // setting on SETTINGS_MAX_HEADER_LIST_SIZE is reduced.
+ settings.values[SETTINGS_MAX_HEADER_LIST_SIZE] = 4;
+ settings.values[256] = 4; // unknown setting
+ session_->OnSettingsFrame(settings);
+}
+
TEST_P(QuicSpdyClientSessionTest, ServerAcceptsZeroRttButOmitSetting) {
if (!session_->version().UsesHttp3()) {
return;
@@ -1402,8 +1435,9 @@
CompleteCryptoHandshake();
EXPECT_TRUE(session_->GetMutableCryptoStream()->EarlyDataAccepted());
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_HTTP_ZERO_RTT_SETTINGS_MISMATCH, _, _))
+ EXPECT_CALL(
+ *connection_,
+ CloseConnection(QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH, _, _))
.WillOnce(testing::Invoke(connection_,
&MockQuicConnection::ReallyCloseConnection));
// Let the session receive a different SETTINGS frame.
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 08f82c8..d6b5a0e 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -910,11 +910,17 @@
// Required Insert Count.
bool success = qpack_encoder_->SetMaximumDynamicTableCapacity(value);
if (GetQuicReloadableFlag(quic_enable_zero_rtt_for_tls) && !success) {
- // TODO(b/153726130): Use different error code for the case of 0-RTT
- // rejection.
CloseConnectionWithDetails(
- QUIC_INTERNAL_ERROR,
- "Server sent an invalid SETTINGS_QPACK_MAX_TABLE_CAPACITY.");
+ was_zero_rtt_rejected()
+ ? QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH
+ : QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH,
+ quiche::QuicheStrCat(
+ was_zero_rtt_rejected()
+ ? "Server rejected 0-RTT, aborting because "
+ : "",
+ "Server sent an SETTINGS_QPACK_MAX_TABLE_CAPACITY: ", value,
+ "while current value is: ",
+ qpack_encoder_->MaximumDynamicTableCapacity()));
return false;
}
// However, limit the dynamic table capacity to
@@ -928,12 +934,20 @@
<< "SETTINGS_MAX_HEADER_LIST_SIZE received with value "
<< value;
if (GetQuicReloadableFlag(quic_enable_zero_rtt_for_tls) &&
- max_outbound_header_list_size_ < value) {
- // TODO(b/153726130): Use different error code for the case of 0-RTT
- // rejection.
+ max_outbound_header_list_size_ !=
+ std::numeric_limits<size_t>::max() &&
+ max_outbound_header_list_size_ > value) {
CloseConnectionWithDetails(
- QUIC_INTERNAL_ERROR,
- "Server sent an invalid SETTINGS_MAX_HEADER_LIST_SIZE.");
+ was_zero_rtt_rejected()
+ ? QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH
+ : QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH,
+ quiche::QuicheStrCat(
+ was_zero_rtt_rejected()
+ ? "Server rejected 0-RTT, aborting because "
+ : "",
+ "Server sent an SETTINGS_MAX_HEADER_LIST_SIZE: ", value,
+ "which reduces current value: ",
+ max_outbound_header_list_size_));
return false;
}
max_outbound_header_list_size_ = value;
@@ -943,12 +957,18 @@
<< "SETTINGS_QPACK_BLOCKED_STREAMS received with value "
<< value;
bool success = qpack_encoder_->SetMaximumBlockedStreams(value);
- // TODO(b/153726130): Use different error code for the case of 0-RTT
- // rejection.
if (GetQuicReloadableFlag(quic_enable_zero_rtt_for_tls) && !success) {
CloseConnectionWithDetails(
- QUIC_INTERNAL_ERROR,
- "Server sent an invalid SETTINGS_QPACK_BLOCKED_STREAMS.");
+ was_zero_rtt_rejected()
+ ? QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH
+ : QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH,
+ quiche::QuicheStrCat(
+ was_zero_rtt_rejected()
+ ? "Server rejected 0-RTT, aborting because "
+ : "",
+ "Server sent an SETTINGS_QPACK_BLOCKED_STREAMS: ", value,
+ "which reduces current value: ",
+ qpack_encoder_->maximum_blocked_streams()));
return false;
}
break;
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index 4887525..1c0984a 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -203,7 +203,8 @@
RETURN_STRING_LITERAL(QUIC_HTTP_DUPLICATE_SETTING_IDENTIFIER);
RETURN_STRING_LITERAL(QUIC_HTTP_INVALID_MAX_PUSH_ID);
RETURN_STRING_LITERAL(QUIC_HTTP_STREAM_LIMIT_TOO_LOW);
- RETURN_STRING_LITERAL(QUIC_HTTP_ZERO_RTT_SETTINGS_MISMATCH);
+ RETURN_STRING_LITERAL(QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH);
+ RETURN_STRING_LITERAL(QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH);
RETURN_STRING_LITERAL(QUIC_HPACK_INDEX_VARINT_ERROR);
RETURN_STRING_LITERAL(QUIC_HPACK_NAME_LENGTH_VARINT_ERROR);
RETURN_STRING_LITERAL(QUIC_HPACK_VALUE_LENGTH_VARINT_ERROR);
@@ -571,8 +572,10 @@
case QUIC_HTTP_STREAM_LIMIT_TOO_LOW:
return {false, static_cast<uint64_t>(
QuicHttp3ErrorCode::GENERAL_PROTOCOL_ERROR)};
- case QUIC_HTTP_ZERO_RTT_SETTINGS_MISMATCH:
+ case QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH:
return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::SETTINGS_ERROR)};
+ case QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH:
+ return {true, static_cast<uint64_t>(INTERNAL_ERROR)};
case QUIC_HPACK_INDEX_VARINT_ERROR:
return {true, static_cast<uint64_t>(INTERNAL_ERROR)};
case QUIC_HPACK_NAME_LENGTH_VARINT_ERROR:
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h
index 60443ce..4c13f2c 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -431,8 +431,12 @@
QUIC_HTTP_INVALID_MAX_PUSH_ID = 159,
// Received unidirectional stream limit is lower than required by HTTP/3.
QUIC_HTTP_STREAM_LIMIT_TOO_LOW = 160,
- // Received mismatched SETTINGS frame from HTTP/3 0-RTT connection.
- QUIC_HTTP_ZERO_RTT_SETTINGS_MISMATCH = 164,
+ // Received mismatched SETTINGS frame from HTTP/3 connection where early data
+ // is accepted. Server violated the HTTP/3 spec.
+ QUIC_HTTP_ZERO_RTT_RESUMPTION_SETTINGS_MISMATCH = 164,
+ // Received mismatched SETTINGS frame from HTTP/3 connection where early data
+ // is rejected. Our implementation currently doesn't support it.
+ QUIC_HTTP_ZERO_RTT_REJECTION_SETTINGS_MISMATCH = 165,
// HPACK header block decoding errors.
// Index varint beyond implementation limit.
@@ -481,7 +485,7 @@
QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED = 163,
// No error. Used as bound while iterating.
- QUIC_LAST_ERROR = 165,
+ QUIC_LAST_ERROR = 166,
};
// QuicErrorCodes is encoded as four octets on-the-wire when doing Google QUIC,
// or a varint62 when doing IETF QUIC. Ensure that its value does not exceed
