add initial support for http-parser permissive parsing mode; refs env…

…oyproxy#19750

Signed-off-by: Adam Meily <[email protected]>

envoy/http/codec.h

@@ -46,6 +46,14 @@ const char MaxResponseHeadersCountOverrideKey[] =
 
 class Stream;
 
+/**
+ * Header validation mode that codecs should use internally.
+ */
+enum class CodecHeaderValidationMode {
+  Enabled,
+  Disabled,
+};
+
 /**
  * Error codes used to convey the reason for a GOAWAY.
  */

source/common/http/codec_client.cc

@@ -183,7 +183,7 @@ NoConnectCodecClientProd::NoConnectCodecClientProd(CodecType type,
   case CodecType::HTTP1: {
     codec_ = std::make_unique<Http1::ClientConnectionImpl>(
         *connection_, host->cluster().http1CodecStats(), *this, host->cluster().http1Settings(),
-        host->cluster().maxResponseHeadersCount());
+        host->cluster().maxResponseHeadersCount(), CodecHeaderValidationMode::Enabled);
     break;
   }
   case CodecType::HTTP2: {

source/common/http/conn_manager_utility.cc

@@ -70,7 +70,8 @@ ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(
     Http1::CodecStats& stats = Http1::CodecStats::atomicGet(http1_codec_stats, scope);
     return std::make_unique<Http1::ServerConnectionImpl>(
         connection, stats, callbacks, http1_settings, max_request_headers_kb,
-        max_request_headers_count, headers_with_underscores_action);
+        max_request_headers_count, headers_with_underscores_action,
+        CodecHeaderValidationMode::Enabled);
   }
 }
 

source/common/http/http1/BUILD

@@ -113,6 +113,7 @@ envoy_cc_library(
     external_deps = ["http_parser"],
     deps = [
         ":parser_interface",
+        "//envoy/http:codec_interface",
         "//source/common/common:assert_lib",
     ],
 )

source/common/http/http1/codec_impl.cc

@@ -472,7 +472,8 @@ int ConnectionImpl::setAndCheckCallbackStatusOr(Envoy::StatusOr<ParserStatus>&&
 
 ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stats,
                                const Http1Settings& settings, MessageType type,
-                               uint32_t max_headers_kb, const uint32_t max_headers_count)
+                               uint32_t max_headers_kb, const uint32_t max_headers_count,
+                               const CodecHeaderValidationMode codec_header_validation_mode)
     : connection_(connection), stats_(stats), codec_settings_(settings),
       encode_only_header_key_formatter_(encodeOnlyFormatterFromSettings(settings)),
       processing_trailers_(false), handling_upgrade_(false), reset_stream_called_(false),
