hcm: forbid use of detection extensions with use_remote_addr/xff_num_trusted_hops (#17558)

Mixing extensions with previously existing knobs leads to undefined behavior,
so this removes the deprecation around xff_num_trusted_hops and ensures that
it's not mixed with extensions.

Note that a unit test already exists for the original bug report, where
use_remote_address is used with xff_num_trusted_hops > 0. However it uses
the XFF extension instead of the old knob. Given this is now forbidden,
there's no need for additional tests wrt that config combination.

Fixes #17554.

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Mirrored from https://github.com/envoyproxy/envoy @ 57c172f8c3595b5f1e4b9ec9d56b7c8d7ec19c90

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",

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.
-  //
+  uint32 xff_num_trusted_hops = 19;
-  // .. 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"];
 
   // 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;
 

envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto

@@ -304,9 +304,9 @@ message HttpConnectionManager {
     type.http.v3.PathTransformation http_filter_transformation = 2;
   }
 
-  reserved 27, 11, 19;
+  reserved 27, 11;
 
-  reserved "idle_timeout", "xff_num_trusted_hops";
+  reserved "idle_timeout";
 
   // Supplies the type of codec that the connection manager should use.
   CodecType codec_type = 1 [(validate.rules).enum = {defined_only: true}];
@@ -498,6 +498,13 @@ message HttpConnectionManager {
   google.protobuf.BoolValue use_remote_address = 14
       [(udpa.annotations.security).configure_for_untrusted_downstream = true];
 
+  // The number of additional ingress proxy hops from the right side of the
+  // :ref:`config_http_conn_man_headers_x-forwarded-for` HTTP header to trust when
+  // 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.
+  uint32 xff_num_trusted_hops = 19;
+
   // The configuration for the original IP detection extensions.
   //
   // When configured the extensions will be called along with the request headers
@@ -508,6 +515,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.v4alpha.TypedExtensionConfig original_ip_detection_extensions = 46;