Skip to content

http: add use_extracted_external_address config option#44933

Open
owenammann wants to merge 3 commits into
envoyproxy:mainfrom
owenammann:feat/use-extracted-external-address
Open

http: add use_extracted_external_address config option#44933
owenammann wants to merge 3 commits into
envoyproxy:mainfrom
owenammann:feat/use-extracted-external-address

Conversation

@owenammann
Copy link
Copy Markdown

@owenammann owenammann commented May 7, 2026

Commit Message:
http: rework as extracted_external_address original IP detection extension

Additional Description:
This PR addresses API-shepherd review feedback from @adisuissa and
@wbpcode on the original revision (which proposed a top-level
HttpConnectionManager.use_extracted_external_address bool). The
functionality has been reworked as a dedicated
envoy.http.original_ip_detection.extracted_external_address extension
under the existing original_ip_detection_extensions framework. The
HCM bool is removed.

The extension reads the x-envoy-external-address request header via
the typed Http::RequestHeaderMap::EnvoyExternalAddress() accessor and
returns the parsed IP as the detected downstream remote address. The
typed accessor binds the extension to the Envoy-managed header at
compile time, so a future rename of the accessor or header becomes a
build break here rather than a silent regression.

A dedicated extension was chosen over reusing the existing
custom_header extension. x-envoy-external-address is not a custom
header — it is an Envoy-managed header populated by the edge HCM as
the result of use_remote_address + XFF resolution. Routing through
custom_header with header_name: x-envoy-external-address would
couple by string name, with no compile-time tie to the accessor.

Use case:
Interior proxies in hierarchical Envoy deployments (sidecars,
waypoints, second-tier gateways) generally cannot statically configure
the correct xff_num_trusted_hops, because the number of upstream
hops varies by call path. The extension lets the interior proxy defer
to the upstream Envoy's already-resolved external address rather than
re-running XFF extraction with a hop count that cannot be calibrated
correctly for all callers.

This unblocks remoteIpBlocks authorization on interior Envoys in
multi-proxy deployments. Related: istio/istio#47947, istio/istio#55722,
istio/istio#30166.

Trust model:
This extension is a consumer of the existing x-envoy-external-address
header trust contract: written by an edge HCM running with
use_remote_address: true, consumed by interior proxies that trust
the upstream Envoy. Operators who need to strip a client-supplied
value at an interior proxy can add x-envoy-external-address to
route_config.internal_only_headers.

Risk Level: Low. New optional extension (status: alpha). No change
in behavior unless explicitly configured under
original_ip_detection_extensions. The previously proposed top-level
HCM bool never shipped in any released Envoy version.

Testing: Unit tests covering header absent, unparseable, empty,
multi-valued, IPv4, and IPv6 paths; plus a factory-registration test.

Docs Changes:

  • New paragraph in docs/root/configuration/http/http_conn_man/headers.rst
    referring interior proxies to the new extension.
  • docs/root/intro/arch_overview/other_features/ip_transparency.rst
    updated to list the new extension alongside custom_header and
    xff.

Release Notes: Entry under new_features in
changelogs/current.yaml (area http).

API Considerations: Adds a new extension package
envoy.extensions.http.original_ip_detection.extracted_external_address.v3
with a single empty ExtractedExternalAddressConfig message. No
existing fields, messages, or services are modified. Wire compat: new
type_url only.

Platform Specific Features: N/A

#44751

Generative AI usage disclosure: Claude Code was used as a coding
assistant to help draft the implementation, tests, and documentation
in this PR. I have read, understand, and take full ownership of every
file in this change.

When enabled, the HTTP connection manager will use an existing
x-envoy-external-address header value as the downstream remote address
instead of re-extracting from XFF. This is useful for interior proxies
in a service mesh where an upstream edge proxy has already extracted
the original client IP.

This enables proper remoteIpBlocks authorization in multi-proxy
deployments (e.g., Istio ambient mode with gateway → waypoint) where
interior proxies cannot be configured with numTrustedProxies.

Related: istio/istio#47947, istio/istio#55722

Signed-off-by: Owen Ammann <owammann@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

Hi @owenammann, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #44933 was opened by owenammann.

see: more, trace.

@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #44933 was opened by owenammann.

see: more, trace.

@kyessenov
Copy link
Copy Markdown
Contributor

Waiting for API review.
/wait

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
2 API questions:

  1. Would it make sense to add this an extension (see original_ip_detection_extensions)?
  2. How does this field interact with other fields such as use_remote_address?

@wbpcode
Copy link
Copy Markdown
Member

wbpcode commented May 14, 2026

same concern with Adi

@tyxia
Copy link
Copy Markdown
Member

tyxia commented May 20, 2026

/wait

Wait for @owenammann to address the API review feedbacks.

Please also merge main.

…external-address

Signed-off-by: Owen Ammann <owammann@gmail.com>

# Conflicts:
#	changelogs/current.yaml
…nsion

Address API-shepherd review feedback (PR envoyproxy#44933) by reworking the
top-level ``HttpConnectionManager.use_extracted_external_address`` bool
into a dedicated ``envoy.http.original_ip_detection.extracted_external_address``
extension under the existing ``original_ip_detection_extensions``
framework. The HCM bool, its config plumbing, and the associated tests
are removed. The new extension provides the same behavior via the
extension chain.

The header is read with the typed
``Http::RequestHeaderMap::EnvoyExternalAddress()`` accessor so the
binding to the Envoy-managed header is enforced at compile time. When
the header is present and parses as a valid IP, the address is treated
as trusted; when absent or unparseable, the extension returns no
detection result and ``skip_xff_append=true``, allowing the chain to
continue without polluting XFF.

A dedicated extension (rather than a configuration of the existing
``custom_header`` extension) was chosen because
``x-envoy-external-address`` is not a custom header: it is an
Envoy-managed header populated by the edge HCM, and the typed accessor
binding is what makes this safe to consume on an interior proxy.
Routing through ``custom_header`` with
``header_name: x-envoy-external-address`` would couple by string name
with no compile-time tie to the accessor.

Interaction with ``use_remote_address`` is enforced by the existing
extension-framework validation in
``source/extensions/filters/network/http_connection_manager/config.cc``:
configuring any ``original_ip_detection_extensions`` alongside
``use_remote_address`` (or ``xff_num_trusted_hops > 0``) is rejected at
config-load.

The proto doc and ``headers.rst`` include an ``.. attention::`` block
warning that ``x-envoy-external-address`` is not stripped on ingress;
operators must ensure the network path to the interior proxy is
integrity-protected (mTLS-authenticated mesh) or that an upstream CDN /
edge load balancer / front Envoy strips client-supplied
``x-envoy-external-address`` before forwarding.

Use case context: this enables ``remoteIpBlocks`` authorization to work
correctly on interior Envoys in multi-proxy deployments (e.g., Istio
ambient gateway -> waypoint) where the interior proxy cannot statically
configure the correct ``xff_num_trusted_hops`` because the number of
upstream hops varies by call path. Related: istio/istio#47947,
istio/istio#55722, istio/istio#30166.

Risk Level: Low. New optional extension (status: alpha). No change in
behavior unless explicitly configured under
``original_ip_detection_extensions``. The previously proposed
top-level HCM bool is removed; that field never shipped in any
released Envoy version.

Testing: Unit tests covering header absent, unparseable, empty,
multi-valued, IPv4, and IPv6 paths; plus a factory-registration test.

Docs Changes: Added a paragraph in
``docs/root/configuration/http/http_conn_man/headers.rst`` referring
interior proxies to the new extension and warning about the spoofing
risk. Updated the ``ip_transparency`` overview to list the new
extension alongside ``custom_header`` and ``xff``.

Release Notes: Entry added under ``new_features`` in
``changelogs/current.yaml`` (area ``http``).

Platform Specific Features: N/A.

Generative AI usage disclosure: Claude Code was used as a coding
assistant to help draft the implementation, tests, and documentation
in this PR. I have read, understand, and take full ownership of every
file in this change.

Signed-off-by: Owen Ammann <owammann@gmail.com>
@owenammann owenammann requested a deployment to external-contributors May 20, 2026 19:05 — with GitHub Actions Waiting
@owenammann
Copy link
Copy Markdown
Author

Thanks for the review. Reworked as envoy.http.original_ip_detection.extracted_external_address, a dedicated extension under original_ip_detection_extensions. PR description and commit message updated to match.

(1) Why a dedicated extension rather than custom_header:
x-envoy-external-address isn't a custom header — it's an Envoy-managed header populated by the edge HCM as the result of use_remote_address + XFF resolution, with a typed accessor at RequestHeaderMap::EnvoyExternalAddress(). Routing through custom_header with header_name: x-envoy-external-address would couple by string name with no compile-time tie to the accessor; a future header rename would be a silent regression. The dedicated extension calls the typed accessor, so the same change is a compile error.

(2) Interaction with use_remote_address:
The existing mutual-exclusion validation at source/extensions/filters/network/http_connection_manager/config.cc:529-539 already covers this — combining any original_ip_detection_extensions with use_remote_address (or xff_num_trusted_hops > 0) is rejected at config-load. The new extension automatically inherits this enforcement.

Merge main:
Merged upstream/main into the branch (per CONTRIBUTING.md guidance to merge rather than rebase under review).

@owenammann
Copy link
Copy Markdown
Author

One thing I'd like a steer on: security_posture. I've matched custom_header's robust_to_untrusted_downstream because the trust contract for x-envoy-external-address is established by core Envoy — written by an edge HCM under use_remote_address: true, consumed by interior proxies that trust the upstream Envoy. Operators who need stricter sanitization on an interior proxy can already add the header to route_config.internal_only_headers.

That's the framing I used in the proto comment and in headers.rst. But I want to flag it explicitly: this extension consumes the existing header, it doesn't change the trust model. If you'd prefer the more conservative requires_trusted_downstream_and_upstream posture, I can switch — just want to surface the choice. Happy to defer to whichever framing matches how you think about other consumers of x-envoy-external-address.

@owenammann
Copy link
Copy Markdown
Author

Per EXTENSION_POLICY.md this needs a sponsoring maintainer plus two reviewers codified in CODEOWNERS. Could one of you point me to the right maintainer to ask, or suggest names for the CODEOWNERS slots? Happy to update CODEOWNERS in this PR once we have names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants