mirror of
https://github.com/TecharoHQ/anubis.git
synced 2026-05-15 19:29:36 +00:00
fix(policy): correctly wire subrequest mode through CEL/path checkers (#1630)
* fix(policy): correctly wire subrequest mode through CEL/path checkers Previously Anubis only checked for the X-Original-Url when using subrequest mode. This header is used by the example nginx config to pass the request path through from the original client request to Anubis in order to do path-based filtering. According to facts and circumstances, Traefik hardcodes its own headers[1]: ```text httpdebug-1 | GET /.within.website/x/cmd/anubis/api/check httpdebug-1 | X-Forwarded-Method: GET httpdebug-1 | X-Forwarded-Proto: http httpdebug-1 | X-Forwarded-Server: b9a5d299c929 httpdebug-1 | X-Forwarded-Port: 8080 httpdebug-1 | X-Forwarded-Uri: / httpdebug-1 | X-Real-Ip: 172.18.0.1 httpdebug-1 | Accept-Encoding: gzip httpdebug-1 | User-Agent: curl/8.20.0 httpdebug-1 | Accept: */* httpdebug-1 | X-Forwarded-For: 172.18.0.1 httpdebug-1 | X-Forwarded-Host: localhost:8080 ``` As a result, this means that path-based filtering did not work. This commit fixes this issue by amending how path based checking logic works: * For CEL based checks, this pipes through the `subrequestMode` flag from main and alters the behaviour if either `X-Original-Url` or `X-Forwarded-Url` are found. These values are currently hardcoded for convenience but probably need to be made configurable in the policy file at a future date. * For path-based checks, this uses the existing `subrequestMode` flag from main and adds `X-Forwarded-Url` to the list of headers it checks. A smoke test was added to make sure that traefik in this mode continues to work. Thank you https://github.com/flifloo for filing a detailed issue with the relevant configuration fragments. Those configuration fragments formed the core of this smoke test. [1]: https://doc.traefik.io/traefik/v3.4/middlewares/http/forwardauth/ Closes: https://github.com/TecharoHQ/anubis/issues/1628 Signed-off-by: Xe Iaso <me@xeiaso.net> Co-Authored-By: flifloo <flifloo@gmail.com> * chore: spelling Signed-off-by: Xe Iaso <me@xeiaso.net> --------- Signed-off-by: Xe Iaso <me@xeiaso.net> Co-authored-by: flifloo <flifloo@gmail.com>
This commit is contained in:
@@ -39,3 +39,5 @@ wenet
|
||||
qwertiko
|
||||
setuplistener
|
||||
mba
|
||||
xfu
|
||||
xou
|
||||
|
||||
@@ -27,6 +27,7 @@ jobs:
|
||||
- palemoon/amd64
|
||||
#- palemoon/i386
|
||||
- robots_txt
|
||||
- traefik
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout code
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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
|
||||
@@ -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);
|
||||
Executable
+22
@@ -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
|
||||
@@ -0,0 +1,8 @@
|
||||
entryPoints:
|
||||
web:
|
||||
address: ":80"
|
||||
|
||||
providers:
|
||||
file:
|
||||
directory: /config
|
||||
watch: false
|
||||
@@ -0,0 +1,2 @@
|
||||
*
|
||||
!.gitignore
|
||||
Reference in New Issue
Block a user