diff --git a/test/common/http/http2/BUILD b/test/common/http/http2/BUILD index 5fd65d1fff706..0c245388afb92 100644 --- a/test/common/http/http2/BUILD +++ b/test/common/http/http2/BUILD @@ -2,6 +2,7 @@ licenses(["notice"]) # Apache 2 load( "//bazel:envoy_build_system.bzl", + "envoy_cc_fuzz_test", "envoy_cc_test", "envoy_cc_test_library", "envoy_package", @@ -55,6 +56,35 @@ envoy_cc_test( ], ) +envoy_cc_test_library( + name = "frame_replay_lib", + srcs = ["frame_replay.cc"], + hdrs = ["frame_replay.h"], + deps = [ + "//source/common/common:hex_lib", + "//source/common/common:macros", + "//source/common/http/http2:codec_lib", + "//test/common/http:common_lib", + "//test/common/http/http2:codec_impl_test_util", + "//test/mocks/http:http_mocks", + "//test/mocks/network:network_mocks", + "//test/test_common:environment_lib", + "//test/test_common:utility_lib", + ], +) + +envoy_cc_test( + name = "frame_replay_test", + srcs = ["frame_replay_test.cc"], + data = [ + "request_header_corpus/simple_example_huffman", + "request_header_corpus/simple_example_plain", + "response_header_corpus/simple_example_huffman", + "response_header_corpus/simple_example_plain", + ], + deps = [":frame_replay_lib"], +) + envoy_cc_test( name = "metadata_encoder_decoder_test", srcs = ["metadata_encoder_decoder_test.cc"], @@ -69,3 +99,17 @@ envoy_cc_test( "//test/test_common:utility_lib", ], ) + +envoy_cc_fuzz_test( + name = "response_header_fuzz_test", + srcs = ["response_header_fuzz_test.cc"], + corpus = "response_header_corpus", + deps = [":frame_replay_lib"], +) + +envoy_cc_fuzz_test( + name = "request_header_fuzz_test", + srcs = ["request_header_fuzz_test.cc"], + corpus = "request_header_corpus", + deps = [":frame_replay_lib"], +) diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index a220413d34969..576516b1662a6 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -34,12 +34,6 @@ namespace Http2 { using Http2SettingsTuple = ::testing::tuple; using Http2SettingsTestParam = ::testing::tuple; -constexpr Http2SettingsTuple - DefaultHttp2SettingsTuple(Http2Settings::DEFAULT_HPACK_TABLE_SIZE, - Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS, - Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS, - Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE); - class Http2CodecImplTestFixture { public: struct ConnectionWrapper { @@ -89,9 +83,6 @@ class Http2CodecImplTestFixture { if (corrupt_metadata_frame_) { corruptMetadataFramePayload(data); } - if (corrupt_at_offset_ >= 0) { - corruptAtOffset(data, corrupt_at_offset_, corrupt_with_char_); - } server_wrapper_.dispatch(data, *server_); })); ON_CALL(server_connection_, write(_, _)) @@ -148,9 +139,6 @@ class Http2CodecImplTestFixture { MockStreamCallbacks server_stream_callbacks_; // Corrupt a metadata frame payload. bool corrupt_metadata_frame_ = false; - // Corrupt frame at a given offset (if positive). - ssize_t corrupt_at_offset_{-1}; - char corrupt_with_char_{'\0'}; uint32_t max_request_headers_kb_ = Http::DEFAULT_MAX_REQUEST_HEADERS_KB; }; @@ -1041,50 +1029,6 @@ TEST_P(Http2CodecImplTest, TestCodecHeaderCompression) { } } -// Validate that nghttp2 rejects NUL/CR/LF as per -// https://httpwg.org/specs/rfc7540.html#rfc.section.10.3. -// TEST_P(Http2CodecImplTest, InvalidHeaderChars) { -// TODO(htuch): Write me. Http2CodecImplMutationTest basically covers this, -// but we could be a bit more specific and add a captured H2 HEADERS frame -// here and inject it with mutation of just the header value, ensuring we get -// the expected codec exception. -// } - -class Http2CodecImplMutationTest : public ::testing::TestWithParam<::testing::tuple>, - protected Http2CodecImplTestFixture { -public: - Http2CodecImplMutationTest() - : Http2CodecImplTestFixture(DefaultHttp2SettingsTuple, DefaultHttp2SettingsTuple) {} - - void initialize() override { - corrupt_with_char_ = ::testing::get<0>(GetParam()); - corrupt_at_offset_ = ::testing::get<1>(GetParam()); - Http2CodecImplTestFixture::initialize(); - } -}; - -INSTANTIATE_TEST_SUITE_P(Http2CodecImplMutationTest, Http2CodecImplMutationTest, - ::testing::Combine(::testing::ValuesIn({'\0', '\r', '\n'}), - ::testing::Range(0, 128))); - -// Mutate an arbitrary offset in the HEADERS frame with NUL/CR/LF. This should -// either throw an exception or continue, but we shouldn't crash due to -// validHeaderString() ASSERTs. -TEST_P(Http2CodecImplMutationTest, HandleInvalidChars) { - initialize(); - - TestHeaderMapImpl request_headers; - request_headers.addCopy("foo", "barbaz"); - HttpTestUtility::addDefaultHeaders(request_headers); - EXPECT_CALL(request_decoder_, decodeHeaders_(_, _)).Times(AnyNumber()); - EXPECT_CALL(client_callbacks_, onGoAway()).Times(AnyNumber()); - try { - request_encoder_->encodeHeaders(request_headers, true); - } catch (const CodecProtocolException& e) { - ENVOY_LOG_MISC(trace, "CodecProtocolException: {}", e.what()); - } -} - } // namespace Http2 } // namespace Http } // namespace Envoy diff --git a/test/common/http/http2/frame_replay.cc b/test/common/http/http2/frame_replay.cc new file mode 100644 index 0000000000000..7d2fe1cc7ae61 --- /dev/null +++ b/test/common/http/http2/frame_replay.cc @@ -0,0 +1,118 @@ +#include "test/common/http/http2/frame_replay.h" + +#include "common/common/hex.h" +#include "common/common/macros.h" + +#include "test/common/http/common.h" +#include "test/test_common/environment.h" + +namespace Envoy { +namespace Http { +namespace Http2 { + +FileFrame::FileFrame(absl::string_view path) : api_(Api::createApiForTest()) { + const std::string contents = api_->fileSystem().fileReadToEnd( + TestEnvironment::runfilesPath("test/common/http/http2/" + std::string(path))); + frame_.resize(contents.size()); + contents.copy(reinterpret_cast(frame_.data()), frame_.size()); +} + +std::unique_ptr FileFrame::istream() { + const std::string frame_string{reinterpret_cast(frame_.data()), frame_.size()}; + return std::make_unique(frame_string); +} + +const Frame& WellKnownFrames::clientConnectionPrefaceFrame() { + CONSTRUCT_ON_FIRST_USE(std::vector, + {0x50, 0x52, 0x49, 0x20, 0x2a, 0x20, 0x48, 0x54, 0x54, 0x50, 0x2f, 0x32, + 0x2e, 0x30, 0x0d, 0x0a, 0x0d, 0x0a, 0x53, 0x4d, 0x0d, 0x0a, 0x0d, 0x0a}); +} + +const Frame& WellKnownFrames::defaultSettingsFrame() { + CONSTRUCT_ON_FIRST_USE(std::vector, + {0x00, 0x00, 0x0c, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, + 0x7f, 0xff, 0xff, 0xff, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00}); +} + +const Frame& WellKnownFrames::initialWindowUpdateFrame() { + CONSTRUCT_ON_FIRST_USE(std::vector, {0x00, 0x00, 0x04, 0x08, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x0f, 0xff, 0x00, 0x01}); +} + +void FrameUtils::fixupHeaders(Frame& frame) { + constexpr size_t frame_header_len = 9; // from RFC 7540 + while (frame.size() < frame_header_len) { + frame.emplace_back(0x00); + } + size_t headers_len = frame.size() - frame_header_len; + frame[2] = headers_len & 0xff; + headers_len >>= 8; + frame[1] = headers_len & 0xff; + headers_len >>= 8; + frame[0] = headers_len & 0xff; + // HEADERS frame with END_STREAM | END_HEADERS for stream 1. + size_t offset = 3; + for (const uint8_t b : {0x01, 0x05, 0x00, 0x00, 0x00, 0x01}) { + frame[offset++] = b; + } +} + +CodecFrameInjector::CodecFrameInjector(const std::string& injector_name) + : injector_name_(injector_name) { + settings_.hpack_table_size_ = Http2Settings::DEFAULT_HPACK_TABLE_SIZE; + settings_.max_concurrent_streams_ = Http2Settings::DEFAULT_MAX_CONCURRENT_STREAMS; + settings_.initial_stream_window_size_ = Http2Settings::DEFAULT_INITIAL_STREAM_WINDOW_SIZE; + settings_.initial_connection_window_size_ = Http2Settings::DEFAULT_INITIAL_CONNECTION_WINDOW_SIZE; + settings_.allow_metadata_ = false; +} + +ClientCodecFrameInjector::ClientCodecFrameInjector() : CodecFrameInjector("server") { + auto client = std::make_unique(client_connection_, client_callbacks_, + stats_store_, settings_, + Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + request_encoder_ = &client->newStream(response_decoder_); + connection_ = std::move(client); + ON_CALL(client_connection_, write(_, _)) + .WillByDefault(Invoke([&](Buffer::Instance& data, bool) -> void { + ENVOY_LOG_MISC( + trace, "client write: {}", + Hex::encode(static_cast(data.linearize(data.length())), data.length())); + data.drain(data.length()); + })); + request_encoder_->getStream().addCallbacks(client_stream_callbacks_); + // Setup a single stream to inject frames as a reply to. + TestHeaderMapImpl request_headers; + HttpTestUtility::addDefaultHeaders(request_headers); + request_encoder_->encodeHeaders(request_headers, true); +} + +ServerCodecFrameInjector::ServerCodecFrameInjector() : CodecFrameInjector("client") { + connection_ = std::make_unique(server_connection_, server_callbacks_, + stats_store_, settings_, + Http::DEFAULT_MAX_REQUEST_HEADERS_KB); + EXPECT_CALL(server_callbacks_, newStream(_, _)) + .WillRepeatedly(Invoke([&](StreamEncoder& encoder, bool) -> StreamDecoder& { + encoder.getStream().addCallbacks(server_stream_callbacks_); + return request_decoder_; + })); + ON_CALL(server_connection_, write(_, _)) + .WillByDefault(Invoke([&](Buffer::Instance& data, bool) -> void { + ENVOY_LOG_MISC( + trace, "server write: {}", + Hex::encode(static_cast(data.linearize(data.length())), data.length())); + data.drain(data.length()); + })); +} + +void CodecFrameInjector::write(const Frame& frame) { + Buffer::OwnedImpl buffer; + buffer.add(frame.data(), frame.size()); + ENVOY_LOG_MISC(trace, "{} write: {}", injector_name_, Hex::encode(frame.data(), frame.size())); + while (buffer.length() > 0) { + connection_->dispatch(buffer); + } +} + +} // namespace Http2 +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/http2/frame_replay.h b/test/common/http/http2/frame_replay.h new file mode 100644 index 0000000000000..fcd750225fcf9 --- /dev/null +++ b/test/common/http/http2/frame_replay.h @@ -0,0 +1,91 @@ +#include +#include +#include + +#include "common/stats/isolated_store_impl.h" + +#include "test/common/http/http2/codec_impl_test_util.h" +#include "test/mocks/http/mocks.h" +#include "test/mocks/network/mocks.h" +#include "test/test_common/utility.h" + +#include "absl/strings/string_view.h" +#include "gmock/gmock.h" + +namespace Envoy { +namespace Http { +namespace Http2 { + +// A byte vector representation of an HTTP/2 frame. +typedef std::vector Frame; + +// An HTTP/2 frame derived from a file location. +class FileFrame { +public: + FileFrame(absl::string_view path); + + Frame& frame() { return frame_; } + std::unique_ptr istream(); + + Frame frame_; + Api::ApiPtr api_; +}; + +// Some standards HTTP/2 frames for setting up a connection. The contents for these and the seed +// corpus were captured via logging the hex bytes in codec_impl_test's write() connection mocks in +// setupDefaultConnectionMocks(). +class WellKnownFrames { +public: + static const Frame& clientConnectionPrefaceFrame(); + static const Frame& defaultSettingsFrame(); + static const Frame& initialWindowUpdateFrame(); +}; + +class FrameUtils { +public: + // Modify a given frame so that it has the HTTP/2 frame header for a valid + // HEADERS frame. + static void fixupHeaders(Frame& frame); +}; + +class CodecFrameInjector { +public: + CodecFrameInjector(const std::string& injector_name); + + void write(const Frame& frame); + + Http2Settings settings_; + std::unique_ptr connection_; + Stats::IsolatedStoreImpl stats_store_; + const std::string injector_name_; +}; + +// Wrapper for HTTP/2 client codec supporting injection of frames and expecting on +// the behaviors of callbacks and the request decoder. +class ClientCodecFrameInjector : public CodecFrameInjector { +public: + ClientCodecFrameInjector(); + + ::testing::NiceMock client_connection_; + MockConnectionCallbacks client_callbacks_; + MockStreamDecoder response_decoder_; + StreamEncoder* request_encoder_; + MockStreamCallbacks client_stream_callbacks_; +}; + +// Wrapper for HTTP/2 server codec supporting injection of frames and expecting on +// the behaviors of callbacks and the request decoder. +class ServerCodecFrameInjector : public CodecFrameInjector { +public: + ServerCodecFrameInjector(); + + ::testing::NiceMock server_connection_; + MockServerConnectionCallbacks server_callbacks_; + std::unique_ptr server_; + MockStreamDecoder request_decoder_; + MockStreamCallbacks server_stream_callbacks_; +}; + +} // namespace Http2 +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/http2/frame_replay_test.cc b/test/common/http/http2/frame_replay_test.cc new file mode 100644 index 0000000000000..de886feaacb97 --- /dev/null +++ b/test/common/http/http2/frame_replay_test.cc @@ -0,0 +1,288 @@ +#include "common/http/exception.h" + +#include "test/common/http/common.h" +#include "test/common/http/http2/frame_replay.h" + +#include "gtest/gtest.h" + +#define EXPECT_NEXT_BYTES(istream, bs...) \ + do { \ + std::vector expected_bytes{bs}; \ + std::vector actual_bytes(expected_bytes.size()); \ + istream->read(reinterpret_cast(actual_bytes.data()), expected_bytes.size()); \ + EXPECT_EQ(actual_bytes, expected_bytes); \ + } while (0) + +using testing::AnyNumber; +using testing::InvokeWithoutArgs; + +namespace Envoy { +namespace Http { +namespace Http2 { +namespace { + +// For organizational purposes only. +class RequestFrameCommentTest : public ::testing::Test {}; +class ResponseFrameCommentTest : public ::testing::Test {}; + +// Validate that a simple Huffman encoded request HEADERS frame can be decoded. +TEST_F(RequestFrameCommentTest, SimpleExampleHuffman) { + FileFrame header{"request_header_corpus/simple_example_huffman"}; + + // Validate HEADERS content matches intent. + auto header_bytes = header.istream(); + // Payload size is 18 bytes. + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x00, 0x12); + // HEADERS frame with END_STREAM | END_HEADERS for stream 1. + EXPECT_NEXT_BYTES(header_bytes, 0x01, 0x05, 0x00, 0x00, 0x00, 0x01); + // Static table :scheme: http, :method: GET + EXPECT_NEXT_BYTES(header_bytes, 0x86, 0x82); + // Static table :authority, Huffman 'host' + EXPECT_NEXT_BYTES(header_bytes, 0x41, 0x83, 0x9c, 0xe8, 0x4f); + // Static table :path: / + EXPECT_NEXT_BYTES(header_bytes, 0x84); + // Huffman foo: barbaz + EXPECT_NEXT_BYTES(header_bytes, 0x40, 0x82, 0x94, 0xe7, 0x85, 0x8c, 0x76, 0x46, 0x3f, 0x7f); + + // Validate HEADERS decode. + ServerCodecFrameInjector codec; + codec.write(WellKnownFrames::clientConnectionPrefaceFrame()); + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + TestHeaderMapImpl expected_headers; + HttpTestUtility::addDefaultHeaders(expected_headers); + expected_headers.addCopy("foo", "barbaz"); + EXPECT_CALL(codec.request_decoder_, decodeHeaders_(HeaderMapEqual(&expected_headers), true)); + codec.write(header.frame()); +} + +// Validate that a simple Huffman encoded response HEADERS frame can be decoded. +TEST_F(ResponseFrameCommentTest, SimpleExampleHuffman) { + FileFrame header{"response_header_corpus/simple_example_huffman"}; + + // Validate HEADERS content matches intent. + auto header_bytes = header.istream(); + + // Payload size is 15 bytes. + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x00, 0x0f); + // HEADERS frame with END_STREAM | END_HEADERS for stream 1. + EXPECT_NEXT_BYTES(header_bytes, 0x01, 0x05, 0x00, 0x00, 0x00, 0x01); + // Static table :status: 200 + EXPECT_NEXT_BYTES(header_bytes, 0x88); + // Huffman compression: test + EXPECT_NEXT_BYTES(header_bytes, 0x40, 0x88, 0x21, 0xe9, 0xae, 0xc2, 0xa1, 0x06, 0x3d, 0x5f, 0x83, + 0x49, 0x50, 0x9f); + + // Validate HEADERS decode. + ClientCodecFrameInjector codec; + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + TestHeaderMapImpl expected_headers; + expected_headers.addCopy(":status", "200"); + expected_headers.addCopy("compression", "test"); + EXPECT_CALL(codec.response_decoder_, decodeHeaders_(HeaderMapEqual(&expected_headers), true)); + codec.write(header.frame()); +} + +// Validate that a simple non-Huffman request HEADERS frame with no static table user either can be +// decoded. +TEST_F(RequestFrameCommentTest, SimpleExamplePlain) { + FileFrame header{"request_header_corpus/simple_example_plain"}; + + // Validate HEADERS content matches intent. + auto header_bytes = header.istream(); + // Payload size is 65 bytes. + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x00, 0x41); + // HEADERS frame with END_STREAM | END_HEADERS for stream 1. + EXPECT_NEXT_BYTES(header_bytes, 0x01, 0x05, 0x00, 0x00, 0x00, 0x01); + // Literal unindexed :scheme: http + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x07, 0x3A, 0x73, 0x63, 0x68, 0x65, 0x6D, 0x65); + EXPECT_NEXT_BYTES(header_bytes, 0x04, 0x68, 0x74, 0x74, 0x70); + // Literal unindexed :method: GET + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x07, 0x3A, 0x6D, 0x65, 0x74, 0x68, 0x6F, 0x64); + EXPECT_NEXT_BYTES(header_bytes, 0x03, 0x47, 0x45, 0x54); + // Literal unindexed :authority: host + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x0A, 0x3A, 0x61, 0x75, 0x74, 0x68, 0x6F, 0x72, 0x69, 0x74, + 0x79); + EXPECT_NEXT_BYTES(header_bytes, 0x04, 0x68, 0x6F, 0x73, 0x74); + // Literal unindexed :path: / + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x05, 0x3A, 0x70, 0x61, 0x74, 0x68); + EXPECT_NEXT_BYTES(header_bytes, 0x01, 0x2F); + // Literal unindexed foo: barbaz + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x03, 0x66, 0x6F, 0x6F); + EXPECT_NEXT_BYTES(header_bytes, 0x06, 0x62, 0x61, 0x72, 0x62, 0x61, 0x7A); + + // Validate HEADERS decode. + ServerCodecFrameInjector codec; + codec.write(WellKnownFrames::clientConnectionPrefaceFrame()); + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + TestHeaderMapImpl expected_headers; + HttpTestUtility::addDefaultHeaders(expected_headers); + expected_headers.addCopy("foo", "barbaz"); + EXPECT_CALL(codec.request_decoder_, decodeHeaders_(HeaderMapEqual(&expected_headers), true)); + codec.write(header.frame()); +} + +// Validate that a simple non-Huffman response HEADERS frame with no static table user either can be +// decoded. +TEST_F(ResponseFrameCommentTest, SimpleExamplePlain) { + FileFrame header{"response_header_corpus/simple_example_plain"}; + + // Validate HEADERS content matches intent. + auto header_bytes = header.istream(); + + // Payload size is 15 bytes. + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x00, 0x1F); + // HEADERS frame with END_STREAM | END_HEADERS for stream 1. + EXPECT_NEXT_BYTES(header_bytes, 0x01, 0x05, 0x00, 0x00, 0x00, 0x01); + // Literal unindexed :status: 200 + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x07, 0x3A, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, 0x03, 0x32, + 0x30, 0x30); + // Literal unindexed compression: test + EXPECT_NEXT_BYTES(header_bytes, 0x00, 0x0B, 0x63, 0x6F, 0x6D, 0x70, 0x72, 0x65, 0x73, 0x73, 0x69, + 0x6F, 0x6E, 0x04, 0x74, 0x65, 0x73, 0x74); + + // Validate HEADERS decode. + ClientCodecFrameInjector codec; + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + TestHeaderMapImpl expected_headers; + expected_headers.addCopy(":status", "200"); + expected_headers.addCopy("compression", "test"); + EXPECT_CALL(codec.response_decoder_, decodeHeaders_(HeaderMapEqual(&expected_headers), true)); + codec.write(header.frame()); +} + +// Validate that corrupting any single byte with {NUL, CR, LF} in a HEADERS frame doesn't crash or +// trigger ASSERTs. This is a litmus test for the HTTP/2 codec (nghttp2) to demonstrate that it +// doesn't suffer from the issue reported for http-parser in CVE-2019-9900. See also +// https://httpwg.org/specs/rfc7540.html#rfc.section.10.3. We use a non-compressed frame with no +// Huffman encoding to simplify. +TEST_F(RequestFrameCommentTest, SingleByteNulCrLfInHeaderFrame) { + FileFrame header{"request_header_corpus/simple_example_plain"}; + + for (size_t offset = 0; offset < header.frame().size(); ++offset) { + for (const char c : {'\0', '\n', '\r'}) { + // Corrupt a single byte in the HEADERS. + const char original = header.frame()[offset]; + header.frame()[offset] = c; + // Play the frames back. + ServerCodecFrameInjector codec; + codec.write(WellKnownFrames::clientConnectionPrefaceFrame()); + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + try { + EXPECT_CALL(codec.request_decoder_, decodeHeaders_(_, _)).Times(AnyNumber()); + EXPECT_CALL(codec.server_stream_callbacks_, onResetStream(_, _)).Times(AnyNumber()); + codec.write(header.frame()); + } catch (const CodecProtocolException& e) { + ENVOY_LOG_MISC(trace, "CodecProtocolException: {}", e.what()); + } + header.frame()[offset] = original; + } + } +} + +// Validate that corrupting any single byte with {NUL, CR, LF} in a HEADERS frame doesn't crash or +// trigger ASSERTs. This is a litmus test for the HTTP/2 codec (nghttp2) to demonsrate that it +// doesn't suffer from the issue reported for http-parser in CVE-2019-9900. See also +// https://httpwg.org/specs/rfc7540.html#rfc.section.10.3. We use a non-compressed frame with no +// Huffman encoding to simplify. +TEST_F(ResponseFrameCommentTest, SingleByteNulCrLfInHeaderFrame) { + FileFrame header{"response_header_corpus/simple_example_plain"}; + + for (size_t offset = 0; offset < header.frame().size(); ++offset) { + for (const char c : {'\0', '\n', '\r'}) { + // Corrupt a single byte in the HEADERS. + const char original = header.frame()[offset]; + header.frame()[offset] = c; + // Play the frames back. + ClientCodecFrameInjector codec; + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + try { + EXPECT_CALL(codec.response_decoder_, decodeHeaders_(_, _)).Times(AnyNumber()); + EXPECT_CALL(codec.client_stream_callbacks_, onResetStream(_, _)).Times(AnyNumber()); + codec.write(header.frame()); + } catch (const CodecProtocolException& e) { + ENVOY_LOG_MISC(trace, "CodecProtocolException: {}", e.what()); + } + header.frame()[offset] = original; + } + } +} + +// Validate that corrupting any single byte with {NUL, CR, LF} in a HEADERS field name or value +// yields a CodecProtocolException or stream reset. This is a litmus test for the HTTP/2 codec +// (nghttp2) to demonsrate that it doesn't suffer from the issue reported for http-parser in +// CVE-2019-9900. See also https://httpwg.org/specs/rfc7540.html#rfc.section.10.3. We use a +// non-compressed frame with no Huffman encoding to simplify. +TEST_F(RequestFrameCommentTest, SingleByteNulCrLfInHeaderField) { + FileFrame header{"request_header_corpus/simple_example_plain"}; + + for (size_t offset = header.frame().size() - 11 /* foo: offset */; offset < header.frame().size(); + ++offset) { + for (const char c : {'\0', '\n', '\r'}) { + // Corrupt a single byte in the HEADERS. + const char original = header.frame()[offset]; + header.frame()[offset] = c; + // Play the frames back. + ServerCodecFrameInjector codec; + codec.write(WellKnownFrames::clientConnectionPrefaceFrame()); + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + bool stream_reset = false; + EXPECT_CALL(codec.request_decoder_, decodeHeaders_(_, _)).Times(0); + EXPECT_CALL(codec.server_stream_callbacks_, onResetStream(_, _)) + .WillRepeatedly(InvokeWithoutArgs([&stream_reset] { stream_reset = true; })); + bool codec_exception = false; + try { + codec.write(header.frame()); + } catch (const CodecProtocolException& e) { + codec_exception = true; + } + EXPECT_TRUE(stream_reset || codec_exception); + header.frame()[offset] = original; + } + } +} + +// Validate that corrupting any single byte with {NUL, CR, LF} in a HEADERS field name or value +// yields a CodecProtocolException or stream reset. This is a litmus test for the HTTP/2 codec +// (nghttp2) to demonsrate that it doesn't suffer from the issue reported for http-parser in +// CVE-2019-9900. See also https://httpwg.org/specs/rfc7540.html#rfc.section.10.3. We use a +// non-compressed frame with no Huffman encoding to simplify. +TEST_F(ResponseFrameCommentTest, SingleByteNulCrLfInHeaderField) { + FileFrame header{"response_header_corpus/simple_example_plain"}; + + for (size_t offset = header.frame().size() - 17 /* test: offset */; + offset < header.frame().size(); ++offset) { + for (const char c : {'\0', '\n', '\r'}) { + // Corrupt a single byte in the HEADERS. + const char original = header.frame()[offset]; + header.frame()[offset] = c; + // Play the frames back. + ClientCodecFrameInjector codec; + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + bool stream_reset = false; + EXPECT_CALL(codec.response_decoder_, decodeHeaders_(_, _)).Times(0); + EXPECT_CALL(codec.client_stream_callbacks_, onResetStream(_, _)) + .WillRepeatedly(InvokeWithoutArgs([&stream_reset] { stream_reset = true; })); + bool codec_exception = false; + try { + codec.write(header.frame()); + } catch (const CodecProtocolException& e) { + codec_exception = true; + } + EXPECT_TRUE(stream_reset || codec_exception); + header.frame()[offset] = original; + } + } +} + +} // namespace +} // namespace Http2 +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/http2/request_header_corpus/simple_example_huffman b/test/common/http/http2/request_header_corpus/simple_example_huffman new file mode 100644 index 0000000000000..90496d83da7be Binary files /dev/null and b/test/common/http/http2/request_header_corpus/simple_example_huffman differ diff --git a/test/common/http/http2/request_header_corpus/simple_example_plain b/test/common/http/http2/request_header_corpus/simple_example_plain new file mode 100644 index 0000000000000..0645c04067f60 Binary files /dev/null and b/test/common/http/http2/request_header_corpus/simple_example_plain differ diff --git a/test/common/http/http2/request_header_fuzz_test.cc b/test/common/http/http2/request_header_fuzz_test.cc new file mode 100644 index 0000000000000..772db68816533 --- /dev/null +++ b/test/common/http/http2/request_header_fuzz_test.cc @@ -0,0 +1,44 @@ +// Fuzzer for H2 response HEADERS frames. Unlike codec_impl_fuzz_test, this is +// stateless and focuses only on HEADERS. This technique also plays well with +// uncompressed HEADERS fuzzing. + +#include "common/http/exception.h" + +#include "test/common/http/http2/frame_replay.h" +#include "test/fuzz/fuzz_runner.h" + +using testing::AnyNumber; + +namespace Envoy { +namespace Http { +namespace Http2 { +namespace { + +void Replay(const Frame& frame) { + ServerCodecFrameInjector codec; + codec.write(WellKnownFrames::clientConnectionPrefaceFrame()); + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + EXPECT_CALL(codec.server_callbacks_, onGoAway()).Times(AnyNumber()); + EXPECT_CALL(codec.request_decoder_, decodeHeaders_(_, _)).Times(AnyNumber()); + EXPECT_CALL(codec.server_stream_callbacks_, onResetStream(_, _)).Times(AnyNumber()); + try { + codec.write(frame); + } catch (const CodecProtocolException& e) { + } +} + +DEFINE_FUZZER(const uint8_t* buf, size_t len) { + Frame frame; + frame.assign(buf, buf + len); + // Replay with the fuzzer bytes. + Replay(frame); + // Try again, but fixup the HEADERS frame to make it a valid HEADERS. + FrameUtils::fixupHeaders(frame); + Replay(frame); +} + +} // namespace +} // namespace Http2 +} // namespace Http +} // namespace Envoy diff --git a/test/common/http/http2/response_header_corpus/simple_example_huffman b/test/common/http/http2/response_header_corpus/simple_example_huffman new file mode 100644 index 0000000000000..b2e452b00b800 Binary files /dev/null and b/test/common/http/http2/response_header_corpus/simple_example_huffman differ diff --git a/test/common/http/http2/response_header_corpus/simple_example_plain b/test/common/http/http2/response_header_corpus/simple_example_plain new file mode 100644 index 0000000000000..2cbf93587aeee Binary files /dev/null and b/test/common/http/http2/response_header_corpus/simple_example_plain differ diff --git a/test/common/http/http2/response_header_fuzz_test.cc b/test/common/http/http2/response_header_fuzz_test.cc new file mode 100644 index 0000000000000..31f6e14b98e06 --- /dev/null +++ b/test/common/http/http2/response_header_fuzz_test.cc @@ -0,0 +1,43 @@ +// Fuzzer for H2 response HEADERS frames. Unlike codec_impl_fuzz_test, this is +// stateless and focuses only on HEADERS. This technique also plays well with +// uncompressed HEADERS fuzzing. + +#include "common/http/exception.h" + +#include "test/common/http/http2/frame_replay.h" +#include "test/fuzz/fuzz_runner.h" + +using testing::AnyNumber; + +namespace Envoy { +namespace Http { +namespace Http2 { +namespace { + +void Replay(const Frame& frame) { + ClientCodecFrameInjector codec; + codec.write(WellKnownFrames::defaultSettingsFrame()); + codec.write(WellKnownFrames::initialWindowUpdateFrame()); + EXPECT_CALL(codec.client_callbacks_, onGoAway()).Times(AnyNumber()); + EXPECT_CALL(codec.response_decoder_, decodeHeaders_(_, _)).Times(AnyNumber()); + EXPECT_CALL(codec.client_stream_callbacks_, onResetStream(_, _)).Times(AnyNumber()); + try { + codec.write(frame); + } catch (const CodecProtocolException& e) { + } +} + +DEFINE_FUZZER(const uint8_t* buf, size_t len) { + Frame frame; + frame.assign(buf, buf + len); + // Replay with the fuzzer bytes. + Replay(frame); + // Try again, but fixup the HEADERS frame to make it a valid HEADERS. + FrameUtils::fixupHeaders(frame); + Replay(frame); +} + +} // namespace +} // namespace Http2 +} // namespace Http +} // namespace Envoy diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 06b17541a93cd..289383e078e12 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -39,6 +39,7 @@ CSV CTX CTXs CVC +CVE CX DCHECK DER @@ -428,6 +429,7 @@ favicon fd fds fileno +fixup fld fmt fmtlib @@ -726,6 +728,7 @@ unejection unescape unescaped unescaping +unindexed uninsantiated uninstantiated unix