diff --git a/bazel/external/http_parser/http_parser.c b/bazel/external/http_parser/http_parser.c index 07c93dce1a697..5d01db6d52aa1 100644 --- a/bazel/external/http_parser/http_parser.c +++ b/bazel/external/http_parser/http_parser.c @@ -1079,6 +1079,10 @@ size_t http_parser_execute (http_parser *parser, break; case CR: case LF: + if (parser->method == HTTP_CONNECT) { + SET_ERRNO(HPE_INVALID_VERSION); + goto error; + } parser->http_major = 0; parser->http_minor = 9; UPDATE_STATE((ch == CR) ? diff --git a/changelogs/current/bug_fixes/http__versionless-connect-response.rst b/changelogs/current/bug_fixes/http__versionless-connect-response.rst new file mode 100644 index 0000000000000..d25a05df969a0 --- /dev/null +++ b/changelogs/current/bug_fixes/http__versionless-connect-response.rst @@ -0,0 +1,3 @@ +Fixed a bug where malformed CONNECT request lines without an authority could cause the legacy +HTTP/1 parser to encode a ``400 Bad Request`` response using HTTP/1.0. Envoy now rejects these +requests without downgrading the response protocol. diff --git a/test/common/http/http1/BUILD b/test/common/http/http1/BUILD index 21e417fda1913..e98b3573cc4a9 100644 --- a/test/common/http/http1/BUILD +++ b/test/common/http/http1/BUILD @@ -32,6 +32,7 @@ envoy_cc_test( "//source/common/http:exception_lib", "//source/common/http:header_map_lib", "//source/common/http/http1:codec_lib", + "//source/common/http/http1:legacy_parser_lib", "//source/extensions/http/header_validators/envoy_default:http1_header_validator", "//test/common/memory:memory_test_utility_lib", "//test/common/stats:stat_test_utility_lib", diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index 17c3a4c8160b3..28150129e5de7 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -14,6 +14,7 @@ #include "source/common/http/exception.h" #include "source/common/http/header_map_impl.h" #include "source/common/http/http1/codec_impl.h" +#include "source/common/http/http1/legacy_parser_impl.h" #include "source/common/runtime/runtime_impl.h" #include "source/extensions/http/header_validators/envoy_default/http1_header_validator.h" @@ -169,6 +170,15 @@ class Http1CodecTestBase : public ::testing::Test { NiceMock overload_manager_; }; +class LegacyParserServerConnectionImpl : public Http1::ServerConnectionImpl { +public: + using Http1::ServerConnectionImpl::ServerConnectionImpl; + + void useLegacyParser() { + parser_ = std::make_unique(Http1::MessageType::Request, this); + } +}; + class Http1ServerConnectionImplTest : public Http1CodecTestBase { public: void initialize() { @@ -947,6 +957,43 @@ TEST_F(Http1ServerConnectionImplTest, Http10) { EXPECT_EQ(Protocol::Http10, codec_->protocol()); } +TEST_F(Http1ServerConnectionImplTest, ConnectWithoutAuthorityUsesHttp11WithLegacyParser) { + codec_settings_.accept_http_10_ = true; + auto legacy_codec = std::make_unique( + connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_, + max_request_headers_count_, headers_with_underscores_action_, overload_manager_); + legacy_codec->useLegacyParser(); + codec_ = std::move(legacy_codec); + + MockRequestDecoder decoder; + setupRequestDecoderMock(decoder); + Http::ResponseEncoder* response_encoder = nullptr; + EXPECT_CALL(callbacks_, newStream(_, _)) + .WillOnce(Invoke([&](ResponseEncoder& encoder, bool) -> RequestDecoder& { + response_encoder = &encoder; + return decoder; + })); + EXPECT_CALL(decoder, + sendLocalReply(Http::Code::BadRequest, "Bad Request", _, _, "http1.codec_error")); + + Buffer::OwnedImpl buffer("CONNECT HTTP/1.1\r\n" + "Host: foo.bar.com:80\r\n" + "foo: bar\r\n" + "baz: fuz\r\n" + "\r\n"); + const auto status = codec_->dispatch(buffer); + + EXPECT_TRUE(isCodecProtocolError(status)); + EXPECT_EQ("http/1.1 protocol error: HPE_INVALID_VERSION", status.message()); + EXPECT_EQ(Protocol::Http11, codec_->protocol()); + EXPECT_EQ("http1.codec_error", response_encoder->getStream().responseDetails()); + + std::string output; + ON_CALL(connection_, write(_, _)).WillByDefault(AddBufferToString(&output)); + response_encoder->encodeHeaders(TestResponseHeaderMapImpl{{":status", "400"}}, true); + EXPECT_EQ("HTTP/1.1 400 Bad Request\r\ncontent-length: 0\r\n\r\n", output); +} + TEST_F(Http1ServerConnectionImplTest, Http10HostAdded) { codec_settings_.accept_http_10_ = true; codec_settings_.default_host_for_http_10_ = "example.com";