mirror of
https://github.com/oauth2-proxy/oauth2-proxy.git
synced 2026-10-18 04:41:19 +02:00
Merge commit from fork
* fix: validate trusted IP proxy headers Respect trusted proxy boundaries before using real-client-IP headers for authentication bypass decisions and safely traverse X-Forwarded-For chains. Signed-off-by: Jan Larwig <jan@larwig.com> * fix: trusted-ip header bypass Signed-off-by: Jan Larwig <jan@larwig.com> * docs: add changelog entry Signed-off-by: Jan Larwig <jan.larwig@digits.schwarz> --------- Signed-off-by: Jan Larwig <jan@larwig.com> Signed-off-by: Jan Larwig <jan.larwig@digits.schwarz>
This commit is contained in:
Jan Larwig
+23
-2
@@ -17,11 +17,12 @@ The Bitbucket provider `--bitbucket-team` flag got deprecated and we added `--bi
|
||||
|
||||
Additionally refer to OAuth client configuration for Bitbucket provider in the [documentation](https://oauth2-proxy.github.io/oauth2-proxy/configuration/providers/bitbucket/). for changes in the scopes (Account>Read) is now required if you restrict by workspace.
|
||||
|
||||
## Breaking Changes
|
||||
## Critical Fixes
|
||||
|
||||
For a small subset of deployments these fixes might be breaking change for the sake of fixing trust/security boundaries.
|
||||
|
||||
### (Critical) [GHSA-63jm-59jj-478j](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-63jm-59jj-478j) Authentication bypass via inconsistent skip-auth path interpretation
|
||||
**Skip-auth path matching is now stricter.** `--skip-auth-route` and
|
||||
**Skip-auth path matching is now stricter**: `--skip-auth-route` and
|
||||
`--skip-auth-regex` no longer grant exemptions for invalid or ambiguous paths,
|
||||
even when a positive or negated rule would otherwise match. This includes dot
|
||||
segments, repeated slashes (including leading `//`), semicolons, backslashes,
|
||||
@@ -41,6 +42,26 @@ matching behavior. The fix does not rewrite upstream request targets or
|
||||
unconditionally reject authenticated requests; existing router behavior and
|
||||
separately configured exemptions remain unchanged.
|
||||
|
||||
### (Critical) [GHSA-wr5q-7wxw-x568](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-wr5q-7wxw-x568) Authentication bypass via spoofed client-IP headers in OAuth2 Proxy
|
||||
**Trusted client-IP resolution now enforces trusted proxy boundaries**: When
|
||||
`--reverse-proxy` is enabled, client-IP headers configured via
|
||||
`--real-client-ip-header` are accepted only from direct peers that match
|
||||
`--trusted-proxy-ip`. If the direct peer is not trusted, the transport peer
|
||||
address is evaluated instead.
|
||||
|
||||
For `X-Forwarded-For`, the chain is traversed from right to left, skipping
|
||||
intermediate proxies in `--trusted-proxy-ip` and treating the first untrusted
|
||||
address as the client. Leftmost entries cannot be spoofed past the trusted proxy
|
||||
boundary. If `--trusted-proxy-ip` is left unset, OAuth2 Proxy preserves backwards
|
||||
compatibility by trusting all source addresses and retaining the leftmost
|
||||
behavior, which does not protect against spoofed XFF headers. To secure
|
||||
`--trusted-ip` exemptions behind a reverse proxy, configure explicit, narrowly
|
||||
scoped `--trusted-proxy-ip` CIDR ranges and ensure the proxy sanitizes client-IP
|
||||
headers. Missing or malformed client-IP headers from a trusted proxy will no
|
||||
longer fall back to granting exemptions based on the proxy's own IP.
|
||||
|
||||
## Breaking Changes
|
||||
|
||||
## Changes since v7.15.4
|
||||
|
||||
- [#3546](https://github.com/oauth2-proxy/oauth2-proxy/pull/3546) fix: strip the port from the request host when matching cookie domains @kirilju
|
||||
|
||||
@@ -32,6 +32,11 @@ When integrating with Kubernetes web UIs, make sure to:
|
||||
Most integrations require the following OAuth2 Proxy configuration:
|
||||
|
||||
- `--reverse-proxy=true`: Required to correctly handle `X-Forwarded-*` headers
|
||||
- `--trusted-proxy-ip=<proxy CIDR>`: Restricts forwarded headers, including
|
||||
client identity used by `--trusted-ip`, to the reverse proxies' addresses or
|
||||
networks. Include the direct peer and every trusted hop that can appear in
|
||||
XFF. Without this option, all source addresses are trusted for backwards
|
||||
compatibility.
|
||||
- **Session storage**: For production deployments with large tokens due to a lot of claims like AD groups, use `--session-store-type=redis`
|
||||
|
||||
For provider-specific configuration, see the [OAuth Provider Configuration](../providers/index.md) documentation.
|
||||
|
||||
@@ -15,6 +15,17 @@ Configure OAuth2 Proxy with Nginx using the `auth_request` directive. Includes e
|
||||
|
||||
**This option requires `--reverse-proxy` option to be set.**
|
||||
|
||||
When Nginx connects from localhost as shown below, also configure OAuth2 Proxy
|
||||
with `--trusted-proxy-ip=127.0.0.1/32`. The example overwrites `X-Real-IP` with
|
||||
Nginx's transport peer address; do not pass through a client-supplied value,
|
||||
especially when `--trusted-ip` is enabled.
|
||||
|
||||
If using `--real-client-ip-header=X-Forwarded-For` with an appending proxy
|
||||
configuration such as `$proxy_add_x_forwarded_for`, list every trusted proxy hop
|
||||
in `--trusted-proxy-ip`. OAuth2 Proxy selects the first untrusted address from
|
||||
the right, not a client-supplied leftmost entry. The trust-all compatibility
|
||||
default cannot provide this protection.
|
||||
|
||||
The [Nginx `auth_request` directive](http://nginx.org/en/docs/http/ngx_http_auth_request_module.html) allows Nginx to authenticate requests via the oauth2-proxy's `/auth` endpoint, which only returns a 202 Accepted response or a 401 Unauthorized response without proxying the request through. For example:
|
||||
|
||||
```nginx
|
||||
|
||||
@@ -194,9 +194,40 @@ Provider specific options can be found on their respective subpages.
|
||||
### Proxy Options
|
||||
|
||||
:::warning
|
||||
When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or CIDR ranges of the reverse proxies that are allowed to send `X-Forwarded-*` headers. If you leave it unset, OAuth2 Proxy currently trusts all source IPs for backwards compatibility, which means a client that can reach OAuth2 Proxy directly may be able to spoof forwarded headers.
|
||||
When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or CIDR ranges of the reverse proxies that are allowed to send `X-Forwarded-*` headers. Include the direct peer and every trusted proxy hop that can appear in `X-Forwarded-For`. If you leave it unset, OAuth2 Proxy currently trusts all source IPs for backwards compatibility, which means a client that can reach OAuth2 Proxy directly may be able to spoof forwarded headers.
|
||||
:::
|
||||
|
||||
### Trusted client IPs behind a reverse proxy
|
||||
|
||||
`--trusted-ip` is an authentication bypass, not a list of trusted reverse
|
||||
proxies. When combining it with `--reverse-proxy`, always configure
|
||||
`--trusted-proxy-ip` with the direct proxy and every trusted intermediate proxy
|
||||
that can appear in XFF, and prevent clients from reaching OAuth2 Proxy directly.
|
||||
|
||||
OAuth2 Proxy accepts `--real-client-ip-header` only from those trusted peers.
|
||||
For `X-Forwarded-For`, it combines repeated header fields in their received order,
|
||||
then walks the chain from right to left and skips addresses covered by
|
||||
`--trusted-proxy-ip`. The first untrusted address is treated as the client;
|
||||
entries to its left are not used. This also applies to client-IP logging and
|
||||
callback diagnostics. For example, if a trusted proxy appends the actual client
|
||||
to `10.0.0.5, 198.51.100.23`, the client is `198.51.100.23`, not the potentially
|
||||
forged `10.0.0.5`. For single-value headers such as `X-Real-IP`, configure the
|
||||
edge proxy to overwrite any value supplied by the client.
|
||||
|
||||
If every address in the chain is in a trusted proxy range, the leftmost address
|
||||
is used for compatibility. Leaving `--trusted-proxy-ip` unset trusts all IPv4 and
|
||||
IPv6 addresses, so it retains this leftmost behavior and does **not** protect
|
||||
against forged XFF entries. Explicit, narrowly scoped proxy ranges are required.
|
||||
|
||||
For a trusted proxy, a missing client-IP header or malformed address encountered
|
||||
before reaching the client does not grant a trusted-IP exemption, even if the
|
||||
proxy's transport IP is in `--trusted-ip`. Logging falls back to the transport IP in these cases;
|
||||
callback diagnostics retain the `transport IP (client IP)` format when both are
|
||||
available. When reverse-proxy mode is disabled or the peer is not trusted, only
|
||||
the transport IP is evaluated. Unix socket connections have no transport IP to
|
||||
exempt; in reverse-proxy mode they retain the existing behavior of accepting
|
||||
client-IP headers, with the same XFF chain resolution.
|
||||
|
||||
| Flag / Config Field | Type | Description | Default |
|
||||
| ----------------------------------------------------------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- |
|
||||
| flag: `--allow-query-semicolons`<br/>toml: `allow_query_semicolons` | bool | allow the use of semicolons in query args ([required for some legacy applications](https://github.com/golang/go/issues/25192)) | `false` |
|
||||
@@ -211,11 +242,11 @@ When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or
|
||||
| flag: `--htpasswd-file`<br/>toml: `htpasswd_file` | string | additionally authenticate against a htpasswd file. Entries must be created with `htpasswd -B` for bcrypt encryption | |
|
||||
| flag: `--htpasswd-user-group`<br/>toml: `htpasswd_user_groups` | string \| list | the groups to be set on sessions for htpasswd users | |
|
||||
| flag: `--proxy-prefix`<br/>toml: `proxy_prefix` | string | the url root path that this proxy should be nested under (e.g. /`<oauth2>/sign_in`) | `"/oauth2"` |
|
||||
| flag: `--real-client-ip-header`<br/>toml: `real_client_ip_header` | string | Header used to determine the real IP of the client, requires `--reverse-proxy` to be set (one of: X-Forwarded-For, X-Real-IP, X-ProxyUser-IP, X-Envoy-External-Address, or CF-Connecting-IP) | X-Real-IP |
|
||||
| flag: `--real-client-ip-header`<br/>toml: `real_client_ip_header` | string | Header used to determine the real IP of the client when the direct caller matches `--trusted-proxy-ip`; requires `--reverse-proxy` (one of: X-Forwarded-For, X-Real-IP, X-ProxyUser-IP, X-Envoy-External-Address, or CF-Connecting-IP) | X-Real-IP |
|
||||
| flag: `--redirect-url`<br/>toml: `redirect_url` | string | the OAuth Redirect URL, e.g. `"https://internalapp.yourcompany.com/oauth2/callback"` | |
|
||||
| flag: `--relative-redirect-url`<br/>toml: `relative_redirect_url` | bool | allow relative OAuth Redirect URL.` | false |
|
||||
| flag: `--reverse-proxy`<br/>toml: `reverse_proxy` | bool | are we running behind a reverse proxy, controls whether headers like X-Real-IP are accepted and allows X-Forwarded-\{Proto,Host,Uri\} headers to be used on redirect selection | false |
|
||||
| flag: `--trusted-proxy-ip`<br/>toml: `trusted_proxy_ips` | string \| list | list of IPs or CIDR ranges allowed to supply `X-Forwarded-*` headers when `--reverse-proxy` is enabled. If not set, OAuth2 Proxy preserves backwards compatibility by trusting all source IPs (`0.0.0.0/0`, `::/0`) and logs a warning at startup. Configure this to your reverse proxy addresses to prevent forwarded header spoofing. | `"0.0.0.0/0", "::/0"` |
|
||||
| flag: `--trusted-proxy-ip`<br/>toml: `trusted_proxy_ips` | string \| list | list of direct proxy IPs or CIDR ranges allowed to supply `X-Forwarded-*` headers when `--reverse-proxy` is enabled. Include every trusted proxy hop that can appear in XFF. If not set, OAuth2 Proxy preserves backwards compatibility by trusting all source IPs (`0.0.0.0/0`, `::/0`) and logs a warning at startup. | `"0.0.0.0/0", "::/0"` |
|
||||
| flag: `--signature-key`<br/>toml: `signature_key` | string | GAP-Signature request signature key (algorithm:secretkey) | |
|
||||
| flag: `--skip-auth-preflight`<br/>toml: `skip_auth_preflight` | bool | will skip authentication for OPTIONS requests | false |
|
||||
| flag: `--skip-auth-regex`<br/>toml: `skip_auth_regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests whose paths match (may be given multiple times). Matching uses the decoded path without query parameters; invalid or ambiguous paths cannot grant a path exemption. See [Skip-auth path matching](#skip-auth-path-matching). | |
|
||||
@@ -223,7 +254,7 @@ When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or
|
||||
| flag: `--skip-jwt-bearer-tokens`<br/>toml: `skip_jwt_bearer_tokens` | bool | will skip requests that have verified JWT bearer tokens (the token must have [`aud`](https://en.wikipedia.org/wiki/JSON_Web_Token#Standard_fields) that matches this client id or one of the extras from `extra-jwt-issuers`) | false |
|
||||
| flag: `--skip-provider-button`<br/>toml: `skip_provider_button` | bool | will skip sign-in-page to directly reach the next step: oauth/start | false |
|
||||
| flag: `--ssl-insecure-skip-verify`<br/>toml: `ssl_insecure_skip_verify` | bool | skip validation of certificates presented when using HTTPS providers | false |
|
||||
| flag: `--trusted-ip`<br/>toml: `trusted_ips` | string \| list | list of IPs or CIDR ranges to allow to bypass authentication (may be given multiple times). When combined with `--reverse-proxy` and optionally `--real-client-ip-header` this will evaluate the trust of the IP stored in an HTTP header by a reverse proxy rather than the layer-3/4 remote address. WARNING: trusting IPs has inherent security flaws, especially when obtaining the IP address from an HTTP header (reverse-proxy mode). Use this option only if you understand the risks and how to manage them. | |
|
||||
| flag: `--trusted-ip`<br/>toml: `trusted_ips` | string \| list | list of client IPs or CIDR ranges allowed to bypass authentication (may be given multiple times). With `--reverse-proxy`, the selected real-client-IP header is accepted only from direct callers matching `--trusted-proxy-ip`; otherwise the transport peer address is used. WARNING: this option bypasses authentication. Configure explicit trusted proxy ranges and ensure the proxy sanitizes client-IP headers. | |
|
||||
| flag: `--whitelist-domain`<br/>toml: `whitelist_domains` | string \| list | allowed domains for redirection after authentication. Prefix domain with a `.` or a `*.` to allow subdomains (e.g. `.example.com`, `*.example.com`) [^2] | |
|
||||
|
||||
[^2]: When using the `whitelist-domain` option, any domain prefixed with a `.` or a `*.` will allow any subdomain of the specified domain as a valid redirect URL. By default, only empty ports are allowed. This translates to allowing the default port of the URL's protocol (80 for HTTP, 443 for HTTPS, etc.) since browsers omit them. To allow only a specific port, add it to the whitelisted domain: `example.com:8080`. To allow any port, use `*`: `example.com:*`.
|
||||
|
||||
@@ -32,6 +32,11 @@ When integrating with Kubernetes web UIs, make sure to:
|
||||
Most integrations require the following OAuth2 Proxy configuration:
|
||||
|
||||
- `--reverse-proxy=true`: Required to correctly handle `X-Forwarded-*` headers
|
||||
- `--trusted-proxy-ip=<proxy CIDR>`: Restricts forwarded headers, including
|
||||
client identity used by `--trusted-ip`, to the reverse proxies' addresses or
|
||||
networks. Include the direct peer and every trusted hop that can appear in
|
||||
XFF. Without this option, all source addresses are trusted for backwards
|
||||
compatibility.
|
||||
- **Session storage**: For production deployments with large tokens due to a lot of claims like AD groups, use `--session-store-type=redis`
|
||||
|
||||
For provider-specific configuration, see the [OAuth Provider Configuration](../providers/index.md) documentation.
|
||||
|
||||
@@ -15,6 +15,17 @@ Configure OAuth2 Proxy with Nginx using the `auth_request` directive. Includes e
|
||||
|
||||
**This option requires `--reverse-proxy` option to be set.**
|
||||
|
||||
When Nginx connects from localhost as shown below, also configure OAuth2 Proxy
|
||||
with `--trusted-proxy-ip=127.0.0.1/32`. The example overwrites `X-Real-IP` with
|
||||
Nginx's transport peer address; do not pass through a client-supplied value,
|
||||
especially when `--trusted-ip` is enabled.
|
||||
|
||||
If using `--real-client-ip-header=X-Forwarded-For` with an appending proxy
|
||||
configuration such as `$proxy_add_x_forwarded_for`, list every trusted proxy hop
|
||||
in `--trusted-proxy-ip`. OAuth2 Proxy selects the first untrusted address from
|
||||
the right, not a client-supplied leftmost entry. The trust-all compatibility
|
||||
default cannot provide this protection.
|
||||
|
||||
The [Nginx `auth_request` directive](http://nginx.org/en/docs/http/ngx_http_auth_request_module.html) allows Nginx to authenticate requests via the oauth2-proxy's `/auth` endpoint, which only returns a 202 Accepted response or a 401 Unauthorized response without proxying the request through. For example:
|
||||
|
||||
```nginx
|
||||
@@ -60,23 +71,23 @@ server {
|
||||
auth_request_set $auth_cookie $upstream_http_set_cookie;
|
||||
add_header Set-Cookie $auth_cookie;
|
||||
|
||||
# When using the --set-authorization-header flag, some provider's cookies can exceed the 4kb
|
||||
# limit and so the OAuth2 Proxy splits these into multiple parts.
|
||||
# Large OIDC tokens can make the session cookie exceed the 4KB limit,
|
||||
# causing OAuth2 Proxy to split the cookie into multiple parts.
|
||||
# Nginx normally only copies the first `Set-Cookie` header from the auth_request to the response,
|
||||
# so if your cookies are larger than 4kb, you will need to extract additional cookies manually.
|
||||
auth_request_set $auth_cookie_name_upstream_1 $upstream_cookie_auth_cookie_name_1;
|
||||
# so extract the second part when using the default cookie name, `_oauth2_proxy`.
|
||||
auth_request_set $auth_cookie_upstream_1 $upstream_cookie__oauth2_proxy_1;
|
||||
|
||||
# Extract the Cookie attributes from the first Set-Cookie header and append them
|
||||
# to the second part ($upstream_cookie_* variables only contain the raw cookie content)
|
||||
if ($auth_cookie ~* "(; .*)") {
|
||||
set $auth_cookie_name_0 $auth_cookie;
|
||||
set $auth_cookie_name_1 "auth_cookie_name_1=$auth_cookie_name_upstream_1$1";
|
||||
set $auth_cookie_0 $auth_cookie;
|
||||
set $auth_cookie_1 "_oauth2_proxy_1=$auth_cookie_upstream_1$1";
|
||||
}
|
||||
|
||||
# Send both Set-Cookie headers now if there was a second part
|
||||
if ($auth_cookie_name_upstream_1) {
|
||||
add_header Set-Cookie $auth_cookie_name_0;
|
||||
add_header Set-Cookie $auth_cookie_name_1;
|
||||
if ($auth_cookie_upstream_1) {
|
||||
add_header Set-Cookie $auth_cookie_0;
|
||||
add_header Set-Cookie $auth_cookie_1;
|
||||
}
|
||||
|
||||
proxy_pass http://backend/;
|
||||
@@ -91,6 +102,24 @@ server {
|
||||
}
|
||||
```
|
||||
|
||||
### Multi-part session cookies
|
||||
|
||||
The example above is ready to use with the default `--cookie-name=_oauth2_proxy` setting and handles a session split into two cookies. If you configure a custom cookie name, update both references to the second cookie part:
|
||||
|
||||
| Cookie configuration | Nginx upstream cookie variable | `Set-Cookie` name |
|
||||
| --- | --- | --- |
|
||||
| Default (`_oauth2_proxy`) | `$upstream_cookie__oauth2_proxy_1` | `_oauth2_proxy_1=` |
|
||||
| `--cookie-name=my_cookie` | `$upstream_cookie_my_cookie_1` | `my_cookie_1=` |
|
||||
|
||||
For example, with `--cookie-name=my_cookie`, change the relevant lines to:
|
||||
|
||||
```nginx
|
||||
auth_request_set $auth_cookie_upstream_1 $upstream_cookie_my_cookie_1;
|
||||
set $auth_cookie_1 "my_cookie_1=$auth_cookie_upstream_1$1";
|
||||
```
|
||||
|
||||
If a session is split into more than two cookies, repeat the extraction for each additional numeric suffix. For consistently large sessions, prefer `--session-store-type=redis` so only a small session ticket is stored in the browser cookie.
|
||||
|
||||
### Understanding the `error_page` redirect pattern
|
||||
|
||||
The `auth_request` directive expects the authentication endpoint (`/oauth2/auth`) to return:
|
||||
@@ -137,12 +166,11 @@ location @oauth2_signin {
|
||||
|
||||
#### API / Machine routes (no redirect)
|
||||
|
||||
For API endpoints where clients expect a 401/403 status code (not a redirect):
|
||||
For API endpoints where clients expect a 401/403 status code (not a redirect), reuse the same `auth_request` header forwarding configuration from the main example as needed for your backend:
|
||||
|
||||
```nginx
|
||||
location /api/ {
|
||||
auth_request /oauth2/auth;
|
||||
error_page 401 =401; # Pass through the 401 status
|
||||
proxy_pass http://backend/;
|
||||
}
|
||||
```
|
||||
@@ -167,8 +195,6 @@ It is recommended to use `--session-store-type=redis` when expecting large sessi
|
||||
For a complete example of integrating oauth2-proxy with Kubernetes Dashboard on AKS using Azure Entra ID, including RBAC configuration and troubleshooting, see the [Kubernetes Dashboard on AKS](../providers/ms_entra_id.md#kubernetes-dashboard-on-aks) section in the Microsoft Entra ID provider documentation.
|
||||
:::
|
||||
|
||||
You have to substitute *name* with the actual cookie name you configured via --cookie-name parameter. If you don't set a custom cookie name the variable should be "$upstream_cookie__oauth2_proxy_1" instead of "$upstream_cookie_name_1" and the new cookie-name should be "_oauth2_proxy_1=" instead of "name_1=".
|
||||
|
||||
:::note
|
||||
If you set up your OAuth2 provider to rotate your client secret, you can use the `client-secret-file` option to reload the secret when it is updated.
|
||||
:::
|
||||
|
||||
@@ -194,9 +194,40 @@ Provider specific options can be found on their respective subpages.
|
||||
### Proxy Options
|
||||
|
||||
:::warning
|
||||
When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or CIDR ranges of the reverse proxies that are allowed to send `X-Forwarded-*` headers. If you leave it unset, OAuth2 Proxy currently trusts all source IPs for backwards compatibility, which means a client that can reach OAuth2 Proxy directly may be able to spoof forwarded headers.
|
||||
When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or CIDR ranges of the reverse proxies that are allowed to send `X-Forwarded-*` headers. Include the direct peer and every trusted proxy hop that can appear in `X-Forwarded-For`. If you leave it unset, OAuth2 Proxy currently trusts all source IPs for backwards compatibility, which means a client that can reach OAuth2 Proxy directly may be able to spoof forwarded headers.
|
||||
:::
|
||||
|
||||
### Trusted client IPs behind a reverse proxy
|
||||
|
||||
`--trusted-ip` is an authentication bypass, not a list of trusted reverse
|
||||
proxies. When combining it with `--reverse-proxy`, always configure
|
||||
`--trusted-proxy-ip` with the direct proxy and every trusted intermediate proxy
|
||||
that can appear in XFF, and prevent clients from reaching OAuth2 Proxy directly.
|
||||
|
||||
OAuth2 Proxy accepts `--real-client-ip-header` only from those trusted peers.
|
||||
For `X-Forwarded-For`, it combines repeated header fields in their received order,
|
||||
then walks the chain from right to left and skips addresses covered by
|
||||
`--trusted-proxy-ip`. The first untrusted address is treated as the client;
|
||||
entries to its left are not used. This also applies to client-IP logging and
|
||||
callback diagnostics. For example, if a trusted proxy appends the actual client
|
||||
to `10.0.0.5, 198.51.100.23`, the client is `198.51.100.23`, not the potentially
|
||||
forged `10.0.0.5`. For single-value headers such as `X-Real-IP`, configure the
|
||||
edge proxy to overwrite any value supplied by the client.
|
||||
|
||||
If every address in the chain is in a trusted proxy range, the leftmost address
|
||||
is used for compatibility. Leaving `--trusted-proxy-ip` unset trusts all IPv4 and
|
||||
IPv6 addresses, so it retains this leftmost behavior and does **not** protect
|
||||
against forged XFF entries. Explicit, narrowly scoped proxy ranges are required.
|
||||
|
||||
For a trusted proxy, a missing client-IP header or malformed address encountered
|
||||
before reaching the client does not grant a trusted-IP exemption, even if the
|
||||
proxy's transport IP is in `--trusted-ip`. Logging falls back to the transport IP in these cases;
|
||||
callback diagnostics retain the `transport IP (client IP)` format when both are
|
||||
available. When reverse-proxy mode is disabled or the peer is not trusted, only
|
||||
the transport IP is evaluated. Unix socket connections have no transport IP to
|
||||
exempt; in reverse-proxy mode they retain the existing behavior of accepting
|
||||
client-IP headers, with the same XFF chain resolution.
|
||||
|
||||
| Flag / Config Field | Type | Description | Default |
|
||||
| ----------------------------------------------------------------------------- | -------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- |
|
||||
| flag: `--allow-query-semicolons`<br/>toml: `allow_query_semicolons` | bool | allow the use of semicolons in query args ([required for some legacy applications](https://github.com/golang/go/issues/25192)) | `false` |
|
||||
@@ -211,19 +242,19 @@ When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or
|
||||
| flag: `--htpasswd-file`<br/>toml: `htpasswd_file` | string | additionally authenticate against a htpasswd file. Entries must be created with `htpasswd -B` for bcrypt encryption | |
|
||||
| flag: `--htpasswd-user-group`<br/>toml: `htpasswd_user_groups` | string \| list | the groups to be set on sessions for htpasswd users | |
|
||||
| flag: `--proxy-prefix`<br/>toml: `proxy_prefix` | string | the url root path that this proxy should be nested under (e.g. /`<oauth2>/sign_in`) | `"/oauth2"` |
|
||||
| flag: `--real-client-ip-header`<br/>toml: `real_client_ip_header` | string | Header used to determine the real IP of the client, requires `--reverse-proxy` to be set (one of: X-Forwarded-For, X-Real-IP, X-ProxyUser-IP, X-Envoy-External-Address, or CF-Connecting-IP) | X-Real-IP |
|
||||
| flag: `--real-client-ip-header`<br/>toml: `real_client_ip_header` | string | Header used to determine the real IP of the client when the direct caller matches `--trusted-proxy-ip`; requires `--reverse-proxy` (one of: X-Forwarded-For, X-Real-IP, X-ProxyUser-IP, X-Envoy-External-Address, or CF-Connecting-IP) | X-Real-IP |
|
||||
| flag: `--redirect-url`<br/>toml: `redirect_url` | string | the OAuth Redirect URL, e.g. `"https://internalapp.yourcompany.com/oauth2/callback"` | |
|
||||
| flag: `--relative-redirect-url`<br/>toml: `relative_redirect_url` | bool | allow relative OAuth Redirect URL.` | false |
|
||||
| flag: `--reverse-proxy`<br/>toml: `reverse_proxy` | bool | are we running behind a reverse proxy, controls whether headers like X-Real-IP are accepted and allows X-Forwarded-\{Proto,Host,Uri\} headers to be used on redirect selection | false |
|
||||
| flag: `--trusted-proxy-ip`<br/>toml: `trusted_proxy_ips` | string \| list | list of IPs or CIDR ranges allowed to supply `X-Forwarded-*` headers when `--reverse-proxy` is enabled. If not set, OAuth2 Proxy preserves backwards compatibility by trusting all source IPs (`0.0.0.0/0`, `::/0`) and logs a warning at startup. Configure this to your reverse proxy addresses to prevent forwarded header spoofing. | `"0.0.0.0/0", "::/0"` |
|
||||
| flag: `--trusted-proxy-ip`<br/>toml: `trusted_proxy_ips` | string \| list | list of direct proxy IPs or CIDR ranges allowed to supply `X-Forwarded-*` headers when `--reverse-proxy` is enabled. Include every trusted proxy hop that can appear in XFF. If not set, OAuth2 Proxy preserves backwards compatibility by trusting all source IPs (`0.0.0.0/0`, `::/0`) and logs a warning at startup. | `"0.0.0.0/0", "::/0"` |
|
||||
| flag: `--signature-key`<br/>toml: `signature_key` | string | GAP-Signature request signature key (algorithm:secretkey) | |
|
||||
| flag: `--skip-auth-preflight`<br/>toml: `skip_auth_preflight` | bool | will skip authentication for OPTIONS requests | false |
|
||||
| flag: `--skip-auth-regex`<br/>toml: `skip_auth_regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests paths that match (may be given multiple times) | |
|
||||
| flag: `--skip-auth-route`<br/>toml: `skip_auth_routes` | string \| list | bypass authentication for requests that match the method & path. Format: method=path_regex OR method!=path_regex. For all methods: path_regex OR !=path_regex | |
|
||||
| flag: `--skip-auth-regex`<br/>toml: `skip_auth_regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests whose paths match (may be given multiple times). Matching uses the decoded path without query parameters; invalid or ambiguous paths cannot grant a path exemption. See [Skip-auth path matching](#skip-auth-path-matching). | |
|
||||
| flag: `--skip-auth-route`<br/>toml: `skip_auth_routes` | string \| list | bypass authentication for requests that match the method & path. Format: method=path_regex OR method!=path_regex. For all methods: path_regex OR !=path_regex. Matching uses the decoded path without query parameters; invalid or ambiguous paths cannot grant a path exemption, including with negated rules. See [Skip-auth path matching](#skip-auth-path-matching). | |
|
||||
| flag: `--skip-jwt-bearer-tokens`<br/>toml: `skip_jwt_bearer_tokens` | bool | will skip requests that have verified JWT bearer tokens (the token must have [`aud`](https://en.wikipedia.org/wiki/JSON_Web_Token#Standard_fields) that matches this client id or one of the extras from `extra-jwt-issuers`) | false |
|
||||
| flag: `--skip-provider-button`<br/>toml: `skip_provider_button` | bool | will skip sign-in-page to directly reach the next step: oauth/start | false |
|
||||
| flag: `--ssl-insecure-skip-verify`<br/>toml: `ssl_insecure_skip_verify` | bool | skip validation of certificates presented when using HTTPS providers | false |
|
||||
| flag: `--trusted-ip`<br/>toml: `trusted_ips` | string \| list | list of IPs or CIDR ranges to allow to bypass authentication (may be given multiple times). When combined with `--reverse-proxy` and optionally `--real-client-ip-header` this will evaluate the trust of the IP stored in an HTTP header by a reverse proxy rather than the layer-3/4 remote address. WARNING: trusting IPs has inherent security flaws, especially when obtaining the IP address from an HTTP header (reverse-proxy mode). Use this option only if you understand the risks and how to manage them. | |
|
||||
| flag: `--trusted-ip`<br/>toml: `trusted_ips` | string \| list | list of client IPs or CIDR ranges allowed to bypass authentication (may be given multiple times). With `--reverse-proxy`, the selected real-client-IP header is accepted only from direct callers matching `--trusted-proxy-ip`; otherwise the transport peer address is used. WARNING: this option bypasses authentication. Configure explicit trusted proxy ranges and ensure the proxy sanitizes client-IP headers. | |
|
||||
| flag: `--whitelist-domain`<br/>toml: `whitelist_domains` | string \| list | allowed domains for redirection after authentication. Prefix domain with a `.` or a `*.` to allow subdomains (e.g. `.example.com`, `*.example.com`) [^2] | |
|
||||
|
||||
[^2]: When using the `whitelist-domain` option, any domain prefixed with a `.` or a `*.` will allow any subdomain of the specified domain as a valid redirect URL. By default, only empty ports are allowed. This translates to allowing the default port of the URL's protocol (80 for HTTP, 443 for HTTPS, etc.) since browsers omit them. To allow only a specific port, add it to the whitelisted domain: `example.com:8080`. To allow any port, use `*`: `example.com:*`.
|
||||
@@ -273,6 +304,34 @@ When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or
|
||||
| flag: `--upstream-timeout`<br/>toml: `upstream_timeout` | duration | maximum amount of time the server will wait for a response from the upstream | 30s |
|
||||
| flag: `--upstream`<br/>toml: `upstreams` | string \| list | the http url(s) of the upstream endpoint, file:// paths for static files or `static://<status_code>` for static response. Routing is based on the path | |
|
||||
|
||||
### Skip-auth path matching
|
||||
|
||||
Both `--skip-auth-route` and the deprecated `--skip-auth-regex` match the
|
||||
percent-decoded path, excluding query parameters. In reverse-proxy mode,
|
||||
`X-Forwarded-Uri` is used only when the existing forwarded-header trust checks
|
||||
permit it. Original-URI metadata must use origin form (`/path?query`), not an
|
||||
absolute URL, authority, or relative path.
|
||||
|
||||
Invalid or ambiguous targets do not grant a path-based authentication exemption.
|
||||
This check happens before any regular expression or its negation is evaluated.
|
||||
In particular, exemptions are declined for malformed path escapes, `.` or `..`
|
||||
path segments, repeated slashes (including leading `//`), semicolons, backslashes,
|
||||
fragment-like `#` content, decoded `?` characters, and control characters.
|
||||
Encoded forms are checked after a single decoding pass; paths are not recursively
|
||||
decoded.
|
||||
|
||||
This intentionally also requires normal authentication for valid semicolon paths
|
||||
such as `/public/file;version=1` and repeated-slash paths such as `/public//file`.
|
||||
Fragments are no longer silently stripped. Ordinary query strings do not affect
|
||||
matching. Unambiguous escaped characters remain supported, and trailing slashes
|
||||
remain significant: `/public` and `/public/` are distinct matching paths.
|
||||
|
||||
These checks do not normalize or rewrite the upstream request target. A declined
|
||||
path exemption follows normal authentication and authorization; it is not an
|
||||
unconditional rejection of authenticated requests using unusual paths. Existing
|
||||
router redirects and separately configured exemptions, such as trusted client
|
||||
IPs or skipped preflight requests, are unchanged.
|
||||
|
||||
## Configuration Validation
|
||||
|
||||
The `--config-test` flag validates your configuration file without starting the proxy server. This is useful for:
|
||||
@@ -361,7 +420,7 @@ Logging of requests to the `/ping` endpoint (or using `--ping-user-agent`) and t
|
||||
|
||||
## Auth Log Format
|
||||
|
||||
Authentication logs are logs which are guaranteed to contain a username or email address of a user attempting to authenticate. These logs are output by default in the below format:
|
||||
Authentication logs describe authentication attempts. They include the user's username or email address when known, or `-` otherwise. OAuth callback diagnostics for missing or invalid CSRF cookies are authentication logs, controlled by `--auth-logging`, and include cookie names but not cookie values. These logs are output by default in the below format:
|
||||
|
||||
```
|
||||
<REMOTE_ADDRESS> - <REQUEST ID> - <user@domain.com> [2015/03/19 17:20:19] [<STATUS>] <MESSAGE>
|
||||
@@ -392,7 +451,7 @@ Available variables for auth logging:
|
||||
| RequestMethod | GET | The request method. |
|
||||
| Timestamp | 2015/03/19 17:20:19 | The date and time of the logging event. |
|
||||
| UserAgent | - | The full user agent as reported by the requesting client. |
|
||||
| Username | username@email.com | The email or username of the auth request. |
|
||||
| Username | username@email.com | The email or username of the auth request, or `-` if not yet known. |
|
||||
| Status | AuthSuccess | The status of the auth request. See above for details. |
|
||||
|
||||
## Request Log Format
|
||||
|
||||
+17
-3
@@ -638,10 +638,16 @@ func (p *OAuthProxy) isTrustedIP(req *http.Request) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
remoteAddr, err := ip.GetClientIP(p.realClientIPParser, req)
|
||||
var remoteAddr net.IP
|
||||
var err error
|
||||
scope := middlewareapi.GetRequestScope(req)
|
||||
if scope != nil && scope.CanTrustForwardedHeaders(req) {
|
||||
remoteAddr, err = ip.GetClientIPFromTrustedProxy(p.realClientIPParser, req, scope.TrustedProxies)
|
||||
} else {
|
||||
remoteAddr, err = ip.GetClientIP(nil, req)
|
||||
}
|
||||
if err != nil {
|
||||
logger.Errorf("Error obtaining real IP for trusted IP list: %v", err)
|
||||
// Possibly spoofed X-Real-IP header
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -891,7 +897,15 @@ func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, ove
|
||||
// OAuthCallback is the OAuth2 authentication flow callback that finishes the
|
||||
// OAuth2 authentication flow
|
||||
func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
|
||||
remoteAddr := ip.GetClientString(p.realClientIPParser, req, true)
|
||||
realClientIPParser := p.realClientIPParser
|
||||
var trustedProxies *ip.NetSet
|
||||
scope := middlewareapi.GetRequestScope(req)
|
||||
if scope != nil && scope.CanTrustForwardedHeaders(req) {
|
||||
trustedProxies = scope.TrustedProxies
|
||||
} else {
|
||||
realClientIPParser = nil
|
||||
}
|
||||
remoteAddr := ip.GetClientString(realClientIPParser, req, trustedProxies, true)
|
||||
|
||||
// finish the oauth cycle
|
||||
// #nosec G120 -- The default max size in Go is already capped at 10MB so this would be the absolute max and is
|
||||
|
||||
+286
-8
@@ -5,6 +5,7 @@ import (
|
||||
"context"
|
||||
"crypto"
|
||||
"encoding/base64"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
@@ -25,6 +26,7 @@ import (
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/hmacauth"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
|
||||
internaloidc "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/providers/oidc"
|
||||
sessionscookie "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/sessions/cookie"
|
||||
@@ -575,6 +577,61 @@ func TestOAuthCallbackCSRFCookieLogging(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
type failingSaveSessionStore struct {
|
||||
sessions.SessionStore
|
||||
}
|
||||
|
||||
func (failingSaveSessionStore) Save(http.ResponseWriter, *http.Request, *sessions.SessionState) error {
|
||||
return errors.New("session save failed")
|
||||
}
|
||||
|
||||
func TestOAuthCallbackClientIPLogging(t *testing.T) {
|
||||
test, err := NewPassAccessTokenTest(PassAccessTokenTestOptions{ValidToken: true})
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(test.Close)
|
||||
t.Cleanup(func() { logger.SetErrOutput(ginkgo.GinkgoWriter) })
|
||||
test.proxy.sessionStore = failingSaveSessionStore{test.proxy.sessionStore}
|
||||
test.proxy.realClientIPParser, err = ip.GetRealClientIPParser("X-Forwarded-For")
|
||||
require.NoError(t, err)
|
||||
trustedProxies, err := ip.ParseNetSet([]string{"192.0.2.0/24"})
|
||||
require.NoError(t, err)
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
remoteAddr string
|
||||
headerValues []string
|
||||
expected string
|
||||
}{
|
||||
{"Untrusted peer", "198.51.100.23:1234", []string{"10.0.0.5"}, "198.51.100.23"},
|
||||
{"Appending trusted proxy", "192.0.2.10:1234", []string{"10.0.0.5", "198.51.100.23, 192.0.2.20"}, "192.0.2.10 (198.51.100.23)"},
|
||||
{"Malformed relevant hop", "192.0.2.10:1234", []string{"10.0.0.5, invalid, 192.0.2.20"}, "192.0.2.10"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
csrf, err := cookies.NewCSRF(test.proxy.CookieOptions, "")
|
||||
require.NoError(t, err)
|
||||
req := httptest.NewRequest(http.MethodGet, "/oauth2/callback?code=callback_code&state="+encodeState(csrf.HashOAuthState(), "%2F", false), nil)
|
||||
req.RemoteAddr = tc.remoteAddr
|
||||
for _, value := range tc.headerValues {
|
||||
req.Header.Add("X-Forwarded-For", value)
|
||||
}
|
||||
req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{
|
||||
ReverseProxy: true,
|
||||
TrustedProxies: trustedProxies,
|
||||
})
|
||||
csrfCookie, err := csrf.SetCookie(httptest.NewRecorder(), req)
|
||||
require.NoError(t, err)
|
||||
req.AddCookie(csrfCookie)
|
||||
var output bytes.Buffer
|
||||
logger.SetErrOutput(&output)
|
||||
rw := httptest.NewRecorder()
|
||||
test.proxy.OAuthCallback(rw, req)
|
||||
assert.Equal(t, http.StatusInternalServerError, rw.Code)
|
||||
assert.Contains(t, output.String(), "Error saving session state for "+tc.expected+": session save failed")
|
||||
assert.NotContains(t, output.String(), "10.0.0.5")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// getEndpointWithCookie makes a requests againt the oauthproxy with passed requestPath
|
||||
// and cookie and returns body and status code.
|
||||
func (patTest *PassAccessTokenTest) getEndpointWithCookie(cookie string, endpoint string) (httpCode int, accessToken string) {
|
||||
@@ -2310,9 +2367,18 @@ func baseTestOptions() *options.Options {
|
||||
}
|
||||
|
||||
func TestTrustedIPs(t *testing.T) {
|
||||
xffRequest := func(remoteAddr string, values ...string) *http.Request {
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.RemoteAddr = remoteAddr
|
||||
for _, value := range values {
|
||||
req.Header.Add("X-Forwarded-For", value)
|
||||
}
|
||||
return req
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
trustedIPs []string
|
||||
trustedProxyIPs []string
|
||||
reverseProxy bool
|
||||
realClientIPHeader string
|
||||
req *http.Request
|
||||
@@ -2386,11 +2452,13 @@ func TestTrustedIPs(t *testing.T) {
|
||||
{
|
||||
name: "TrustsLocalhostInReverseProxyMode",
|
||||
trustedIPs: []string{"127.0.0.0/8", "::1"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Add("X-Forwarded-For", "127.0.0.1")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: true,
|
||||
@@ -2399,11 +2467,13 @@ func TestTrustedIPs(t *testing.T) {
|
||||
{
|
||||
name: "TrustsIP6LocalostInReverseProxyMode",
|
||||
trustedIPs: []string{"127.0.0.0/8", "::1"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Add("X-Forwarded-For", "::1")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: true,
|
||||
@@ -2412,11 +2482,13 @@ func TestTrustedIPs(t *testing.T) {
|
||||
{
|
||||
name: "DoesNotTrustRandomIP4Address",
|
||||
trustedIPs: []string{"127.0.0.0/8", "::1"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Add("X-Forwarded-For", "12.34.56.78")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: false,
|
||||
@@ -2425,11 +2497,13 @@ func TestTrustedIPs(t *testing.T) {
|
||||
{
|
||||
name: "DoesNotTrustRandomIP6Address",
|
||||
trustedIPs: []string{"127.0.0.0/8", "::1"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Add("X-Forwarded-For", "::2")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: false,
|
||||
@@ -2438,11 +2512,13 @@ func TestTrustedIPs(t *testing.T) {
|
||||
{
|
||||
name: "RespectsCorrectHeaderInReverseProxyMode",
|
||||
trustedIPs: []string{"127.0.0.0/8", "::1"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Add("X-Real-IP", "::1")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: false,
|
||||
@@ -2451,15 +2527,164 @@ func TestTrustedIPs(t *testing.T) {
|
||||
{
|
||||
name: "DoesNotTrustGarbageInReverseProxyMode",
|
||||
trustedIPs: []string{"127.0.0.0/8", "::1"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Add("X-Forwarded-For", "adsfljk29242as!!")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: false,
|
||||
},
|
||||
{
|
||||
name: "IgnoresRealClientHeaderFromUntrustedPeer",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Real-IP",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set("X-Real-IP", "10.0.0.5")
|
||||
req.RemoteAddr = "198.51.100.23:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: false,
|
||||
},
|
||||
{
|
||||
name: "UsesTransportAddressForUntrustedPeer",
|
||||
trustedIPs: []string{"198.51.100.23"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Real-IP",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set("X-Real-IP", "10.0.0.5")
|
||||
req.RemoteAddr = "198.51.100.23:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: true,
|
||||
},
|
||||
{
|
||||
name: "UsesRealClientHeaderFromTrustedPeer",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Real-IP",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set("X-Real-IP", "10.0.0.5")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: true,
|
||||
},
|
||||
{
|
||||
name: "RejectsSpoofedLeftmostXFFEntry",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.10"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: func() *http.Request {
|
||||
req, _ := http.NewRequest(http.MethodGet, "/", nil)
|
||||
req.Header.Set("X-Forwarded-For", "10.0.0.5, 198.51.100.23")
|
||||
req.RemoteAddr = "192.0.2.10:1234"
|
||||
return req
|
||||
}(),
|
||||
expectTrusted: false,
|
||||
},
|
||||
{
|
||||
name: "IgnoresXFFFromUntrustedPeer",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("198.51.100.23:1234", "10.0.0.5"),
|
||||
},
|
||||
{
|
||||
name: "RejectsSpoofedRepeatedXFF",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("192.0.2.10:1234", "10.0.0.5", "198.51.100.23, 192.0.2.20"),
|
||||
},
|
||||
{
|
||||
name: "TrustsClientThroughMultipleProxies",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("192.0.2.10:1234", " 10.0.0.5:1234 , 192.0.2.20", "192.0.2.21:443"),
|
||||
expectTrusted: true,
|
||||
},
|
||||
{
|
||||
name: "TrustsIPv6ClientThroughIPv6Proxy",
|
||||
trustedIPs: []string{"2001:db8:2::/48"},
|
||||
trustedProxyIPs: []string{"2001:db8:1::/48"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("[2001:db8:1::10]:1234", "[2001:db8:2::23]:1234, 2001:db8:1::20"),
|
||||
expectTrusted: true,
|
||||
},
|
||||
{
|
||||
name: "MalformedRelevantHopIsNotExempt",
|
||||
trustedIPs: []string{"10.0.0.0/8", "192.0.2.0/24"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("192.0.2.10:1234", "10.0.0.5, invalid, 192.0.2.20"),
|
||||
},
|
||||
{
|
||||
name: "MissingXFFDoesNotFallBackToExemptPeer",
|
||||
trustedIPs: []string{"192.0.2.0/24"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("192.0.2.10:1234"),
|
||||
},
|
||||
{
|
||||
name: "EntireChainTrustedUsesLeftmost",
|
||||
trustedIPs: []string{"192.0.2.30"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("192.0.2.10:1234", "192.0.2.30, 192.0.2.20"),
|
||||
expectTrusted: true,
|
||||
},
|
||||
{
|
||||
name: "TrustAllDefaultRetainsLeftmostCompatibility",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("192.0.2.10:1234", "10.0.0.5, 198.51.100.23"),
|
||||
expectTrusted: true,
|
||||
},
|
||||
{
|
||||
name: "ReverseProxyOffIgnoresXFF",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("192.0.2.10:1234", "10.0.0.5"),
|
||||
},
|
||||
{
|
||||
name: "UnixSocketUsesTrustedProxyChain",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("@", "10.0.0.5, 192.0.2.20"),
|
||||
expectTrusted: true,
|
||||
},
|
||||
{
|
||||
name: "UnixSocketRejectsSpoofedChain",
|
||||
trustedIPs: []string{"10.0.0.0/8"},
|
||||
trustedProxyIPs: []string{"192.0.2.0/24"},
|
||||
reverseProxy: true,
|
||||
realClientIPHeader: "X-Forwarded-For",
|
||||
req: xffRequest("@", "10.0.0.5, 198.51.100.23"),
|
||||
},
|
||||
// Check doesn't trust if garbage is provided (no reverse-proxy).
|
||||
{
|
||||
name: "DoesNotTrustGarbage",
|
||||
@@ -2488,25 +2713,78 @@ func TestTrustedIPs(t *testing.T) {
|
||||
},
|
||||
}
|
||||
opts.TrustedIPs = tt.trustedIPs
|
||||
opts.TrustedProxyIPs = tt.trustedProxyIPs
|
||||
opts.ReverseProxy = tt.reverseProxy
|
||||
opts.RealClientIPHeader = tt.realClientIPHeader
|
||||
err := validation.Validate(opts)
|
||||
assert.NoError(t, err)
|
||||
require.NoError(t, err)
|
||||
|
||||
proxy, err := NewOAuthProxy(opts, func(string) bool { return true })
|
||||
assert.NoError(t, err)
|
||||
rw := httptest.NewRecorder()
|
||||
require.NoError(t, err)
|
||||
|
||||
proxy.ServeHTTP(rw, tt.req)
|
||||
if tt.expectTrusted {
|
||||
assert.Equal(t, 200, rw.Code)
|
||||
} else {
|
||||
assert.Equal(t, 403, rw.Code)
|
||||
for _, endpoint := range []struct {
|
||||
name string
|
||||
path string
|
||||
trustedStatus int
|
||||
deniedStatus int
|
||||
}{
|
||||
{"Proxy", "/", http.StatusOK, http.StatusForbidden},
|
||||
{"AuthOnly", "/oauth2/auth", http.StatusAccepted, http.StatusUnauthorized},
|
||||
} {
|
||||
t.Run(endpoint.name, func(t *testing.T) {
|
||||
req := tt.req.Clone(tt.req.Context())
|
||||
req.URL.Path = endpoint.path
|
||||
rw := httptest.NewRecorder()
|
||||
proxy.ServeHTTP(rw, req)
|
||||
if tt.expectTrusted {
|
||||
assert.Equal(t, endpoint.trustedStatus, rw.Code)
|
||||
} else {
|
||||
assert.Equal(t, endpoint.deniedStatus, rw.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestTrustedIPsMissingRequestConfiguration(t *testing.T) {
|
||||
trustedIPs, err := ip.ParseNetSet([]string{"10.0.0.0/8", "192.0.2.0/24"})
|
||||
require.NoError(t, err)
|
||||
trustedProxies, err := ip.ParseNetSet([]string{"192.0.2.0/24"})
|
||||
require.NoError(t, err)
|
||||
parser, err := ip.GetRealClientIPParser("X-Forwarded-For")
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
scope *middlewareapi.RequestScope
|
||||
remoteAddr string
|
||||
nilParser bool
|
||||
expectTrusted bool
|
||||
}{
|
||||
{"Missing scope ignores header", nil, "198.51.100.23:1234", false, false},
|
||||
{"Missing scope permits direct trusted client", nil, "10.0.0.5:1234", false, true},
|
||||
{"Missing trusted proxies ignores header", &middlewareapi.RequestScope{ReverseProxy: true}, "198.51.100.23:1234", false, false},
|
||||
{"Missing parser does not fall back to exempt peer", &middlewareapi.RequestScope{ReverseProxy: true, TrustedProxies: trustedProxies}, "192.0.2.10:1234", true, false},
|
||||
{"Missing parser permits direct trusted client", &middlewareapi.RequestScope{ReverseProxy: true, TrustedProxies: trustedProxies}, "10.0.0.5:1234", true, true},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
proxy := &OAuthProxy{trustedIPs: trustedIPs, realClientIPParser: parser}
|
||||
if test.nilParser {
|
||||
proxy.realClientIPParser = nil
|
||||
}
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.RemoteAddr = test.remoteAddr
|
||||
req.Header.Set("X-Forwarded-For", "10.0.0.5")
|
||||
if test.scope != nil {
|
||||
req = middlewareapi.AddRequestScope(req, test.scope)
|
||||
}
|
||||
assert.Equal(t, test.expectTrusted, proxy.isTrustedIP(req))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func Test_buildRoutesAllowlist(t *testing.T) {
|
||||
type expectedAllowedRoute struct {
|
||||
method string
|
||||
|
||||
@@ -119,9 +119,9 @@ func NewFlagSet() *pflag.FlagSet {
|
||||
flagSet := pflag.NewFlagSet("oauth2-proxy", pflag.ExitOnError)
|
||||
|
||||
flagSet.Bool("reverse-proxy", false, "are we running behind a reverse proxy, controls whether headers like X-Real-Ip are accepted")
|
||||
flagSet.String("real-client-ip-header", "X-Real-IP", "Header used to determine the real IP of the client (one of: X-Forwarded-For, X-Real-IP, X-ProxyUser-IP, X-Envoy-External-Address, or CF-Connecting-IP)")
|
||||
flagSet.StringSlice("trusted-proxy-ip", []string{}, "list of IPs or CIDR ranges that are allowed to set X-Forwarded-* headers when --reverse-proxy is enabled. Defaults to trusting all IPs for backwards compatibility; configure this to your reverse proxy addresses to prevent header spoofing.")
|
||||
flagSet.StringSlice("trusted-ip", []string{}, "list of IPs or CIDR ranges to allow to bypass authentication. WARNING: trusting by IP has inherent security flaws, read the configuration documentation for more information.")
|
||||
flagSet.String("real-client-ip-header", "X-Real-IP", "Header used to determine the real IP of the client when the direct caller is a trusted proxy (one of: X-Forwarded-For, X-Real-IP, X-ProxyUser-IP, X-Envoy-External-Address, or CF-Connecting-IP)")
|
||||
flagSet.StringSlice("trusted-proxy-ip", []string{}, "list of direct proxy IPs or CIDR ranges allowed to set X-Forwarded-* headers when --reverse-proxy is enabled, including every trusted proxy hop in X-Forwarded-For. Defaults to trusting all IPs for backwards compatibility.")
|
||||
flagSet.StringSlice("trusted-ip", []string{}, "list of IPs or CIDR ranges to allow to bypass authentication. WARNING: trusting by IP has inherent security flaws; with --reverse-proxy, configure --trusted-proxy-ip and read the configuration documentation.")
|
||||
flagSet.Bool("force-https", false, "force HTTPS redirect for HTTP requests")
|
||||
flagSet.String("redirect-url", "", "the OAuth Redirect URL. ie: \"https://internalapp.yourcompany.com/oauth2/callback\"")
|
||||
flagSet.Bool("relative-redirect-url", false, "allow relative OAuth Redirect URL.")
|
||||
|
||||
+50
-8
@@ -49,18 +49,58 @@ func (p xForwardedForClientIPParser) GetRealClientIP(h http.Header) (net.IP, err
|
||||
if commaIndex := strings.IndexRune(ipStr, ','); commaIndex != -1 {
|
||||
ipStr = ipStr[:commaIndex]
|
||||
}
|
||||
ipStr = strings.TrimSpace(ipStr)
|
||||
|
||||
return parseClientIP(ipStr, p.header)
|
||||
}
|
||||
|
||||
// GetClientIPFromTrustedProxy obtains the client IP from a header supplied by
|
||||
// a caller that has already been verified as a trusted proxy.
|
||||
func GetClientIPFromTrustedProxy(p ipapi.RealClientIPParser, req *http.Request, trustedProxies *NetSet) (net.IP, error) {
|
||||
if p == nil {
|
||||
return nil, fmt.Errorf("real client IP parser is required")
|
||||
}
|
||||
|
||||
xffParser, ok := p.(*xForwardedForClientIPParser)
|
||||
if !ok || xffParser.header != http.CanonicalHeaderKey("X-Forwarded-For") {
|
||||
return p.GetRealClientIP(req.Header)
|
||||
}
|
||||
|
||||
if trustedProxies == nil {
|
||||
return nil, fmt.Errorf("trusted proxy list is required to parse X-Forwarded-For")
|
||||
}
|
||||
|
||||
xff := strings.Join(req.Header.Values(xffParser.header), ",")
|
||||
if xff == "" {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
chain := strings.Split(xff, ",")
|
||||
for i := len(chain) - 1; i >= 0; i-- {
|
||||
clientIP, err := parseClientIP(chain[i], xffParser.header)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if i == 0 || !trustedProxies.Has(clientIP) {
|
||||
return clientIP, nil
|
||||
}
|
||||
}
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func parseClientIP(ipStr string, header string) (net.IP, error) {
|
||||
ipStr = strings.TrimSpace(ipStr)
|
||||
if ipHost, _, err := net.SplitHostPort(ipStr); err == nil {
|
||||
ipStr = ipHost
|
||||
}
|
||||
|
||||
ip := net.ParseIP(ipStr)
|
||||
if ip == nil {
|
||||
return nil, fmt.Errorf("unable to parse ip (%s) from %s header", ipStr, http.CanonicalHeaderKey(p.header))
|
||||
clientIP := net.ParseIP(ipStr)
|
||||
if clientIP == nil {
|
||||
return nil, fmt.Errorf("unable to parse ip (%s) from %s header", ipStr, http.CanonicalHeaderKey(header))
|
||||
}
|
||||
|
||||
return ip, nil
|
||||
return clientIP, nil
|
||||
}
|
||||
|
||||
// GetClientIP obtains the perceived end-user IP address from headers if p != nil else from req.RemoteAddr.
|
||||
@@ -90,11 +130,13 @@ func getRemoteIP(req *http.Request) (net.IP, error) {
|
||||
//revive:enable:indent-error-flow
|
||||
}
|
||||
|
||||
// GetClientString obtains the human readable string of the remote IP and optionally the real client IP if available
|
||||
func GetClientString(p ipapi.RealClientIPParser, req *http.Request, full bool) (s string) {
|
||||
// GetClientString obtains the human readable string of the remote IP and optionally
|
||||
// the real client IP. Callers must pass a nil parser unless the peer is trusted.
|
||||
// Missing or invalid client headers leave only the transport IP in the output.
|
||||
func GetClientString(p ipapi.RealClientIPParser, req *http.Request, trustedProxies *NetSet, full bool) (s string) {
|
||||
var realClientIPStr string
|
||||
if p != nil {
|
||||
if realClientIP, err := p.GetRealClientIP(req.Header); err == nil && realClientIP != nil {
|
||||
if realClientIP, err := GetClientIPFromTrustedProxy(p, req, trustedProxies); err == nil && realClientIP != nil {
|
||||
realClientIPStr = realClientIP.String()
|
||||
}
|
||||
}
|
||||
|
||||
+100
-3
@@ -8,6 +8,7 @@ import (
|
||||
|
||||
ipapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/ip"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestGetRealClientIPParser(t *testing.T) {
|
||||
@@ -106,6 +107,93 @@ func TestXForwardedForClientIPParserIgnoresOthers(t *testing.T) {
|
||||
assert.Equal(t, ip, net.ParseIP(expectedIPString))
|
||||
}
|
||||
|
||||
func TestGetClientIPFromTrustedProxy(t *testing.T) {
|
||||
parser, err := GetRealClientIPParser("X-Forwarded-For")
|
||||
require.NoError(t, err)
|
||||
trustedProxies, err := ParseNetSet([]string{"192.0.2.0/24", "2001:db8:1::/48"})
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
headerValues []string
|
||||
expectedIP string
|
||||
errString string
|
||||
}{
|
||||
{"Ignores spoofed leftmost entry", []string{"10.0.0.5, 198.51.100.23"}, "198.51.100.23", ""},
|
||||
{"Walks past trusted intermediate proxies", []string{"198.51.100.23, 192.0.2.20, 192.0.2.21"}, "198.51.100.23", ""},
|
||||
{"Uses leftmost entry when entire chain is trusted", []string{"192.0.2.30, 192.0.2.20"}, "192.0.2.30", ""},
|
||||
{"Repeated fields preserve chain order", []string{"10.0.0.5", "198.51.100.23, 192.0.2.20", "192.0.2.21"}, "198.51.100.23", ""},
|
||||
{"Whitespace and IPv4 port", []string{" 198.51.100.23:1234 ,\t192.0.2.20:443 "}, "198.51.100.23", ""},
|
||||
{"IPv6 client and proxies", []string{"2001:db8:2::23, 2001:db8:1::20"}, "2001:db8:2::23", ""},
|
||||
{"IPv6 port and mixed chain", []string{" [2001:db8:2::23]:1234 , [2001:db8:1::20]:443, 192.0.2.20"}, "2001:db8:2::23", ""},
|
||||
{"Stops at first untrusted hop", []string{"10.0.0.5, 198.51.100.22, 198.51.100.23, 192.0.2.20"}, "198.51.100.23", ""},
|
||||
{"Ignores malformed entry beyond boundary", []string{"invalid, 198.51.100.23, 192.0.2.20"}, "198.51.100.23", ""},
|
||||
{"Missing header", nil, "", ""},
|
||||
{"Empty header", []string{""}, "", ""},
|
||||
{"Whitespace only", []string{" \t"}, "", "unable to parse ip () from X-Forwarded-For header"},
|
||||
{"Malformed rightmost hop", []string{"10.0.0.5, invalid"}, "", "unable to parse ip (invalid) from X-Forwarded-For header"},
|
||||
{"Malformed relevant intermediate hop", []string{"10.0.0.5, invalid, 192.0.2.20"}, "", "unable to parse ip (invalid) from X-Forwarded-For header"},
|
||||
{"Malformed leftmost in otherwise trusted chain", []string{"invalid, 192.0.2.20"}, "", "unable to parse ip (invalid) from X-Forwarded-For header"},
|
||||
{"Empty intermediate hop", []string{"10.0.0.5,,192.0.2.20"}, "", "unable to parse ip () from X-Forwarded-For header"},
|
||||
{"Empty last field", []string{"10.0.0.5", ""}, "", "unable to parse ip () from X-Forwarded-For header"},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
req := &http.Request{Header: http.Header{}}
|
||||
for _, value := range test.headerValues {
|
||||
req.Header.Add("X-Forwarded-For", value)
|
||||
}
|
||||
|
||||
clientIP, err := GetClientIPFromTrustedProxy(parser, req, trustedProxies)
|
||||
if test.errString != "" {
|
||||
assert.EqualError(t, err, test.errString)
|
||||
} else {
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
assert.Equal(t, net.ParseIP(test.expectedIP), clientIP)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetClientIPFromTrustedProxyConfiguration(t *testing.T) {
|
||||
xff, err := GetRealClientIPParser("X-Forwarded-For")
|
||||
require.NoError(t, err)
|
||||
realIP, err := GetRealClientIPParser("X-Real-IP")
|
||||
require.NoError(t, err)
|
||||
trustAll, err := ParseNetSet([]string{"0.0.0.0/0", "::/0"})
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
parser ipapi.RealClientIPParser
|
||||
trustedProxies *NetSet
|
||||
expectedIP string
|
||||
errString string
|
||||
}{
|
||||
{"Nil parser", nil, trustAll, "", "real client IP parser is required"},
|
||||
{"Nil trusted proxies", xff, nil, "", "trusted proxy list is required to parse X-Forwarded-For"},
|
||||
{"Empty trusted proxies", xff, NewNetSet(), "2001:db8::23", ""},
|
||||
{"Trust all retains leftmost compatibility", xff, trustAll, "10.0.0.5", ""},
|
||||
{"Single value header", realIP, trustAll, "198.51.100.23", ""},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
req := &http.Request{Header: http.Header{
|
||||
"X-Forwarded-For": {"10.0.0.5, 2001:db8::23"},
|
||||
"X-Real-Ip": {"198.51.100.23"},
|
||||
}}
|
||||
clientIP, err := GetClientIPFromTrustedProxy(test.parser, req, test.trustedProxies)
|
||||
if test.errString != "" {
|
||||
assert.EqualError(t, err, test.errString)
|
||||
} else {
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
assert.Equal(t, net.ParseIP(test.expectedIP), clientIP)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetRemoteIP(t *testing.T) {
|
||||
tests := []struct {
|
||||
remoteAddr string
|
||||
@@ -147,6 +235,8 @@ func TestGetRemoteIP(t *testing.T) {
|
||||
|
||||
func TestGetClientString(t *testing.T) {
|
||||
p := &xForwardedForClientIPParser{header: http.CanonicalHeaderKey("X-Forwarded-For")}
|
||||
trustedProxies, err := ParseNetSet([]string{"192.0.2.0/24", "2001:db8:1::/48"})
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
parser ipapi.RealClientIPParser
|
||||
@@ -155,7 +245,7 @@ func TestGetClientString(t *testing.T) {
|
||||
expectedClient string
|
||||
expectedClientFull string
|
||||
}{
|
||||
// Should fail quietly, only printing warnings to the log
|
||||
// Preserve transport-only output when no client address is available.
|
||||
{nil, "", "", "", ""},
|
||||
// Unix domain socket — no IP available
|
||||
{nil, "@", "", "", ""},
|
||||
@@ -164,6 +254,13 @@ func TestGetClientString(t *testing.T) {
|
||||
{nil, "10.254.244.165:62750", "", "10.254.244.165", "10.254.244.165"},
|
||||
// Parser is nil, the contents of X-Forwarded-For should be ignored in all cases.
|
||||
{nil, "[2001:470:26:307:a5a1:1177:2ae3:e9c3]:48290", "127.0.0.1", "2001:470:26:307:a5a1:1177:2ae3:e9c3", "2001:470:26:307:a5a1:1177:2ae3:e9c3"},
|
||||
{p, "192.0.2.10:443", "10.0.0.5, 198.51.100.23, 192.0.2.20", "198.51.100.23", "192.0.2.10 (198.51.100.23)"},
|
||||
{p, "[2001:db8:1::10]:443", "[2001:db8:2::23]:1234, 2001:db8:1::20", "2001:db8:2::23", "2001:db8:1::10 (2001:db8:2::23)"},
|
||||
{p, "192.0.2.10:443", "10.0.0.5, invalid, 192.0.2.20", "192.0.2.10", "192.0.2.10"},
|
||||
{p, "192.0.2.10:443", "invalid, 198.51.100.23", "198.51.100.23", "192.0.2.10 (198.51.100.23)"},
|
||||
{p, "192.0.2.10:443", "192.0.2.30, 192.0.2.20", "192.0.2.30", "192.0.2.10 (192.0.2.30)"},
|
||||
{p, "@", "198.51.100.23, 192.0.2.20", "198.51.100.23", " (198.51.100.23)"},
|
||||
{p, "@", "invalid", "", ""},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
@@ -174,10 +271,10 @@ func TestGetClientString(t *testing.T) {
|
||||
RemoteAddr: test.remoteAddr,
|
||||
}
|
||||
|
||||
client := GetClientString(test.parser, req, false)
|
||||
client := GetClientString(test.parser, req, trustedProxies, false)
|
||||
assert.Equal(t, test.expectedClient, client)
|
||||
|
||||
clientFull := GetClientString(test.parser, req, true)
|
||||
clientFull := GetClientString(test.parser, req, trustedProxies, true)
|
||||
assert.Equal(t, test.expectedClientFull, clientFull)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -20,7 +20,7 @@ func validateAllowlists(o *options.Options) []string {
|
||||
msgs = append(msgs, validateTrustedIPs(o)...)
|
||||
|
||||
if len(o.TrustedIPs) > 0 && o.ReverseProxy {
|
||||
_, err := fmt.Fprintln(os.Stderr, "WARNING: mixing --trusted-ip with --reverse-proxy is a potential security vulnerability. An attacker can inject a trusted IP into an X-Real-IP or X-Forwarded-For header if they aren't properly protected outside of oauth2-proxy")
|
||||
_, err := fmt.Fprintln(os.Stderr, "WARNING: --trusted-ip bypasses authentication. When using it with --reverse-proxy, configure --trusted-proxy-ip and ensure the proxy overwrites the selected real-client-IP header (or safely appends X-Forwarded-For).")
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"net/url"
|
||||
"strings"
|
||||
|
||||
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/hmacauth"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
|
||||
@@ -89,7 +90,15 @@ func Validate(o *options.Options) error {
|
||||
|
||||
// Allow the logger to get client IPs
|
||||
logger.SetGetClientFunc(func(r *http.Request) string {
|
||||
return ip.GetClientString(o.GetRealClientIPParser(), r, false)
|
||||
parser := o.GetRealClientIPParser()
|
||||
var trustedProxies *ip.NetSet
|
||||
scope := middlewareapi.GetRequestScope(r)
|
||||
if scope != nil && scope.CanTrustForwardedHeaders(r) {
|
||||
trustedProxies = scope.TrustedProxies
|
||||
} else {
|
||||
parser = nil
|
||||
}
|
||||
return ip.GetClientString(parser, r, trustedProxies, false)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -1,16 +1,24 @@
|
||||
package validation
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"crypto"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
|
||||
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr"
|
||||
"github.com/onsi/ginkgo/v2"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -224,6 +232,65 @@ func TestRealClientIPHeader(t *testing.T) {
|
||||
assert.Nil(t, o.GetRealClientIPParser())
|
||||
}
|
||||
|
||||
func TestRealClientIPLogging(t *testing.T) {
|
||||
t.Cleanup(func() {
|
||||
logger.SetOutput(ginkgo.GinkgoWriter)
|
||||
logger.SetAuthTemplate(logger.DefaultAuthLoggingFormat)
|
||||
logger.SetReqTemplate(logger.DefaultRequestLoggingFormat)
|
||||
logger.SetGetClientFunc(func(r *http.Request) string { return r.RemoteAddr })
|
||||
})
|
||||
tests := []struct {
|
||||
name string
|
||||
remoteAddr string
|
||||
headerValues []string
|
||||
reverseProxy bool
|
||||
nilParser bool
|
||||
expected string
|
||||
}{
|
||||
{"Untrusted peer", "198.51.100.23:1234", []string{"10.0.0.5"}, true, false, "198.51.100.23"},
|
||||
{"Appending trusted proxy", "192.0.2.10:1234", []string{"10.0.0.5, 198.51.100.23"}, true, false, "198.51.100.23"},
|
||||
{"Repeated fields and multiple proxies", "192.0.2.10:1234", []string{"10.0.0.5", "198.51.100.23, 192.0.2.20", "192.0.2.21"}, true, false, "198.51.100.23"},
|
||||
{"Legitimate client", "192.0.2.10:1234", []string{"10.0.0.5, 192.0.2.20"}, true, false, "10.0.0.5"},
|
||||
{"Missing header", "192.0.2.10:1234", nil, true, false, "192.0.2.10"},
|
||||
{"Malformed relevant hop", "192.0.2.10:1234", []string{"10.0.0.5, invalid, 192.0.2.20"}, true, false, "192.0.2.10"},
|
||||
{"Nil parser", "192.0.2.10:1234", []string{"10.0.0.5"}, true, true, "192.0.2.10"},
|
||||
{"Reverse proxy disabled in scope", "192.0.2.10:1234", []string{"10.0.0.5"}, false, false, "192.0.2.10"},
|
||||
{"IPv6", "[2001:db8:1::10]:1234", []string{"10.0.0.5", " [2001:db8:2::23]:443 , 2001:db8:1::20"}, true, false, "2001:db8:2::23"},
|
||||
{"Unix socket", "@", []string{"10.0.0.5, 198.51.100.23"}, true, false, "198.51.100.23"},
|
||||
{"Entire chain trusted", "192.0.2.10:1234", []string{"192.0.2.30, 192.0.2.20"}, true, false, "192.0.2.30"},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
o := testOptions()
|
||||
o.ReverseProxy = true
|
||||
o.RealClientIPHeader = "X-Forwarded-For"
|
||||
o.TrustedProxyIPs = []string{"192.0.2.0/24", "2001:db8:1::/48"}
|
||||
o.Logging.AuthFormat = "{{.Client}}"
|
||||
o.Logging.RequestFormat = "{{.Client}}"
|
||||
require.NoError(t, Validate(o))
|
||||
if test.nilParser {
|
||||
o.SetRealClientIPParser(nil)
|
||||
}
|
||||
trustedProxies, err := ip.ParseNetSet(o.TrustedProxyIPs)
|
||||
require.NoError(t, err)
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.RemoteAddr = test.remoteAddr
|
||||
for _, value := range test.headerValues {
|
||||
req.Header.Add("X-Forwarded-For", value)
|
||||
}
|
||||
req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{
|
||||
ReverseProxy: test.reverseProxy,
|
||||
TrustedProxies: trustedProxies,
|
||||
})
|
||||
var output bytes.Buffer
|
||||
logger.SetOutput(&output)
|
||||
logger.PrintAuthf("", req, logger.AuthFailure, "denied")
|
||||
logger.PrintReq("", "", req, *req.URL, time.Now(), http.StatusForbidden, 0)
|
||||
assert.Equal(t, test.expected+"\n"+test.expected+"\n", output.String())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestProviderCAFilesError(t *testing.T) {
|
||||
file, err := os.CreateTemp("", "absent.*.crt")
|
||||
assert.NoError(t, err)
|
||||
|
||||
Reference in New Issue
Block a user