Ensure QUIC crypto send buffer does not exceed its size limit
Note to reviewer(s): Please double check that the new error code looks good and that it should indeed map to FLOW_CONTROL_ERROR (wasn't sure).
Protected by FLAGS_quic_reloadable_flag_quic_bounded_crypto_send_buffer.
PiperOrigin-RevId: 451167627
diff --git a/quiche/quic/core/quic_crypto_stream.cc b/quiche/quic/core/quic_crypto_stream.cc
index 3839db3..c85b9ab 100644
--- a/quiche/quic/core/quic_crypto_stream.cc
+++ b/quiche/quic/core/quic_crypto_stream.cc
@@ -13,6 +13,7 @@
#include "quiche/quic/core/crypto/crypto_utils.h"
#include "quiche/quic/core/frames/quic_crypto_frame.h"
#include "quiche/quic/core/quic_connection.h"
+#include "quiche/quic/core/quic_error_codes.h"
#include "quiche/quic/core/quic_session.h"
#include "quiche/quic/core/quic_types.h"
#include "quiche/quic/core/quic_utils.h"
@@ -145,17 +146,41 @@
return;
}
const bool had_buffered_data = HasBufferedCryptoFrames();
- // Append |data| to the send buffer for this encryption level.
QuicStreamSendBuffer* send_buffer =
&substreams_[QuicUtils::GetPacketNumberSpace(level)].send_buffer;
QuicStreamOffset offset = send_buffer->stream_offset();
+
+ // Ensure this data does not cause the send buffer for this encryption level
+ // to exceed its size limit.
+ if (GetQuicReloadableFlag(quic_bounded_crypto_send_buffer)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_bounded_crypto_send_buffer);
+ QUIC_BUG_IF(quic_crypto_stream_offset_lt_bytes_written,
+ offset < send_buffer->stream_bytes_written());
+ uint64_t current_buffer_size =
+ offset - std::min(offset, send_buffer->stream_bytes_written());
+ if (current_buffer_size > 0) {
+ QUIC_CODE_COUNT(quic_received_crypto_data_with_non_empty_send_buffer);
+ if (BufferSizeLimitForLevel(level) <
+ (current_buffer_size + data.length())) {
+ QUIC_BUG(quic_crypto_send_buffer_overflow)
+ << absl::StrCat("Too much data for crypto send buffer with level: ",
+ EncryptionLevelToString(level),
+ ", current_buffer_size: ", current_buffer_size,
+ ", data length: ", data.length());
+ OnUnrecoverableError(QUIC_INTERNAL_ERROR,
+ "Too much data for crypto send buffer");
+ return;
+ }
+ }
+ }
+
+ // Append |data| to the send buffer for this encryption level.
send_buffer->SaveStreamData(data);
if (kMaxStreamLength - offset < data.length()) {
QUIC_BUG(quic_bug_10322_2) << "Writing too much crypto handshake data";
- // TODO(nharper): Switch this to an IETF QUIC error code, possibly
- // INTERNAL_ERROR?
- OnUnrecoverableError(QUIC_STREAM_LENGTH_OVERFLOW,
+ OnUnrecoverableError(QUIC_INTERNAL_ERROR,
"Writing too much crypto handshake data");
+ return;
}
if (had_buffered_data) {
// Do not try to write if there is buffered data.
@@ -228,7 +253,6 @@
QuicStream::OnStreamDataConsumed(bytes_consumed);
}
-
bool QuicCryptoStream::HasPendingCryptoRetransmission() const {
if (!QuicVersionUsesCryptoFrames(session()->transport_version())) {
return false;
diff --git a/quiche/quic/core/quic_crypto_stream_test.cc b/quiche/quic/core/quic_crypto_stream_test.cc
index ea99926..f5d1f2e 100644
--- a/quiche/quic/core/quic_crypto_stream_test.cc
+++ b/quiche/quic/core/quic_crypto_stream_test.cc
@@ -13,8 +13,10 @@
#include "quiche/quic/core/crypto/crypto_handshake.h"
#include "quiche/quic/core/crypto/crypto_protocol.h"
#include "quiche/quic/core/crypto/null_encrypter.h"
+#include "quiche/quic/core/quic_error_codes.h"
#include "quiche/quic/core/quic_types.h"
#include "quiche/quic/core/quic_utils.h"
+#include "quiche/quic/platform/api/quic_expect_bug.h"
#include "quiche/quic/platform/api/quic_socket_address.h"
#include "quiche/quic/platform/api/quic_test.h"
#include "quiche/quic/test_tools/crypto_test_utils.h"
@@ -652,6 +654,50 @@
}
}
+TEST_F(QuicCryptoStreamTest, WriteCryptoDataExceedsSendBufferLimit) {
+ if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) {
+ return;
+ }
+ EXPECT_EQ(ENCRYPTION_INITIAL, connection_->encryption_level());
+ int32_t buffer_limit = GetQuicFlag(FLAGS_quic_max_buffered_crypto_bytes);
+
+ // Write data larger than the buffer limit, when there is no existing data in
+ // the buffer. Data is sent rather than closing the connection.
+ EXPECT_FALSE(stream_->HasBufferedCryptoFrames());
+ int32_t over_limit = buffer_limit + 1;
+ EXPECT_CALL(*connection_, SendCryptoData(ENCRYPTION_INITIAL, over_limit, 0))
+ // All the data is sent, no resulting buffer.
+ .WillOnce(Return(over_limit));
+ std::string large_data(over_limit, 'a');
+ stream_->WriteCryptoData(ENCRYPTION_INITIAL, large_data);
+
+ // Write data to the buffer up to the limit. One byte gets sent.
+ EXPECT_FALSE(stream_->HasBufferedCryptoFrames());
+ EXPECT_CALL(*connection_,
+ SendCryptoData(ENCRYPTION_INITIAL, buffer_limit, over_limit))
+ .WillOnce(Return(1));
+ std::string data(buffer_limit, 'a');
+ stream_->WriteCryptoData(ENCRYPTION_INITIAL, data);
+ EXPECT_TRUE(stream_->HasBufferedCryptoFrames());
+
+ // Write another byte that is not sent (due to there already being data in the
+ // buffer); send buffer is now full.
+ EXPECT_CALL(*connection_, SendCryptoData(_, _, _)).Times(0);
+ std::string data2(1, 'a');
+ stream_->WriteCryptoData(ENCRYPTION_INITIAL, data2);
+ EXPECT_TRUE(stream_->HasBufferedCryptoFrames());
+
+ // Writing an additional byte to the send buffer closes the connection.
+ if (GetQuicReloadableFlag(quic_bounded_crypto_send_buffer)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_bounded_crypto_send_buffer);
+ EXPECT_CALL(*connection_, CloseConnection(QUIC_INTERNAL_ERROR, _, _));
+ EXPECT_QUIC_BUG(
+ stream_->WriteCryptoData(ENCRYPTION_INITIAL, data2),
+ "Too much data for crypto send buffer with level: ENCRYPTION_INITIAL, "
+ "current_buffer_size: 16384, data length: 1");
+ }
+}
+
TEST_F(QuicCryptoStreamTest, WriteBufferedCryptoFrames) {
if (!QuicVersionUsesCryptoFrames(connection_->transport_version())) {
return;
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index ddb7948..c4f9f91 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -31,6 +31,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_allow_client_enabled_bbr_v2, true)
// If true, close read side but not write side in QuicSpdyStream::OnStreamReset().
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_fix_on_stream_reset, true)
+// If true, close the connection if a crypto send buffer exceeds its size limit.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_bounded_crypto_send_buffer, false)
// If true, consider original connection ID as active before handshake completes.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_consider_original_connection_id_as_active_pre_handshake, false)
// If true, consolidate more logic into SetRetransmissionAlarm to ensure the logic is applied consistently.
