diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index 40463afe..5dcb7bfb 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Log weight when issuing challenge. - Gate pprof endpoints behind `metrics.debug` in the policy file. - Limit naive honeypot r9k delay to one second. +- Fix an obscure case where adding query values to a subrequest match could cause an invalid rule match when using path based matching for protected resources. - Fix `path_regex` and CEL `path` rules not matching when using Traefik `forwardAuth` middleware. Anubis now checks `X-Forwarded-Uri` (Traefik) in addition to `X-Original-URI` (nginx) when resolving the request path in subrequest mode ([#1628](https://github.com/TecharoHQ/anubis/issues/1628)). ## v1.25.0: Necron diff --git a/lib/policy/checker.go b/lib/policy/checker.go index a8032c83..d20c067b 100644 --- a/lib/policy/checker.go +++ b/lib/policy/checker.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/netip" + "net/url" "regexp" "strings" @@ -114,6 +115,9 @@ func (pc *PathChecker) Check(r *http.Request) (bool, error) { originalUrl = r.Header.Get("X-Forwarded-Uri") } if originalUrl != "" { + if parsed, err := url.ParseRequestURI(originalUrl); err == nil { + originalUrl = parsed.Path + } if pc.regexp.MatchString(originalUrl) { return true, nil } diff --git a/lib/policy/policy_test.go b/lib/policy/policy_test.go index ff38fb36..e2c4d0e4 100644 --- a/lib/policy/policy_test.go +++ b/lib/policy/policy_test.go @@ -1,6 +1,8 @@ package policy import ( + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -85,3 +87,27 @@ func TestBadConfigs(t *testing.T) { }) } } + +func TestPathCheckerStripsForwardedURIQuery(t *testing.T) { + checker, err := NewPathChecker("^/admin$", true) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodGet, "https://anubis.local/.within.website/x/cmd/anubis/api/check", nil) + req.Header.Set("X-Forwarded-Uri", "/admin?x=1") + matched, err := checker.Check(req) + if err != nil { + t.Fatal(err) + } + if !matched { + t.Fatalf("expected exact path checker to match forwarded URI when query string is appended") + } + req.Header.Set("X-Forwarded-Uri", "/admin") + matched, err = checker.Check(req) + if err != nil { + t.Fatal(err) + } + if !matched { + t.Fatalf("expected exact path checker to match forwarded URI without query string") + } +}