The`xff_num_trusted_hops` in XffIPDetection is not aligned with the doc and the `xffNumTrustedHops` option in HCM

[zhaohuabing]

It seems that there is an inconsistency between the two approaches of getting remote IP from the XFF header.

The same request:

curl  --header "X-Forwarded-For: 10.0.0.4,10.0.2.1,10.0.0.5" http://172.18.255.203:80/protected2

With this configuration

"name": "envoy.filters.network.http_connection_manager",
"typedConfig": {
  "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager",
  // omitted for brevity
  ...

  "originalIpDetectionExtensions": [
    {
      "name": "envoy.extensions.http.original_ip_detection.xff",
      "typedConfig": {
        "@type": "type.googleapis.com/envoy.extensions.http.original_ip_detection.xff.v3.XffConfig",
        "xffNumTrustedHops": 2
      }
    }
  ]

The remoteIP is the 10.0.0.4:0, the third rightmost IP, as the following log shows:

[2024-05-20 00:34:03.604][62][debug][rbac] [source/extensions/filters/http/rbac/rbac_filter.cc:131] checking request: requestedServerName: , sourceIP: 172.18.0.1:50822, directRemoteIP: 172.18.0.1:50822, remoteIP: 10.0.0.4:0,localAddress: 10.244.0.101:10080, ssl: none, headers: ':authority', 'www.example.com'

But with this configuration:

"name": "envoy.filters.network.http_connection_manager",
"typedConfig": {
  "@type": "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager",
  // omitted for brevity
  ...
   
  "useRemoteAddress": true,
  "xffNumTrustedHops": 2
}

The remoteIP is the 10.0.2.1, the second rightmost IP, as the following log shows:

[2024-05-20 00:33:18.581][98][debug][rbac] [source/extensions/filters/http/rbac/rbac_filter.cc:131] checking request: requestedServerName: , sourceIP: 172.18.0.1:46138, directRemoteIP: 172.18.0.1:46138, remoteIP: 10.0.2.1:0,localAddress: 10.244.0.101:10080, ssl: none, headers: ':authority', 'www.example.com'

According to the Envoy docs, the correct xxfNumTrustedHops should be 2 here.

Example 3: Envoy as edge proxy, with two trusted external proxies in front of it
Settings:
use_remote_address = true
xff_num_trusted_hops = 2

The inconsistency seems comes from:

envoy/source/extensions/http/original_ip_detection/xff/xff.cc

Line 21 in b65de1f

Envoy::Http::Utility::getLastAddressFromXFF(params.request_headers, xff_num_trusted_hops_);

envoy/source/common/http/conn_manager_utility.cc

Line 128 in b65de1f

Utility::getLastAddressFromXFF(request_headers, xff_num_trusted_hops - 1).address_;

[zuercher]

cc @alyssawilk

[alyssawilk]

are you setting use remote address in both cases? AFIK if you set num hops =2 through either method it should be using exactly the same code

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[zhaohuabing]

@alyssawilk

The inconsistency seems to come from the below code. Even though they call the same function, but the input parameters are different xff_num_trusted_hops_ vs xff_num_trusted_hops -1:

envoy/source/extensions/http/original_ip_detection/xff/xff.cc

Line 21 in b65de1f

Envoy::Http::Utility::getLastAddressFromXFF(params.request_headers, xff_num_trusted_hops_);

envoy/source/common/http/conn_manager_utility.cc

Line 128 in b65de1f

Utility::getLastAddressFromXFF(request_headers, xff_num_trusted_hops - 1).address_;

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.