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