diff --git a/CHANGELOG.md b/CHANGELOG.md index bfce323a..9f227932 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - [GHSA-5hvv-m4w4-gf6v](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-5hvv-m4w4-gf6v) fix: health check user-agent authentication bypass (@tuunit) - [GHSA-7x63-xv5r-3p2x](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-7x63-xv5r-3p2x) fix: authentication bypass via X-Forwarded-Uri header spoofing (@tuunit) - [GHSA-c5c4-8r6x-56w3](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-c5c4-8r6x-56w3) fix: email validation bypass via malformed multi-@ email claims (@tuunit) +- [GHSA-pxq7-h93f-9jrg](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-pxq7-h93f-9jrg) fix: fragment evaluation as part of the allowed routes (@tuunit) # V7.15.1 diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index b145065a..965953fa 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -218,8 +218,8 @@ When `--reverse-proxy` is enabled, configure `--trusted-proxy-ip` to the IPs or | flag: `--trusted-proxy-ip`
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: `--signature-key`
toml: `signature_key` | string | GAP-Signature request signature key (algorithm:secretkey) | | | flag: `--skip-auth-preflight`
toml: `skip_auth_preflight` | bool | will skip authentication for OPTIONS requests | false | -| flag: `--skip-auth-regex`
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`
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`
toml: `skip_auth_regex` | string \| list | (DEPRECATED for `--skip-auth-route`) bypass authentication for requests paths that match (may be given multiple times). Path matching is performed against the normalized path only; fragment identifiers (`#`) and their URL-encoded form (`%23`) are stripped before evaluation. | | +| flag: `--skip-auth-route`
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. Path matching is performed against the normalized path only; fragment identifiers (`#`) and their URL-encoded form (`%23`) are stripped before evaluation. | | | flag: `--skip-jwt-bearer-tokens`
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`
toml: `skip_provider_button` | bool | will skip sign-in-page to directly reach the next step: oauth/start | false | | flag: `--ssl-insecure-skip-verify`
toml: `ssl_insecure_skip_verify` | bool | skip validation of certificates presented when using HTTPS providers | false | diff --git a/oauthproxy_test.go b/oauthproxy_test.go index fc9c34bd..e1235a4e 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -2679,9 +2679,11 @@ func TestAllowedRequest(t *testing.T) { } opts.SkipAuthRegex = []string{ "^/skip/auth/regex$", + "^/public/.*/endpoint$", } opts.SkipAuthRoutes = []string{ "GET=^/skip/auth/routes/get", + "^/foo/.*/bar$", } err := validation.Validate(opts) assert.NoError(t, err) @@ -2714,6 +2716,18 @@ func TestAllowedRequest(t *testing.T) { url: "/wrong/denied", allowed: false, }, + { + name: "Regex allowed with fragment-free path", + method: "GET", + url: "/public/legit/endpoint", + allowed: true, + }, + { + name: "Regex denied when path contains encoded fragment suffix", + method: "GET", + url: "/public/secret%23/endpoint", + allowed: false, + }, { name: "Route allowed", method: "GET", @@ -2738,6 +2752,18 @@ func TestAllowedRequest(t *testing.T) { url: "/skip/auth/routes/wrong/path", allowed: false, }, + { + name: "Route allowed with fragment-free path", + method: "GET", + url: "/foo/public/bar", + allowed: true, + }, + { + name: "Route denied when path contains encoded fragment suffix", + method: "GET", + url: "/foo/secret%23/bar", + allowed: false, + }, } for _, tc := range testCases { @@ -2778,9 +2804,11 @@ func TestAllowedRequestWithForwardedUriHeader(t *testing.T) { } opts.SkipAuthRegex = []string{ "^/skip/auth/regex$", + "^/public/.*/endpoint$", } opts.SkipAuthRoutes = []string{ "GET=^/skip/auth/routes/get", + "^/foo/.*/bar$", } err := validation.Validate(opts) assert.NoError(t, err) @@ -2813,6 +2841,18 @@ func TestAllowedRequestWithForwardedUriHeader(t *testing.T) { url: "/wrong/denied", allowed: false, }, + { + name: "Regex allowed with fragment-free path", + method: "GET", + url: "/public/legit/endpoint", + allowed: true, + }, + { + name: "Regex denied when X-Forwarded-Uri contains an encoded fragment suffix", + method: "GET", + url: "/public/secret%23/endpoint", + allowed: false, + }, { name: "Route allowed", method: "GET", @@ -2837,6 +2877,18 @@ func TestAllowedRequestWithForwardedUriHeader(t *testing.T) { url: "/skip/auth/routes/wrong/path", allowed: false, }, + { + name: "Route allowed with fragment-free path", + method: "GET", + url: "/foo/public/bar", + allowed: true, + }, + { + name: "Route denied when X-Forwarded-Uri contains an encoded fragment suffix", + method: "GET", + url: "/foo/secret%23/bar", + allowed: false, + }, } for _, tc := range testCases { diff --git a/pkg/requests/util/util.go b/pkg/requests/util/util.go index 7b9b4919..568ebcc6 100644 --- a/pkg/requests/util/util.go +++ b/pkg/requests/util/util.go @@ -47,16 +47,28 @@ func GetRequestURI(req *http.Request) string { // GetRequestPath returns the request URI or X-Forwarded-Uri if present and the // request came from a trusted reverse proxy but always strips the query -// parameters and only returns the pure path. +// parameters and fragment suffixes and only returns the pure path. func GetRequestPath(req *http.Request) string { - uri := GetRequestURI(req) + uri := stripRequestFragment(GetRequestURI(req)) // Parse URI and return only the path component if parsedURL, err := url.Parse(uri); err == nil { - return parsedURL.Path + return stripRequestFragment(parsedURL.Path) } // Fallback: strip query parameters manually + return stripRequestQuery(uri) +} + +func stripRequestFragment(uri string) string { + if idx := strings.Index(uri, "#"); idx != -1 { + return uri[:idx] + } + + return uri +} + +func stripRequestQuery(uri string) string { if idx := strings.Index(uri, "?"); idx != -1 { return uri[:idx] } diff --git a/pkg/requests/util/util_test.go b/pkg/requests/util/util_test.go index ed7a88a6..c4185b35 100644 --- a/pkg/requests/util/util_test.go +++ b/pkg/requests/util/util_test.go @@ -152,6 +152,23 @@ var _ = Describe("Util Suite", func() { Expect(util.GetRequestPath(req)).To(Equal(uriNoQueryParams)) }) + It("drops fragment content from a parsed request path", func() { + // Simulate net/http ParseRequestURI preserving '#' in URL.Path. + req.URL.Path = "/foo/secret#/bar" + req.URL.RawPath = "/foo/secret%23/bar" + Expect(util.GetRequestPath(req)).To(Equal("/foo/secret")) + }) + + It("drops fragment-like suffixes from encoded number signs", func() { + req = httptest.NewRequest( + http.MethodGet, + fmt.Sprintf("%s://%s/foo/secret%%23/bar?query=param", proto, host), + nil, + ) + req = middleware.AddRequestScope(req, &middleware.RequestScope{}) + Expect(util.GetRequestPath(req)).To(Equal("/foo/secret")) + }) + It("ignores X-Forwarded-Uri and returns the URI (without query params)", func() { req.Header.Add("X-Forwarded-Uri", "/some/other/path?query=param") Expect(util.GetRequestPath(req)).To(Equal(uriNoQueryParams)) @@ -175,6 +192,11 @@ var _ = Describe("Util Suite", func() { req.Header.Add("X-Forwarded-Uri", "/some/other/path?query=param") Expect(util.GetRequestPath(req)).To(Equal("/some/other/path")) }) + + It("drops fragment-like suffixes from the X-Forwarded-Uri", func() { + req.Header.Add("X-Forwarded-Uri", "/foo/secret%23/bar?query=param") + Expect(util.GetRequestPath(req)).To(Equal("/foo/secret")) + }) }) })