@@ -483,7 +484,7 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, CodecStats& stat
           []() -> void { /* TODO(adisuissa): Handle overflow watermark */ })),
       max_headers_kb_(max_headers_kb), max_headers_count_(max_headers_count) {
   output_buffer_->setWatermarks(connection.bufferLimit());
-  parser_ = std::make_unique<LegacyHttpParserImpl>(type, this);
+  parser_ = std::make_unique<LegacyHttpParserImpl>(type, this, codec_header_validation_mode);
 }
 
 Status ConnectionImpl::completeLastHeader() {
@@ -957,9 +958,10 @@ ServerConnectionImpl::ServerConnectionImpl(
     const Http1Settings& settings, uint32_t max_request_headers_kb,
     const uint32_t max_request_headers_count,
     envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
-        headers_with_underscores_action)
+        headers_with_underscores_action,
+    const CodecHeaderValidationMode codec_header_validation_mode)
     : ConnectionImpl(connection, stats, settings, MessageType::Request, max_request_headers_kb,
-                     max_request_headers_count),
+                     max_request_headers_count, codec_header_validation_mode),
       callbacks_(callbacks),
       response_buffer_releasor_([this](const Buffer::OwnedBufferFragmentImpl* fragment) {
         releaseOutboundResponse(fragment);
@@ -1241,11 +1243,12 @@ void ServerConnectionImpl::ActiveRequest::dumpState(std::ostream& os, int indent
   os << DUMP_MEMBER(response_encoder_.local_end_stream_);
 }
 
-ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, CodecStats& stats,
-                                           ConnectionCallbacks&, const Http1Settings& settings,
-                                           const uint32_t max_response_headers_count)
+ClientConnectionImpl::ClientConnectionImpl(
+    Network::Connection& connection, CodecStats& stats, ConnectionCallbacks&,
+    const Http1Settings& settings, const uint32_t max_response_headers_count,
+    const CodecHeaderValidationMode codec_header_validation_mode)
     : ConnectionImpl(connection, stats, settings, MessageType::Response, MAX_RESPONSE_HEADERS_KB,
-                     max_response_headers_count) {}
+                     max_response_headers_count, codec_header_validation_mode) {}
 
 bool ClientConnectionImpl::cannotHaveBody() {
   if (pending_response_.has_value() && pending_response_.value().encoder_.headRequest()) {

source/common/http/http1/codec_impl.h

@@ -254,7 +254,8 @@ class ConnectionImpl : public virtual Connection,
 
 protected:
   ConnectionImpl(Network::Connection& connection, CodecStats& stats, const Http1Settings& settings,
-                 MessageType type, uint32_t max_headers_kb, const uint32_t max_headers_count);
+                 MessageType type, uint32_t max_headers_kb, const uint32_t max_headers_count,
+                 CodecHeaderValidationMode codec_header_validation_mode);
 
   bool resetStreamCalled() { return reset_stream_called_; }
 
@@ -436,7 +437,8 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
                        ServerConnectionCallbacks& callbacks, const Http1Settings& settings,
                        uint32_t max_request_headers_kb, const uint32_t max_request_headers_count,
                        envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
-                           headers_with_underscores_action);
+                           headers_with_underscores_action,
+                       const CodecHeaderValidationMode codec_header_validation_mode);
   bool supportsHttp10() override { return codec_settings_.accept_http_10_; }
 
 protected:
@@ -549,7 +551,8 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl {
 public:
   ClientConnectionImpl(Network::Connection& connection, CodecStats& stats,
                        ConnectionCallbacks& callbacks, const Http1Settings& settings,
-                       const uint32_t max_response_headers_count);
+                       const uint32_t max_response_headers_count,
+                       const CodecHeaderValidationMode codec_header_validation_mode);
   // Http::ClientConnection
   RequestEncoder& newStream(ResponseDecoder& response_decoder) override;
 

source/common/http/http1/legacy_parser_impl.cc

@@ -38,7 +38,11 @@ class LegacyHttpParserImpl::Impl {
     parser_.allow_chunked_length = 1;
   }
 
