Reject HTTP/2 only frames in HTTP/3.
Protected by quic_reloadable_flag_quic_reject_spdy_frames.
PiperOrigin-RevId: 334660786
Change-Id: Id0362f27f34e8a0f10caf3dfb6c8f4554cb56dda
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index 3e06703..d142201 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -6,8 +6,10 @@
#include <cstdint>
+#include "net/third_party/quiche/src/http2/http2_constants.h"
#include "net/third_party/quiche/src/quic/core/http/http_frames.h"
#include "net/third_party/quiche/src/quic/core/quic_data_reader.h"
+#include "net/third_party/quiche/src/quic/core/quic_error_codes.h"
#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_fallthrough.h"
@@ -93,7 +95,7 @@
switch (state_) {
case STATE_READING_FRAME_TYPE:
- ReadFrameType(&reader);
+ continue_processing = ReadFrameType(&reader);
break;
case STATE_READING_FRAME_LENGTH:
continue_processing = ReadFrameLength(&reader);
@@ -114,7 +116,7 @@
return len - reader.BytesRemaining();
}
-void HttpDecoder::ReadFrameType(QuicDataReader* reader) {
+bool HttpDecoder::ReadFrameType(QuicDataReader* reader) {
DCHECK_NE(0u, reader->BytesRemaining());
if (current_type_field_length_ == 0) {
// A new frame is coming.
@@ -124,7 +126,7 @@
// Buffer a new type field.
remaining_type_field_length_ = current_type_field_length_;
BufferFrameType(reader);
- return;
+ return true;
}
// The reader has all type data needed, so no need to buffer.
bool success = reader->ReadVarInt62(¤t_frame_type_);
@@ -134,14 +136,34 @@
BufferFrameType(reader);
// The frame is still not buffered completely.
if (remaining_type_field_length_ != 0) {
- return;
+ return true;
}
QuicDataReader type_reader(type_buffer_.data(), current_type_field_length_);
bool success = type_reader.ReadVarInt62(¤t_frame_type_);
DCHECK(success);
}
+ if (GetQuicReloadableFlag(quic_reject_spdy_frames)) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_reject_spdy_frames);
+ // https://tools.ietf.org/html/draft-ietf-quic-http-31#section-7.2.8
+ // specifies that the following frames are treated as errors.
+ if (current_frame_type_ ==
+ static_cast<uint64_t>(http2::Http2FrameType::PRIORITY) ||
+ current_frame_type_ ==
+ static_cast<uint64_t>(http2::Http2FrameType::PING) ||
+ current_frame_type_ ==
+ static_cast<uint64_t>(http2::Http2FrameType::WINDOW_UPDATE) ||
+ current_frame_type_ ==
+ static_cast<uint64_t>(http2::Http2FrameType::CONTINUATION)) {
+ RaiseError(
+ QUIC_HTTP_RECEIVE_SPDY_FRAME,
+ quiche::QuicheStrCat("HTTP/2 frame received in a HTTP/3 connection: ",
+ current_frame_type_));
+ return false;
+ }
+ }
state_ = STATE_READING_FRAME_LENGTH;
+ return true;
}
bool HttpDecoder::ReadFrameLength(QuicDataReader* reader) {
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h
index 1551aa7..82535f7 100644
--- a/quic/core/http/http_decoder.h
+++ b/quic/core/http/http_decoder.h
@@ -158,8 +158,9 @@
// Reads the type of a frame from |reader|. Sets error_ and error_detail_
// if there are any errors. Also calls OnDataFrameStart() or
- // OnHeadersFrameStart() for appropriate frame types.
- void ReadFrameType(QuicDataReader* reader);
+ // OnHeadersFrameStart() for appropriate frame types. Returns whether the
+ // processing should continue.
+ bool ReadFrameType(QuicDataReader* reader);
// Reads the length of a frame from |reader|. Sets error_ and error_detail_
// if there are any errors. Returns whether processing should continue.
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index 01f0a5f..fb29d7b 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -192,7 +192,7 @@
const QuicByteCount payload_lengths[] = {0, 14, 100};
const uint64_t frame_types[] = {
0x21, 0x40, 0x5f, 0x7e, 0x9d, // some reserved frame types
- 0x06, 0x6f, 0x14 // some unknown, not reserved frame types
+ 0x6f, 0x14 // some unknown, not reserved frame types
};
for (auto payload_length : payload_lengths) {
@@ -791,6 +791,21 @@
EXPECT_EQ("Frame is too large.", decoder_.error_detail());
}
+TEST_F(HttpDecoderTest, Http2Frame) {
+ SetQuicReloadableFlag(quic_reject_spdy_frames, true);
+ std::string input = quiche::QuicheTextUtils::HexDecode(
+ "06" // PING in HTTP/2 but not supported in HTTP/3.
+ "05" // length
+ "15"); // random payload
+
+ // Process the full frame.
+ EXPECT_CALL(visitor_, OnError(&decoder_));
+ EXPECT_EQ(1u, ProcessInput(input));
+ EXPECT_THAT(decoder_.error(), IsError(QUIC_HTTP_RECEIVE_SPDY_FRAME));
+ EXPECT_EQ("HTTP/2 frame received in a HTTP/3 connection: 6",
+ decoder_.error_detail());
+}
+
TEST_F(HttpDecoderTest, HeadersPausedThenData) {
InSequence s;
std::string input = quiche::QuicheStrCat(
diff --git a/quic/core/http/quic_server_session_base_test.cc b/quic/core/http/quic_server_session_base_test.cc
index 61303de..a0ac44a 100644
--- a/quic/core/http/quic_server_session_base_test.cc
+++ b/quic/core/http/quic_server_session_base_test.cc
@@ -305,7 +305,7 @@
QuicStreamFrame frame1(GetNthClientInitiatedBidirectionalId(0), false, 0,
quiche::QuicheStringPiece("\1\0\0\0\0\0\0\0HT"));
QuicStreamFrame frame2(GetNthClientInitiatedBidirectionalId(1), false, 0,
- quiche::QuicheStringPiece("\2\0\0\0\0\0\0\0HT"));
+ quiche::QuicheStringPiece("\3\0\0\0\0\0\0\0HT"));
session_->OnStreamFrame(frame1);
session_->OnStreamFrame(frame2);
EXPECT_EQ(2u, QuicSessionPeer::GetNumOpenDynamicStreams(session_.get()));
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index 8a07598..63d96f1 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -209,6 +209,7 @@
RETURN_STRING_LITERAL(QUIC_HTTP_GOAWAY_INVALID_STREAM_ID);
RETURN_STRING_LITERAL(QUIC_HTTP_GOAWAY_ID_LARGER_THAN_PREVIOUS);
RETURN_STRING_LITERAL(QUIC_HTTP_RECEIVE_SPDY_SETTING);
+ RETURN_STRING_LITERAL(QUIC_HTTP_RECEIVE_SPDY_FRAME);
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);
@@ -590,6 +591,9 @@
return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::ID_ERROR)};
case QUIC_HTTP_RECEIVE_SPDY_SETTING:
return {false, static_cast<uint64_t>(QuicHttp3ErrorCode::SETTINGS_ERROR)};
+ case QUIC_HTTP_RECEIVE_SPDY_FRAME:
+ return {false,
+ static_cast<uint64_t>(QuicHttp3ErrorCode::FRAME_UNEXPECTED)};
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 180dff7..85fb7f2 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -445,6 +445,8 @@
// HTTP/3 session received SETTINGS frame which contains HTTP/2 specific
// settings.
QUIC_HTTP_RECEIVE_SPDY_SETTING = 169,
+ // HTTP/3 session received an HTTP/2 only frame.
+ QUIC_HTTP_RECEIVE_SPDY_FRAME = 171,
// HPACK header block decoding errors.
// Index varint beyond implementation limit.
@@ -499,7 +501,7 @@
QUIC_MISSING_WRITE_KEYS = 170,
// No error. Used as bound while iterating.
- QUIC_LAST_ERROR = 171,
+ QUIC_LAST_ERROR = 172,
};
// 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/tools/quic_simple_server_session_test.cc b/quic/tools/quic_simple_server_session_test.cc
index 780ff9d..c591f81 100644
--- a/quic/tools/quic_simple_server_session_test.cc
+++ b/quic/tools/quic_simple_server_session_test.cc
@@ -395,7 +395,7 @@
QuicStreamFrame frame1(GetNthClientInitiatedBidirectionalId(0), false, 0,
quiche::QuicheStringPiece("\1\0\0\0\0\0\0\0HT"));
QuicStreamFrame frame2(GetNthClientInitiatedBidirectionalId(1), false, 0,
- quiche::QuicheStringPiece("\2\0\0\0\0\0\0\0HT"));
+ quiche::QuicheStringPiece("\3\0\0\0\0\0\0\0HT"));
session_->OnStreamFrame(frame1);
session_->OnStreamFrame(frame2);
EXPECT_EQ(2u, QuicSessionPeer::GetNumOpenDynamicStreams(session_.get()));
