From b727015bfcceaf67d572eb3fc0d4a5f6957cbf12 Mon Sep 17 00:00:00 2001 From: marcoome Date: Tue, 15 Sep 2026 17:41:50 +0200 Subject: [PATCH] fix(waf): push rule suppressions to nodes as SecRuleRemoveById Suppressing a rule in Admin -> Security -> WAF only hid its events; in blocking mode the node kept answering 403. Active suppressions (global and per-route) are now appended to every WAF-enabled route's directives after the CRS include, and saving or deleting one re-pushes all nodes. Rule IDs are whitelisted (NNN or NNN-MMM) before they reach SecLang since the form stores free text. Root cause of the report: not SSE. Beszel's dashboard opens /api/realtime fine, then POSTs subscription topics like "systems/*"; CRS 942100 (libinjection) reads "/*" as a SQL comment, the anomaly score hits the threshold and the POST gets 403, so the SDK reconnects forever. Verified in headless Chrome against a local Beszel hub behind caddy+coraza v2.5.0: blocking mode fails, blocking mode + SecRuleRemoveById 942100 passes. Documented in docs/WAF.md. Fixes #14 --- CHANGELOG.md | 4 ++ docs/WAF.md | 21 +++++++ internal/domain/routes/build.go | 5 ++ internal/domain/routes/waf_suppress.go | 62 +++++++++++++++++++ internal/domain/routes/waf_suppress_test.go | 35 +++++++++++ .../httpserver/handlers/admin_waf_events.go | 10 +++ 6 files changed, 137 insertions(+) create mode 100644 internal/domain/routes/waf_suppress.go create mode 100644 internal/domain/routes/waf_suppress_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ff8220c..6205a6d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project. Format: [Keep a Changelog](https://keepacha ## [Unreleased] +### Fixed + +- **WAF suppressions now reach the nodes** ([#14](https://github.com/host-yt/caddy-proxy-manager/issues/14)). Suppressing a rule only hid its events; in blocking mode the node kept returning 403. Active suppressions (global and per-route) are now emitted as `SecRuleRemoveById` after the CRS include on every WAF-enabled route, and saving or deleting one re-pushes all nodes. Rule IDs are validated (`NNN` or `NNN-MMM`) before they reach SecLang. The report itself was a CRS `942100` false positive on PocketBase/Beszel realtime subscription topics (`systems/*`), not an SSE transport problem; documented in `docs/WAF.md`. + ## [1.4.8] - 2026-08-29 Stabilization pass over the findings from an external review: the tenant diff --git a/docs/WAF.md b/docs/WAF.md index 03d6b975..e7d72bc7 100644 --- a/docs/WAF.md +++ b/docs/WAF.md @@ -109,6 +109,24 @@ Two consequences worth knowing: request normally, it would be uninspected. Turn WebSocket support off on routes that do not need it; the WAF then covers every request with no bypass. +### Blocking mode and JSON API false positives + +Detection-only mode never changes a response, so an app that "breaks when the WAF +is on" is almost always in blocking mode with a CRS rule matching legitimate +traffic. The Events page shows which rule; suppress it for that route. + +Known case - PocketBase-based apps such as Beszel (#14): the dashboard opens its +`/api/realtime` event stream (SSE) fine, then registers subscriptions with a POST +whose JSON body contains topics like `systems/*`. Rule `942100` (libinjection SQLi) +reads the `/*` as a SQL comment, the anomaly score reaches the blocking threshold +and the POST gets a 403. The SDK reconnects and retries forever, which looks like a +hung EventSource. Suppress `942100` on that route, or add the equivalent directive +by hand: + +``` +SecRuleRemoveById 942100 +``` + ## Events Every rule match is stored in the `waf_events` table: @@ -129,6 +147,9 @@ Every rule match is stored in the `waf_events` table: View events at Admin → Security → WAF Events, filterable by route and severity. Individual events can be acknowledged. Frequent false-positive rules can be suppressed globally or per-route in `waf_rule_suppressions` (Admin → Security → WAF Suppressions). +A suppression does two things: it hides the rule's events, and it is emitted as +`SecRuleRemoveById ` on every affected route, so in blocking mode the rule stops +blocking as well. Saving or deleting a suppression re-pushes every node. Export: the WAF Events page has an "Export CSV" button. diff --git a/internal/domain/routes/build.go b/internal/domain/routes/build.go index a9ac4dcf..940f95fe 100644 --- a/internal/domain/routes/build.go +++ b/internal/domain/routes/build.go @@ -215,6 +215,8 @@ func (s *Service) buildRoutesForNode(ctx context.Context, nodeID int64) ([]caddy // Operator's fail-open choice governs whether a route whose mTLS enforcement // cannot be emitted is served open or denied. Loaded once per build. mtlsFailOpen := s.loadMTLSFailOpen(ctx) + // Suppressed WAF rules are removed from every route's ruleset (#14). + wafSups := s.loadWAFSuppressions(ctx) rows, err := s.DB.QueryContext(ctx, `SELECT r.id, r.domain, COALESCE(r.aliases,''), COALESCE(r.aliases_verified,''), r.path_prefix, r.upstream_port, r.upstream_scheme, r.upstream_skip_tls_verify, r.websocket, r.force_https, @@ -540,6 +542,9 @@ func (s *Service) buildRoutesForNode(ctx context.Context, nodeID int64) ([]caddy // mTLS respects the operator's mtls.fail_open; the portal never does. portalReady := s.PanelInternalHost != "" && s.PanelInternalPort != 0 mtlsEnforceable := sslEnabled && mtlsCACertPEM != "" && caddyapi.MTLSCAUsable(mtlsCACertPEM) + if wafEnabled { + wafDirectives = appendWAFDirectives(wafDirectives, wafSuppressionDirectives(wafSups, id)) + } built = append(built, caddyapi.Route{ ID: fmt.Sprintf("%d", id), Hosts: hosts, diff --git a/internal/domain/routes/waf_suppress.go b/internal/domain/routes/waf_suppress.go new file mode 100644 index 00000000..2d995ad3 --- /dev/null +++ b/internal/domain/routes/waf_suppress.go @@ -0,0 +1,62 @@ +package routes + +import ( + "context" + "database/sql" + "regexp" + "strings" + + "github.com/host-yt/caddy-proxy-manager/internal/wafevents" +) + +// rule_id is free text in the suppress form; only a plain ID or ID range may +// reach seclang, anything else could smuggle directives into the node config. +var wafRuleIDRe = regexp.MustCompile(`^[0-9]{1,9}(-[0-9]{1,9})?$`) + +// loadWAFSuppressions returns the active suppressions once per build. A read +// error degrades to "none" so a push never fails on this. +func (s *Service) loadWAFSuppressions(ctx context.Context) []wafevents.Suppression { + if s.DB == nil { + return nil + } + sups, err := wafevents.New(func() *sql.DB { return s.DB }).ListSuppressions(ctx, nil) + if err != nil && s.Logger != nil { + s.Logger.Warn("waf suppressions load", "err", err) + } + return sups +} + +// wafSuppressionDirectives renders SecRuleRemoveById lines for the suppressions +// that apply to routeID (global or scoped to it). A suppressed rule must stop +// blocking on the node, not just disappear from the events page (#14). +func wafSuppressionDirectives(sups []wafevents.Suppression, routeID int64) string { + var b strings.Builder + seen := map[string]bool{} + for _, sup := range sups { + if sup.RouteID.Valid && sup.RouteID.Int64 != routeID { + continue + } + id := strings.TrimSpace(sup.RuleID) + if seen[id] || !wafRuleIDRe.MatchString(id) { + continue + } + seen[id] = true + b.WriteString("SecRuleRemoveById ") + b.WriteString(id) + b.WriteString("\n") + } + return strings.TrimRight(b.String(), "\n") +} + +// appendWAFDirectives joins the operator's directives with generated ones, +// keeping both after the CRS include (see caddyapi.BuildRoute). +func appendWAFDirectives(base, extra string) string { + base = strings.TrimSpace(base) + if extra == "" { + return base + } + if base == "" { + return extra + } + return base + "\n" + extra +} diff --git a/internal/domain/routes/waf_suppress_test.go b/internal/domain/routes/waf_suppress_test.go new file mode 100644 index 00000000..466a652e --- /dev/null +++ b/internal/domain/routes/waf_suppress_test.go @@ -0,0 +1,35 @@ +package routes + +import ( + "database/sql" + "testing" + + "github.com/host-yt/caddy-proxy-manager/internal/wafevents" +) + +func TestWAFSuppressionDirectives(t *testing.T) { + route := func(id int64) sql.NullInt64 { return sql.NullInt64{Int64: id, Valid: true} } + sups := []wafevents.Suppression{ + {RuleID: "942100"}, // global + {RuleID: "920350", RouteID: route(7)}, // this route + {RuleID: "941100", RouteID: route(8)}, // other route + {RuleID: "942100"}, // duplicate + {RuleID: "930100-930199"}, // range is valid seclang + {RuleID: "942100; SecRuleEngine Off"}, // injection attempt + {RuleID: "abc"}, + } + got := wafSuppressionDirectives(sups, 7) + want := "SecRuleRemoveById 942100\nSecRuleRemoveById 920350\nSecRuleRemoveById 930100-930199" + if got != want { + t.Fatalf("got\n%s\nwant\n%s", got, want) + } + if got := wafSuppressionDirectives(nil, 7); got != "" { + t.Fatalf("no suppressions must yield empty, got %q", got) + } + if got := appendWAFDirectives(" SecRuleRemoveById 1 ", "SecRuleRemoveById 2"); got != "SecRuleRemoveById 1\nSecRuleRemoveById 2" { + t.Fatalf("append: %q", got) + } + if got := appendWAFDirectives("", ""); got != "" { + t.Fatalf("append empty: %q", got) + } +} diff --git a/internal/httpserver/handlers/admin_waf_events.go b/internal/httpserver/handlers/admin_waf_events.go index 9d69a48e..483291b2 100644 --- a/internal/httpserver/handlers/admin_waf_events.go +++ b/internal/httpserver/handlers/admin_waf_events.go @@ -268,6 +268,11 @@ func (h *AdminHandlers) WAFSuppressRule(w http.ResponseWriter, r *http.Request) Meta: map[string]any{"rule_id": ruleID, "route_id": routeID, "reason": reason}, }) } + // Suppressions are baked into node config as SecRuleRemoveById; without a + // push a blocking rule keeps firing until an unrelated route change. + if h.Routes != nil { + h.Routes.SchedulePushAllNodes(h.Routes.BackgroundCtx()) + } redirectWithFlash(w, r, "/admin/waf", "Rule suppressed", "") } @@ -314,6 +319,11 @@ func (h *AdminHandlers) WAFDeleteSuppression(w http.ResponseWriter, r *http.Requ EntityID: strconv.FormatInt(id, 10), }) } + // Suppressions are baked into node config as SecRuleRemoveById; without a + // push a blocking rule keeps firing until an unrelated route change. + if h.Routes != nil { + h.Routes.SchedulePushAllNodes(h.Routes.BackgroundCtx()) + } redirectWithFlash(w, r, "/admin/waf", "Suppression deleted", "") }