envoyproxy · ggreenway · May 6, 2022 · Nov 9, 2021 · Nov 10, 2021 · Nov 10, 2021
diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto
@@ -40,4 +40,14 @@ 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::
+  //
+  //   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/1.9/doc/proxy-protocol.txt
+  //
+  bool allow_requests_without_proxy_protocol = 2;
 }
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -65,6 +65,7 @@ New Features
 * listener: added support for :ref:`MPTCP <envoy_v3_api_field_config.listener.v3.Listener.enable_mptcp>` (multipath TCP).
 * oauth filter: added :ref:`cookie_names <envoy_v3_api_field_extensions.filters.http.oauth2.v3.OAuth2Credentials.cookie_names>` to allow overriding (default) cookie names (``BearerToken``, ``OauthHMAC``, and ``OauthExpires``) set by the filter.
 * oauth filter: setting IdToken and RefreshToken cookies if they are provided by Identity provider along with AccessToken.
+* proxy protocol: added support for allowing requests without proxy protocol on the listener from trusted downstreams as an opt-in flag.
 * tcp: added a :ref:`FilterState <envoy_v3_api_msg_type.v3.HashPolicy.FilterState>` :ref:`hash policy <envoy_v3_api_msg_type.v3.HashPolicy>`, used by :ref:`TCP proxy <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.hash_policy>` to allow hashing load balancer algorithms to hash on objects in filter state.
 * tcp_proxy: added support to populate upstream http connect header values from stream info.
 * thrift_proxy: add header to metadata filter for turning headers into dynamic metadata.

@@ -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");
   Network::ConnectionSocket& socket = cb.socket();
@@ -77,11 +82,19 @@ Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) {
   return Network::FilterStatus::StopIteration;
 }
 
+void Filter::resetAndContinue(Network::IoHandle& io_handle) {
+  // Release the file event so that we do not interfere with the connection read events.
+  io_handle.resetFileEvents();
+  cb_->continueFilterChain(true);
+}
+
 void Filter::onRead() {
   const ReadOrParseState read_state = onReadWorker();
   if (read_state == ReadOrParseState::Error) {
     config_->stats_.downstream_cx_proxy_proto_error_.inc();
     cb_->continueFilterChain(false);
+  } else if (read_state == ReadOrParseState::SkipFilterError) {
+    resetAndContinue(cb_->socket().ioHandle());
   }
 }
 
@@ -135,9 +148,7 @@ ReadOrParseState Filter::onReadWorker() {
         proxy_protocol_header_.value().remote_address_);
   }
 
-  // Release the file event so that we do not interfere with the connection read events.
-  socket.ioHandle().resetFileEvents();
-  cb_->continueFilterChain(true);
+  resetAndContinue(socket.ioHandle());
   return ReadOrParseState::Done;
 }
 
@@ -309,7 +320,7 @@ ReadOrParseState Filter::parseExtensions(Network::IoHandle& io_handle, uint8_t*
     const auto recv_result = io_handle.recv(buf, to_read, 0);
     if (!recv_result.ok()) {
       if (recv_result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again) {
-        return ReadOrParseState::TryAgainLater;
+        return ReadOrParseState::TryAgainLaterError;
       }
       ENVOY_LOG(debug, "failed to read proxy protocol (no bytes avail)");
       return ReadOrParseState::Error;
@@ -424,7 +435,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) {
 
     if (!result.ok()) {
       if (result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again) {
-        return ReadOrParseState::TryAgainLater;
+        return ReadOrParseState::TryAgainLaterError;
       }
       ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)");
       return ReadOrParseState::Error;
@@ -434,6 +445,13 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) {
     if (nread < 1) {
       ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)");
       return ReadOrParseState::Error;
+    } else if (nread < PROXY_PROTO_V2_HEADER_LEN &&
+               config_.get()->allowRequestsWithoutProxyProtocol()) {
+      if (nread < PROXY_PROTO_V1_SIGNATURE_LEN ||
+          memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) {
+        ENVOY_LOG(debug, "need more bytes to determine if we have a proxy protocol header");
+        return ReadOrParseState::SkipFilterError;
+      }
     }
 
     if (buf_off_ + nread >= PROXY_PROTO_V2_HEADER_LEN) {

@@ -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_;
+  bool allow_requests_without_proxy_protocol_{};
 };
 
 using ConfigSharedPtr = std::shared_ptr<Config>;
 
 enum ProxyProtocolVersion { Unknown = -1, InProgress = -2, V1 = 1, V2 = 2 };
 
-enum class ReadOrParseState { Done, TryAgainLater, Error };
+enum class ReadOrParseState { Done, TryAgainLaterError, Error, SkipFilterError };
 
 /**
  * Implementation the PROXY Protocol listener filter
@@ -98,7 +105,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 Done if successfully parsed, or the error type.
    */
   ReadOrParseState readProxyHeader(Network::IoHandle& io_handle);
 
@@ -118,6 +125,8 @@ class Filter : public Network::ListenerFilter, Logger::Loggable<Logger::Id::filt
   bool parseV2Header(char* buf);
   absl::optional<size_t> lenV2Address(char* buf);
 
+  void resetAndContinue(Network::IoHandle& io_handle);
+
   Network::ListenerFilterCallbacks* cb_{};
 
   // The offset in buf_ that has been fully read

@@ -222,6 +222,19 @@ TEST_P(ProxyProtocolTest, V1Basic) {
   disconnect();
 }
 
+TEST_P(ProxyProtocolTest, AllowNoProxyProtocol) {
+  // allows request 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);
+
+  write("more data");
+
+  expectData("more data");
+
+  disconnect();
+}
+
 TEST_P(ProxyProtocolTest, V1Minimal) {
   connect();
   write("PROXY UNKNOWN\r\nmore data");
@@ -927,6 +940,30 @@ 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");
+
+  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, V2PartialRead) {
   // A well-formed ipv4/tcp header, delivered with part of the signature,
   // part of the header, rest of header + body