1
0
mirror of https://github.com/oauth2-proxy/oauth2-proxy.git synced 2025-08-06 22:42:56 +02:00

Merge commit from fork

Signed-off-by: Jan Larwig <jan@larwig.com>
This commit is contained in:
Jan Larwig
2025-07-30 19:46:58 +02:00
committed by GitHub
parent f4b33b64bd
commit 9ffafad4b2
4 changed files with 85 additions and 14 deletions

View File

@ -4,8 +4,22 @@
## Important Notes ## Important Notes
Fixed critical vulnerability where `skip_auth_routes` regex patterns matched against the full request URI (path + query parameters) instead of just the path, allowing authentication bypass attacks.
## Breaking Changes ## Breaking Changes
If your configuration relies on matching query parameters in `skip_auth_routes` patterns, you must update your regex patterns to match paths only. Review all `skip_auth_routes` entries for potential impact.
**Example of affected configuration:**
```yaml
# This pattern previously matched both:
# - /api/foo/status (intended)
# - /api/private/sensitive?path=/status (bypass - now fixed)
skip_auth_routes: ["^/api/.*/status"]
```
For detailed information, migration guidance, and security implications, see the [security advisory](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-7rh7-c77v-6434).
## Changes since v7.10.0 ## Changes since v7.10.0
- [#2615](https://github.com/oauth2-proxy/oauth2-proxy/pull/2615) feat(cookies): add option to set a limit on the number of per-request CSRF cookies oauth2-proxy sets (@bh-tt) - [#2615](https://github.com/oauth2-proxy/oauth2-proxy/pull/2615) feat(cookies): add option to set a limit on the number of per-request CSRF cookies oauth2-proxy sets (@bh-tt)
@ -17,6 +31,7 @@
- [#3055](https://github.com/oauth2-proxy/oauth2-proxy/pull/3055) feat: support non-default authorization request response mode also for OIDC providers (@stieler-it) - [#3055](https://github.com/oauth2-proxy/oauth2-proxy/pull/3055) feat: support non-default authorization request response mode also for OIDC providers (@stieler-it)
- [#3138](https://github.com/oauth2-proxy/oauth2-proxy/pull/3138) feat: make google_groups argument optional when using google provider (@sourava01) - [#3138](https://github.com/oauth2-proxy/oauth2-proxy/pull/3138) feat: make google_groups argument optional when using google provider (@sourava01)
- [#3093](https://github.com/oauth2-proxy/oauth2-proxy/pull/3093) feat: differentiate between "no available key" and error for redis sessions (@nobletrout) - [#3093](https://github.com/oauth2-proxy/oauth2-proxy/pull/3093) feat: differentiate between "no available key" and error for redis sessions (@nobletrout)
- [GHSA-7rh7-c77v-6434](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-7rh7-c77v-6434) fix: skip_auth_routes bypass through query parameter inclusion
# V7.10.0 # V7.10.0

View File

@ -580,7 +580,7 @@ func isAllowedMethod(req *http.Request, route allowedRoute) bool {
} }
func isAllowedPath(req *http.Request, route allowedRoute) bool { func isAllowedPath(req *http.Request, route allowedRoute) bool {
matches := route.pathRegex.MatchString(requestutil.GetRequestURI(req)) matches := route.pathRegex.MatchString(requestutil.GetRequestPath(req))
if route.negate { if route.negate {
return !matches return !matches

View File

@ -2,6 +2,8 @@ package util
import ( import (
"net/http" "net/http"
"net/url"
"strings"
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
) )
@ -43,6 +45,25 @@ func GetRequestURI(req *http.Request) string {
return uri return uri
} }
// GetRequestPath returns the request URI or X-Forwarded-Uri if present and the
// request is proxied but always strips the query parameters and only returns
// the pure path
func GetRequestPath(req *http.Request) string {
uri := GetRequestURI(req)
// Parse URI and return only the path component
if parsedURL, err := url.Parse(uri); err == nil {
return parsedURL.Path
}
// Fallback: strip query parameters manually
if idx := strings.Index(uri, "?"); idx != -1 {
return uri[:idx]
}
return uri
}
// IsProxied determines if a request was from a proxy based on the RequestScope // IsProxied determines if a request was from a proxy based on the RequestScope
// ReverseProxy tracker. // ReverseProxy tracker.
func IsProxied(req *http.Request) bool { func IsProxied(req *http.Request) bool {

View File

@ -13,16 +13,17 @@ import (
var _ = Describe("Util Suite", func() { var _ = Describe("Util Suite", func() {
const ( const (
proto = "http" proto = "http"
host = "www.oauth2proxy.test" host = "www.oauth2proxy.test"
uri = "/test/endpoint" uriWithQueryParams = "/test/endpoint?query=param"
uriNoQueryParams = "/test/endpoint"
) )
var req *http.Request var req *http.Request
BeforeEach(func() { BeforeEach(func() {
req = httptest.NewRequest( req = httptest.NewRequest(
http.MethodGet, http.MethodGet,
fmt.Sprintf("%s://%s%s", proto, host, uri), fmt.Sprintf("%s://%s%s", proto, host, uriWithQueryParams),
nil, nil,
) )
}) })
@ -101,13 +102,13 @@ var _ = Describe("Util Suite", func() {
req = middleware.AddRequestScope(req, &middleware.RequestScope{}) req = middleware.AddRequestScope(req, &middleware.RequestScope{})
}) })
It("returns the URI", func() { It("returns the URI (with query params)", func() {
Expect(util.GetRequestURI(req)).To(Equal(uri)) Expect(util.GetRequestURI(req)).To(Equal(uriWithQueryParams))
}) })
It("ignores X-Forwarded-Uri and returns the URI", func() { It("ignores X-Forwarded-Uri and returns the URI (with query params)", func() {
req.Header.Add("X-Forwarded-Uri", "/some/other/path") req.Header.Add("X-Forwarded-Uri", "/some/other/path")
Expect(util.GetRequestURI(req)).To(Equal(uri)) Expect(util.GetRequestURI(req)).To(Equal(uriWithQueryParams))
}) })
}) })
@ -118,13 +119,47 @@ var _ = Describe("Util Suite", func() {
}) })
}) })
It("returns the URI if X-Forwarded-Uri is not present", func() { It("returns the URI if X-Forwarded-Uri is not present (with query params)", func() {
Expect(util.GetRequestURI(req)).To(Equal(uri)) Expect(util.GetRequestURI(req)).To(Equal(uriWithQueryParams))
}) })
It("returns the X-Forwarded-Uri when present", func() { It("returns the X-Forwarded-Uri when present (with query params)", func() {
req.Header.Add("X-Forwarded-Uri", "/some/other/path") req.Header.Add("X-Forwarded-Uri", "/some/other/path?query=param")
Expect(util.GetRequestURI(req)).To(Equal("/some/other/path")) Expect(util.GetRequestURI(req)).To(Equal("/some/other/path?query=param"))
})
})
})
Context("GetRequestPath", func() {
Context("IsProxied is false", func() {
BeforeEach(func() {
req = middleware.AddRequestScope(req, &middleware.RequestScope{})
})
It("returns the URI (without query params)", func() {
Expect(util.GetRequestPath(req)).To(Equal(uriNoQueryParams))
})
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))
})
})
Context("IsProxied is true", func() {
BeforeEach(func() {
req = middleware.AddRequestScope(req, &middleware.RequestScope{
ReverseProxy: true,
})
})
It("returns the URI if X-Forwarded-Uri is not present (without query params)", func() {
Expect(util.GetRequestPath(req)).To(Equal(uriNoQueryParams))
})
It("returns the X-Forwarded-Uri when present (without query params)", func() {
req.Header.Add("X-Forwarded-Uri", "/some/other/path?query=param")
Expect(util.GetRequestPath(req)).To(Equal("/some/other/path"))
}) })
}) })
}) })