diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 4bbdfe98..806a44f4 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -39,3 +39,5 @@ wenet qwertiko setuplistener mba +xfu +xou diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 89d60394..993df189 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -27,6 +27,7 @@ jobs: - palemoon/amd64 #- palemoon/i386 - robots_txt + - traefik runs-on: ubuntu-latest steps: - name: Checkout code diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index ba4bec51..d72bc943 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix a bug in the dataset poisoning maze that could allow denial of service [#1580](https://github.com/TecharoHQ/anubis/issues/1580). - Add config option to add ASN to logs/metrics. - Log weight when issuing challenge +- 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/celchecker.go b/lib/policy/celchecker.go index c313cd43..2bf0a23c 100644 --- a/lib/policy/celchecker.go +++ b/lib/policy/celchecker.go @@ -13,11 +13,12 @@ import ( ) type CELChecker struct { - program cel.Program - src string + program cel.Program + src string + subRequestMode bool } -func NewCELChecker(cfg *config.ExpressionOrList, dnsObj *dns.Dns) (*CELChecker, error) { +func NewCELChecker(cfg *config.ExpressionOrList, dnsObj *dns.Dns, subRequestMode bool) (*CELChecker, error) { env, err := expressions.BotEnvironment(dnsObj) if err != nil { return nil, err @@ -29,8 +30,9 @@ func NewCELChecker(cfg *config.ExpressionOrList, dnsObj *dns.Dns) (*CELChecker, } return &CELChecker{ - src: cfg.String(), - program: program, + src: cfg.String(), + program: program, + subRequestMode: subRequestMode, }, nil } @@ -39,7 +41,7 @@ func (cc *CELChecker) Hash() string { } func (cc *CELChecker) Check(r *http.Request) (bool, error) { - result, _, err := cc.program.ContextEval(r.Context(), &CELRequest{r}) + result, _, err := cc.program.ContextEval(r.Context(), &CELRequest{r, cc.subRequestMode}) if err != nil { return false, err @@ -54,6 +56,7 @@ func (cc *CELChecker) Check(r *http.Request) (bool, error) { type CELRequest struct { *http.Request + subRequestMode bool } func (cr *CELRequest) Parent() cel.Activation { return nil } @@ -71,6 +74,14 @@ func (cr *CELRequest) ResolveName(name string) (any, bool) { case "userAgent": return cr.UserAgent(), true case "path": + if cr.subRequestMode { + if xou := cr.Header.Get("X-Original-URI"); xou != "" { + return xou, true + } + if xfu := cr.Header.Get("X-Forwarded-Uri"); xfu != "" { + return xfu, true + } + } return cr.URL.Path, true case "query": return expressions.URLValues{Values: cr.URL.Query()}, true diff --git a/lib/policy/celchecker_test.go b/lib/policy/celchecker_test.go index 61c90819..a9cabdef 100644 --- a/lib/policy/celchecker_test.go +++ b/lib/policy/celchecker_test.go @@ -23,7 +23,7 @@ func TestCELChecker_MapIterationWrappers(t *testing.T) { Expression: `headers.exists(k, k == "Accept") && query.exists(k, k == "format")`, } - checker, err := NewCELChecker(cfg, newTestDNS(t)) + checker, err := NewCELChecker(cfg, newTestDNS(t), false) if err != nil { t.Fatalf("creating CEL checker failed: %v", err) } @@ -42,3 +42,77 @@ func TestCELChecker_MapIterationWrappers(t *testing.T) { t.Fatal("expected expression to evaluate true") } } + +func TestCELChecker_PathWithForwardedUri(t *testing.T) { + tests := []struct { + name string + expression string + xForwardedUri string + urlPath string + subRequestMode bool + want bool + }{ + { + name: "path matches X-Forwarded-Uri in subrequest mode", + expression: `path.startsWith("/admin")`, + xForwardedUri: "/admin/secret", + urlPath: "/.within.website/x/cmd/anubis/api/check", + subRequestMode: true, + want: true, + }, + { + name: "path with query string", + expression: `path.startsWith("/api/secret")`, + xForwardedUri: "/api/secret?token=abc", + urlPath: "/.within.website/x/cmd/anubis/api/check", + subRequestMode: true, + want: true, + }, + { + name: "path falls back to url path when no header", + expression: `path == "/public/page"`, + urlPath: "/public/page", + subRequestMode: true, + want: true, + }, + { + name: "non-subrequest mode ignores X-Forwarded-Uri", + expression: `path.startsWith("/admin")`, + xForwardedUri: "/admin/secret", + urlPath: "/public/page", + subRequestMode: false, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &config.ExpressionOrList{ + Expression: tt.expression, + } + checker, err := NewCELChecker(cfg, newTestDNS(t), tt.subRequestMode) + if err != nil { + t.Fatalf("NewCELChecker() error: %v", err) + } + + req, err := http.NewRequest(http.MethodGet, "http://example.com"+tt.urlPath, nil) + if err != nil { + t.Fatalf("http.NewRequest: %v", err) + } + + if tt.xForwardedUri != "" { + req.Header.Set("X-Forwarded-Uri", tt.xForwardedUri) + } + + got, err := checker.Check(req) + if err != nil { + t.Fatalf("Check() error: %v", err) + } + + if got != tt.want { + t.Errorf("Check() = %v, want %v (subRequestMode=%v, urlPath=%q, X-Forwarded-Uri=%q)", + got, tt.want, tt.subRequestMode, tt.urlPath, tt.xForwardedUri) + } + }) + } +} diff --git a/lib/policy/checker.go b/lib/policy/checker.go index 38003164..a8032c83 100644 --- a/lib/policy/checker.go +++ b/lib/policy/checker.go @@ -110,6 +110,9 @@ func NewPathChecker(rexStr string, subrequestMode bool) (checker.Impl, error) { func (pc *PathChecker) Check(r *http.Request) (bool, error) { if pc.subRequestMode { originalUrl := r.Header.Get("X-Original-URI") + if originalUrl == "" { + originalUrl = r.Header.Get("X-Forwarded-Uri") + } if originalUrl != "" { if pc.regexp.MatchString(originalUrl) { return true, nil diff --git a/lib/policy/checker_test.go b/lib/policy/checker_test.go index 59ee03fb..07403ebf 100644 --- a/lib/policy/checker_test.go +++ b/lib/policy/checker_test.go @@ -410,3 +410,119 @@ func TestPathChecker_GHSA_6wcg_mqvh_fcvg(t *testing.T) { }) } } + +func TestPathChecker_XForwardedUri(t *testing.T) { + tests := []struct { + name string + regex string + xForwardedUri string + xOriginalURI string + urlPath string + subRequestMode bool + want bool + }{ + { + name: "X-Forwarded-Uri matches regex in subrequest mode", + regex: "^/admin/.*", + xForwardedUri: "/admin/users", + urlPath: "/.within.website/x/cmd/anubis/api/check", + subRequestMode: true, + want: true, + }, + { + name: "X-Forwarded-Uri with query string", + regex: "^/admin/.*", + xForwardedUri: "/admin/users?page=1", + urlPath: "/.within.website/x/cmd/anubis/api/check", + subRequestMode: true, + want: true, + }, + { + name: "X-Original-URI takes priority over X-Forwarded-Uri", + regex: "^/admin/.*", + xForwardedUri: "/public/page", + xOriginalURI: "/admin/users", + urlPath: "/.within.website/x/cmd/anubis/api/check", + subRequestMode: true, + want: true, + }, + { + name: "falls back to X-Forwarded-Uri when no X-Original-URI", + regex: "^/admin/.*", + xForwardedUri: "/admin/dashboard", + urlPath: "/.within.website/x/cmd/anubis/api/check", + subRequestMode: true, + want: true, + }, + { + name: "neither header matches, url path matches", + regex: "^/public/.*", + xForwardedUri: "/admin/users", + urlPath: "/public/page", + subRequestMode: true, + want: true, + }, + { + name: "nothing matches", + regex: "^/admin/.*", + xForwardedUri: "/public/page", + urlPath: "/.within.website/x/cmd/anubis/api/check", + subRequestMode: true, + want: false, + }, + { + name: "non-subrequest mode ignores X-Forwarded-Uri", + regex: "^/admin/.*", + xForwardedUri: "/admin/users", + urlPath: "/public/page", + subRequestMode: false, + want: false, + }, + { + name: "non-subrequest mode uses url path", + regex: "^/admin/.*", + xForwardedUri: "/public/page", + urlPath: "/admin/secret", + subRequestMode: false, + want: true, + }, + { + name: "empty X-Forwarded-Uri falls back to url path", + regex: "^/check$", + urlPath: "/check", + subRequestMode: true, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pc, err := NewPathChecker(tt.regex, tt.subRequestMode) + if err != nil { + t.Fatalf("NewPathChecker(%q, %v) returned error: %v", tt.regex, tt.subRequestMode, err) + } + + req, err := http.NewRequest(http.MethodGet, "http://example.com"+tt.urlPath, nil) + if err != nil { + t.Fatalf("http.NewRequest: %v", err) + } + + if tt.xForwardedUri != "" { + req.Header.Set("X-Forwarded-Uri", tt.xForwardedUri) + } + if tt.xOriginalURI != "" { + req.Header.Set("X-Original-URI", tt.xOriginalURI) + } + + got, err := pc.Check(req) + if err != nil { + t.Fatalf("Check() unexpected error: %v", err) + } + + if got != tt.want { + t.Errorf("Check() = %v, want %v (subRequestMode=%v, urlPath=%q, X-Forwarded-Uri=%q, X-Original-URI=%q)", + got, tt.want, tt.subRequestMode, tt.urlPath, tt.xForwardedUri, tt.xOriginalURI) + } + }) + } +} diff --git a/lib/policy/policy.go b/lib/policy/policy.go index 3889dc8e..6b329a86 100644 --- a/lib/policy/policy.go +++ b/lib/policy/policy.go @@ -170,7 +170,7 @@ func ParseConfig(ctx context.Context, fin io.Reader, fname string, defaultDiffic } if b.Expression != nil { - c, err := NewCELChecker(b.Expression, result.Dns) + c, err := NewCELChecker(b.Expression, result.Dns, subrequestMode) if err != nil { validationErrs = append(validationErrs, fmt.Errorf("while processing rule %s expressions: %w", b.Name, err)) } else { diff --git a/test/traefik/anubis.yaml b/test/traefik/anubis.yaml new file mode 100644 index 00000000..12ec4567 --- /dev/null +++ b/test/traefik/anubis.yaml @@ -0,0 +1,16 @@ +bots: + - name: block-admin-via-regex + path_regex: ^/admin(/.*)?$ + action: DENY + + - name: block-secret-via-cel + expression: + all: + - 'path.startsWith("/api/secret")' + action: DENY + + - import: (data)/meta/default-config.yaml + +status_codes: + CHALLENGE: 200 + DENY: 403 diff --git a/test/traefik/docker-compose.yaml b/test/traefik/docker-compose.yaml new file mode 100644 index 00000000..9640baa8 --- /dev/null +++ b/test/traefik/docker-compose.yaml @@ -0,0 +1,27 @@ +services: + traefik: + image: traefik:v3.3 + restart: always + ports: + - 8080:80 + volumes: + - ./traefik.yml:/etc/traefik/traefik.yml:ro + - ./http.yaml:/config/http.yaml:ro + + anubis: + image: ko.local/anubis + restart: always + environment: + BIND: ":8080" + TARGET: " " + POLICY_FNAME: /etc/techaro/anubis.yaml + PUBLIC_URL: http://localhost:8080/.within.website/x/cmd/anubis + COOKIE_DOMAIN: localhost + USE_REMOTE_ADDRESS: "true" + volumes: + - ./anubis.yaml:/etc/techaro/anubis.yaml + + backend: + image: ghcr.io/xe/x/httpdebug + pull_policy: always + restart: always diff --git a/test/traefik/http.yaml b/test/traefik/http.yaml new file mode 100644 index 00000000..426aa278 --- /dev/null +++ b/test/traefik/http.yaml @@ -0,0 +1,30 @@ +http: + middlewares: + anubis: + forwardAuth: + address: http://anubis:8080/.within.website/x/cmd/anubis/api/check + trustForwardHeader: true + + routers: + anubis-assets: + rule: Host(`localhost`) && PathPrefix(`/.within.website/x/cmd/anubis`) + entryPoints: + - web + service: anubis + backend: + rule: Host(`localhost`) + entryPoints: + - web + service: backend + middlewares: + - anubis + + services: + anubis: + loadBalancer: + servers: + - url: http://anubis:8080 + backend: + loadBalancer: + servers: + - url: http://backend:3000 diff --git a/test/traefik/test.mjs b/test/traefik/test.mjs new file mode 100644 index 00000000..4bf7c09f --- /dev/null +++ b/test/traefik/test.mjs @@ -0,0 +1,33 @@ +// Smoke test for https://github.com/TecharoHQ/anubis/issues/1628 +// +// Traefik's forwardAuth middleware calls Anubis at the literal path +// /.within.website/x/cmd/anubis/api/check and conveys the original URL in the +// X-Forwarded-Uri header. Path-targeting policy rules must match that header +// (not r.URL.Path), otherwise every request looks like a request to /check. + +const BASE = "http://localhost:8080"; +const UA = "Mozilla/5.0 (compatible; AnubisTraefikSmoke/1.0)"; + +const cases = [ + { path: "/", expected: 307, why: "control: no DENY rule, default challenge redirect" }, + { path: "/free", expected: 307, why: "control: no DENY rule, default challenge redirect" }, + { path: "/admin", expected: 403, why: "path_regex must match X-Forwarded-Uri, not 307 or 200" }, + { path: "/admin/users", expected: 403, why: "path_regex must match X-Forwarded-Uri, not 307 or 200" }, + { path: "/api/secret", expected: 403, why: "CEL path must match X-Forwarded-Uri, not 307 or 200" }, +]; + +let failed = false; + +for (const c of cases) { + const resp = await fetch(`${BASE}${c.path}`, { + headers: { "User-Agent": UA }, + redirect: "manual", + }); + const ok = resp.status === c.expected; + console.log( + `${ok ? "PASS" : "FAIL"}: GET ${c.path} → ${resp.status} (want ${c.expected}: ${c.why})`, + ); + if (!ok) failed = true; +} + +process.exit(failed ? 1 : 0); diff --git a/test/traefik/test.sh b/test/traefik/test.sh new file mode 100755 index 00000000..f876acc6 --- /dev/null +++ b/test/traefik/test.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash + +set -eo pipefail + +export VERSION=${GITHUB_SHA:-devel}-test +export KO_DOCKER_REPO=ko.local + +set -u + +source ../lib/lib.sh + +build_anubis_ko + +function cleanup() { + docker compose down -t 1 || : +} + +trap cleanup EXIT SIGINT + +docker compose up -d + +backoff-retry --try-count 20 node ./test.mjs diff --git a/test/traefik/traefik.yml b/test/traefik/traefik.yml new file mode 100644 index 00000000..7aac016e --- /dev/null +++ b/test/traefik/traefik.yml @@ -0,0 +1,8 @@ +entryPoints: + web: + address: ":80" + +providers: + file: + directory: /config + watch: false diff --git a/test/traefik/var/.gitignore b/test/traefik/var/.gitignore new file mode 100644 index 00000000..d6b7ef32 --- /dev/null +++ b/test/traefik/var/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore