security/oidc: route discovery and JWKS through a forward proxy#30268
Open
pgellert wants to merge 5 commits intoredpanda-data:devfrom
Open
security/oidc: route discovery and JWKS through a forward proxy#30268pgellert wants to merge 5 commits intoredpanda-data:devfrom
pgellert wants to merge 5 commits intoredpanda-data:devfrom
Conversation
pgellert
requested review from
Copilot and
nguyen-andrew
and removed request for
a team
Contributor
There was a problem hiding this comment.
Pull request overview
Adds forward-proxy (HTTP CONNECT) support for OIDC discovery + JWKS retrieval so brokers can reach external IdPs from corporate-proxy environments, and introduces ducktape coverage to validate proxy routing + misconfiguration rejection.
Changes:
- Add optional forward-proxy CONNECT tunneling support to
net::base_transportand thread it into OIDC HTTP fetches. - Introduce new cluster config
oidc_http_proxyplus commit-time cross-field validation against plaintextoidc_discovery_url. - Add ducktape tests + a
MitmproxyService(and docker dependency) to validate HTTP/HTTPS proxy variants and rejection paths.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rptest/tests/redpanda_oauth_proxy_test.py | New ducktape tests for OIDC discovery/JWKS via proxy + rejection behavior |
| tests/rptest/services/mitmproxy.py | New ducktape service wrapper to run mitmproxy as a CONNECT-only forward proxy |
| tests/docker/ducktape-deps/mitmproxy | Installs mitmproxy in the ducktape image for proxy-based tests |
| tests/docker/Dockerfile | Wires mitmproxy dependency install into ducktape image build |
| src/v/security/oidc_service.h | Adds http_proxy binding to OIDC service constructor |
| src/v/security/oidc_service.cc | Routes OIDC discovery/JWKS HTTP client through forward proxy and adds runtime backstop rejection |
| src/v/redpanda/admin/server.cc | Adds cross-field validator call for oidc_http_proxy on config patch |
| src/v/net/transport.h | Adds optional proxy configuration and proxy_connect_error for CONNECT failures |
| src/v/net/transport.cc | Implements proxy connect flow (connect to proxy, optional TLS, CONNECT handshake, then origin TLS) |
| src/v/config/validators.h | Declares validate_oidc_http_proxy |
| src/v/config/validators.cc | Implements commit-time rejection for proxy + plaintext discovery URL |
| src/v/config/configuration.h | Adds oidc_http_proxy property to configuration |
| src/v/config/configuration.cc | Defines oidc_http_proxy property and per-property parsing validator |
| src/v/cluster/controller.cc | Plumbs oidc_http_proxy binding into OIDC service wiring |
Comment on lines
+491
to
+496
| vlog( | ||
| seclog.debug, | ||
| "OIDC: routing request to {} via HTTP proxy {}", | ||
| url, | ||
| proxy_url_str); | ||
| } |
There was a problem hiding this comment.
proxy_url_str (sourced from oidc_http_proxy) is logged verbatim and also included in error details. If an operator provides a proxy URL with userinfo (e.g. http://user:pass@proxy:port), this would leak credentials into logs and error messages. Consider either rejecting URLs containing userinfo in validation, or redacting userinfo before logging/formatting error strings.
pgellert
force-pushed
the
feat/http-proxy-2
branch
from
50ff440 to
6f25e75
Compare
Collaborator
Retry command for Build#83584please wait until all jobs are finished before running the slash command |
Collaborator
pgellert
force-pushed
the
feat/http-proxy-2
branch
from
6f25e75 to
d818bab
Compare
Contributor
Author
|
force-push: address CI failure (add a |
Extends base_transport::configuration with an optional proxy_config
sub-struct and a matching proxy_connect_error exception type. When
configuration::proxy is set, do_connect now:
1. TCP-connects to the proxy (not the origin).
2. Optionally TLS-wraps the proxy socket for https:// proxies, with
SNI derived from the proxy address. Skipped for http:// proxies.
3. Sends an HTTP CONNECT request for the origin; requires a 200
response. Throws proxy_connect_error on failure, naming the
proxy and origin for operator-visible diagnostics.
4. TLS-wraps the tunnelled socket to the origin as today (gated on
configuration::credentials), with SNI set to the origin hostname.
The ordering is non-commutative and matches RFC 9110 §9.3.6:
- Plaintext proxy, HTTPS origin: TCP → CONNECT → TLS(origin)
- TLS proxy, HTTPS origin: TCP → TLS(proxy) → CONNECT → TLS(origin)
The CONNECT response parser bounds both per-line length and total
response-header bytes so a misbehaving proxy cannot force unbounded
allocation on this control-plane path.
The v1 caller is the OIDC service; other callers (RPC, cloud storage,
metrics reporter, etc.) remain unaware of the field.
parse_url silently drops userinfo (user:pass@) when extracting host/port, so embedded credentials never reach the wire. Rejecting at the parser covers every oidc URL caller, including oidc_discovery_url — which previously accepted (and silently stripped) such URLs.
New cluster config consumed in the next commit by the OIDC service to route discovery and JWKS fetches through an HTTP forward proxy. The property is live-reloadable (needs_restart::no) and user-visible. Commit-time validation: - per-property: rejects values that do not parse as valid absolute URLs. - cross-field: validate_oidc_http_proxy_url rejects a PATCH that would leave oidc_http_proxy_url set alongside a non-https oidc_discovery_url. Follows the existing config_multi_property_validation pattern used for cloud storage, iceberg REST catalog, partition balancer, and default storage mode.
When oidc_http_proxy_url is set, the OIDC service parses the URL and attaches it to the base_transport configuration for each discovery / JWKS fetch. Both http:// and https:// proxy URL schemes are supported; an https:// proxy triggers a TLS handshake with the proxy using the OIDC service's system-trust credentials before CONNECT is sent, with the origin TLS handshake tunnelled inside. The combination of oidc_http_proxy_url + plaintext oidc_discovery_url is rejected at request time with a named operator-facing error (CONNECT tunneling cannot be used for plaintext origins; RFC 9112 §3.2.2). The commit-time cross-field validator in config/validators.cc is the primary gate; the runtime rejection here is defense-in-depth for the bootstrap-config path. Live-reloadable: changing oidc_http_proxy_url triggers a metadata refresh without a broker restart.
Five ducktape tests covering the behavioural matrix: - OIDCViaProxyTest: http:// proxy + https:// origin, OAUTHBEARER end-to-end via mitmproxy. iptables DROP on direct Keycloak egress enforces the proxy path. - OIDCViaHttpsProxyTest: https:// proxy + https:// origin (nested TLS). mitmproxy's regular-mode listener autodetects TLS via --certs; the cert is signed by the existing TLSCertManager CA. - OIDCProxyRejectsPlaintextOriginTest: runtime rejection via the bootstrap-config path. - OIDCProxyRejectedAtConfigCommitTest: cross-field validator via admin PATCH, both directions (adding proxy while discovery is http, and reverting discovery to http while proxy is set). The two new test files are added to the pyright "standard" list in type-check-strictness.json, matching the strictness of the sibling redpanda_oauth_test.py / keycloak.py they inherit from.
pgellert
force-pushed
the
feat/http-proxy-2
branch
from
d818bab to
1a7dc20
Compare
Contributor
Author
|
force-push: address copilot code review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds forward-proxy support for OIDC discovery and JWKS fetches so Redpanda brokers in corporate-proxy environments can reach external IdPs (Azure AD / Entra ID, Okta, Keycloak behind a proxy, etc.).
The transport layer (net::base_transport) learns HTTP CONNECT tunneling via an optional proxy_config on its configuration struct. A new cluster config oidc_http_proxy_url is the only opt-in caller in v1: OIDC's
discovery and JWKS fetches route through the proxy when it's set. Both http:// and https:// proxy URL schemes are supported; origins must be https:// (CONNECT tunneling requires a TLS origin, and
plaintext-origin proxying would need absolute-form HTTP request rewriting per RFC 9112 §3.2.2, not implemented). The bad combination of oidc_http_proxy_url set with a non-https oidc_discovery_url is rejected at
cluster-config commit time via a cross-field validator in config::validate_oidc_http_proxy, plus a runtime backstop in oidc_service::make_request for the bootstrap-config path that bypasses the admin API.
Review guidance
This is a good intro/recap of how forward proxies work: https://docs.mitmproxy.org/stable/concepts/how-mitmproxy-works/
Fixes https://redpandadata.atlassian.net/browse/CORE-16095
Backports Required
Release Notes
Features