proxy protocol: new feature auto-detect proxy protocol

api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto

@@ -41,4 +41,21 @@ message ProxyProtocol {
 
   // The list of rules to apply to requests.
   repeated Rule rules = 1;
+
+  // Allow requests through that don't use proxy protocol. Defaults to false.
+  //
+  // .. attention::
+  //
+  //   This breaks conformance with the specification.
+  //   Only enable if ALL traffic to the listener comes from a trusted source.
+  //   For more information on the security implications of this feature, see
+  //   https://www.haproxy.org/download/2.1/doc/proxy-protocol.txt
+  //
+  // .. attention::
+  //
+  //   Requests of 12 or fewer bytes that match the proxy protocol v2 signature
+  //   and requests of 6 or fewer bytes that match the proxy protocol v1
+  //   signature will timeout (Envoy is unable to differentiate these requests
+  //   from incomplete proxy protocol requests).
+  bool allow_requests_without_proxy_protocol = 2;
 }

changelogs/1.23.0.yaml

@@ -75,6 +75,9 @@ new_features:
 - area: on_demand
   change: |
     :ref:`OnDemand <envoy_v3_api_msg_extensions.filters.http.on_demand.v3.OnDemand>` got extended to hold configuration for on-demand cluster discovery. A similar message for :ref:`per-route configuration <envoy_v3_api_msg_extensions.filters.http.on_demand.v3.PerRouteConfig>` is also added.
+- area: proxy_protcol
+  change: |
+    added :ref:`allow_requests_without_proxy_protocol<envoy_v3_api_field_extensions.filters.listener.proxy_protocol.v3.ProxyProtocol.allow_requests_without_proxy_protocol>` to allow requests without proxy protocol on the listener from trusted downstreams as an opt-in flag.
 - area: build
   change: |
     enabled building arm64 envoy-distroless and envoy-tools :ref:`docker images <install_binaries>`.

source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc

@@ -46,7 +46,8 @@ namespace ProxyProtocol {
 Config::Config(
     Stats::Scope& scope,
     const envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol& proto_config)
-    : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))} {
+    : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))},
+      allow_requests_without_proxy_protocol_(proto_config.allow_requests_without_proxy_protocol()) {
   for (const auto& rule : proto_config.rules()) {
     tlv_types_[0xFF & rule.tlv_type()] = rule.on_tlv_present();
   }
@@ -63,6 +64,10 @@ const KeyValuePair* Config::isTlvTypeNeeded(uint8_t type) const {
 
 size_t Config::numberOfNeededTlvTypes() const { return tlv_types_.size(); }
 
+bool Config::allowRequestsWithoutProxyProtocol() const {
+  return allow_requests_without_proxy_protocol_;
+}
+
 Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) {
   ENVOY_LOG(debug, "proxy_protocol: New connection accepted");
   cb_ = &cb;
@@ -72,12 +77,17 @@ Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) {
 
 Network::FilterStatus Filter::onData(Network::ListenerFilterBuffer& buffer) {
   const ReadOrParseState read_state = parseBuffer(buffer);
-  if (read_state == ReadOrParseState::Error) {
+  switch (read_state) {
+  case ReadOrParseState::Error:
     config_->stats_.downstream_cx_proxy_proto_error_.inc();
     cb_->socket().ioHandle().close();
     return Network::FilterStatus::StopIteration;
-  } else if (read_state == ReadOrParseState::TryAgainLater) {
+  case ReadOrParseState::TryAgainLater:
     return Network::FilterStatus::StopIteration;
+  case ReadOrParseState::SkipFilter:
+    return Network::FilterStatus::Continue;
+  default:
+    return Network::FilterStatus::Continue;
   }
   return Network::FilterStatus::Continue;
 }
@@ -399,6 +409,19 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer)
   auto raw_slice = buffer.rawSlice();
   const char* buf = static_cast<const char*>(raw_slice.mem_);
 
+  if (config_.get()->allowRequestsWithoutProxyProtocol()) {
+    auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE,
+                           std::min<size_t>(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_));
+    auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE,
+                           std::min<size_t>(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_));
+    if (!matchv2 && !matchv1) {
+      // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely
+      // short-circuit
+      ENVOY_LOG(trace, "request does not use v1 or v2 proxy protocol, forwarding as is");
+      return ReadOrParseState::SkipFilter;
+    }
+  }
+
   if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) {
     const char* sig = PROXY_PROTO_V2_SIGNATURE;
     if (!memcmp(buf, sig, PROXY_PROTO_V2_SIGNATURE_LEN)) {

source/extensions/filters/listener/proxy_protocol/proxy_protocol.h

@@ -60,15 +60,22 @@ class Config : public Logger::Loggable<Logger::Id::filter> {
    */
   size_t numberOfNeededTlvTypes() const;
 
+  /**
+   * Filter configuration that determines if we should pass-through requests without
+   * proxy protocol. Should only be configured to true for trusted downstreams.
+   */
+  bool allowRequestsWithoutProxyProtocol() const;
+
 private:
   absl::flat_hash_map<uint8_t, KeyValuePair> tlv_types_;
+  const bool allow_requests_without_proxy_protocol_;
 };
 
 using ConfigSharedPtr = std::shared_ptr<Config>;
 
 enum ProxyProtocolVersion { Unknown = 0, V1 = 1, V2 = 2 };
 
-enum class ReadOrParseState { Done, TryAgainLater, Error };
+enum class ReadOrParseState { Done, TryAgainLater, Error, SkipFilter };
 
 /**
  * Implementation the PROXY Protocol listener filter
@@ -100,7 +107,7 @@ class Filter : public Network::ListenerFilter, Logger::Loggable<Logger::Id::filt
   /**
    * Helper function that attempts to read the proxy header
    * (delimited by \r\n if V1 format, or with length if V2)
-   * @return bool true valid header, false if more data is needed or socket errors occurred.
+   * @return ReadOrParseState
    */
   ReadOrParseState readProxyHeader(Network::ListenerFilterBuffer& buffer);
 

test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc

@@ -31,6 +31,8 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
+using Envoy::Extensions::Common::ProxyProtocol::PROXY_PROTO_V1_SIGNATURE_LEN;
+using Envoy::Extensions::Common::ProxyProtocol::PROXY_PROTO_V2_SIGNATURE_LEN;
 using testing::_;
 using testing::AnyNumber;
 using testing::AtLeast;
@@ -230,6 +232,78 @@ TEST_P(ProxyProtocolTest, V1Basic) {
   disconnect();
 }
 
+TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) {
+  // Allows a small request (less bytes than v1/v2 signature) through even though it doesn't use
+  // proxy protocol
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  std::string msg = "data";
+  ASSERT_GT(PROXY_PROTO_V1_SIGNATURE_LEN,
+            msg.length()); // Ensure we attempt parsing byte by byte using `search_index_`
+  ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length());
+
+  write(msg);
+  expectData(msg);
+  disconnect();
+}
+
+TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV1First) {
+  // Allows a small request (less bytes than v1/v2 signature) through even though it doesn't use
+  // proxy protocol v1/v2 (but it does match parts of both signatures)
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  // First two bytes are proxy protocol v1, second two bytes are proxy protocol v2.
+  // This ensures our byte by byte parsing (`search_index_`) has persistence built-in to
+  // remember whether the previous bytes were also valid for the signature
+  std::string msg = "PR\r\n";
+  ASSERT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length());
+  ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length());
+
+  write(msg);
+  expectData(msg);
+  disconnect();
+}
+
+TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV2First) {
+  // Allows a small request (less bytes than v1/v2 signature) through even though it doesn't use
+  // proxy protocol v1/v2 (but it does match parts of both signatures)
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  // First two bytes are proxy protocol v2, second two bytes are proxy protocol v1.
+  // This ensures our byte by byte parsing (`search_index_`) has persistence built-in to
+  // remember whether the previous bytes were also valid for the signature
+  std::string msg = "\r\nOX";
+  ASSERT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length());
+  ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length());
+
+  write(msg);
+  expectData(msg);
+  disconnect();
+}
+
+TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) {
+  // Allows a large request (more bytes than v1/v2 signature) through even though it doesn't use
+  // proxy protocol
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  std::string msg = "more data more data more data";
+  ASSERT_GT(msg.length(),
+            PROXY_PROTO_V2_HEADER_LEN); // Ensure we attempt parsing as v2 proxy protocol up front
+                                        // rather than parsing byte by byte using `search_index_`
+
+  write(msg);
+  expectData(msg);
+  disconnect();
+}
+
 TEST_P(ProxyProtocolTest, V1Minimal) {
   connect();
   write("PROXY UNKNOWN\r\nmore data");
@@ -526,6 +600,19 @@ TEST_P(ProxyProtocolTest, V2ShortV4) {
   expectProxyProtoError();
 }
 
+TEST_P(ProxyProtocolTest, V2ShortV4WithAllowNoProxyProtocol) {
+  // An ipv4/tcp PROXY header that has incorrect addr-len encoded
+  constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49,
+                                0x54, 0x0a, 0x21, 0x21, 0x00, 0x04, 0x00, 0x08, 0x00, 0x02,
+                                'm',  'o',  'r',  'e',  ' ',  'd',  'a',  't',  'a'};
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(false, &proto_config);
+
+  write(buffer, sizeof(buffer));
+  expectProxyProtoError();
+}
+
 TEST_P(ProxyProtocolTest, V2ShortAddrV4) {
   // An ipv4/tcp connection that has insufficient header-length encoded
   constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49,
@@ -609,6 +696,18 @@ TEST_P(ProxyProtocolTest, V1TooLong) {
   expectProxyProtoError();
 }
 
+TEST_P(ProxyProtocolTest, V1TooLongWithAllowNoProxyProtocol) {
+  constexpr uint8_t buffer[] = {' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '};
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(false, &proto_config);
+  write("PROXY TCP4 1.2.3.4 2.3.4.5 100 100");
+  for (size_t i = 0; i < 256; i += sizeof(buffer)) {
+    write(buffer, sizeof(buffer));
+  }
+  expectProxyProtoError();
+}
+
 TEST_P(ProxyProtocolTest, V2ParseExtensions) {
   // A well-formed ipv4/tcp with a pair of TLV extensions is accepted
   constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49,
@@ -1035,6 +1134,52 @@ TEST_P(ProxyProtocolTest, PartialRead) {
   disconnect();
 }
 
+TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) {
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  write("PROXY TCP4"); // Intentionally larger than the size of v1 proxy protocol signature
+
+  dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
+
+  write(" 254.254.2");
+  write("54.254 1.2");
+  write(".3.4 65535");
+  write(" 1234\r\n...");
+
+  expectData("...");
+  EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(),
+            "254.254.254.254");
+  EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored());
+  disconnect();
+}
+
+TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) {
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  write("PRO"); // Intentionally smaller than the size of v1 proxy protocol signature
+
+  dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
+
+  write("XY TCP4 25");
+
+  dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
+
+  write("4.254.2");
+  write("54.254 1.2");
+  write(".3.4 65535");
+  write(" 1234\r\n...");
+
+  expectData("...");
+  EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(),
+            "254.254.254.254");
+  EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored());
+  disconnect();
+}
+
 TEST_P(ProxyProtocolTest, V2PartialRead) {
   // A well-formed ipv4/tcp header, delivered with part of the signature,
   // part of the header, rest of header + body
@@ -1060,6 +1205,64 @@ TEST_P(ProxyProtocolTest, V2PartialRead) {
   disconnect();
 }
 
+TEST_P(ProxyProtocolTest, PartialV2ReadWithAllowNoProxyProtocol) {
+  // A well-formed ipv4/tcp header, delivered with part of the signature,
+  // part of the header, rest of header + body
+  constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55,
+                                0x49, 0x54, 0x0a, 0x21, 0x11, 0x00, 0x0c, 0x01, 0x02,
+                                0x03, 0x04, 0x00, 0x01, 0x01, 0x02, 0x03, 0x05, 0x00,
+                                0x02, 'm',  'o',  'r',  'e',  'd',  'a',  't',  'a'};
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  // Using 18 intentionally as it is larger than v2 signature length and divides evenly into
+  // len(buffer)
+  auto buffer_incr_size = 18;
+  ASSERT_LT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size);
+  for (size_t i = 0; i < sizeof(buffer); i += buffer_incr_size) {
+    write(&buffer[i], buffer_incr_size);
+    if (i == 0) {
+      dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
+    }
+  }
+
+  expectData("moredata");
+  EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(),
+            "1.2.3.4");
+  EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored());
+  disconnect();
+}
+
+TEST_P(ProxyProtocolTest, TinyPartialV2ReadWithAllowNoProxyProtocol) {
+  // A well-formed ipv4/tcp header, delivered with part of the signature,
+  // part of the header, rest of header + body
+  constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55,
+                                0x49, 0x54, 0x0a, 0x21, 0x11, 0x00, 0x0c, 0x01, 0x02,
+                                0x03, 0x04, 0x00, 0x01, 0x01, 0x02, 0x03, 0x05, 0x00,
+                                0x02, 'm',  'o',  'r',  'e',  'd',  'a',  't',  'a'};
+  envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config;
+  proto_config.set_allow_requests_without_proxy_protocol(true);
+  connect(true, &proto_config);
+
+  // Using 3 intentionally as it is smaller than v2 signature length and divides evenly into
+  // len(buffer)
+  auto buffer_incr_size = 3;
+  ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size);
+  for (size_t i = 0; i < sizeof(buffer); i += buffer_incr_size) {
+    write(&buffer[i], buffer_incr_size);
+    if (i == 0) {
+      dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
+    }
+  }
+
+  expectData("moredata");
+  EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(),
+            "1.2.3.4");
+  EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored());
+  disconnect();
+}
+
 const std::string ProxyProtocol = "envoy.filters.listener.proxy_protocol";
 
 TEST_P(ProxyProtocolTest, V2ParseExtensionsLargeThanInitMaxReadBytes) {