-  Impl(http_parser_type type, void* data) : Impl(type) {
+  Impl(http_parser_type type, void* data,
+       const Http::CodecHeaderValidationMode codec_header_validation_mode)
+      : Impl(type) {
+    unsigned permissive_parsing =
+        codec_header_validation_mode == Http::CodecHeaderValidationMode::Disabled ? 1 : 0;
     parser_.data = data;
     settings_ = {
         [](http_parser* parser) -> int {
@@ -89,7 +93,8 @@ class LegacyHttpParserImpl::Impl {
           static_cast<ParserCallbacks*>(parser->data)->onChunkHeader(is_final_chunk);
           return 0;
         },
-        nullptr // on_chunk_complete
+        nullptr,           // on_chunk_complete
+        permissive_parsing // permissive_parsing
     };
   }
 
@@ -136,7 +141,9 @@ class LegacyHttpParserImpl::Impl {
   http_parser_settings settings_;
 };
 
-LegacyHttpParserImpl::LegacyHttpParserImpl(MessageType type, ParserCallbacks* data) {
+LegacyHttpParserImpl::LegacyHttpParserImpl(
+    MessageType type, ParserCallbacks* data,
+    const Http::CodecHeaderValidationMode codec_header_validation_mode) {
   http_parser_type parser_type;
   switch (type) {
   case MessageType::Request:
@@ -147,7 +154,7 @@ LegacyHttpParserImpl::LegacyHttpParserImpl(MessageType type, ParserCallbacks* da
     break;
   }
 
-  impl_ = std::make_unique<Impl>(parser_type, data);
+  impl_ = std::make_unique<Impl>(parser_type, data, codec_header_validation_mode);
 }
 
 // Because we have a pointer-to-impl using std::unique_ptr, we must place the destructor in the

source/common/http/http1/legacy_parser_impl.h

@@ -2,6 +2,8 @@
 
 #include <memory>
 
+#include "envoy/http/codec.h"
+
 #include "source/common/http/http1/parser.h"
 
 namespace Envoy {
@@ -10,7 +12,8 @@ namespace Http1 {
 
 class LegacyHttpParserImpl : public Parser {
 public:
-  LegacyHttpParserImpl(MessageType type, ParserCallbacks* data);
+  LegacyHttpParserImpl(MessageType type, ParserCallbacks* data,
+                       const CodecHeaderValidationMode codec_header_validation_mode);
   ~LegacyHttpParserImpl() override;
 
   // Http1::Parser

source/common/http/utility.cc

@@ -333,7 +333,7 @@ bool Utility::Url::initialize(absl::string_view absolute_url, bool is_connect) {
   struct http_parser_url u;
   http_parser_url_init(&u);
   const int result =
-      http_parser_parse_url(absolute_url.data(), absolute_url.length(), is_connect, &u);
+      http_parser_parse_url(absolute_url.data(), absolute_url.length(), is_connect, &u, 0);
 
   if (result != 0) {
     return false;

source/extensions/filters/listener/http_inspector/http_inspector.cc

@@ -28,9 +28,8 @@ Filter::Filter(const ConfigSharedPtr config) : config_(config) {
   http_parser_init(&parser_, HTTP_REQUEST);
 }
 
-http_parser_settings Filter::settings_{
-    nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
-};
+http_parser_settings Filter::settings_{nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
+                                       nullptr, nullptr, nullptr, nullptr, 0};
 
 Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) {
   ENVOY_LOG(debug, "http inspector: new connection accepted");

source/extensions/filters/network/http_connection_manager/config.cc

@@ -696,7 +696,7 @@ HttpConnectionManagerConfig::createCodec(Network::Connection& connection,
     return std::make_unique<Http::Http1::ServerConnectionImpl>(
         connection, Http::Http1::CodecStats::atomicGet(http1_codec_stats_, context_.scope()),
         callbacks, http1_settings_, maxRequestHeadersKb(), maxRequestHeadersCount(),
-        headersWithUnderscoresAction());
+        headersWithUnderscoresAction(), Http::CodecHeaderValidationMode::Enabled);
   }
   case CodecType::HTTP2: {
     return std::make_unique<Http::Http2::ServerConnectionImpl>(

test/common/http/codec_impl_fuzz_test.cc

@@ -509,7 +509,7 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
   } else {
     client = std::make_unique<Http1::ClientConnectionImpl>(
         client_connection, Http1::CodecStats::atomicGet(http1_stats, stats_store), client_callbacks,
-        client_http1settings, max_response_headers_count);
+        client_http1settings, max_response_headers_count, CodecHeaderValidationMode::Enabled);
   }
 
   if (http2) {
@@ -524,7 +524,7 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
     server = std::make_unique<Http1::ServerConnectionImpl>(
         server_connection, Http1::CodecStats::atomicGet(http1_stats, stats_store), server_callbacks,
         server_http1settings, max_request_headers_kb, max_request_headers_count,
-        headers_with_underscores_action);
+        headers_with_underscores_action, CodecHeaderValidationMode::Enabled);
   }
 
   // We track whether the connection should be closed for HTTP/1, since stream resets imply

test/common/http/http1/codec_impl_test.cc

@@ -83,7 +83,8 @@ class Http1ServerConnectionImplTest : public Http1CodecTestBase {
   void initialize() {
     codec_ = std::make_unique<Http1::ServerConnectionImpl>(
         connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-        max_request_headers_count_, headers_with_underscores_action_);
+        max_request_headers_count_, headers_with_underscores_action_,
+        codec_header_validation_mode_);
   }
 
   ~Http1ServerConnectionImplTest() override {
@@ -96,6 +97,8 @@ class Http1ServerConnectionImplTest : public Http1CodecTestBase {
   NiceMock<Http::MockServerConnectionCallbacks> callbacks_;
   NiceMock<Http1Settings> codec_settings_;
   Http::ServerConnectionPtr codec_;
+  Http::CodecHeaderValidationMode codec_header_validation_mode_{
+      Http::CodecHeaderValidationMode::Enabled};
 
   void expectHeadersTest(Protocol p, bool allow_absolute_url, Buffer::OwnedImpl& buffer,
                          TestRequestHeaderMapImpl& expected_headers);
@@ -153,7 +156,8 @@ void Http1ServerConnectionImplTest::expect400(Protocol p, bool allow_absolute_ur
     codec_settings_.allow_absolute_url_ = allow_absolute_url;
     codec_ = std::make_unique<Http1::ServerConnectionImpl>(
         connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-        max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+        max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW,
+        CodecHeaderValidationMode::Enabled);
   }
 
   MockRequestDecoder decoder;
@@ -183,7 +187,8 @@ void Http1ServerConnectionImplTest::expectHeadersTest(Protocol p, bool allow_abs
     codec_settings_.allow_absolute_url_ = allow_absolute_url;
     codec_ = std::make_unique<Http1::ServerConnectionImpl>(
         connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-        max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+        max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW,
+        CodecHeaderValidationMode::Enabled);
   }
 
   MockRequestDecoder decoder;
@@ -204,7 +209,8 @@ void Http1ServerConnectionImplTest::expectTrailersTest(bool enable_trailers) {
     codec_settings_.enable_trailers_ = enable_trailers;
     codec_ = std::make_unique<Http1::ServerConnectionImpl>(
         connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-        max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+        max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW,
+        CodecHeaderValidationMode::Enabled);
   }
 
   InSequence sequence;
@@ -241,7 +247,8 @@ void Http1ServerConnectionImplTest::testTrailersExceedLimit(std::string trailer_
   codec_settings_.enable_trailers_ = enable_trailers;
   codec_ = std::make_unique<Http1::ServerConnectionImpl>(
       connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW,
+      CodecHeaderValidationMode::Enabled);
   std::string exception_reason;
   NiceMock<MockRequestDecoder> decoder;
   EXPECT_CALL(callbacks_, newStream(_, _))
@@ -325,7 +332,8 @@ void Http1ServerConnectionImplTest::testServerAllowChunkedContentLength(uint32_t
   codec_settings_.allow_chunked_length_ = allow_chunked_length;
   codec_ = std::make_unique<Http1::ServerConnectionImpl>(
       connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW,
+      CodecHeaderValidationMode::Enabled);
 
   MockRequestDecoder decoder;
   Http::ResponseEncoder* response_encoder = nullptr;
@@ -710,7 +718,8 @@ TEST_F(Http1ServerConnectionImplTest, CodecHasCorrectStreamErrorIfTrue) {
   codec_settings_.stream_error_on_invalid_http_message_ = true;
   codec_ = std::make_unique<Http1::ServerConnectionImpl>(
       connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW,
+      CodecHeaderValidationMode::Enabled);
 
   Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n");
   NiceMock<MockRequestDecoder> decoder;
@@ -729,7 +738,8 @@ TEST_F(Http1ServerConnectionImplTest, CodecHasCorrectStreamErrorIfFalse) {
   codec_settings_.stream_error_on_invalid_http_message_ = false;
   codec_ = std::make_unique<Http1::ServerConnectionImpl>(
       connection_, http1CodecStats(), callbacks_, codec_settings_, max_request_headers_kb_,
-      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+      max_request_headers_count_, envoy::config::core::v3::HttpProtocolOptions::ALLOW,
+      CodecHeaderValidationMode::Enabled);
 
   Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n");
   NiceMock<MockRequestDecoder> decoder;
@@ -2127,7 +2137,8 @@ class Http1ClientConnectionImplTest : public Http1CodecTestBase {
 public:
   void initialize() {
     codec_ = std::make_unique<Http1::ClientConnectionImpl>(
-        connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_);
+        connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_,
+        codec_header_validation_mode_);
   }
 
   void readDisableOnRequestEncoder(RequestEncoder* request_encoder, bool disable) {
@@ -2138,6 +2149,8 @@ class Http1ClientConnectionImplTest : public Http1CodecTestBase {
   NiceMock<Http::MockConnectionCallbacks> callbacks_;
   NiceMock<Http1Settings> codec_settings_;
   Http::ClientConnectionPtr codec_;
+  Http::CodecHeaderValidationMode codec_header_validation_mode_{
+      Http::CodecHeaderValidationMode::Enabled};
 
   void testClientAllowChunkedContentLength(uint32_t content_length, bool allow_chunked_length);
 
@@ -2150,7 +2163,8 @@ void Http1ClientConnectionImplTest::testClientAllowChunkedContentLength(uint32_t
                                                                         bool allow_chunked_length) {
   codec_settings_.allow_chunked_length_ = allow_chunked_length;
   codec_ = std::make_unique<Http1::ClientConnectionImpl>(
-      connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_);
+      connection_, http1CodecStats(), callbacks_, codec_settings_, max_response_headers_count_,
+      CodecHeaderValidationMode::Enabled);
 
   NiceMock<MockResponseDecoder> response_decoder;
   Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder);
@@ -3006,6 +3020,39 @@ TEST_F(Http1ServerConnectionImplTest, ManyLargeRequestHeadersAccepted) {
   testRequestHeadersAccepted(createLargeHeaderFragment(64));
 }
 
+TEST_F(Http1ServerConnectionImplTest, PermissiveParsing) {
+  codec_header_validation_mode_ = CodecHeaderValidationMode::Disabled;
+  initialize();
+
+  InSequence sequence;
+
+  MockRequestDecoder decoder;
+  EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder));
+
+  TestRequestHeaderMapImpl expected_headers{
+      {":path", "/δ¶/δt/pope?q=1#narf"},
+      {":method", "GET"},
+  };
+  EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_headers), true));
+
+  Buffer::OwnedImpl buffer("GET /δ¶/δt/pope?q=1#narf HXXP/1.1\r\n\r\n");
+  auto status = codec_->dispatch(buffer);
+  EXPECT_TRUE(status.ok());
+  EXPECT_EQ(0U, buffer.length());
+}
+
+TEST_F(Http1ServerConnectionImplTest, StrictParsing) {
+  initialize();
+
+  MockRequestDecoder decoder;
+  EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder));
+
+  Buffer::OwnedImpl buffer("GET /δ¶/δt/pope?q=1#narf HXXP/1.1\r\n\r\n");
+  EXPECT_CALL(decoder, sendLocalReply(_, _, _, _, _));
+  auto status = codec_->dispatch(buffer);
+  EXPECT_TRUE(isCodecProtocolError(status));
+}
+
 // Tests that incomplete response headers of 80 kB header value fails.
 TEST_F(Http1ClientConnectionImplTest, ResponseHeadersWithLargeValueRejected) {
   initialize();