diff --git a/CLA.md b/CLA.md new file mode 100644 index 0000000..899193d --- /dev/null +++ b/CLA.md @@ -0,0 +1,103 @@ +# Contributor License Agreement + +> **This is a template and has not been reviewed by a lawyer.** It follows the +> structure of the widely used Apache Individual CLA, but you should have it +> reviewed by qualified counsel before relying on it. Whoever adopts it is +> responsible for its adequacy in their jurisdiction. + +## Why this exists + +Forms WAF is MIT licensed today. This agreement grants Dobrev IT Ltd a licence +broad enough to relicense or dual-license the project in future — for example +under an open-core model — without having to locate and obtain permission from +every past contributor. + +**You keep the copyright in your contribution.** This is a licence grant, not an +assignment. You remain free to use your own work however you wish, including in +other projects. + +If you would rather not sign, that is entirely reasonable. Please open an issue +describing what you want to change; a maintainer may be able to implement it +independently. + +--- + +## Agreement + +By submitting a Contribution to this project, You accept and agree to the +following terms for Your present and future Contributions. + +**1. Definitions** + +"You" means the copyright owner, or the legal entity authorised by the copyright +owner, entering into this agreement. + +"Contribution" means any original work of authorship, including any +modification of or addition to an existing work, that is intentionally submitted +by You to the project for inclusion in or documentation of any of its products. +"Submitted" means any form of electronic, verbal or written communication sent +to the project or its maintainers, including but not limited to pull requests, +issues and electronic mailing lists, excluding communication conspicuously +marked or otherwise designated in writing by You as "Not a Contribution". + +**2. Grant of Copyright Licence** + +You grant to Dobrev IT Ltd and to recipients of software distributed by it a +perpetual, worldwide, non-exclusive, no-charge, royalty-free, irrevocable +copyright licence to reproduce, prepare derivative works of, publicly display, +publicly perform, sublicense and distribute Your Contributions and such +derivative works, **under any licensing terms, including proprietary terms**. + +**3. Grant of Patent Licence** + +You grant to Dobrev IT Ltd and to recipients of software distributed by it a +perpetual, worldwide, non-exclusive, no-charge, royalty-free, irrevocable +(except as stated in this section) patent licence to make, have made, use, offer +to sell, sell, import and otherwise transfer the Work, where such licence +applies only to those patent claims licensable by You that are necessarily +infringed by Your Contribution alone or by combination of Your Contribution with +the Work. + +If any entity institutes patent litigation alleging that Your Contribution, or +the Work to which You contributed, constitutes direct or contributory patent +infringement, then any patent licences granted to that entity under this +agreement terminate as of the date such litigation is filed. + +**4. Your Representations** + +You represent that You are legally entitled to grant the above licences. If Your +employer has rights to intellectual property that You create, You represent that +You have received permission to make the Contribution on behalf of that +employer, that Your employer has waived such rights, or that Your employer has +executed a separate corporate CLA. + +You represent that each Contribution is Your original creation, and that Your +Contribution submissions include complete details of any third-party licence or +other restriction of which You are personally aware and which is associated with +any part of Your Contribution. + +**5. No Warranty** + +You are not expected to provide support for Your Contributions, except to the +extent You desire to do so. Contributions are provided "AS IS", without +warranty of any kind, express or implied, including any warranty of +merchantability or fitness for a particular purpose. + +**6. Notification** + +You agree to notify the project of any facts or circumstances of which You become +aware that would make these representations inaccurate in any respect. + +--- + +## How to sign + +Comment on your first pull request with: + +``` +I have read the CLA document and I hereby sign the CLA. +``` + +Include the name and email address you use for commits. A maintainer will record +it. If you are contributing on behalf of an employer, say so — a corporate CLA +may be needed instead. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..adff893 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,87 @@ +# Contributing + +Thanks for considering a contribution. + +## Before you start + +For anything beyond a small fix, open an issue first. This is a security product +with a defence engine whose scoring is calibrated against real traffic patterns, +and a change that looks harmless can shift the boundary between "flag" and +"block" for every deployment. Discussing the shape of a change first saves +rework. + +**Security issues do not belong in pull requests or public issues.** See +[SECURITY.md](SECURITY.md). + +## Development + +The stack runs under Docker Compose; see [README.md](README.md). `CLAUDE.md`, if +present in your checkout, carries working notes on the architecture. + +```bash +docker compose up -d --build +docker compose exec redis sh /init-data.sh # seed defaults +``` + +## The checks your change must pass + +CI runs these on every pull request, and all of them block: + +```bash +# Lua unit tests (busted, on LuaJIT -- see openresty/spec/README.md) +docker build -t forms-waf-lua-test -f openresty/Dockerfile.test ./openresty +docker run --rm -v "$PWD/openresty/lua:/app/lua:ro" \ + -v "$PWD/openresty/spec:/app/spec:ro" \ + forms-waf-lua-test --verbose /app/spec + +# Admin UI +cd admin-ui && npm ci && npm run typecheck && npm run lint && npm run audit:gate + +# Integration suite (needs a running stack, and admin credentials so it can +# raise the rate limits it would otherwise trip) +WAF_ADMIN_USER=admin WAF_ADMIN_PASS=... ./scripts/test-waf.sh + +# API contract, against a running stack +python3 scripts/check-api-contract.py +``` + +Syntax-check Lua with **LuaJIT**, not the system `luac`. Several modules use +`goto`/`::continue::`, which PUC Lua 5.1 rejects and Lua 5.4 accepts — neither +matches the runtime. + +## Things that are easy to get wrong + +These have each caused a shipped defect, so they are worth knowing up front. + +**A new endpoint config key must be added to `config_resolver.resolve()`.** That +function builds its result from an explicit allowlist, and anything not named +there is silently dropped before the request path sees it. Three features were +inert for exactly this reason. `openresty/tests/config_contract_spec.lua` guards +the seam — add your key to it. + +**A new Admin API route needs an RBAC entry.** `rbac.check_permission()` +default-denies any route with no mapping, so a registered-but-unmapped handler +returns 403 for every role including admin. The startup audit will log the gap; +do not ignore it. + +**A change to an API response shape needs the OpenAPI spec updated.** +`docs/openapi.yaml` is validated against a live server, and the admin UI's types +are generated from it. Both directions are enforced. + +**Defaults are a security decision.** Shipping something permissive "so the +tests pass" is how the allowlist ended up disabling the WAF. If a test and a +default disagree, work out which one is wrong. + +## Commits and pull requests + +- `type: Subject` — `feat:`, `fix:`, `docs:`, `test:`, `chore:`, `ci:` +- Explain *why*, not just what. A commit that says what the diff already says is + a wasted opportunity. +- Say what you verified, and how. "Tests pass" is weaker than the numbers. +- Branch from `main`, open a PR against `main`. + +## Contributor License Agreement + +Contributions require a signed CLA — see [CLA.md](CLA.md). This lets the project +relicense or dual-license in future without tracking down every past +contributor. You keep the copyright in your contribution. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..0a5d1e0 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,53 @@ +# Security Policy + +## Reporting a vulnerability + +**Please do not open a public issue for security problems.** + +Report privately through either channel: + +- GitHub's [private vulnerability reporting](https://github.com/dobrevit/forms-waf/security/advisories/new) (preferred — it keeps the discussion attached to the repository) +- Email **security@dobrev.eu** + +Please include enough detail to reproduce: affected version or commit, configuration relevant to the issue, and the steps or request that triggers it. A proof of concept helps, but a clear description is enough to get started. + +### What to expect + +| Stage | Target | +|---|---| +| Acknowledgement | 3 working days | +| Initial assessment | 10 working days | +| Fix or mitigation for a confirmed high-severity issue | 30 days | + +If a report is disputed, we will explain the reasoning rather than closing it silently. If a fix will take longer than the target, we will say so and why. + +## Scope + +In scope: + +- The WAF request path (`openresty/lua/`) — bypasses, request smuggling, injection, denial of service +- The Admin API and its authentication, session handling and RBAC +- The admin UI (`admin-ui/`) +- Default configuration shipped in `redis/init-data.sh` and `helm/` +- The Helm chart and container images + +Out of scope: + +- Vulnerabilities in a deployer's own upstream application +- Findings that require a misconfiguration explicitly warned against in the documentation +- Automated scanner output with no demonstrated impact +- Denial of service through sheer traffic volume against an under-provisioned deployment + +## A note on defaults + +This is a WAF, so its shipped defaults are part of its security posture. Reports +about defaults that weaken protection are in scope and welcome — a previous +release seeded an IP allowlist covering every RFC1918 range, which disabled +inspection entirely in the topology the product targets. That class of issue +matters as much as a code-level bug. + +## Disclosure + +We aim to publish an advisory once a fix is available, crediting the reporter +unless anonymity is requested. If you intend to publish independently, please +give us a reasonable window to ship a fix first. diff --git a/admin-ui/src/App.tsx b/admin-ui/src/App.tsx index ba0b8c4..7f76210 100644 --- a/admin-ui/src/App.tsx +++ b/admin-ui/src/App.tsx @@ -27,6 +27,7 @@ import AttackSignatures from '@/pages/security/AttackSignatures' import AttackSignatureEditor from '@/pages/security/AttackSignatureEditor' import BehavioralAnalytics from '@/pages/analytics/BehavioralAnalytics' import ClusterStatus from '@/pages/cluster/ClusterStatus' +import ShadowMode from '@/pages/shadow/ShadowMode' import { About } from '@/pages/About' import { Users } from '@/pages/admin/Users' import { AuthProviders } from '@/pages/admin/AuthProviders' @@ -87,6 +88,7 @@ export default function App() { } /> } /> } /> + } /> } /> } /> } /> diff --git a/admin-ui/src/api/client.ts b/admin-ui/src/api/client.ts index c486b17..11538f8 100644 --- a/admin-ui/src/api/client.ts +++ b/admin-ui/src/api/client.ts @@ -1409,3 +1409,98 @@ export const backupApi = { return backup }, } + +// --------------------------------------------------------------------------- +// Shadow mode +// +// What monitoring mode *would* have blocked. The point of these is to let an +// operator see the consequences of a rule set before it starts rejecting real +// traffic, then promote it once the sample looks right. +// --------------------------------------------------------------------------- + +export interface ShadowCount { + name: string + count: number +} + +export interface ShadowDecision { + ts: number + vhost_id: string + endpoint_id: string + client_ip?: string + host?: string + path?: string + method?: string + score: number + blocked_by?: string[] + flags?: string[] +} + +export interface ShadowScope { + vhost_id: string + endpoint_id: string + would_block_count: number +} + +export interface ShadowSummary { + would_block_total: number + // Non-zero means the recorder's buffer overflowed and every count below + // understates reality. + dropped_total: number + retained_decisions: number + top_rules: ShadowCount[] + top_flags: ShadowCount[] + scopes: ShadowScope[] +} + +export interface ShadowImpact { + vhost_id: string + endpoint_id?: string + would_block_count: number + unique_client_ips: number + average_score: number + window_start?: number + window_end?: number + sample_incomplete: boolean + dropped_total?: number + top_rules: ShadowCount[] + top_flags: ShadowCount[] + affected_endpoints: ShadowCount[] +} + +export interface ShadowPromoteResponse { + promoted: boolean + vhost_id: string + mode?: string + previous_mode?: string + message?: string +} + +export const shadowApi = { + summary: () => request('/shadow/summary'), + + decisions: (params?: { limit?: number; vhost_id?: string; endpoint_id?: string }) => { + const q = new URLSearchParams() + if (params?.limit) q.set('limit', String(params.limit)) + if (params?.vhost_id) q.set('vhost_id', params.vhost_id) + if (params?.endpoint_id) q.set('endpoint_id', params.endpoint_id) + const qs = q.toString() + return request<{ decisions: ShadowDecision[]; count: number; limit?: number }>( + `/shadow/decisions${qs ? `?${qs}` : ''}` + ) + }, + + impact: (vhostId: string, endpointId?: string) => { + const q = new URLSearchParams({ vhost_id: vhostId }) + if (endpointId) q.set('endpoint_id', endpointId) + return request(`/shadow/impact?${q.toString()}`) + }, + + promote: (vhostId: string) => + request('/shadow/promote', { + method: 'POST', + body: JSON.stringify({ vhost_id: vhostId }), + }), + + clear: () => request<{ cleared: boolean }>('/shadow/decisions', { method: 'DELETE' }), +} diff --git a/admin-ui/src/api/generated.ts b/admin-ui/src/api/generated.ts index d3d4979..6c6323f 100644 --- a/admin-ui/src/api/generated.ts +++ b/admin-ui/src/api/generated.ts @@ -195,6 +195,121 @@ export interface paths { patch?: never; trace?: never; }; + "/shadow/summary": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** What monitoring mode would have blocked */ + get: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Aggregate view over the retained window */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ShadowSummary"]; + }; + }; + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/shadow/decisions": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Individual would-block decisions */ + get: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Recent decisions, newest first */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + decisions: components["schemas"]["ShadowDecision"][]; + count: number; + limit?: number; + }; + }; + }; + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/shadow/impact": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Pre-flight impact of promoting a scope to blocking */ + get: { + parameters: { + query: { + vhost_id: string; + endpoint_id?: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description What promoting this scope would have blocked */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ShadowImpact"]; + }; + }; + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/keywords/blocked": { parameters: { query?: never; @@ -374,6 +489,52 @@ export interface components { high_event_rate?: number; }; }; + ShadowDecision: { + /** @description Unix time the decision was made */ + ts: number; + vhost_id: string; + endpoint_id: string; + client_ip?: string; + host?: string; + path?: string; + method?: string; + score: number; + blocked_by?: string[]; + flags?: string[]; + }; + ShadowCount: { + name: string; + count: number; + }; + ShadowSummary: { + would_block_total: number; + /** @description Records lost to the recorder's buffer cap. Non-zero means the sample is incomplete and the counts understate reality. */ + dropped_total: number; + retained_decisions: number; + top_rules: components["schemas"]["ShadowCount"][]; + /** @description Detection-level attribution; this is what identifies a rule to suppress. */ + top_flags: components["schemas"]["ShadowCount"][]; + scopes: { + vhost_id: string; + endpoint_id: string; + would_block_count: number; + }[]; + }; + ShadowImpact: { + vhost_id: string; + endpoint_id?: string; + would_block_count: number; + unique_client_ips: number; + average_score: number; + window_start?: number; + window_end?: number; + /** @description True when records were dropped, so this understates the impact. */ + sample_incomplete: boolean; + dropped_total?: number; + top_rules?: components["schemas"]["ShadowCount"][]; + top_flags?: components["schemas"]["ShadowCount"][]; + affected_endpoints?: components["schemas"]["ShadowCount"][]; + }; WafStatus: { redis_host?: string; redis_port?: number; diff --git a/admin-ui/src/components/layout/Sidebar.tsx b/admin-ui/src/components/layout/Sidebar.tsx index 24b177c..d3f6e8a 100644 --- a/admin-ui/src/components/layout/Sidebar.tsx +++ b/admin-ui/src/components/layout/Sidebar.tsx @@ -23,6 +23,7 @@ import { Activity, Server, Fingerprint, + EyeOff, MessageSquare, Workflow, Target, @@ -50,6 +51,7 @@ const navigation = [ { name: 'Security', children: [ + { name: 'Shadow Mode', href: '/security/shadow', icon: EyeOff }, { name: 'Form Timing', href: '/security/timing', icon: Clock }, { name: 'Defense Profiles', href: '/security/defense-profiles', icon: Workflow }, { name: 'Attack Signatures', href: '/security/attack-signatures', icon: Target }, diff --git a/admin-ui/src/pages/shadow/ShadowMode.tsx b/admin-ui/src/pages/shadow/ShadowMode.tsx new file mode 100644 index 0000000..7199ee7 --- /dev/null +++ b/admin-ui/src/pages/shadow/ShadowMode.tsx @@ -0,0 +1,434 @@ +import { useState } from 'react' +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' +import { shadowApi } from '@/api/client' +import type { ShadowCount, ShadowImpact, ShadowScope } from '@/api/client' +import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card' +import { Badge } from '@/components/ui/badge' +import { Button } from '@/components/ui/button' +import { Alert, AlertTitle, AlertDescription } from '@/components/ui/alert' +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from '@/components/ui/alert-dialog' +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from '@/components/ui/table' +import { useToast } from '@/components/ui/use-toast' +import { + EyeOff, + ShieldCheck, + AlertTriangle, + RefreshCw, + Trash2, + ArrowUpCircle, + Fingerprint, +} from 'lucide-react' + +function formatWhen(ts: number): string { + if (!ts) return '-' + const secs = Math.floor(Date.now() / 1000 - ts) + if (secs < 60) return `${secs}s ago` + if (secs < 3600) return `${Math.floor(secs / 60)}m ago` + if (secs < 86400) return `${Math.floor(secs / 3600)}h ago` + return `${Math.floor(secs / 86400)}d ago` +} + +/** + * A count bar. The relative width is what makes a list of flags readable at a + * glance -- "which rule is responsible for most of this" is the whole question. + */ +function CountBars({ items, empty }: { items: ShadowCount[]; empty: string }) { + if (!items?.length) { + return

{empty}

+ } + const max = Math.max(...items.map((i) => i.count)) + return ( +
+ {items.map((item) => ( +
+
+ {item.name} + {item.count} +
+
+
+
+
+ ))} +
+ ) +} + +export default function ShadowMode() { + const { toast } = useToast() + const queryClient = useQueryClient() + const [pending, setPending] = useState(null) + + const { data: summary, isLoading, refetch } = useQuery({ + queryKey: ['shadow', 'summary'], + queryFn: shadowApi.summary, + refetchInterval: 15000, + }) + + const { data: recent } = useQuery({ + queryKey: ['shadow', 'decisions'], + queryFn: () => shadowApi.decisions({ limit: 25 }), + refetchInterval: 15000, + }) + + // Promotion is the one destructive-ish action here: it starts rejecting live + // traffic. So the impact is fetched first and shown for confirmation rather + // than promoting straight from the table. + const impactMutation = useMutation({ + mutationFn: (scope: ShadowScope) => shadowApi.impact(scope.vhost_id), + onSuccess: (impact) => setPending(impact), + onError: (err: Error) => + toast({ title: 'Could not load impact', description: err.message, variant: 'destructive' }), + }) + + const promoteMutation = useMutation({ + mutationFn: (vhostId: string) => shadowApi.promote(vhostId), + onSuccess: (res) => { + setPending(null) + toast({ + title: res.promoted ? `${res.vhost_id} is now blocking` : 'No change', + description: res.promoted + ? `Was ${res.previous_mode}. This vhost now rejects matching traffic.` + : res.message, + }) + queryClient.invalidateQueries({ queryKey: ['shadow'] }) + queryClient.invalidateQueries({ queryKey: ['vhosts'] }) + }, + onError: (err: Error) => + toast({ title: 'Promote failed', description: err.message, variant: 'destructive' }), + }) + + const clearMutation = useMutation({ + mutationFn: shadowApi.clear, + onSuccess: () => { + toast({ title: 'Sample discarded' }) + queryClient.invalidateQueries({ queryKey: ['shadow'] }) + }, + onError: (err: Error) => + toast({ title: 'Could not clear', description: err.message, variant: 'destructive' }), + }) + + if (isLoading) { + return
Loading shadow decisions...
+ } + + const incomplete = (summary?.dropped_total ?? 0) > 0 + + return ( +
+
+
+

+ + Shadow Mode +

+

+ What monitoring mode would have blocked. Review the sample, then promote a + vhost to blocking once it looks right. +

+
+
+ + +
+
+ + {incomplete && ( + + + This sample is incomplete + + {summary?.dropped_total} decision(s) were dropped because the recorder's buffer + filled up. Every count below understates the real impact — treat them as a floor, + not a total. + + + )} + +
+ + + Would have blocked + + {summary?.would_block_total ?? 0} + + + +

+ requests allowed through that a blocking vhost would have rejected +

+ + + + + Decisions retained + + {summary?.retained_decisions ?? 0} + + + +

individually inspectable below

+ + + + + Scopes affected + {summary?.scopes?.length ?? 0} + + +

vhost / endpoint combinations

+ + +
+ +
+ + + + + Top detections + + + The specific signal that fired. This is what identifies a rule to keep or + suppress — the profile name alone does not. + + + + + + + + + + Top rules + The profile or mechanism that produced the decision. + + + + + +
+ + + + Affected scopes + + Promote a vhost once its sample looks right. You will see the impact before + anything changes. + + + + {summary?.scopes?.length ? ( + + + + Vhost + Endpoint + Would block + + + + + {summary.scopes.map((scope) => ( + + {scope.vhost_id} + + {scope.endpoint_id || '-'} + + + {scope.would_block_count} + + + + + + ))} + +
+ ) : ( +

+ No shadow decisions recorded. A vhost in monitoring mode records what it would + have blocked; a vhost already in blocking mode records nothing here. +

+ )} + + + + + + Recent decisions + Newest first. + + + {recent?.decisions?.length ? ( +
+ + + + When + Client + Request + Score + Detections + + + + {recent.decisions.map((d, i) => ( + + + {formatWhen(d.ts)} + + + {d.client_ip || '-'} + + + + {d.method} {d.path} + + + {d.score} + +
+ {(d.flags ?? []).slice(0, 4).map((f) => ( + + {f} + + ))} + {(d.flags?.length ?? 0) > 4 && ( + + +{(d.flags?.length ?? 0) - 4} + + )} +
+ + + ))} + +
+
+ ) : ( +

No decisions recorded.

+ )} + + + + !open && setPending(null)}> + + + + + Promote {pending?.vhost_id} to blocking? + + +
+

+ This vhost will start rejecting traffic immediately. Based on the recorded + sample, it would have blocked: +

+
+
+
+ {pending?.would_block_count ?? 0} +
+
requests
+
+
+
+ {pending?.unique_client_ips ?? 0} +
+
distinct clients
+
+
+
+ {pending?.average_score ?? 0} +
+
average score
+
+
+ {pending?.sample_incomplete && ( + + + + Records were dropped, so the real impact is larger than these figures. + + + )} + {!!pending?.top_flags?.length && ( +
+

Mostly from

+ +
+ )} + {!!pending?.affected_endpoints?.length && ( +
+

Affected endpoints

+
+ {pending.affected_endpoints.map((e) => ( + + {e.name} ({e.count}) + + ))} +
+
+ )} +
+ + + + Cancel + { + e.preventDefault() + if (pending) promoteMutation.mutate(pending.vhost_id) + }} + disabled={promoteMutation.isPending} + > + {promoteMutation.isPending ? 'Promoting...' : 'Promote to blocking'} + + + + +
+ ) +} diff --git a/docs/API_HANDLERS.md b/docs/API_HANDLERS.md index f53e2c9..4fc48be 100644 --- a/docs/API_HANDLERS.md +++ b/docs/API_HANDLERS.md @@ -335,6 +335,24 @@ local ok, err = utils.validate_required(data, {"field1", "field2"}) --- +### Shadow Mode (`api_handlers/shadow.lua`) + +What monitoring mode *would* have blocked, so a rule set can be proven before it +starts rejecting traffic. `shadow_recorder.lua` buffers decisions in a shared +dict; `redis_sync` drains them. + +| Method | Endpoint | Description | +|--------|----------|-------------| +| GET | /shadow/summary | Would-block totals, top rules, top flags, per-scope counts | +| GET | /shadow/decisions | Individual decisions, newest first (`limit`, `vhost_id`, `endpoint_id`) | +| GET | /shadow/impact | Pre-flight impact of promoting one scope (`vhost_id` required) | +| POST | /shadow/promote | Switch a vhost from monitoring to blocking | +| DELETE | /shadow/decisions | Discard the recorded sample | + +`dropped_total` is non-zero when the recorder's buffer overflowed; the sample is +then incomplete and every count understates reality. `sample_incomplete` on the +impact response carries the same warning. + ## RBAC Permission Mapping Each endpoint requires specific permissions. See [RBAC Guide](RBAC.md) for full details. @@ -353,6 +371,7 @@ Each endpoint requires specific permissions. See [RBAC Guide](RBAC.md) for full | captcha | read, update | | security | read, update | | slack | read, update, test | +| shadow | read, promote, delete | --- diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 3309cb4..ad98122 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -113,6 +113,73 @@ components: high_event_count: { type: integer } high_event_rate: { type: integer } + ShadowDecision: + type: object + required: [ts, vhost_id, endpoint_id, score] + properties: + ts: { type: integer, description: Unix time the decision was made } + vhost_id: { type: string } + endpoint_id: { type: string } + client_ip: { type: string } + host: { type: string } + path: { type: string } + method: { type: string } + score: { type: integer } + blocked_by: { type: array, items: { type: string } } + flags: { type: array, items: { type: string } } + + ShadowCount: + type: object + required: [name, count] + properties: + name: { type: string } + count: { type: integer } + + ShadowSummary: + type: object + required: [would_block_total, dropped_total, retained_decisions, top_rules, top_flags, scopes] + properties: + would_block_total: { type: integer } + dropped_total: + type: integer + description: > + Records lost to the recorder's buffer cap. Non-zero means the sample + is incomplete and the counts understate reality. + retained_decisions: { type: integer } + top_rules: { type: array, items: { $ref: "#/components/schemas/ShadowCount" } } + top_flags: + type: array + description: Detection-level attribution; this is what identifies a rule to suppress. + items: { $ref: "#/components/schemas/ShadowCount" } + scopes: + type: array + items: + type: object + required: [vhost_id, endpoint_id, would_block_count] + properties: + vhost_id: { type: string } + endpoint_id: { type: string } + would_block_count: { type: integer } + + ShadowImpact: + type: object + required: [vhost_id, would_block_count, unique_client_ips, average_score, sample_incomplete] + properties: + vhost_id: { type: string } + endpoint_id: { type: string } + would_block_count: { type: integer } + unique_client_ips: { type: integer } + average_score: { type: integer } + window_start: { type: integer } + window_end: { type: integer } + sample_incomplete: + type: boolean + description: True when records were dropped, so this understates the impact. + dropped_total: { type: integer } + top_rules: { type: array, items: { $ref: "#/components/schemas/ShadowCount" } } + top_flags: { type: array, items: { $ref: "#/components/schemas/ShadowCount" } } + affected_endpoints: { type: array, items: { $ref: "#/components/schemas/ShadowCount" } } + WafStatus: type: object required: [redis_connected, blocked_hashes_count, whitelisted_ips_count, endpoints_count, vhosts_count] @@ -197,6 +264,54 @@ paths: emitted_events: { type: array, items: { type: string } } unavailable_events: { type: array, items: { type: string } } + /shadow/summary: + get: + summary: What monitoring mode would have blocked + responses: + "200": + description: Aggregate view over the retained window + content: + application/json: + schema: { $ref: "#/components/schemas/ShadowSummary" } + + /shadow/decisions: + get: + summary: Individual would-block decisions + responses: + "200": + description: Recent decisions, newest first + content: + application/json: + schema: + type: object + required: [decisions, count] + properties: + decisions: + type: array + items: { $ref: "#/components/schemas/ShadowDecision" } + count: { type: integer } + limit: { type: integer } + + /shadow/impact: + get: + summary: Pre-flight impact of promoting a scope to blocking + x-contract-example-query: "vhost_id=_default" + parameters: + - name: vhost_id + in: query + required: true + schema: { type: string } + - name: endpoint_id + in: query + required: false + schema: { type: string } + responses: + "200": + description: What promoting this scope would have blocked + content: + application/json: + schema: { $ref: "#/components/schemas/ShadowImpact" } + /keywords/blocked: get: summary: Blocked keywords diff --git a/openresty/conf/nginx.conf b/openresty/conf/nginx.conf index f804f9e..45892ec 100644 --- a/openresty/conf/nginx.conf +++ b/openresty/conf/nginx.conf @@ -135,6 +135,7 @@ http { lua_shared_dict waf_metrics 10m; # WAF metrics counters (per vhost/endpoint) lua_shared_dict waf_timing 1m; # Timing token encryption keys lua_shared_dict coordinator_cache 1m; # Instance coordinator state + lua_shared_dict shadow_cache 10m; # Shadow-mode would-block buffer (flushed to Redis by a timer) # Initialize modules on worker start init_worker_by_lua_block { diff --git a/openresty/lua/admin_api.lua b/openresty/lua/admin_api.lua index 5fc19ae..5aa4e31 100644 --- a/openresty/lua/admin_api.lua +++ b/openresty/lua/admin_api.lua @@ -19,6 +19,7 @@ local keywords_handler = require "api_handlers.keywords" local config_handler = require "api_handlers.config" local webhooks_handler = require "api_handlers.webhooks" local slack_handler = require "api_handlers.slack" +local shadow_handler = require "api_handlers.shadow" local geoip_handler = require "api_handlers.geoip" local reputation_handler = require "api_handlers.reputation" local bulk_handler = require "api_handlers.bulk" @@ -68,6 +69,7 @@ register_handlers(keywords_handler) register_handlers(config_handler) register_handlers(webhooks_handler) register_handlers(slack_handler) +register_handlers(shadow_handler) register_handlers(geoip_handler) register_handlers(reputation_handler) register_handlers(bulk_handler) diff --git a/openresty/lua/api_handlers/shadow.lua b/openresty/lua/api_handlers/shadow.lua new file mode 100644 index 0000000..79b0728 --- /dev/null +++ b/openresty/lua/api_handlers/shadow.lua @@ -0,0 +1,284 @@ +-- api_handlers/shadow.lua +-- Shadow mode: what monitoring mode would have blocked, and promoting a scope +-- to blocking once you can see it. + +local _M = {} + +local utils = require "api_handlers.utils" +local cjson = require "cjson.safe" +local redis_sync = require "redis_sync" + +local KEYS = { + decisions = "waf:shadow:decisions", + rules = "waf:shadow:rules", + flags = "waf:shadow:flags", + endpoints = "waf:shadow:endpoints", + stats = "waf:shadow:stats", +} + +local DEFAULT_LIMIT = 100 +local MAX_LIMIT = 500 + +-- An empty Lua table encodes as a JSON object. The UI treats these as arrays and +-- calls .map on them, so force array encoding (same defect as the Slack handler). +local function as_array(t) + if type(t) ~= "table" then + return setmetatable({}, cjson.array_mt) + end + return setmetatable(t, cjson.array_mt) +end + +local function redis_hash_to_table(raw) + local out = {} + if type(raw) ~= "table" then return out end + for i = 1, #raw, 2 do + out[raw[i]] = tonumber(raw[i + 1]) or 0 + end + return out +end + +--- Sort a name->count map into a descending array, capped. +local function top_n(counts, limit) + local list = {} + for name, count in pairs(counts) do + table.insert(list, { name = name, count = count }) + end + table.sort(list, function(a, b) + if a.count == b.count then return a.name < b.name end + return a.count > b.count + end) + while #list > (limit or 10) do table.remove(list) end + return as_array(list) +end + +local function query_arg(name, default) + local args = ngx.req.get_uri_args() + local value = args[name] + if type(value) == "table" then value = value[1] end + if value == nil or value == "" then return default end + return value +end + +--- Decode the stored decision list, optionally narrowed to one scope. +local function read_decisions(red, vhost_id, endpoint_id, limit) + -- Unfiltered, every entry read is an entry returned, so ask for exactly the + -- page wanted: a limit=25 UI poll was pulling and decoding 500 JSON blobs. + -- Filtered, the matches can sit anywhere in the list, so the wider read is + -- what makes the filter meaningful. + local scan_to = (vhost_id or endpoint_id) and MAX_LIMIT or limit + local raw = red:lrange(KEYS.decisions, 0, scan_to - 1) + local out = {} + if type(raw) ~= "table" then return out end + + for _, entry in ipairs(raw) do + local decision = cjson.decode(entry) + if decision then + local matches = true + if vhost_id and decision.vhost_id ~= vhost_id then matches = false end + if endpoint_id and decision.endpoint_id ~= endpoint_id then matches = false end + if matches then + table.insert(out, decision) + if #out >= limit then break end + end + end + end + return out +end + +_M.handlers = {} + +-- GET /shadow/summary - what monitoring mode has withheld so far +_M.handlers["GET:/shadow/summary"] = function() + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + local stats = redis_hash_to_table(red:hgetall(KEYS.stats)) + local rules = redis_hash_to_table(red:hgetall(KEYS.rules)) + local flags = redis_hash_to_table(red:hgetall(KEYS.flags)) + local endpoints = redis_hash_to_table(red:hgetall(KEYS.endpoints)) + local retained = red:llen(KEYS.decisions) + utils.close_redis(red) + + -- Split the "vhost|endpoint" aggregate key back into its parts so the UI does + -- not have to know the encoding. + local scopes = {} + for scope, count in pairs(endpoints) do + local vhost_id, endpoint_id = scope:match("^(.-)|(.*)$") + table.insert(scopes, { + vhost_id = vhost_id or scope, + endpoint_id = endpoint_id or "global", + would_block_count = count, + }) + end + table.sort(scopes, function(a, b) return a.would_block_count > b.would_block_count end) + + return utils.json_response({ + would_block_total = stats.would_block_total or 0, + -- Non-zero means the buffer overflowed and the sample is incomplete. The + -- UI must say so rather than presenting the counts as exhaustive. + dropped_total = stats.dropped_total or 0, + retained_decisions = tonumber(retained) or 0, + top_rules = top_n(rules, 10), + -- The actionable breakdown: which detections drove the would-blocks. + top_flags = top_n(flags, 15), + scopes = as_array(scopes), + }) +end + +-- GET /shadow/decisions - the individual records behind the summary +_M.handlers["GET:/shadow/decisions"] = function() + -- Clamped at both ends. Without the floor, limit=0 or a negative made + -- read_decisions return nothing at all, which reads as "no decisions + -- recorded" rather than as bad input. + local limit = tonumber(query_arg("limit", DEFAULT_LIMIT)) or DEFAULT_LIMIT + limit = math.max(1, math.min(math.floor(limit), MAX_LIMIT)) + local vhost_id = query_arg("vhost_id") + local endpoint_id = query_arg("endpoint_id") + + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + local decisions = read_decisions(red, vhost_id, endpoint_id, limit) + utils.close_redis(red) + + return utils.json_response({ + decisions = as_array(decisions), + count = #decisions, + limit = limit, + }) +end + +-- GET /shadow/impact - the pre-flight answer to "what happens if I promote this?" +_M.handlers["GET:/shadow/impact"] = function() + local vhost_id = query_arg("vhost_id") + if not vhost_id then + return utils.error_response("vhost_id is required") + end + local endpoint_id = query_arg("endpoint_id") + + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + local decisions = read_decisions(red, vhost_id, endpoint_id, MAX_LIMIT) + local stats = redis_hash_to_table(red:hgetall(KEYS.stats)) + utils.close_redis(red) + + local rules, flags, ips, endpoints_hit = {}, {}, {}, {} + local score_total, oldest, newest = 0, nil, nil + for _, d in ipairs(decisions) do + for _, rule in ipairs(d.blocked_by or {}) do + rules[rule] = (rules[rule] or 0) + 1 + end + for _, flag in ipairs(d.flags or {}) do + flags[flag] = (flags[flag] or 0) + 1 + end + if d.client_ip then ips[d.client_ip] = true end + local ep = d.endpoint_id or "global" + endpoints_hit[ep] = (endpoints_hit[ep] or 0) + 1 + score_total = score_total + (d.score or 0) + if not oldest or (d.ts or 0) < oldest then oldest = d.ts end + if not newest or (d.ts or 0) > newest then newest = d.ts end + end + + local unique_ips = 0 + for _ in pairs(ips) do unique_ips = unique_ips + 1 end + + return utils.json_response({ + vhost_id = vhost_id, + endpoint_id = endpoint_id, + -- What promoting this scope would have blocked, over the retained window. + would_block_count = #decisions, + unique_client_ips = unique_ips, + average_score = (#decisions > 0) and math.floor(score_total / #decisions) or 0, + window_start = oldest, + window_end = newest, + top_rules = top_n(rules, 10), + top_flags = top_n(flags, 15), + affected_endpoints = top_n(endpoints_hit, 20), + -- Carried through so the caller cannot mistake a truncated sample for the + -- full picture when deciding to promote. + sample_incomplete = (stats.dropped_total or 0) > 0, + dropped_total = stats.dropped_total or 0, + }) +end + +-- POST /shadow/promote - switch a vhost from monitoring to blocking +_M.handlers["POST:/shadow/promote"] = function() + local data, err = utils.get_json_body() + if not data then + return utils.error_response(err or "Invalid JSON") + end + local ok, verr = utils.validate_required(data, { "vhost_id" }) + if not ok then + return utils.error_response(verr) + end + + local red, rerr = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (rerr or "unknown")) + end + + local key = "waf:vhosts:config:" .. data.vhost_id + local raw = red:get(key) + if not raw or raw == ngx.null then + utils.close_redis(red) + return utils.error_response("Virtual host not found: " .. data.vhost_id, 404) + end + + local config = cjson.decode(raw) + if not config then + utils.close_redis(red) + return utils.error_response("Stored vhost configuration is not valid JSON", 500) + end + + config.waf = config.waf or {} + local previous_mode = config.waf.mode + if previous_mode == "blocking" then + utils.close_redis(red) + return utils.json_response({ + promoted = false, + vhost_id = data.vhost_id, + mode = "blocking", + message = "Already in blocking mode; nothing to do", + }) + end + + config.waf.mode = "blocking" + local saved, serr = red:set(key, cjson.encode(config)) + utils.close_redis(red) + if not saved then + return utils.error_response("Failed to save configuration: " .. (serr or "unknown")) + end + + redis_sync.sync_now() + + ngx.log(ngx.WARN, string.format( + "SHADOW PROMOTE: vhost=%s mode %s -> blocking by user=%s", + data.vhost_id, tostring(previous_mode), + (ngx.ctx.admin_user and ngx.ctx.admin_user.username) or "unknown")) + + return utils.json_response({ + promoted = true, + vhost_id = data.vhost_id, + previous_mode = previous_mode, + mode = "blocking", + }) +end + +-- DELETE /shadow/decisions - reset the observation window +_M.handlers["DELETE:/shadow/decisions"] = function() + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + red:del(KEYS.decisions, KEYS.rules, KEYS.flags, KEYS.endpoints, KEYS.stats) + utils.close_redis(red) + return utils.json_response({ cleared = true }) +end + +return _M diff --git a/openresty/lua/rbac.lua b/openresty/lua/rbac.lua index 0794158..e1475dd 100644 --- a/openresty/lua/rbac.lua +++ b/openresty/lua/rbac.lua @@ -37,6 +37,7 @@ local DEFAULT_ROLES = { captcha = {"create", "read", "update", "delete", "enable", "disable", "test"}, webhooks = {"read", "update", "test"}, slack = {"read", "update", "test", "reset"}, + shadow = {"read", "promote", "delete"}, geoip = {"read", "update", "reload"}, reputation = {"read", "update"}, timing = {"read", "update"}, @@ -68,6 +69,7 @@ local DEFAULT_ROLES = { captcha = {"read"}, webhooks = {"read"}, slack = {"read"}, + shadow = {"read"}, geoip = {"read"}, reputation = {"read"}, timing = {"read"}, @@ -96,6 +98,7 @@ local DEFAULT_ROLES = { captcha = {"read"}, webhooks = {"read"}, slack = {"read"}, + shadow = {"read"}, geoip = {"read"}, reputation = {"read"}, timing = {"read"}, @@ -183,6 +186,13 @@ local ENDPOINT_PERMISSIONS = { -- Timing (per-vhost listing) ["GET:/timing/vhosts"] = {resource = "timing", action = "read"}, + -- Shadow mode (what monitoring would have blocked, and promoting a scope) + ["GET:/shadow/summary"] = {resource = "shadow", action = "read"}, + ["GET:/shadow/decisions"] = {resource = "shadow", action = "read"}, + ["GET:/shadow/impact"] = {resource = "shadow", action = "read"}, + ["POST:/shadow/promote"] = {resource = "shadow", action = "promote"}, + ["DELETE:/shadow/decisions"] = {resource = "shadow", action = "delete"}, + -- Slack notifications ["GET:/slack/config"] = {resource = "slack", action = "read"}, ["PUT:/slack/config"] = {resource = "slack", action = "update"}, diff --git a/openresty/lua/redis_sync.lua b/openresty/lua/redis_sync.lua index d28df1a..d0ee1e6 100644 --- a/openresty/lua/redis_sync.lua +++ b/openresty/lua/redis_sync.lua @@ -1155,6 +1155,19 @@ local function do_sync() -- Sync Slack notification configuration sync_slack(red) + -- Drain the shadow-mode buffer here rather than on its own timer: this + -- function already holds an open connection and runs on a predictable + -- interval. + local ok_shadow, shadow_recorder = pcall(require, "shadow_recorder") + if ok_shadow and shadow_recorder then + local ok_flush, flushed = pcall(shadow_recorder.flush, red) + if not ok_flush then + ngx.log(ngx.ERR, "shadow recorder flush failed: ", tostring(flushed)) + elseif flushed > 0 then + ngx.log(ngx.INFO, "shadow recorder: flushed ", flushed, " would-block decisions") + end + end + close_redis(red) ngx.log(ngx.INFO, "Redis sync completed") diff --git a/openresty/lua/shadow_recorder.lua b/openresty/lua/shadow_recorder.lua new file mode 100644 index 0000000..02e6342 --- /dev/null +++ b/openresty/lua/shadow_recorder.lua @@ -0,0 +1,227 @@ +--[[ + Shadow mode recorder + ==================== + Monitoring mode already decides what it *would* have blocked -- waf_handler + computes is_monitoring_would_block, writes a line to the error log and bumps a + counter. Then it throws the detail away, so the only way to answer "what + happens if I turn blocking on?" is to grep logs. + + This keeps that decision, with attribution, so the answer becomes a query. + + Two constraints shape the design, both learned from defects in this codebase: + + * Nothing in the request path may read or write Redis (R-04). Recording + therefore writes to a shared dict and a timer flushes it. + * One timer per request is how you exhaust lua_max_running_timers and + starve redis_sync (R-03). A single periodic flush is used instead, the + same shape field_learner already uses for its batching. + + Storage is bounded on purpose. A WAF in monitoring mode on a busy site sees + everything, and an unbounded recorder becomes its own outage. +]] + +local cjson = require "cjson.safe" + +local _M = {} + +local shadow_cache = ngx.shared.shadow_cache + +-- Redis keys +local KEYS = { + decisions = "waf:shadow:decisions", -- capped list of recent records + rules = "waf:shadow:rules", -- HASH profile/rule -> count + flags = "waf:shadow:flags", -- HASH detection flag -> count + endpoints = "waf:shadow:endpoints", -- HASH vhost|endpoint -> count + stats = "waf:shadow:stats", -- HASH totals +} + +-- Bounds. These are deliberate ceilings, not tuning knobs to raise casually: +-- every one of them is what stops a busy site's monitoring mode from becoming an +-- incident of its own. +local MAX_BUFFERED = 500 -- records held in the shared dict between flushes +local MAX_DECISIONS = 2000 -- records retained in Redis for the detail view +local DECISION_TTL = 7 * 24 * 3600 +local MAX_FLAG_LENGTH = 120 +local MAX_PATH_LENGTH = 256 + +local BUFFER_HEAD = "shadow:head" -- next write slot +local BUFFER_TAIL = "shadow:tail" -- last slot successfully flushed +local DROPPED = "shadow:dropped" +local DRAIN_LOCK = "shadow:draining" +-- Long enough that a real drain never trips it, short enough that a worker +-- killed mid-drain cannot wedge the buffer for more than one sync interval or two. +local DRAIN_LOCK_TTL = 30 + +--- Trim a value that originates from the request, so a long or hostile field +--- cannot bloat storage. +local function clip(value, limit) + if value == nil then return nil end + value = tostring(value) + if #value > limit then + return value:sub(1, limit) .. "..." + end + return value +end + +--- Record one would-block decision. Called from the request path, so this must +--- stay allocation-light and must never touch Redis. +-- @param decision table vhost_id, endpoint_id, client_ip, host, path, method, +-- score, blocked_by (array), flags (array) +function _M.record(decision) + if not shadow_cache or type(decision) ~= "table" then + return false + end + + local head = shadow_cache:incr(BUFFER_HEAD, 1, 0) + if not head then + return false + end + + local tail = shadow_cache:get(BUFFER_TAIL) or 0 + if head - tail > MAX_BUFFERED then + -- Buffer is full: drop this record rather than evicting an older one, and + -- count the loss so the UI can say "showing a sample" instead of implying + -- completeness. + shadow_cache:incr(DROPPED, 1, 0) + return false, "buffer full" + end + + local record = cjson.encode({ + ts = ngx.time(), + vhost_id = decision.vhost_id or "unknown", + endpoint_id = decision.endpoint_id or "global", + client_ip = decision.client_ip, + host = clip(decision.host, MAX_PATH_LENGTH), + path = clip(decision.path, MAX_PATH_LENGTH), + method = decision.method, + score = decision.score or 0, + blocked_by = decision.blocked_by, + flags = decision.flags, + }) + if not record then + return false + end + + -- Slots expire well after a flush would have consumed them, so a stalled + -- flush cannot pin memory indefinitely. + shadow_cache:set("shadow:rec:" .. head, record, 600) + return true +end + +--- Write one slot range to Redis. Called under the drain lock. +-- Separated out so flush() can run it under pcall and still release the lock: +-- otherwise a failure part-way wedges the buffer until the lock TTL expires. +local function drain_range(red, from, to) + local flushed = 0 + for slot = from, to do + local key = "shadow:rec:" .. slot + local record = shadow_cache:get(key) + if record then + local decoded = cjson.decode(record) + if decoded then + red:lpush(KEYS.decisions, record) + + local scope = (decoded.vhost_id or "unknown") .. "|" .. (decoded.endpoint_id or "global") + red:hincrby(KEYS.endpoints, scope, 1) + red:hincrby(KEYS.stats, "would_block_total", 1) + + for _, rule in ipairs(decoded.blocked_by or {}) do + red:hincrby(KEYS.rules, clip(rule, MAX_FLAG_LENGTH), 1) + end + + -- blocked_by names the profile ("legacy"), which tells an + -- operator nothing about what to suppress. The flags carry the + -- actual reason -- kw:viagra, fp_flag:suspicious-bot -- so count + -- those too; they are what the diff view is for. + for _, flag in ipairs(decoded.flags or {}) do + red:hincrby(KEYS.flags, clip(flag, MAX_FLAG_LENGTH), 1) + end + flushed = flushed + 1 + end + shadow_cache:delete(key) + end + -- Advance one slot at a time rather than jumping the tail at the end: a + -- drain that dies part-way resumes from the last completed slot, with + -- neither a replayed record nor a lost one. + shadow_cache:set(BUFFER_TAIL, slot) + end + return flushed +end + +--- Drain the buffer into Redis. Timer context only. +-- @param red an open Redis connection +-- @return number of records flushed +function _M.flush(red) + if not shadow_cache or not red then + return 0 + end + + local head = shadow_cache:get(BUFFER_HEAD) or 0 + local tail = shadow_cache:get(BUFFER_TAIL) or 0 + if head <= tail then + return 0 + end + + -- redis_sync's timer runs on EVERY worker, so several of them reach this at + -- the same moment. Reading the tail, draining, then writing the tail back + -- lets each worker drain the same slots -- three workers turned one + -- would-block decision into three recorded copies, which overstated the + -- impact of promoting an endpoint by 3x. + -- + -- Claiming a range with incr(TAIL, pending) fixed the duplicates and + -- introduced something worse: a second worker claiming the same `pending` + -- pushed the tail past the head, into slots the writer had not filled yet. + -- Those claims drained nothing, and every record written into that gap was + -- then skipped for good, because the head <= tail guard above reports an + -- empty buffer. Silent loss in the thing whose whole job is to be a + -- trustworthy sample. + -- + -- add() is atomic and fails when the key already exists, so exactly one + -- worker drains and the rest return immediately. + if not shadow_cache:add(DRAIN_LOCK, 1, DRAIN_LOCK_TTL) then + return 0 + end + + -- The TTL on the lock only covers a worker that dies outright. A drain that + -- merely fails -- Redis going away mid-range -- releases here instead, so the + -- next sync interval retries rather than waiting the TTL out. + local ok, flushed = pcall(drain_range, red, tail + 1, head) + if not ok then + shadow_cache:delete(DRAIN_LOCK) + ngx.log(ngx.ERR, "shadow flush failed, resuming from last completed slot: ", + tostring(flushed)) + return 0 + end + + if flushed > 0 then + red:ltrim(KEYS.decisions, 0, MAX_DECISIONS - 1) + red:expire(KEYS.decisions, DECISION_TTL) + red:expire(KEYS.rules, DECISION_TTL) + red:expire(KEYS.flags, DECISION_TTL) + red:expire(KEYS.endpoints, DECISION_TTL) + red:expire(KEYS.stats, DECISION_TTL) + end + + local dropped = shadow_cache:get(DROPPED) + if dropped and dropped > 0 then + red:hincrby(KEYS.stats, "dropped_total", dropped) + shadow_cache:set(DROPPED, 0) + end + + shadow_cache:delete(DRAIN_LOCK) + return flushed +end + +function _M.get_keys() + return KEYS +end + +--- Exposed for tests and for the API's "is anything buffered" check. +function _M.buffer_depth() + if not shadow_cache then return 0 end + local head = shadow_cache:get(BUFFER_HEAD) or 0 + local tail = shadow_cache:get(BUFFER_TAIL) or 0 + return math.max(0, head - tail) +end + +return _M diff --git a/openresty/lua/waf_handler.lua b/openresty/lua/waf_handler.lua index 4df5d52..1bdaa31 100644 --- a/openresty/lua/waf_handler.lua +++ b/openresty/lua/waf_handler.lua @@ -47,6 +47,49 @@ end -- F08: HMAC key for log integrity (optional) local LOG_HMAC_KEY = os.getenv("WAF_LOG_HMAC_KEY") +-- Keep a would-block decision so "what happens if I switch this to blocking?" +-- becomes a query rather than a log grep. +-- +-- There are two would-block branches in process_request and both must record. +-- Capturing only one would silently understate the impact of promoting an +-- endpoint, which is the one thing this feature exists to get right. +-- +-- The recorder buffers in a shared dict and a timer drains it: the request path +-- must not touch Redis, and a timer per request would exhaust the timer pool. +-- Resolved once and cached, following get_multi_executor above. This runs on +-- every would-block request, and pcall(require, ...) on a hot path is overhead +-- for a lookup whose answer never changes. _shadow_tried keeps a failed require +-- from being retried on every request too. +local _shadow_recorder +local _shadow_tried = false + +local function get_shadow_recorder() + if not _shadow_tried then + _shadow_tried = true + local ok, mod = pcall(require, "shadow_recorder") + if ok then _shadow_recorder = mod end + end + return _shadow_recorder +end + +local function record_shadow_decision(summary, client_ip, host, path, method, profile_result) + local shadow_recorder = get_shadow_recorder() + if not shadow_recorder then + return + end + shadow_recorder.record({ + vhost_id = summary.vhost_id, + endpoint_id = summary.endpoint_id, + client_ip = client_ip, + host = host, + path = path, + method = method, + score = profile_result.score or 0, + blocked_by = profile_result.blocked_by, + flags = profile_result.flags, + }) +end + -- Structured audit logging for security events -- Outputs JSON formatted log entries for easy parsing by log aggregation tools -- F08: Optionally adds HMAC signature for log integrity verification @@ -732,6 +775,7 @@ function _M.process_request() profile_result.flags and table.concat(profile_result.flags, ",") or "none" )) metrics.record_request(summary.vhost_id, summary.endpoint_id, "monitored", profile_result.score or 0) + record_shadow_decision(summary, client_ip, host, path, method, profile_result) -- Return to continue to proxy_pass - HAProxy will use the request headers we set return end @@ -826,6 +870,7 @@ function _M.process_request() profile_result.score or 0 )) metrics.record_request(summary.vhost_id, summary.endpoint_id, "monitored", profile_result.score or 0) + record_shadow_decision(summary, client_ip, host, path, method, profile_result) else metrics.record_request(summary.vhost_id, summary.endpoint_id, "allowed", profile_result.score or 0) end diff --git a/openresty/spec/shadow_recorder_spec.lua b/openresty/spec/shadow_recorder_spec.lua new file mode 100644 index 0000000..a4c8340 --- /dev/null +++ b/openresty/spec/shadow_recorder_spec.lua @@ -0,0 +1,207 @@ +--[[ + The recorder buffers would-block decisions in a shared dict and a timer + drains them. Two properties matter more than the rest: + + * The drain is claimed atomically. redis_sync's timer runs on every + worker, and this box has 24 of them. Reading the tail, draining, then + writing it back let every worker drain the same slots -- one decision + became one record per worker, which would have overstated the impact of + promoting an endpoint by up to 24x. That was a real defect, caught by + counting records rather than trusting the code. + + * The buffer is bounded and says so. A WAF in monitoring mode on a busy + site sees everything; silently dropping records would make the diff view + claim completeness it does not have. +]] +local helper = require "spec_helper" + +describe("shadow_recorder", function() + local recorder + + -- Minimal Redis double that records what would have been written. + local function fake_redis() + local calls = { lpush = {}, hincrby = {}, ltrim = 0, expire = 0 } + return { + calls = calls, + lpush = function(_, _, v) table.insert(calls.lpush, v); return 1 end, + hincrby = function(_, key, field, n) + calls.hincrby[key .. "/" .. field] = (calls.hincrby[key .. "/" .. field] or 0) + n + return 1 + end, + ltrim = function() calls.ltrim = calls.ltrim + 1; return true end, + expire = function() calls.expire = calls.expire + 1; return true end, + } + end + + local function a_decision(overrides) + local d = { + vhost_id = "example-com", endpoint_id = "contact", + client_ip = "203.0.113.9", host = "example.com", path = "/contact", + method = "POST", score = 85, + blocked_by = { "keyword_filter" }, flags = { "kw:viagra" }, + } + for k, v in pairs(overrides or {}) do d[k] = v end + return d + end + + before_each(function() + helper.install_ngx() + helper.stub_external_modules() + package.loaded["shadow_recorder"] = nil + recorder = require "shadow_recorder" + end) + + it("buffers a decision without touching Redis", function() + assert.is_true(recorder.record(a_decision())) + assert.equals(1, recorder.buffer_depth()) + end) + + it("ignores anything that is not a decision table", function() + assert.is_false(recorder.record(nil)) + assert.is_false(recorder.record("not a table")) + assert.equals(0, recorder.buffer_depth()) + end) + + it("flushes buffered decisions to Redis and empties the buffer", function() + for i = 1, 3 do recorder.record(a_decision({ score = 80 + i })) end + local red = fake_redis() + assert.equals(3, recorder.flush(red)) + assert.equals(3, #red.calls.lpush) + assert.equals(0, recorder.buffer_depth()) + end) + + it("does not re-flush what a previous drain already claimed", function() + -- The defect this guards: a second drain seeing the same slots. + for i = 1, 4 do recorder.record(a_decision()) end + local first, second = fake_redis(), fake_redis() + assert.equals(4, recorder.flush(first)) + assert.equals(0, recorder.flush(second)) + assert.equals(4, #first.calls.lpush) + assert.equals(0, #second.calls.lpush) + end) + + it("only one worker drains at a time", function() + for i = 1, 5 do recorder.record(a_decision()) end + assert.equals(5, recorder.buffer_depth()) + recorder.flush(fake_redis()) + assert.equals(0, recorder.buffer_depth(), + "a completed drain must leave nothing for another worker") + end) + + it("a concurrent drain never pushes the tail past the head", function() + -- The defect this guards, and it is subtler than the duplicate it + -- replaced. Claiming a range with incr(TAIL, pending) let a second + -- worker claim slots the writer had not filled yet, leaving tail > head. + -- Records written into that gap were then skipped for good: no + -- duplicates, so the obvious test passed, while the sample silently lost + -- records -- the one thing a shadow sample must not do. + for i = 1, 5 do recorder.record(a_decision()) end + + -- Two workers reach flush together. The first holds the drain lock, so + -- simulate the second arriving before the first has released it by + -- calling flush again from inside the fake connection's first lpush. + local second_result + local red = fake_redis() + local real_lpush = red.lpush + local reentered = false + red.lpush = function(self, ...) + if not reentered then + reentered = true + second_result = recorder.flush(fake_redis()) + end + return real_lpush(self, ...) + end + + assert.equals(5, recorder.flush(red)) + assert.equals(0, second_result, "the second worker must drain nothing") + assert.is_true(reentered, "the concurrent drain did not run; test is not proving anything") + + -- The real assertion: the buffer still works afterwards. + for i = 1, 3 do recorder.record(a_decision()) end + assert.equals(3, recorder.buffer_depth(), + "records written after a concurrent drain must still be visible") + local after = fake_redis() + assert.equals(3, recorder.flush(after), + "records written after a concurrent drain must still be flushed") + end) + + it("resumes from the last completed slot rather than replaying or skipping", function() + for i = 1, 6 do recorder.record(a_decision()) end + -- A drain that dies part-way: fail the connection after 2 records. + local red = fake_redis() + local n = 0 + local real_lpush = red.lpush + red.lpush = function(self, ...) + n = n + 1 + if n > 2 then error("connection lost") end + return real_lpush(self, ...) + end + pcall(recorder.flush, red) + + -- The 2 that landed must not come back; the other 4 must not be lost. + local resumed = fake_redis() + assert.equals(4, recorder.flush(resumed), + "a resumed drain must pick up exactly the slots that did not complete") + end) + + it("flushing an empty buffer is a no-op", function() + local red = fake_redis() + assert.equals(0, recorder.flush(red)) + assert.equals(0, #red.calls.lpush) + assert.equals(0, red.calls.ltrim) + end) + + it("counts each rule that contributed to the decision", function() + recorder.record(a_decision({ blocked_by = { "keyword_filter", "honeypot" } })) + local red = fake_redis() + recorder.flush(red) + assert.equals(1, red.calls.hincrby["waf:shadow:rules/keyword_filter"]) + assert.equals(1, red.calls.hincrby["waf:shadow:rules/honeypot"]) + end) + + it("aggregates by vhost and endpoint", function() + recorder.record(a_decision()) + recorder.record(a_decision({ endpoint_id = "signup" })) + local red = fake_redis() + recorder.flush(red) + assert.equals(1, red.calls.hincrby["waf:shadow:endpoints/example-com|contact"]) + assert.equals(1, red.calls.hincrby["waf:shadow:endpoints/example-com|signup"]) + end) + + it("counts detection flags, which are what name the rule to suppress", function() + recorder.record(a_decision({ flags = { "legacy:kw:viagra", "legacy:fp_flag:bot" } })) + recorder.record(a_decision({ flags = { "legacy:kw:viagra" } })) + local red = fake_redis() + recorder.flush(red) + -- "legacy" as a rule name tells an operator nothing; the flag does. + assert.equals(2, red.calls.hincrby["waf:shadow:flags/legacy:kw:viagra"]) + assert.equals(1, red.calls.hincrby["waf:shadow:flags/legacy:fp_flag:bot"]) + end) + + it("clips a flag, since part of it comes from the request", function() + recorder.record(a_decision({ flags = { "legacy:kw:" .. string.rep("A", 900) } })) + local red = fake_redis() + recorder.flush(red) + for field in pairs(red.calls.hincrby) do + if field:find("^waf:shadow:flags/") then + assert.is_true(#field < 300, "an unclipped flag would let one request bloat the hash") + end + end + end) + + it("clips attacker-controlled values so one request cannot bloat storage", function() + recorder.record(a_decision({ path = string.rep("A", 5000) })) + local red = fake_redis() + recorder.flush(red) + assert.is_true(#red.calls.lpush[1] < 2000, + "a 5000-character path must not be stored whole") + end) + + it("drops rather than growing without bound, and counts what it dropped", function() + for _ = 1, 600 do recorder.record(a_decision()) end -- cap is 500 + local red = fake_redis() + recorder.flush(red) + assert.is_true(red.calls.hincrby["waf:shadow:stats/dropped_total"] > 0, + "dropped records must be counted so the UI can say it is showing a sample") + end) +end) diff --git a/scripts/check-api-contract.py b/scripts/check-api-contract.py index 42107d5..25e6e7e 100755 --- a/scripts/check-api-contract.py +++ b/scripts/check-api-contract.py @@ -117,6 +117,12 @@ def main(): if "get" not in methods: continue url = f"{ADMIN_URL}{base}{path}" + # Some endpoints require a query parameter. The spec carries an example + # under x-contract-example-query so this can exercise them rather than + # skipping the ones most likely to drift. + example_query = methods["get"].get("x-contract-example-query") + if example_query: + url = f"{url}?{example_query}" try: status, raw, _ = request(url, cookie=cookie) except urllib.error.HTTPError as exc: diff --git a/scripts/test-waf.sh b/scripts/test-waf.sh index c5fa200..5981663 100755 --- a/scripts/test-waf.sh +++ b/scripts/test-waf.sh @@ -549,17 +549,19 @@ if echo "$WP_RESPONSE" | grep -q '"endpoint":"wp-login"' || [ "$WP_PROBE_STATUS" sleep 1 - # Test 6: Common username "admin" should be blocked (from builtin_credential_stuffing signature) - test_request "WP Login - Credential stuffing (admin)" "403" "POST" "/wp-login.php" \ - -A "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" \ - -d "log=admin&pwd=somepassword" - - sleep 1 - - # Test 7: Common password should be blocked - test_request "WP Login - Credential stuffing (password123)" "403" "POST" "/wp-login.php" \ - -A "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" \ - -d "log=someuser&pwd=password123" + # KNOWN GAP: these asserted that a single common-credential POST returns 403, + # and they passed -- but for the wrong reason. Bare curl sends a Chrome + # User-Agent with none of Chrome's headers, so the fake-modern-browser check + # scored them, not credential-stuffing detection. Given a realistic browser + # client both requests are allowed. + # + # Credential stuffing is by definition automated and repetitive, so the honest + # assertion is that a burst of common-credential attempts from one source is + # blocked while a single attempt from a real browser is not. That needs a + # product decision on where the line sits, so the assertions are parked rather + # than rewritten to match whatever the code happens to do today. + log_known_gap "WP Login - Credential stuffing (admin) - assertion passed for the wrong reason; needs a burst-based test" + log_known_gap "WP Login - Credential stuffing (password123) - assertion passed for the wrong reason; needs a burst-based test" else log_info "WP Login endpoint not configured - skipping defense line tests" @@ -593,7 +595,7 @@ echo "========================================" echo "Test Summary" echo "========================================" echo -e "Passed: ${GREEN}$PASS${NC}" -echo -e "Known gaps: ${YELLOW}${KNOWN_GAPS}${NC} (see R-27)" +echo -e "Known gaps: ${YELLOW}${KNOWN_GAPS}${NC} (assertions parked deliberately - see the [KNOWN GAP] lines above)" echo -e "Failed: ${RED}$FAIL${NC}" echo ""