hcm: forbid use of detection extensions with use_remote_addr/xff_num_trusted_hops

api/envoy/extensions/filters/network/http_connection_manager/v3/BUILD

@@ -6,7 +6,6 @@ licenses(["notice"])  # Apache 2
 
 api_proto_package(
     deps = [
-        "//envoy/annotations:pkg",
         "//envoy/config/accesslog/v3:pkg",
         "//envoy/config/core/v3:pkg",
         "//envoy/config/filter/network/http_connection_manager/v2:pkg",

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -19,7 +19,6 @@ import "google/protobuf/any.proto";
 import "google/protobuf/duration.proto";
 import "google/protobuf/wrappers.proto";
 
-import "envoy/annotations/deprecation.proto";
 import "udpa/annotations/migrate.proto";
 import "udpa/annotations/security.proto";
 import "udpa/annotations/status.proto";
@@ -501,24 +500,7 @@ message HttpConnectionManager {
   // determining the origin client's IP address. The default is zero if this option
   // is not specified. See the documentation for
   // :ref:`config_http_conn_man_headers_x-forwarded-for` for more information.
-  //
-  // .. note::
-  //    This field is deprecated and instead :ref:`original_ip_detection_extensions
-  //    <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.original_ip_detection_extensions>`
-  //    should be used to configure the :ref:`xff extension <envoy_v3_api_msg_extensions.http.original_ip_detection.xff.v3.XffConfig>`
-  //    to configure IP detection using the :ref:`config_http_conn_man_headers_x-forwarded-for` header. To replace
-  //    this field use a config like the following:
-  //
-  // .. code-block:: yaml
-  //
-  //    original_ip_detection_extensions:
-  //    - name: envoy.http.original_ip_detection.xff
-  //      typed_config:
-  //        "@type": type.googleapis.com/envoy.extensions.http.original_ip_detection.xff.v3.XffConfig
-  //        xff_num_trusted_hops: 1
-  //
-  uint32 xff_num_trusted_hops = 19
-      [deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"];
+  uint32 xff_num_trusted_hops = 19;
 
   // The configuration for the original IP detection extensions.
   //
@@ -530,6 +512,12 @@ message HttpConnectionManager {
   // the request. If the request isn't rejected nor any extension succeeds, the HCM will
   // fallback to using the remote address.
   //
+  // .. WARNING::
+  //    Extensions cannot be used in conjunction with :ref:`use_remote_address
+  //    <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.use_remote_address>`
+  //    nor :ref:`xff_num_trusted_hops
+  //    <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.xff_num_trusted_hops>`.
+  //
   // [#extension-category: envoy.http.original_ip_detection]
   repeated config.core.v3.TypedExtensionConfig original_ip_detection_extensions = 46;
 

docs/root/configuration/http/http_conn_man/headers.rst

@@ -210,10 +210,16 @@ Given an HTTP request that has traveled through a series of zero or more proxies
 Envoy, the trusted client address is the earliest source IP address that is known to be
 accurate. The source IP address of the immediate downstream node's connection to Envoy is
 trusted. XFF *sometimes* can be trusted. Malicious clients can forge XFF, but the last
-address in XFF can be trusted if it was put there by a trusted proxy. Alternatively, Envoy
-supports :ref:`extensions <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.original_ip_detection_extensions>`
+address in XFF can be trusted if it was put there by a trusted proxy.
+
+Alternatively, Envoy supports
+:ref:`extensions <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.original_ip_detection_extensions>`
 for determining the *trusted client address* or original IP address.
 
+.. note::
+
+ The use of such extensions cannot be mixed with *use_remote_address* nor *xff_num_trusted_hops*.
+
 Envoy's default rules for determining the trusted client address (*before* appending anything
 to XFF) are:
 
@@ -223,11 +229,8 @@ to XFF) are:
   node's connection to Envoy.
 
 In an environment where there are one or more trusted proxies in front of an edge
-Envoy instance, the :ref:`XFF extension <envoy_v3_api_msg_extensions.http.original_ip_detection.xff.v3.XffConfig>`
-can be configured via the :ref:`original_ip_detection_extensions field
-<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.original_ip_detection_extensions>`
-to set the *xff_num_trusted_hops* option which controls the number of additional
-addresses that are to be trusted:
+Envoy instance, the *xff_num_trusted_hops* configuration option can be used to trust
+additional addresses from XFF:
 
 * If *use_remote_address* is false and *xff_num_trusted_hops* is set to a value *N* that is
   greater than zero, the trusted client address is the (N+1)th address from the right end

docs/root/version_history/current.rst

@@ -42,6 +42,7 @@ Bug Fixes
 * access log: fix ``%UPSTREAM_CLUSTER%`` when used in http upstream access logs. Previously, it was always logging as an unset value.
 * aws request signer: fix the AWS Request Signer extension to correctly normalize the path and query string to be signed according to AWS' guidelines, so that the hash on the server side matches. See `AWS SigV4 documentaion <https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html>`_.
 * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false.
+* hcm: remove deprecation for :ref:`xff_num_trusted_hops <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.xff_num_trusted_hops>` and forbid mixing ip detection extensions with old related knobs.
 * listener: fixed an issue on Windows where connections are not handled by all worker threads.
 * xray: fix the AWS X-Ray tracer bug where span's error, fault and throttle information was not reported properly as per the `AWS X-Ray documentation <https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html>`_. Before this fix, server error was reported under 'annotations' section of the segment data.
 

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

@@ -373,6 +373,16 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
     auto* extension = ip_detection_extensions.Add();
     extension->set_name("envoy.http.original_ip_detection.xff");
     extension->mutable_typed_config()->PackFrom(xff_config);
+  } else {
+    if (use_remote_address_) {
+      throw EnvoyException(
+          "Original IP detection extensions and use_remote_address may not be mixed");
+    }
+
+    if (xff_num_trusted_hops_ > 0) {
+      throw EnvoyException(
+          "Original IP detection extensions and xff_num_trusted_hops may not be mixed");
+    }
   }
 
   original_ip_detection_extensions_.reserve(ip_detection_extensions.size());

test/extensions/filters/network/http_connection_manager/config_test.cc

@@ -1996,6 +1996,46 @@ TEST_F(HttpConnectionManagerConfigTest, OriginalIPDetectionExtension) {
   EXPECT_EQ(1, original_ip_detection_extensions.size());
 }
 
+TEST_F(HttpConnectionManagerConfigTest, OriginalIPDetectionExtensionMixedWithUseRemoteAddress) {
+  const std::string yaml_string = R"EOF(
+  stat_prefix: ingress_http
+  route_config:
+    name: local_route
+  use_remote_address: true
+  original_ip_detection_extensions:
+  - name: envoy.http.original_ip_detection.custom_header
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.http.original_ip_detection.custom_header.v3.CustomHeaderConfig
+      header_name: x-ip-header
+  http_filters:
+  - name: envoy.filters.http.router
+  )EOF";
+
+  EXPECT_THROW_WITH_REGEX(
+      createHttpConnectionManagerConfig(yaml_string), EnvoyException,
+      "Original IP detection extensions and use_remote_address may not be mixed");
+}
+
+TEST_F(HttpConnectionManagerConfigTest, OriginalIPDetectionExtensionMixedWithNumTrustedHops) {
+  const std::string yaml_string = R"EOF(
+  stat_prefix: ingress_http
+  route_config:
+    name: local_route
+  xff_num_trusted_hops: 1
+  original_ip_detection_extensions:
+  - name: envoy.http.original_ip_detection.custom_header
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.http.original_ip_detection.custom_header.v3.CustomHeaderConfig
+      header_name: x-ip-header
+  http_filters:
+  - name: envoy.filters.http.router
+  )EOF";
+
+  EXPECT_THROW_WITH_REGEX(
+      createHttpConnectionManagerConfig(yaml_string), EnvoyException,
+      "Original IP detection extensions and xff_num_trusted_hops may not be mixed");
+}
+
 TEST_F(HttpConnectionManagerConfigTest, DynamicFilterWarmingNoDefault) {
   const std::string yaml_string = R"EOF(
 codec_type: http1

test/integration/header_integration_test.cc

@@ -43,11 +43,7 @@ const std::string http_connection_mgr_config = R"EOF(
   - name: envoy.filters.http.router
 codec_type: HTTP1
 use_remote_address: false
-original_ip_detection_extensions:
-- name: envoy.http.original_ip_detection.xff
-  typed_config:
-    "@type": type.googleapis.com/envoy.extensions.http.original_ip_detection.xff.v3.XffConfig
-    xff_num_trusted_hops: 1
+xff_num_trusted_hops: 1
 stat_prefix: header_test
 route_config:
   virtual_hosts: