diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ff8220..6205a6d 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 03d6b97..e7d72bc 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 a9ac4dc..940f95f 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 0000000..2d995ad --- /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 0000000..466a652 --- /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 9d69a48..483291b 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", "") }