diff --git a/cmd/anubis/main.go b/cmd/anubis/main.go index d6ce5000..b1a906ab 100644 --- a/cmd/anubis/main.go +++ b/cmd/anubis/main.go @@ -259,7 +259,7 @@ func main() { } lg.Info("loading policy file", "fname", *policyFname) - policy, err := libanubis.LoadPoliciesOrDefault(ctx, *policyFname, *challengeDifficulty, *slogLevel) + policy, err := libanubis.LoadPoliciesOrDefault(ctx, *policyFname, *challengeDifficulty, *slogLevel, strings.TrimSpace(*target) == "") if err != nil { log.Fatalf("can't parse policy file: %v", err) } diff --git a/docs/docs/CHANGELOG.md b/docs/docs/CHANGELOG.md index 2175ad86..ba4bec51 100644 --- a/docs/docs/CHANGELOG.md +++ b/docs/docs/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +- Patch [GHSA-6wcg-mqvh-fcvg](https://github.com/TecharoHQ/anubis/security/advisories/GHSA-6wcg-mqvh-fcvg) by containing subrequest logic to Anubis instances in subrequest mode. - Move metrics server configuration to [the policy file](./admin/policies.mdx#metrics-server). - Expose [pprof endpoints](https://pkg.go.dev/net/http/pprof) on the metrics listener to enable profiling Anubis in production. - fix: prevent nil pointer panic in challenge validation when threshold rules match during PassChallenge (#1463) diff --git a/internal/test/playwright_test.go b/internal/test/playwright_test.go index 4d6355ba..6961dc02 100644 --- a/internal/test/playwright_test.go +++ b/internal/test/playwright_test.go @@ -595,7 +595,7 @@ func spawnAnubisWithOptions(t *testing.T, basePrefix string) string { fmt.Fprintf(w, "%d", time.Now().Unix()) }) - policy, err := libanubis.LoadPoliciesOrDefault(t.Context(), "", anubis.DefaultDifficulty, "info") + policy, err := libanubis.LoadPoliciesOrDefault(t.Context(), "", anubis.DefaultDifficulty, "info", false) if err != nil { t.Fatal(err) } diff --git a/lib/anubis_test.go b/lib/anubis_test.go index 7c0f87d3..3ea07199 100644 --- a/lib/anubis_test.go +++ b/lib/anubis_test.go @@ -58,7 +58,7 @@ func loadPolicies(t *testing.T, fname string, difficulty int) *policy.ParsedConf t.Logf("loading policy file: %s", fname) - anubisPolicy, err := LoadPoliciesOrDefault(ctx, fname, difficulty, "info") + anubisPolicy, err := LoadPoliciesOrDefault(ctx, fname, difficulty, "info", false) if err != nil { t.Fatal(err) } @@ -250,7 +250,7 @@ func TestLoadPolicies(t *testing.T) { } defer fin.Close() - if _, err := policy.ParseConfig(t.Context(), fin, fname, 4, "info"); err != nil { + if _, err := policy.ParseConfig(t.Context(), fin, fname, 4, "info", false); err != nil { t.Fatal(err) } }) diff --git a/lib/config.go b/lib/config.go index e8b7626d..f577ecf7 100644 --- a/lib/config.go +++ b/lib/config.go @@ -55,7 +55,7 @@ type Options struct { DifficultyInJWT bool } -func LoadPoliciesOrDefault(ctx context.Context, fname string, defaultDifficulty int, logLevel string) (*policy.ParsedConfig, error) { +func LoadPoliciesOrDefault(ctx context.Context, fname string, defaultDifficulty int, logLevel string, subrequestMode bool) (*policy.ParsedConfig, error) { var fin io.ReadCloser var err error @@ -79,7 +79,7 @@ func LoadPoliciesOrDefault(ctx context.Context, fname string, defaultDifficulty } }(fin) - anubisPolicy, err := policy.ParseConfig(ctx, fin, fname, defaultDifficulty, logLevel) + anubisPolicy, err := policy.ParseConfig(ctx, fin, fname, defaultDifficulty, logLevel, subrequestMode) if err != nil { return nil, fmt.Errorf("can't parse policy file %s: %w", fname, err) } diff --git a/lib/config_test.go b/lib/config_test.go index 99aab736..3fdc3259 100644 --- a/lib/config_test.go +++ b/lib/config_test.go @@ -12,7 +12,7 @@ import ( ) func TestInvalidChallengeMethod(t *testing.T) { - if _, err := LoadPoliciesOrDefault(t.Context(), "testdata/invalid-challenge-method.yaml", 4, "info"); !errors.Is(err, policy.ErrChallengeRuleHasWrongAlgorithm) { + if _, err := LoadPoliciesOrDefault(t.Context(), "testdata/invalid-challenge-method.yaml", 4, "info", false); !errors.Is(err, policy.ErrChallengeRuleHasWrongAlgorithm) { t.Fatalf("wanted error %v but got %v", policy.ErrChallengeRuleHasWrongAlgorithm, err) } } @@ -25,7 +25,7 @@ func TestBadConfigs(t *testing.T) { for _, st := range finfos { t.Run(st.Name(), func(t *testing.T) { - if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "bad", st.Name()), anubis.DefaultDifficulty, "info"); err == nil { + if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "bad", st.Name()), anubis.DefaultDifficulty, "info", false); err == nil { t.Fatal(err) } else { t.Log(err) @@ -44,13 +44,13 @@ func TestGoodConfigs(t *testing.T) { t.Run(st.Name(), func(t *testing.T) { t.Run("with-thoth", func(t *testing.T) { ctx := thothmock.WithMockThoth(t) - if _, err := LoadPoliciesOrDefault(ctx, filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := LoadPoliciesOrDefault(ctx, filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) t.Run("without-thoth", func(t *testing.T) { - if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := LoadPoliciesOrDefault(t.Context(), filepath.Join("config", "testdata", "good", st.Name()), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) diff --git a/lib/policy/checker.go b/lib/policy/checker.go index 8b88a5be..38003164 100644 --- a/lib/policy/checker.go +++ b/lib/policy/checker.go @@ -94,23 +94,26 @@ func (hmc *HeaderMatchesChecker) Hash() string { } type PathChecker struct { - regexp *regexp.Regexp - hash string + regexp *regexp.Regexp + hash string + subRequestMode bool } -func NewPathChecker(rexStr string) (checker.Impl, error) { +func NewPathChecker(rexStr string, subrequestMode bool) (checker.Impl, error) { rex, err := regexp.Compile(strings.TrimSpace(rexStr)) if err != nil { return nil, fmt.Errorf("%w: regex %s failed parse: %w", ErrMisconfiguration, rexStr, err) } - return &PathChecker{rex, internal.FastHash(rexStr)}, nil + return &PathChecker{rex, internal.FastHash(rexStr), subrequestMode}, nil } func (pc *PathChecker) Check(r *http.Request) (bool, error) { - originalUrl := r.Header.Get("X-Original-URI") - if originalUrl != "" { - if pc.regexp.MatchString(originalUrl) { - return true, nil + if pc.subRequestMode { + originalUrl := r.Header.Get("X-Original-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 ce27a782..59ee03fb 100644 --- a/lib/policy/checker_test.go +++ b/lib/policy/checker_test.go @@ -272,8 +272,8 @@ func TestPathChecker_XOriginalURI(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create the PathChecker - pc, err := NewPathChecker(tt.regex) + // Create the PathChecker in subrequest mode so X-Original-URI is honored. + pc, err := NewPathChecker(tt.regex, true) if err != nil { if !tt.expectError { t.Fatalf("NewPathChecker() unexpected error: %v", err) @@ -305,3 +305,108 @@ func TestPathChecker_XOriginalURI(t *testing.T) { }) } } + +// TestPathChecker_GHSA_6wcg_mqvh_fcvg is a regression test for +// https://github.com/TecharoHQ/anubis/security/advisories/GHSA-6wcg-mqvh-fcvg. +// +// PR https://github.com/TecharoHQ/anubis/pull/1015 added the ability for +// reverse proxies using Anubis in subrequest auth mode to look at the path +// of a request as there are many rules in the wild that rely on checking +// the path. This is how access to things like robots.txt or anything in the +// .well-known directory is unaffected by Anubis. +// +// However this logic was also enabled for non-subrequest deployments of Anubis, +// meaning that a specially crafted request could include a /.well-known/ +// path in it and then get around Anubis with little effort. +// +// This fix gates the logic behind a new plumbed variable named subrequestMode +// that only fires when Anubis is running in subrequest auth mode. This +// properly contains that workaround so that the logic does not fire in +// most deployments. +func TestPathChecker_GHSA_6wcg_mqvh_fcvg(t *testing.T) { + tests := []struct { + name string + regex string + urlPath string + xOriginalURI string + subRequestMode bool + want bool + }{ + { + name: "default mode ignores spoofed X-Original-URI when real path matches", + regex: "^/admin/.*", + urlPath: "/admin/secret", + xOriginalURI: "/public/index", + subRequestMode: false, + want: true, + }, + { + name: "default mode ignores spoofed X-Original-URI when real path does not match", + regex: "^/admin/.*", + urlPath: "/public/index", + xOriginalURI: "/admin/secret", + subRequestMode: false, + want: false, + }, + { + name: "default mode without X-Original-URI matches real path", + regex: "^/admin/.*", + urlPath: "/admin/dashboard", + xOriginalURI: "", + subRequestMode: false, + want: true, + }, + { + name: "subrequest mode honors X-Original-URI", + regex: "^/admin/.*", + urlPath: "/auth", + xOriginalURI: "/admin/secret", + subRequestMode: true, + want: true, + }, + { + name: "subrequest mode falls back to URL.Path when X-Original-URI does not match", + regex: "^/admin/.*", + urlPath: "/admin/dashboard", + xOriginalURI: "/public/index", + subRequestMode: true, + want: true, + }, + { + name: "subrequest mode with empty X-Original-URI uses URL.Path", + regex: "^/admin/.*", + urlPath: "/admin/dashboard", + xOriginalURI: "", + 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.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-Original-URI=%q)", + got, tt.want, tt.subRequestMode, tt.urlPath, tt.xOriginalURI) + } + }) + } +} diff --git a/lib/policy/policy.go b/lib/policy/policy.go index fd38079f..3889dc8e 100644 --- a/lib/policy/policy.go +++ b/lib/policy/policy.go @@ -60,7 +60,7 @@ func newParsedConfig(orig *config.Config) *ParsedConfig { } } -func ParseConfig(ctx context.Context, fin io.Reader, fname string, defaultDifficulty int, logLevel string) (*ParsedConfig, error) { +func ParseConfig(ctx context.Context, fin io.Reader, fname string, defaultDifficulty int, logLevel string, subrequestMode bool) (*ParsedConfig, error) { c, err := config.Load(fin, fname) if err != nil { return nil, err @@ -152,7 +152,7 @@ func ParseConfig(ctx context.Context, fin io.Reader, fname string, defaultDiffic } if b.PathRegex != nil { - c, err := NewPathChecker(*b.PathRegex) + c, err := NewPathChecker(*b.PathRegex, subrequestMode) if err != nil { validationErrs = append(validationErrs, fmt.Errorf("while processing rule %s path regex: %w", b.Name, err)) } else { diff --git a/lib/policy/policy_test.go b/lib/policy/policy_test.go index 7f14d3f9..ff38fb36 100644 --- a/lib/policy/policy_test.go +++ b/lib/policy/policy_test.go @@ -19,7 +19,7 @@ func TestDefaultPolicyMustParse(t *testing.T) { } defer fin.Close() - if _, err := ParseConfig(ctx, fin, "botPolicies.yaml", anubis.DefaultDifficulty, "info"); err != nil { + if _, err := ParseConfig(ctx, fin, "botPolicies.yaml", anubis.DefaultDifficulty, "info", false); err != nil { t.Fatalf("can't parse config: %v", err) } } @@ -41,7 +41,7 @@ func TestGoodConfigs(t *testing.T) { defer fin.Close() ctx := thothmock.WithMockThoth(t) - if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) @@ -53,7 +53,7 @@ func TestGoodConfigs(t *testing.T) { } defer fin.Close() - if _, err := ParseConfig(t.Context(), fin, fin.Name(), anubis.DefaultDifficulty, "info"); err != nil { + if _, err := ParseConfig(t.Context(), fin, fin.Name(), anubis.DefaultDifficulty, "info", false); err != nil { t.Fatal(err) } }) @@ -77,7 +77,7 @@ func TestBadConfigs(t *testing.T) { } defer fin.Close() - if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info"); err == nil { + if _, err := ParseConfig(ctx, fin, fin.Name(), anubis.DefaultDifficulty, "info", false); err == nil { t.Fatal(err) } else { t.Log(err)