Use detailed error code when client cached 0-RTT stream/flow control limit is reduced by server. Protected by quic_enable_zero_rtt_for_tls PiperOrigin-RevId: 316531305 Change-Id: Ia9bf39307b4e19b345605cbd8a13ed4874e7b3b8
diff --git a/quic/core/http/quic_spdy_client_session_test.cc b/quic/core/http/quic_spdy_client_session_test.cc index 0a0f5e9..5c0fdb8 100644 --- a/quic/core/http/quic_spdy_client_session_test.cc +++ b/quic/core/http/quic_spdy_client_session_test.cc
@@ -1142,7 +1142,7 @@ EXPECT_CALL( *connection_, CloseConnection( - QUIC_INTERNAL_ERROR, + QUIC_ZERO_RTT_UNRETRANSMITTABLE, "Server rejected 0-RTT, aborting because new bidirectional initial " "stream limit 0 is less than current open streams: 1", _)) @@ -1190,14 +1190,15 @@ if (session_->version().UsesHttp3()) { // Both control stream and the request stream will report errors. - EXPECT_CALL(*connection_, CloseConnection(QUIC_INTERNAL_ERROR, _, _)) + EXPECT_CALL(*connection_, + CloseConnection(QUIC_ZERO_RTT_UNRETRANSMITTABLE, _, _)) .Times(2) .WillOnce(testing::Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); } else { EXPECT_CALL(*connection_, CloseConnection( - QUIC_INTERNAL_ERROR, + QUIC_ZERO_RTT_UNRETRANSMITTABLE, "Server rejected 0-RTT, aborting because new stream max " "data 1 for stream 3 is less than currently used: 5", _)) @@ -1236,7 +1237,8 @@ // Let the stream write some data. stream->WriteOrBufferData(data_to_send, true, nullptr); - EXPECT_CALL(*connection_, CloseConnection(QUIC_INTERNAL_ERROR, _, _)) + EXPECT_CALL(*connection_, + CloseConnection(QUIC_ZERO_RTT_UNRETRANSMITTABLE, _, _)) .WillOnce(testing::Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); EXPECT_CALL(*connection_, CloseConnection(QUIC_HANDSHAKE_FAILED, _, _));
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc index 6afccfe..4b2c737 100644 --- a/quic/core/quic_error_codes.cc +++ b/quic/core/quic_error_codes.cc
@@ -221,6 +221,9 @@ RETURN_STRING_LITERAL(QUIC_HPACK_TRUNCATED_BLOCK); RETURN_STRING_LITERAL(QUIC_HPACK_FRAGMENT_TOO_LONG); RETURN_STRING_LITERAL(QUIC_HPACK_COMPRESSED_HEADER_SIZE_EXCEEDS_LIMIT); + RETURN_STRING_LITERAL(QUIC_ZERO_RTT_UNRETRANSMITTABLE); + RETURN_STRING_LITERAL(QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED); + RETURN_STRING_LITERAL(QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED); RETURN_STRING_LITERAL(QUIC_LAST_ERROR); // Intentionally have no default case, so we'll break the build @@ -599,6 +602,12 @@ return {true, static_cast<uint64_t>(INTERNAL_ERROR)}; case QUIC_HPACK_COMPRESSED_HEADER_SIZE_EXCEEDS_LIMIT: return {true, static_cast<uint64_t>(INTERNAL_ERROR)}; + case QUIC_ZERO_RTT_UNRETRANSMITTABLE: + return {true, static_cast<uint64_t>(INTERNAL_ERROR)}; + case QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED: + return {true, static_cast<uint64_t>(INTERNAL_ERROR)}; + case QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED: + return {true, static_cast<uint64_t>(PROTOCOL_VIOLATION)}; case QUIC_LAST_ERROR: return {false, static_cast<uint64_t>(QUIC_LAST_ERROR)}; }
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h index f057fea..f5c5007 100644 --- a/quic/core/quic_error_codes.h +++ b/quic/core/quic_error_codes.h
@@ -466,8 +466,20 @@ // Total compressed HPACK data size exceeds limit. QUIC_HPACK_COMPRESSED_HEADER_SIZE_EXCEEDS_LIMIT = 150, + // Stream/flow control limit from 1-RTT handshake is too low to retransmit + // 0-RTT data. This is our implentation error. We could in theory keep the + // connection alive but chose not to for simplicity. + QUIC_ZERO_RTT_UNRETRANSMITTABLE = 161, + // Stream/flow control limit from 0-RTT rejection reduces cached limit. + // This is our implentation error. We could in theory keep the connection + // alive but chose not to for simplicity. + QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED = 162, + // Stream/flow control limit from 0-RTT resumption reduces cached limit. + // This is the peer violating QUIC spec. + QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED = 163, + // No error. Used as bound while iterating. - QUIC_LAST_ERROR = 161, + QUIC_LAST_ERROR = 164, }; // 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
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 5043f1f..b697457 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -1027,7 +1027,7 @@ max_streams < v99_streamid_manager_.outgoing_bidirectional_stream_count()) { connection_->CloseConnection( - QUIC_INTERNAL_ERROR, + QUIC_ZERO_RTT_UNRETRANSMITTABLE, quiche::QuicheStrCat( "Server rejected 0-RTT, aborting because new bidirectional " "initial stream limit ", @@ -1043,8 +1043,12 @@ max_streams < v99_streamid_manager_.max_outgoing_bidirectional_streams()) { connection_->CloseConnection( - QUIC_MAX_STREAMS_ERROR, + was_zero_rtt_rejected_ ? QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED + : QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED, quiche::QuicheStrCat( + was_zero_rtt_rejected_ + ? "Server rejected 0-RTT, aborting because " + : "", "new bidirectional limit ", max_streams, " decreases the current limit: ", v99_streamid_manager_.max_outgoing_bidirectional_streams()), @@ -1065,7 +1069,7 @@ max_streams < v99_streamid_manager_.outgoing_unidirectional_stream_count()) { connection_->CloseConnection( - QUIC_INTERNAL_ERROR, + QUIC_ZERO_RTT_UNRETRANSMITTABLE, quiche::QuicheStrCat( "Server rejected 0-RTT, aborting because new unidirectional " "initial stream limit ", @@ -1078,8 +1082,12 @@ if (max_streams < v99_streamid_manager_.max_outgoing_unidirectional_streams()) { connection_->CloseConnection( - QUIC_MAX_STREAMS_ERROR, + was_zero_rtt_rejected_ ? QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED + : QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED, quiche::QuicheStrCat( + was_zero_rtt_rejected_ + ? "Server rejected 0-RTT, aborting because " + : "", "new unidirectional limit ", max_streams, " decreases the current limit: ", v99_streamid_manager_.max_outgoing_unidirectional_streams()), @@ -1321,16 +1329,7 @@ } QUIC_DVLOG(1) << ENDPOINT << "Informing unidirectional stream " << id << " of new stream flow control window " << new_window; - if (was_zero_rtt_rejected_ && - new_window < kv.second->flow_controller()->bytes_sent()) { - connection_->CloseConnection( - QUIC_INTERNAL_ERROR, - quiche::QuicheStrCat( - "Server rejected 0-RTT, aborting because new stream max data ", - new_window, " for stream ", kv.first, - " is less than currently used: ", - kv.second->flow_controller()->bytes_sent()), - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + if (!ValidateStreamFlowControlLimit(new_window, kv.second.get())) { return; } if (!kv.second->ConfigSendWindowOffset(new_window)) { @@ -1357,16 +1356,7 @@ } QUIC_DVLOG(1) << ENDPOINT << "Informing outgoing bidirectional stream " << id << " of new stream flow control window " << new_window; - if (was_zero_rtt_rejected_ && - new_window < kv.second->flow_controller()->bytes_sent()) { - connection_->CloseConnection( - QUIC_INTERNAL_ERROR, - quiche::QuicheStrCat( - "Server rejected 0-RTT, aborting because new stream max data ", - new_window, " for stream ", kv.first, - " is less than currently used: ", - kv.second->flow_controller()->bytes_sent()), - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + if (!ValidateStreamFlowControlLimit(new_window, kv.second.get())) { return; } if (!kv.second->ConfigSendWindowOffset(new_window)) { @@ -1393,16 +1383,7 @@ } QUIC_DVLOG(1) << ENDPOINT << "Informing incoming bidirectional stream " << id << " of new stream flow control window " << new_window; - if (was_zero_rtt_rejected_ && - new_window < kv.second->flow_controller()->bytes_sent()) { - connection_->CloseConnection( - QUIC_INTERNAL_ERROR, - quiche::QuicheStrCat( - "Server rejected 0-RTT, aborting because new stream max data ", - new_window, " for stream ", kv.first, - " is less than currently used: ", - kv.second->flow_controller()->bytes_sent()), - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + if (!ValidateStreamFlowControlLimit(new_window, kv.second.get())) { return; } if (!kv.second->ConfigSendWindowOffset(new_window)) { @@ -1411,6 +1392,41 @@ } } +bool QuicSession::ValidateStreamFlowControlLimit(QuicStreamOffset new_window, + const QuicStream* stream) { + if (was_zero_rtt_rejected_ && + new_window < stream->flow_controller()->bytes_sent()) { + QUIC_BUG_IF(perspective() == Perspective::IS_SERVER) + << "Server should never receive configs twice."; + connection_->CloseConnection( + QUIC_ZERO_RTT_UNRETRANSMITTABLE, + quiche::QuicheStrCat( + "Server rejected 0-RTT, aborting because new stream max data ", + new_window, " for stream ", stream->id(), + " is less than currently used: ", + stream->flow_controller()->bytes_sent()), + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return false; + } + + if (version().AllowsLowFlowControlLimits() && + new_window < stream->flow_controller()->send_window_offset()) { + QUIC_BUG_IF(perspective() == Perspective::IS_SERVER) + << "Server should never receive configs twice."; + connection_->CloseConnection( + was_zero_rtt_rejected_ ? QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED + : QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED, + quiche::QuicheStrCat( + was_zero_rtt_rejected_ ? "Server rejected 0-RTT, aborting because " + : "", + "new stream max data ", new_window, " decreases current limit: ", + stream->flow_controller()->send_window_offset()), + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + return false; + } + return true; +} + void QuicSession::OnNewSessionFlowControlWindow(QuicStreamOffset new_window) { QUIC_DVLOG(1) << ENDPOINT << "OnNewSessionFlowControlWindow " << new_window; @@ -1422,7 +1438,7 @@ ", which is below currently used: ", flow_controller_.bytes_sent()); QUIC_LOG(ERROR) << error_details; connection_->CloseConnection( - QUIC_INTERNAL_ERROR, error_details, + QUIC_ZERO_RTT_UNRETRANSMITTABLE, error_details, ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); return; } @@ -1442,12 +1458,15 @@ // The client receives a lower limit than remembered, violating // https://tools.ietf.org/html/draft-ietf-quic-transport-27#section-7.3.1 std::string error_details = quiche::QuicheStrCat( - "Peer sent us an invalid session flow control send window: ", - new_window, ", below current: ", flow_controller_.send_window_offset()); + was_zero_rtt_rejected_ ? "Server rejected 0-RTT, aborting because " + : "", + "new session max data ", new_window, + " decreases current limit: ", flow_controller_.send_window_offset()); QUIC_LOG(ERROR) << error_details; connection_->CloseConnection( - QUIC_FLOW_CONTROL_INVALID_WINDOW, error_details, - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + was_zero_rtt_rejected_ ? QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED + : QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED, + error_details, ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); return; }
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index e1e5188..81ba099 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -691,6 +691,12 @@ QuicRstStreamErrorCode error, QuicStreamOffset bytes_written); + // Closes the connection and returns false if |new_window| is lower than + // |stream|'s current flow control window. + // Returns true otherwise. + bool ValidateStreamFlowControlLimit(QuicStreamOffset new_window, + const QuicStream* stream); + // Sends a STOP_SENDING frame if the stream type allows. void MaybeSendStopSendingFrame(QuicStreamId id, QuicRstStreamErrorCode error);
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 70fda35..a400d97 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -2070,8 +2070,12 @@ const uint32_t kInvalidWindow = kMinimumFlowControlSendWindow - 1; QuicConfigPeer::SetReceivedInitialSessionFlowControlWindow(session_.config(), kInvalidWindow); - EXPECT_CALL(*connection_, - CloseConnection(QUIC_FLOW_CONTROL_INVALID_WINDOW, _, _)); + EXPECT_CALL( + *connection_, + CloseConnection(connection_->version().AllowsLowFlowControlLimits() + ? QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED + : QUIC_FLOW_CONTROL_INVALID_WINDOW, + _, _)); connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); session_.OnConfigNegotiated(); } @@ -2083,7 +2087,8 @@ } QuicConfigPeer::SetReceivedMaxBidirectionalStreams( session_.config(), kDefaultMaxStreamsPerConnection - 1); - EXPECT_CALL(*connection_, CloseConnection(QUIC_MAX_STREAMS_ERROR, _, _)); + EXPECT_CALL(*connection_, + CloseConnection(QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED, _, _)); connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); session_.OnConfigNegotiated(); } @@ -2095,7 +2100,8 @@ } QuicConfigPeer::SetReceivedMaxUnidirectionalStreams( session_.config(), kDefaultMaxStreamsPerConnection - 1); - EXPECT_CALL(*connection_, CloseConnection(QUIC_MAX_STREAMS_ERROR, _, _)); + EXPECT_CALL(*connection_, + CloseConnection(QUIC_ZERO_RTT_RESUMPTION_LIMIT_REDUCED, _, _)); connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); session_.OnConfigNegotiated(); }
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc index 895cf4c..9903781 100644 --- a/quic/core/quic_stream.cc +++ b/quic/core/quic_stream.cc
@@ -929,16 +929,11 @@ << "ConfigSendWindowOffset called on stream without flow control"; return false; } - if (perspective_ == Perspective::IS_CLIENT && - session()->version().AllowsLowFlowControlLimits() && - new_offset < flow_controller_->send_window_offset()) { - OnUnrecoverableError( - QUIC_FLOW_CONTROL_INVALID_WINDOW, - quiche::QuicheStrCat("New stream max data ", new_offset, - " decreases current limit: ", - flow_controller_->send_window_offset())); - return false; - } + + QUIC_BUG_IF(session()->version().AllowsLowFlowControlLimits() && + new_offset < flow_controller_->send_window_offset()) + << ENDPOINT << "The new offset " << new_offset + << " decreases current offset " << flow_controller_->send_window_offset(); if (flow_controller_->UpdateSendWindowOffset(new_offset)) { // Let session unblock this stream. session_->MarkConnectionLevelWriteBlocked(id_); @@ -1285,6 +1280,22 @@ session_->SendStopSending(code, id_); } +QuicFlowController* QuicStream::flow_controller() { + if (flow_controller_.has_value()) { + return &flow_controller_.value(); + } + QUIC_BUG << "Trying to access non-existent flow controller."; + return nullptr; +} + +const QuicFlowController* QuicStream::flow_controller() const { + if (flow_controller_.has_value()) { + return &flow_controller_.value(); + } + QUIC_BUG << "Trying to access non-existent flow controller."; + return nullptr; +} + // static spdy::SpdyStreamPrecedence QuicStream::CalculateDefaultPriority( const QuicSession* session) {
diff --git a/quic/core/quic_stream.h b/quic/core/quic_stream.h index 80b2fb0..7a847f0 100644 --- a/quic/core/quic_stream.h +++ b/quic/core/quic_stream.h
@@ -229,7 +229,9 @@ int num_frames_received() const; int num_duplicate_frames_received() const; - QuicFlowController* flow_controller() { return &*flow_controller_; } + QuicFlowController* flow_controller(); + + const QuicFlowController* flow_controller() const; // Called when endpoint receives a frame which could increase the highest // offset.
diff --git a/quic/core/tls_client_handshaker_test.cc b/quic/core/tls_client_handshaker_test.cc index 1ed2042..1ec3d39 100644 --- a/quic/core/tls_client_handshaker_test.cc +++ b/quic/core/tls_client_handshaker_test.cc
@@ -8,6 +8,7 @@ #include "net/third_party/quiche/src/quic/core/crypto/quic_decrypter.h" #include "net/third_party/quiche/src/quic/core/crypto/quic_encrypter.h" +#include "net/third_party/quiche/src/quic/core/quic_error_codes.h" #include "net/third_party/quiche/src/quic/core/quic_packets.h" #include "net/third_party/quiche/src/quic/core/quic_server_id.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" @@ -515,7 +516,8 @@ config.SetMaxBidirectionalStreamsToSend( config.GetMaxBidirectionalStreamsToSend() - 1); - EXPECT_CALL(*connection_, CloseConnection(QUIC_MAX_STREAMS_ERROR, _, _)) + EXPECT_CALL(*connection_, + CloseConnection(QUIC_ZERO_RTT_REJECTION_LIMIT_REDUCED, _, _)) .WillOnce(testing::Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); // Close connection will be called again in the handshaker, but this will be