diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 6b7ffb0..a401626 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -107,6 +107,13 @@ jobs: -v "$PWD/openresty/spec:/app/spec:ro" \ forms-waf-lua-test:ci --verbose /app/spec + # The chart ships its own copy of nginx.conf, so an env declaration or a + # shared dict added in one place and not the other fails silently and only + # on Kubernetes. Shadow mode shipped missing its dict; HAPROXY_TIMEOUT was + # set by the deployment but undeclared, so Lua never saw it. + - name: Helm / nginx.conf drift + run: python3 scripts/check-helm-drift.py + # Guards the seam between config_resolver and the defense mechanisms, where # six shipped features were silently inert. - name: Config contract diff --git a/admin-ui/src/App.tsx b/admin-ui/src/App.tsx index 7f76210..47afcc3 100644 --- a/admin-ui/src/App.tsx +++ b/admin-ui/src/App.tsx @@ -28,6 +28,7 @@ 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 Suppressions from '@/pages/security/Suppressions' import { About } from '@/pages/About' import { Users } from '@/pages/admin/Users' import { AuthProviders } from '@/pages/admin/AuthProviders' @@ -89,6 +90,7 @@ export default function App() { } /> } /> } /> + } /> } /> } /> } /> diff --git a/admin-ui/src/api/client.ts b/admin-ui/src/api/client.ts index 11538f8..1d127bd 100644 --- a/admin-ui/src/api/client.ts +++ b/admin-ui/src/api/client.ts @@ -1504,3 +1504,47 @@ export const shadowApi = { clear: () => request<{ cleared: boolean }>('/shadow/decisions', { method: 'DELETE' }), } + +// --------------------------------------------------------------------------- +// Rule suppressions +// +// The counterpart to shadow mode: shadow names the rule that would have +// blocked, a suppression says that rule is wrong for this scope. +// --------------------------------------------------------------------------- + +export type SuppressionScope = 'global' | 'vhost' | 'endpoint' + +export interface Suppression { + id: string + scope_type: SuppressionScope + scope_id?: string + flag: string + reason?: string + created_at?: number + created_by?: string +} + +export interface NewSuppression { + flag: string + scope_type?: SuppressionScope + scope_id?: string + reason?: string +} + +export const suppressionsApi = { + list: () => + request<{ suppressions: Suppression[]; count: number; max?: number }>('/suppressions'), + + create: (body: NewSuppression) => + request<{ suppression: Suppression; created: boolean }>('/suppressions', { + method: 'POST', + body: JSON.stringify(body), + }), + + remove: (id: string) => + request<{ deleted: boolean; id: string }>(`/suppressions/${encodeURIComponent(id)}`, { + method: 'DELETE', + }), + + clear: () => request<{ cleared: boolean }>('/suppressions', { method: 'DELETE' }), +} diff --git a/admin-ui/src/api/generated.ts b/admin-ui/src/api/generated.ts index 6c6323f..2782024 100644 --- a/admin-ui/src/api/generated.ts +++ b/admin-ui/src/api/generated.ts @@ -195,6 +195,168 @@ export interface paths { patch?: never; trace?: never; }; + "/suppressions": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** Detections that no longer count, and where */ + get: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Every suppression currently in force */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + suppressions: components["schemas"]["Suppression"][]; + count: number; + /** @description Ceiling on entries; this list is walked on every defense node. */ + max?: number; + }; + }; + }; + }; + }; + put?: never; + /** Stop a detection counting for one scope */ + post: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + "application/json": { + /** @description Exact detection flag, or a trailing "*" for a family (kw:*). This is the flag the mechanism emits, without the profile prefix that appears on recorded shadow decisions. */ + flag: string; + /** + * @default global + * @enum {string} + */ + scope_type?: "global" | "vhost" | "endpoint"; + /** @description Required unless scope_type is global. */ + scope_id?: string; + reason?: string; + }; + }; + }; + responses: { + /** @description Already present. The id is a digest of scope and flag, so re-adding the same suppression updates it rather than creating a duplicate. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["SuppressionCreated"]; + }; + }; + /** @description Created */ + 201: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["SuppressionCreated"]; + }; + }; + /** @description Invalid input. Includes a global "*", which would disable all detection and is refused deliberately. */ + 400: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + /** Remove every suppression */ + delete: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Cleared */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + cleared: boolean; + }; + }; + }; + }; + }; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/suppressions/{id}": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post?: never; + /** Remove one suppression */ + delete: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Removed */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + deleted: boolean; + id: string; + }; + }; + }; + /** @description No suppression with that id */ + 404: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/shadow/summary": { parameters: { query?: never; @@ -489,6 +651,24 @@ export interface components { high_event_rate?: number; }; }; + Suppression: { + /** @description Digest of scope and flag, so adding the same one twice is idempotent. */ + id: string; + /** @enum {string} */ + scope_type: "global" | "vhost" | "endpoint"; + /** @description Absent for global scope; the vhost or endpoint id otherwise. */ + scope_id?: string; + /** @description Exact detection flag, or a trailing "*" to cover a family (kw:*). */ + flag: string; + reason?: string; + created_at?: number; + created_by?: string; + }; + SuppressionCreated: { + suppression: components["schemas"]["Suppression"]; + /** @description False when the suppression already existed and was updated. */ + created: boolean; + }; ShadowDecision: { /** @description Unix time the decision was made */ ts: number; diff --git a/admin-ui/src/components/layout/Sidebar.tsx b/admin-ui/src/components/layout/Sidebar.tsx index d3f6e8a..7f73f10 100644 --- a/admin-ui/src/components/layout/Sidebar.tsx +++ b/admin-ui/src/components/layout/Sidebar.tsx @@ -24,6 +24,7 @@ import { Server, Fingerprint, EyeOff, + BellOff, MessageSquare, Workflow, Target, @@ -52,6 +53,7 @@ const navigation = [ name: 'Security', children: [ { name: 'Shadow Mode', href: '/security/shadow', icon: EyeOff }, + { name: 'Suppressions', href: '/security/suppressions', icon: BellOff }, { 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/security/Suppressions.tsx b/admin-ui/src/pages/security/Suppressions.tsx new file mode 100644 index 0000000..f0f0c95 --- /dev/null +++ b/admin-ui/src/pages/security/Suppressions.tsx @@ -0,0 +1,286 @@ +import { useState } from 'react' +import { useSearchParams } from 'react-router-dom' +import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' +import { suppressionsApi, vhostsApi, endpointsApi } from '@/api/client' +import type { Suppression, SuppressionScope } 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 { Input } from '@/components/ui/input' +import { Label } from '@/components/ui/label' +import { Alert, AlertDescription } from '@/components/ui/alert' +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select' +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from '@/components/ui/table' +import { useToast } from '@/components/ui/use-toast' +import { BellOff, Trash2, Info, Plus } from 'lucide-react' + +function formatWhen(ts?: number): string { + if (!ts) return '-' + return new Date(ts * 1000).toLocaleString() +} + +function scopeLabel(s: Suppression): string { + if (s.scope_type === 'global') return 'Everywhere' + return `${s.scope_type}: ${s.scope_id}` +} + +export default function Suppressions() { + const { toast } = useToast() + const queryClient = useQueryClient() + + // Arriving from Shadow Mode's "suppress" action, with the detection and the + // vhost it fired on already chosen. Landing on an empty form and being asked + // to retype a flag you were just looking at is how a good idea gets abandoned. + const [searchParams] = useSearchParams() + const [flag, setFlag] = useState(searchParams.get('flag') ?? '') + // Validated rather than cast: a stale or hand-edited link carrying + // ?scope_type=foo would put the form into a state no Select option matches, + // leaving a control that looks set and submits something else. + const [scopeType, setScopeType] = useState(() => { + const fromUrl = searchParams.get('scope_type') + return fromUrl === 'global' || fromUrl === 'vhost' || fromUrl === 'endpoint' + ? fromUrl + : 'vhost' + }) + const [scopeId, setScopeId] = useState(searchParams.get('scope_id') ?? '') + const [reason, setReason] = useState('') + + const { data, isLoading } = useQuery({ + queryKey: ['suppressions'], + queryFn: suppressionsApi.list, + }) + + const { data: vhostsData } = useQuery({ queryKey: ['vhosts'], queryFn: vhostsApi.list }) + const { data: endpointsData } = useQuery({ + queryKey: ['endpoints'], + queryFn: () => endpointsApi.list(), + }) + + // Both list endpoints wrap their array, and the items come back untyped. + // Only id and name are needed to populate the scope picker. + type ScopeOption = { id: string; name?: string } + const rawVhosts = (vhostsData as { vhosts?: ScopeOption[] } | undefined)?.vhosts + const vhosts = Array.isArray(rawVhosts) ? rawVhosts : [] + const rawEndpoints = (endpointsData as { endpoints?: ScopeOption[] } | undefined)?.endpoints + const endpoints = Array.isArray(rawEndpoints) ? rawEndpoints : [] + + const createMutation = useMutation({ + mutationFn: suppressionsApi.create, + onSuccess: (res) => { + toast({ + title: res.created ? 'Suppression added' : 'Already suppressed', + description: `${res.suppression.flag} no longer counts for ${scopeLabel(res.suppression)}.`, + }) + setFlag('') + setReason('') + queryClient.invalidateQueries({ queryKey: ['suppressions'] }) + }, + onError: (err: Error) => + toast({ title: 'Could not add suppression', description: err.message, variant: 'destructive' }), + }) + + const removeMutation = useMutation({ + mutationFn: suppressionsApi.remove, + onSuccess: () => { + toast({ title: 'Suppression removed', description: 'That detection counts again.' }) + queryClient.invalidateQueries({ queryKey: ['suppressions'] }) + }, + onError: (err: Error) => + toast({ title: 'Could not remove', description: err.message, variant: 'destructive' }), + }) + + const scopeOptions = scopeType === 'vhost' ? vhosts : endpoints + const needsScopeId = scopeType !== 'global' + const canSubmit = flag.trim().length > 0 && (!needsScopeId || scopeId.length > 0) + + return ( +
+
+

+ + Rule Suppressions +

+

+ Stop a detection counting where it is wrong, without turning the endpoint off. +

+
+ + + + + A suppression removes one detection from the result. A request that also triggered + something you did not suppress is still blocked — so suppressing{' '} + kw:viagra will not let spam through that also + matched kw:casino. Use a trailing{' '} + * to cover a family, e.g.{' '} + kw:*. + + + + + + Add a suppression + + The flag is the detection name shown in Shadow Mode, e.g.{' '} + kw:viagra or{' '} + fp_flag:suspicious-bot. + + + +
+
+ + setFlag(e.target.value)} + placeholder="kw:viagra" + /> +
+
+ + +
+
+ + +
+
+ + setReason(e.target.value)} + placeholder="Pharmacy client, legitimate term" + /> +
+
+
+ +
+
+
+ + + + + In force{' '} + {data?.count !== undefined && ( + + ({data.count} + {data.max ? ` of ${data.max}` : ''}) + + )} + + + Every one of these is a detection that no longer contributes to a block. + + + + {isLoading ? ( +

Loading...

+ ) : data?.suppressions?.length ? ( + + + + Detection + Scope + Reason + Added + + + + + {data.suppressions.map((s) => ( + + + {s.flag} + + + + {scopeLabel(s)} + + + + {s.reason || '-'} + + + {formatWhen(s.created_at)} + {s.created_by ? ` by ${s.created_by}` : ''} + + + + + + ))} + +
+ ) : ( +

+ Nothing suppressed. Every detection counts. +

+ )} +
+
+
+ ) +} diff --git a/admin-ui/src/pages/shadow/ShadowMode.tsx b/admin-ui/src/pages/shadow/ShadowMode.tsx index 7199ee7..8c81629 100644 --- a/admin-ui/src/pages/shadow/ShadowMode.tsx +++ b/admin-ui/src/pages/shadow/ShadowMode.tsx @@ -1,4 +1,5 @@ import { useState } from 'react' +import { Link } from 'react-router-dom' import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' import { shadowApi } from '@/api/client' import type { ShadowCount, ShadowImpact, ShadowScope } from '@/api/client' @@ -33,6 +34,7 @@ import { Trash2, ArrowUpCircle, Fingerprint, + BellOff, } from 'lucide-react' function formatWhen(ts: number): string { @@ -44,11 +46,41 @@ function formatWhen(ts: number): string { return `${Math.floor(secs / 86400)}d ago` } +/** + * Recorded flags are usually profile-prefixed (`legacy:kw:viagra`), but not + * always: defense-line flags are merged into the result without a prefix. A + * suppression matches the flag the mechanism itself emits, so the prefix has to + * come off -- and only when it really is a prefix. + * + * Stripping up to the first colon unconditionally mangles an unprefixed + * `kw:viagra` into `viagra`, which matches nothing. Requiring the remainder to + * still contain a colon fails the other way, on single-segment flags like + * `legacy:thread_error`. So the decision is made against the profile names the + * API already reports in top_rules: strip a leading segment only if it is one + * of them. + */ +function detectionFlag(recorded: string, profileNames: Set): string { + const firstColon = recorded.indexOf(':') + if (firstColon === -1) return recorded + const head = recorded.slice(0, firstColon) + return profileNames.has(head) ? recorded.slice(firstColon + 1) : recorded +} + /** * 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 }) { +function CountBars({ + items, + empty, + suppressScope, + profileNames, +}: { + items: ShadowCount[] + empty: string + suppressScope?: string + profileNames?: Set +}) { if (!items?.length) { return

{empty}

} @@ -59,7 +91,25 @@ function CountBars({ items, empty }: { items: ShadowCount[]; empty: string }) {
{item.name} - {item.count} + + {item.count} + {suppressScope && profileNames && ( + + )} +
0 + // The summary spans every scope, so a flag in it cannot always be attributed + // to one vhost. Offer the suppress shortcut only when every recorded decision + // came from the same vhost -- otherwise the prefilled scope would be a guess, + // and a suppression applied to the wrong vhost is a hole, not an annoyance. + const scopeVhosts = new Set((summary?.scopes ?? []).map((s) => s.vhost_id)) + const singleVhost = scopeVhosts.size === 1 ? [...scopeVhosts][0] : undefined + + // top_rules is the set of things that produced a decision -- profile ids for a + // profile block. That is what tells detectionFlag which leading segment is a + // prefix rather than part of the flag. + const summaryProfiles = new Set((summary?.top_rules ?? []).map((r) => r.name)) + return (
@@ -227,6 +289,8 @@ export default function ShadowMode() { @@ -397,7 +461,12 @@ export default function ShadowMode() { {!!pending?.top_flags?.length && (

Mostly from

- + r.name))} + />
)} {!!pending?.affected_endpoints?.length && ( diff --git a/docs/API_HANDLERS.md b/docs/API_HANDLERS.md index 4fc48be..13c3608 100644 --- a/docs/API_HANDLERS.md +++ b/docs/API_HANDLERS.md @@ -335,6 +335,32 @@ local ok, err = utils.validate_required(data, {"field1", "field2"}) --- +### Rule Suppressions (`api_handlers/suppressions.lua`) + +The counterpart to shadow mode. Shadow mode names the rule that would have +blocked; a suppression is how an operator says that rule is wrong *here*, +without turning the endpoint off. + +| Method | Endpoint | Description | +|--------|----------|-------------| +| GET | /suppressions | Everything currently in force | +| POST | /suppressions | Stop a detection counting for one scope | +| DELETE | /suppressions | Remove all suppressions | +| DELETE | /suppressions/{id} | Remove one | + +Applied in `defense_profile_executor.execute_defense_node`, which every +mechanism returns through. Suppressed flags are removed from the node result; +a node is neutralised only when *every* one of its flags was suppressed. A +mechanism reports one aggregate score for all its flags, so there is no +per-flag score to subtract — which is why a node that also fired for an +unsuppressed reason keeps its verdict. `kw:*` covers a family; a global `*` +is refused, because that is a WAF that is on and does nothing. + +Scopes accumulate (global + vhost + endpoint all apply) rather than the +narrower replacing the broader, which is the opposite of the deep-merge +`config_resolver` uses — deliberately, since a narrow entry silently dropping +the global ones is the wrong default for a safety control. + ### Shadow Mode (`api_handlers/shadow.lua`) What monitoring mode *would* have blocked, so a rule set can be proven before it @@ -372,6 +398,7 @@ Each endpoint requires specific permissions. See [RBAC Guide](RBAC.md) for full | security | read, update | | slack | read, update, test | | shadow | read, promote, delete | +| suppressions | create, read, delete | --- diff --git a/docs/openapi.yaml b/docs/openapi.yaml index ad98122..ddb3e31 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -113,6 +113,33 @@ components: high_event_count: { type: integer } high_event_rate: { type: integer } + Suppression: + type: object + required: [id, scope_type, flag] + properties: + id: + type: string + description: Digest of scope and flag, so adding the same one twice is idempotent. + scope_type: { type: string, enum: [global, vhost, endpoint] } + scope_id: + type: string + description: Absent for global scope; the vhost or endpoint id otherwise. + flag: + type: string + description: Exact detection flag, or a trailing "*" to cover a family (kw:*). + reason: { type: string } + created_at: { type: integer } + created_by: { type: string } + + SuppressionCreated: + type: object + required: [suppression, created] + properties: + suppression: { $ref: "#/components/schemas/Suppression" } + created: + type: boolean + description: False when the suppression already existed and was updated. + ShadowDecision: type: object required: [ts, vhost_id, endpoint_id, score] @@ -264,6 +291,102 @@ paths: emitted_events: { type: array, items: { type: string } } unavailable_events: { type: array, items: { type: string } } + /suppressions: + get: + summary: Detections that no longer count, and where + responses: + "200": + description: Every suppression currently in force + content: + application/json: + schema: + type: object + required: [suppressions, count] + properties: + suppressions: + type: array + items: { $ref: "#/components/schemas/Suppression" } + count: { type: integer } + max: + type: integer + description: Ceiling on entries; this list is walked on every defense node. + post: + summary: Stop a detection counting for one scope + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [flag] + properties: + flag: + type: string + maxLength: 120 + description: > + Exact detection flag, or a trailing "*" for a family (kw:*). + This is the flag the mechanism emits, without the profile + prefix that appears on recorded shadow decisions. + scope_type: + type: string + enum: [global, vhost, endpoint] + default: global + scope_id: + type: string + description: Required unless scope_type is global. + reason: { type: string, maxLength: 500 } + responses: + "201": + description: Created + content: + application/json: + schema: { $ref: "#/components/schemas/SuppressionCreated" } + "200": + description: > + Already present. The id is a digest of scope and flag, so re-adding + the same suppression updates it rather than creating a duplicate. + content: + application/json: + schema: { $ref: "#/components/schemas/SuppressionCreated" } + "400": + description: > + Invalid input. Includes a global "*", which would disable all + detection and is refused deliberately. + delete: + summary: Remove every suppression + responses: + "200": + description: Cleared + content: + application/json: + schema: + type: object + required: [cleared] + properties: + cleared: { type: boolean } + + /suppressions/{id}: + delete: + summary: Remove one suppression + parameters: + - name: id + in: path + required: true + schema: { type: string } + responses: + "200": + description: Removed + content: + application/json: + schema: + type: object + required: [deleted, id] + properties: + deleted: { type: boolean } + id: { type: string } + "404": + description: No suppression with that id + /shadow/summary: get: summary: What monitoring mode would have blocked diff --git a/helm/forms-waf/templates/openresty-configmap.yaml b/helm/forms-waf/templates/openresty-configmap.yaml index 081e3a3..7ff1866 100644 --- a/helm/forms-waf/templates/openresty-configmap.yaml +++ b/helm/forms-waf/templates/openresty-configmap.yaml @@ -24,6 +24,19 @@ data: env WAF_ADMIN_PASSWORD; env HOSTNAME; env WAF_USE_LEADER_ELECTION; + env HAPROXY_TIMEOUT; + env REDIS_TLS; + env WAF_ADMIN_COOKIE; + env WAF_ADMIN_URL; + env WAF_ALLOW_INTERNAL_URLS; + env WAF_DISABLE_SSRF_PROTECTION; + env WAF_LOCAL_AUTH; + env WAF_LOG_HMAC_KEY; + env WAF_SESSION_TTL; + env WAF_TRUSTED_PROXIES; + env http_proxy; + env https_proxy; + env no_proxy; worker_processes auto; error_log /var/log/nginx/error.log warn; @@ -92,6 +105,8 @@ data: lua_shared_dict waf_metrics 10m; lua_shared_dict waf_timing 1m; lua_shared_dict coordinator_cache 1m; + lua_shared_dict shadow_cache 10m; + lua_shared_dict suppression_cache 1m; # Initialize modules on worker start init_worker_by_lua_block { diff --git a/openresty/conf/nginx.conf b/openresty/conf/nginx.conf index 45892ec..5c557f5 100644 --- a/openresty/conf/nginx.conf +++ b/openresty/conf/nginx.conf @@ -136,6 +136,7 @@ http { 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) + lua_shared_dict suppression_cache 1m; # Rule suppressions, read on every defense node # 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 5aa4e31..15fb377 100644 --- a/openresty/lua/admin_api.lua +++ b/openresty/lua/admin_api.lua @@ -20,6 +20,7 @@ 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 suppressions_handler = require "api_handlers.suppressions" local geoip_handler = require "api_handlers.geoip" local reputation_handler = require "api_handlers.reputation" local bulk_handler = require "api_handlers.bulk" @@ -70,6 +71,7 @@ register_handlers(config_handler) register_handlers(webhooks_handler) register_handlers(slack_handler) register_handlers(shadow_handler) +register_handlers(suppressions_handler) register_handlers(geoip_handler) register_handlers(reputation_handler) register_handlers(bulk_handler) @@ -201,6 +203,15 @@ function _M.handle_request() end end + -- Check for parameterized suppression routes: DELETE /suppressions/{id} + local suppression_id = path:match("^/suppressions/([a-fA-F0-9]+)$") + if suppression_id then + local handler = suppressions_handler.resource_handlers[method] + if handler then + return handler(suppression_id) + end + end + -- Check for parameterized vhost routes: /vhosts/{id} or /vhosts/{id}/action local vhost_id, vhost_action = path:match("^/vhosts/([a-zA-Z0-9_-]+)/?([a-z]*)$") diff --git a/openresty/lua/api_handlers/suppressions.lua b/openresty/lua/api_handlers/suppressions.lua new file mode 100644 index 0000000..82c740b --- /dev/null +++ b/openresty/lua/api_handlers/suppressions.lua @@ -0,0 +1,226 @@ +-- api_handlers/suppressions.lua +-- Rule suppressions: "this detection does not count for this scope". +-- +-- The counterpart to shadow mode. Shadow mode names the rule that would have +-- blocked; this is how an operator says that rule is wrong here, without +-- turning the whole endpoint off. + +local _M = {} + +local utils = require "api_handlers.utils" +local cjson = require "cjson.safe" +local redis_sync = require "redis_sync" +local suppressions = require "suppressions" + +local REDIS_KEY = "waf:suppressions" + +local VALID_SCOPES = { global = true, vhost = true, endpoint = true } + +-- An empty Lua table encodes as a JSON object; the UI calls .map on these. +local function as_array(t) + if type(t) ~= "table" then + return setmetatable({}, cjson.array_mt) + end + return setmetatable(t, cjson.array_mt) +end + +--- A stable id from the scope and flag, so the same suppression added twice is +--- the same entry rather than a duplicate that has to be deleted twice. +local function suppression_id(scope_type, scope_id, flag) + return ngx.md5(scope_type .. "|" .. (scope_id or "") .. "|" .. flag) +end + +local function read_all(red) + local raw, err = red:hgetall(REDIS_KEY) + if not raw then return nil, err end + + local out = {} + if type(raw) == "table" then + for i = 1, #raw, 2 do + local decoded = cjson.decode(raw[i + 1]) + if type(decoded) == "table" then + decoded.id = decoded.id or raw[i] + out[#out + 1] = decoded + end + end + end + table.sort(out, function(a, b) + return (a.created_at or 0) > (b.created_at or 0) + end) + return out +end + +_M.handlers = {} + +-- GET /suppressions - everything currently in force +_M.handlers["GET:/suppressions"] = function() + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + local entries, read_err = read_all(red) + utils.close_redis(red) + if not entries then + return utils.error_response("Failed to read suppressions: " .. (read_err or "unknown")) + end + + return utils.json_response({ + suppressions = as_array(entries), + count = #entries, + max = suppressions.get_max(), + }) +end + +-- POST /suppressions - stop a detection counting for one scope +_M.handlers["POST:/suppressions"] = function() + local body, err = utils.get_json_body() + if not body then + return utils.error_response(err or "Invalid JSON body", 400) + end + + local valid, missing = utils.validate_required(body, { "flag" }) + if not valid then + return utils.error_response("Missing required field: " .. missing, 400) + end + + local flag = tostring(body.flag) + if #flag == 0 or #flag > 120 then + return utils.error_response("flag must be 1-120 characters", 400) + end + + local scope_type = body.scope_type or "global" + if not VALID_SCOPES[scope_type] then + return utils.error_response("scope_type must be one of: global, vhost, endpoint", 400) + end + + local scope_id = body.scope_id + if scope_type ~= "global" then + if not scope_id or scope_id == "" then + return utils.error_response("scope_id is required when scope_type is " .. scope_type, 400) + end + scope_id = tostring(scope_id) + else + scope_id = nil + end + + -- A bare "*" would suppress every detection everywhere, which is a WAF that + -- is on but does nothing -- the failure mode this feature must not enable by + -- a single typo. Turning the vhost off is the honest way to express that. + if flag == "*" and scope_type == "global" then + return utils.error_response( + "Refusing a global '*' suppression: that disables all detection. " .. + "Scope it to a vhost or endpoint, or set the vhost mode to passthrough.", 400) + end + + local red, conn_err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (conn_err or "unknown")) + end + + -- A typo in scope_id would be accepted and then simply never match anything, + -- which is the failure mode this codebase keeps being bitten by: config that + -- looks applied and is inert. Cheaper to refuse it than to debug it later. + if scope_type ~= "global" then + local scope_key = (scope_type == "vhost" and "waf:vhosts:config:" or "waf:endpoints:config:") + .. scope_id + local exists = red:exists(scope_key) + if exists == 0 then + utils.close_redis(red) + return utils.error_response( + "No " .. scope_type .. " with id '" .. scope_id .. + "'. A suppression on a scope that does not exist would never apply.", 400) + end + end + + local existing = read_all(red) + local id = suppression_id(scope_type, scope_id, flag) + local already = false + for _, entry in ipairs(existing or {}) do + if entry.id == id then already = true break end + end + + -- Bounded: this list is walked on every defense node of every request. The + -- scan above is only used for this cap; whether the entry already existed + -- comes from HSET below, which cannot race with a concurrent write. + if not already and existing and #existing >= suppressions.get_max() then + utils.close_redis(red) + return utils.error_response( + "Suppression limit reached (" .. suppressions.get_max() .. "). " .. + "Delete unused entries, or use a trailing '*' to cover a family in one rule.", 400) + end + + local entry = { + id = id, + scope_type = scope_type, + scope_id = scope_id, + flag = flag, + reason = body.reason and tostring(body.reason):sub(1, 500) or nil, + created_at = ngx.time(), + created_by = ngx.ctx.admin_user and ngx.ctx.admin_user.username or "unknown", + } + + local encoded = cjson.encode(entry) + -- HSET returns 1 when it created the field and 0 when it replaced one, which + -- is the authoritative answer to "did this already exist?". The earlier scan + -- is a best-effort read that a concurrent write can invalidate. + local created_field, set_err = red:hset(REDIS_KEY, id, encoded) + utils.close_redis(red) + if not created_field then + return utils.error_response("Failed to store suppression: " .. (set_err or "unknown")) + end + local created = tonumber(created_field) == 1 + + -- Push it to this pod's workers now; other pods pick it up on their timer. + redis_sync.sync_now() + + ngx.log(ngx.WARN, "SUPPRESSION_ADDED: flag=", flag, " scope=", scope_type, + ":", tostring(scope_id), " by=", entry.created_by) + + return utils.json_response({ + suppression = entry, + created = created, + }, created and 201 or 200) +end + +-- DELETE /suppressions - remove every suppression at once +_M.handlers["DELETE:/suppressions"] = function() + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + red:del(REDIS_KEY) + utils.close_redis(red) + redis_sync.sync_now() + + ngx.log(ngx.WARN, "SUPPRESSIONS_CLEARED by=", + ngx.ctx.admin_user and ngx.ctx.admin_user.username or "unknown") + + return utils.json_response({ cleared = true }) +end + +-- Parametric: DELETE /suppressions/{id} +_M.resource_handlers = {} + +_M.resource_handlers["DELETE"] = function(id) + local red, err = utils.get_redis() + if not red then + return utils.error_response("Redis connection failed: " .. (err or "unknown")) + end + + local removed = red:hdel(REDIS_KEY, id) + utils.close_redis(red) + + if not removed or removed == 0 then + return utils.error_response("Suppression not found: " .. tostring(id), 404) + end + + redis_sync.sync_now() + ngx.log(ngx.WARN, "SUPPRESSION_REMOVED: id=", id, " by=", + ngx.ctx.admin_user and ngx.ctx.admin_user.username or "unknown") + + return utils.json_response({ deleted = true, id = id }) +end + +return _M diff --git a/openresty/lua/defense_profile_executor.lua b/openresty/lua/defense_profile_executor.lua index 3dea3ea..8de65be 100644 --- a/openresty/lua/defense_profile_executor.lua +++ b/openresty/lua/defense_profile_executor.lua @@ -423,6 +423,27 @@ local function execute_defense_node(node, request_context) return _M.result_score(0, {"defense_error:" .. defense_name}, {error = result}) end + -- Every mechanism returns through here, which is why suppression hooks in at + -- this point rather than in each mechanism. A suppression an operator added + -- after seeing a false positive in the shadow view takes effect here. + local suppressions = require "suppressions" + local _, removed, neutralised = suppressions.apply(result, request_context.vhost_id, + request_context.endpoint_id) + if removed then + -- WARN only when the suppression actually changed the verdict, which is + -- the case an operator has to be able to find when asking why something + -- got through. Dropping a flag from a node that was not going to block + -- either way is routine and stays at INFO -- a suppression exists + -- precisely because its rule fires often, so logging every hit at WARN + -- would bury the consequential ones. Note the default error_log level is + -- warn, so the INFO line needs the level raised to be seen at all. + ngx.log(neutralised and ngx.WARN or ngx.INFO, + "SUPPRESSED", neutralised and " (block prevented)" or "", ": ", + defense_name, " flags=", table.concat(removed, ","), + " vhost=", tostring(request_context.vhost_id), + " endpoint=", tostring(request_context.endpoint_id)) + end + return result end diff --git a/openresty/lua/rbac.lua b/openresty/lua/rbac.lua index e1475dd..537147b 100644 --- a/openresty/lua/rbac.lua +++ b/openresty/lua/rbac.lua @@ -38,6 +38,7 @@ local DEFAULT_ROLES = { webhooks = {"read", "update", "test"}, slack = {"read", "update", "test", "reset"}, shadow = {"read", "promote", "delete"}, + suppressions = {"create", "read", "delete"}, geoip = {"read", "update", "reload"}, reputation = {"read", "update"}, timing = {"read", "update"}, @@ -70,6 +71,7 @@ local DEFAULT_ROLES = { webhooks = {"read"}, slack = {"read"}, shadow = {"read"}, + suppressions = {"create", "read", "delete"}, geoip = {"read"}, reputation = {"read"}, timing = {"read"}, @@ -99,6 +101,7 @@ local DEFAULT_ROLES = { webhooks = {"read"}, slack = {"read"}, shadow = {"read"}, + suppressions = {"read"}, geoip = {"read"}, reputation = {"read"}, timing = {"read"}, @@ -192,6 +195,9 @@ local ENDPOINT_PERMISSIONS = { ["GET:/shadow/impact"] = {resource = "shadow", action = "read"}, ["POST:/shadow/promote"] = {resource = "shadow", action = "promote"}, ["DELETE:/shadow/decisions"] = {resource = "shadow", action = "delete"}, + ["GET:/suppressions"] = {resource = "suppressions", action = "read"}, + ["POST:/suppressions"] = {resource = "suppressions", action = "create"}, + ["DELETE:/suppressions"] = {resource = "suppressions", action = "delete"}, -- Slack notifications ["GET:/slack/config"] = {resource = "slack", action = "read"}, @@ -297,6 +303,12 @@ local ENDPOINT_PERMISSIONS = { -- Parametric endpoint permissions (for /endpoints/{id}, /vhosts/{id}, etc.) local PARAMETRIC_PERMISSIONS = { + -- Suppressions. Not vhost-scoped: the id is a digest of the scope and flag, + -- not a vhost, so a scoped check would have nothing to match against. The + -- scope is enforced when the suppression is created, not when it is deleted. + suppressions = { + ["DELETE"] = {resource = "suppressions", action = "delete"}, + }, -- Endpoints endpoints = { ["GET"] = {resource = "endpoints", action = "read", scoped = true}, @@ -530,6 +542,11 @@ function _M.get_endpoint_permission(method, path) return PARAMETRIC_PERMISSIONS.endpoints[handler_key], endpoint_id, "endpoint" end + local suppression_id = path:match("^/suppressions/([a-fA-F0-9]+)$") + if suppression_id then + return PARAMETRIC_PERMISSIONS.suppressions[method], suppression_id, "suppression" + end + local vhost_id = path:match("^/vhosts/([a-zA-Z0-9_-]+)/?([a-z]*)$") if vhost_id then local action = path:match("^/vhosts/[a-zA-Z0-9_-]+/([a-z]+)$") diff --git a/openresty/lua/redis_sync.lua b/openresty/lua/redis_sync.lua index d0ee1e6..dd2befd 100644 --- a/openresty/lua/redis_sync.lua +++ b/openresty/lua/redis_sync.lua @@ -104,6 +104,7 @@ local KEYS = { thresholds = "waf:config:thresholds", routing = "waf:config:routing", ip_whitelist = "waf:whitelist:ips", + suppressions = "waf:suppressions", -- Endpoint configuration keys endpoint_index = "waf:endpoints:index", endpoint_config_prefix = "waf:endpoints:config:", @@ -284,6 +285,51 @@ local function sync_thresholds(red) end end +-- Sync rule suppressions +-- Read on every defense node of every request, so it lands in its own dict and +-- is stored pre-decoded as one JSON blob rather than as N keys to look up. +local function sync_suppressions(red) + local suppression_cache = ngx.shared.suppression_cache + if not suppression_cache then + return + end + + local entries, err = red:hgetall(KEYS.suppressions) + if not entries then + ngx.log(ngx.WARN, "Failed to get suppressions: ", err) + return + end + + local cjson = require "cjson.safe" + local list = {} + if type(entries) == "table" then + for i = 1, #entries, 2 do + local decoded = cjson.decode(entries[i + 1]) + -- Objects only. cjson.decode happily returns a number or a string for + -- valid-but-wrong JSON, and both are truthy; assigning .id to one + -- raises here, and letting one through would arm the same failure in + -- the request path. + if type(decoded) == "table" then + decoded.id = decoded.id or entries[i] + list[#list + 1] = decoded + else + ngx.log(ngx.WARN, "Ignoring malformed suppression: ", tostring(entries[i])) + end + end + end + + local encoded = cjson.encode(list) + if encoded then + -- No TTL: an expiring suppression would silently start blocking traffic + -- an operator deliberately allowed. It is replaced on every sync instead. + local ok, set_err = suppression_cache:set("suppressions", encoded) + if not ok then + ngx.log(ngx.ERR, "Failed to cache suppressions: ", set_err) + end + end + ngx.log(ngx.DEBUG, "Synced ", #list, " suppressions") +end + -- Sync routing configuration local function sync_routing(red) local config, err = red:hgetall(KEYS.routing) @@ -1130,6 +1176,7 @@ local function do_sync() sync_thresholds(red) sync_routing(red) sync_ip_whitelist(red) + sync_suppressions(red) -- Sync endpoint configurations sync_endpoints(red) diff --git a/openresty/lua/suppressions.lua b/openresty/lua/suppressions.lua new file mode 100644 index 0000000..4caded0 --- /dev/null +++ b/openresty/lua/suppressions.lua @@ -0,0 +1,182 @@ +--[[ + Rule suppression + ================ + Shadow mode answers "what would blocking have rejected?". It names the rule + responsible -- kw:viagra, fp_flag:suspicious-bot -- but naming it is only half + an answer. Without a way to say "that one is wrong for this form", the + operator's only choices are to promote and accept the false positives, or not + to promote at all. That is the choice that stops WAFs being turned on. + + A suppression says: for this scope, this detection does not count. + + Applied at the node result + -------------------------- + Every defense mechanism returns through execute_defense_node, so that is the + single place this has to hook into. Suppressed flags are removed from the + node's result; if a node's flags are *entirely* suppressed, the node is + neutralised -- score zeroed, block cleared. + + The honest limitation: a mechanism reports one aggregate score for all its + flags, so there is no per-flag score to subtract. Suppressing one of several + flags on a node therefore removes the flag but leaves the score, and the + remaining detections still stand on their own. That is the conservative + direction -- a partially-suppressed node can still block, and cannot be used + to silently defeat a rule that is also matching something else. + + Scope precedence is additive, not overriding. A global suppression, a vhost + one and an endpoint one all apply to a request in that endpoint; the narrower + scope adds to the broader rather than replacing it. Deep-merge semantics + (as config_resolver uses) would let an endpoint entry silently drop the + global ones, which is the wrong default for a safety control. +]] + +local cjson = require "cjson.safe" + +local _M = {} + +local CACHE_KEY = "suppressions" + +-- Bounded on purpose: this list is read on every defense node of every request. +local MAX_SUPPRESSIONS = 200 + +local function cache() + return ngx.shared.suppression_cache +end + +-- apply() runs on every defense node, so several times per request. Decoding the +-- same JSON blob each time is work with no new answer. Keyed on the raw string, +-- so a redis_sync write is picked up on the very next call rather than after a +-- TTL -- a stale suppression is either traffic wrongly blocked or wrongly +-- allowed, and neither should wait. +local _cached_raw, _cached_list + +--- Every suppression currently in force, as stored by redis_sync. +-- @return array of {id, scope_type, scope_id, flag, reason, created_at, created_by} +function _M.get_all() + local dict = cache() + if not dict then return {} end + + local raw = dict:get(CACHE_KEY) + if not raw then return {} end + + if raw == _cached_raw then + return _cached_list + end + + local decoded = cjson.decode(raw) + if type(decoded) ~= "table" then return {} end + + _cached_raw, _cached_list = raw, decoded + return decoded +end + +--- Does `flag` match `pattern`? +-- Exact match, or a trailing `*` covering a whole family: `kw:*` suppresses +-- every keyword detection without needing one entry per word. Deliberately not +-- a Lua pattern -- an operator typing a rule name should not have to know that +-- `-` and `.` mean something, and a bad pattern here fails open. +function _M.matches(flag, pattern) + if type(flag) ~= "string" or type(pattern) ~= "string" then return false end + if pattern == flag then return true end + + local prefix = pattern:match("^(.-)%*$") + if prefix then + return prefix == "" or flag:sub(1, #prefix) == prefix + end + return false +end + +--- Is this suppression in force for this scope? +local function applies_to_scope(entry, vhost_id, endpoint_id) + local scope_type = entry.scope_type or "global" + if scope_type == "global" then + return true + elseif scope_type == "vhost" then + return entry.scope_id == vhost_id + elseif scope_type == "endpoint" then + return entry.scope_id == endpoint_id + end + return false +end + +--- The flag patterns in force for one scope. +function _M.active_patterns(vhost_id, endpoint_id) + local patterns = {} + for _, entry in ipairs(_M.get_all()) do + -- Type-guarded because this runs in the request path, outside the pcall + -- that wraps the mechanism itself. A single malformed entry -- anything + -- written straight into the Redis hash that is not a JSON object -- + -- would otherwise raise on entry.flag and take the request down with it. + if type(entry) == "table" and entry.flag + and applies_to_scope(entry, vhost_id, endpoint_id) then + patterns[#patterns + 1] = entry.flag + end + end + return patterns +end + +--- Apply suppressions to one defense node's result. +-- @return result (mutated in place), the flags removed, and whether the node +-- lost its verdict entirely. The caller logs the third case louder: a +-- suppression that merely drops a flag is routine, one that turns a +-- block into a pass is the answer to "why did this get through?". +function _M.apply(result, vhost_id, endpoint_id) + if type(result) ~= "table" then return result, nil, false end + + local original = result.flags + if type(original) ~= "table" or #original == 0 then + return result, nil, false + end + + local patterns = _M.active_patterns(vhost_id, endpoint_id) + if #patterns == 0 then + return result, nil, false + end + + local kept, removed = {}, {} + for _, flag in ipairs(original) do + local suppressed = false + for _, pattern in ipairs(patterns) do + if _M.matches(flag, pattern) then + suppressed = true + break + end + end + if suppressed then + removed[#removed + 1] = flag + else + kept[#kept + 1] = flag + end + end + + if #removed == 0 then + return result, nil, false + end + + result.flags = kept + result.details = result.details or {} + result.details.suppressed = removed + + -- Only a node whose every detection was suppressed loses its verdict. One + -- surviving flag means something the operator did not suppress still fired, + -- and that finding is left intact. + local neutralised = false + if #kept == 0 then + neutralised = result.blocked or (result.score or 0) > 0 + result.score = 0 + result.blocked = false + result.block_reason = nil + end + + return result, removed, neutralised +end + +function _M.get_cache_key() + return CACHE_KEY +end + +function _M.get_max() + return MAX_SUPPRESSIONS +end + +return _M diff --git a/openresty/spec/suppressions_spec.lua b/openresty/spec/suppressions_spec.lua new file mode 100644 index 0000000..1db0cf1 --- /dev/null +++ b/openresty/spec/suppressions_spec.lua @@ -0,0 +1,216 @@ +--[[ + Suppression is a safety control that makes the WAF do less, so its failure + modes run in the dangerous direction. Two properties carry the weight: + + * A suppression must not reach beyond its scope. An entry added for one + vhost silently applying to another is a hole opened by accident. + + * Suppressing one detection must not clear a node that also fired for a + reason nobody suppressed. Mechanisms report one aggregate score for all + their flags, so there is no per-flag score to subtract -- which means the + only safe rule is that a node keeps its verdict while any flag survives. +]] +local helper = require "spec_helper" + +describe("suppressions", function() + local suppressions + + local function install(entries) + local cjson = require "cjson.safe" + ngx.shared.suppression_cache:set("suppressions", cjson.encode(entries or {})) + end + + before_each(function() + helper.install_ngx() + helper.stub_external_modules() + package.loaded["suppressions"] = nil + suppressions = require "suppressions" + install({}) + end) + + describe("pattern matching", function() + it("matches an exact flag", function() + assert.is_true(suppressions.matches("kw:viagra", "kw:viagra")) + assert.is_false(suppressions.matches("kw:viagra", "kw:casino")) + end) + + it("matches a family with a trailing star", function() + assert.is_true(suppressions.matches("kw:viagra", "kw:*")) + assert.is_true(suppressions.matches("kw:casino", "kw:*")) + assert.is_false(suppressions.matches("fp_flag:bot", "kw:*")) + end) + + it("treats regex metacharacters as literals", function() + -- An operator typing a rule name should not have to know that "-" and + -- "." mean something. A flag containing them must match itself, and a + -- pattern using them must not match anything else. + assert.is_true(suppressions.matches("kw:crypto-investment", "kw:crypto-investment")) + assert.is_false(suppressions.matches("kw:viagra", "kw:.")) + assert.is_false(suppressions.matches("kwXviagra", "kw.viagra")) + end) + + it("rejects non-string input rather than erroring", function() + assert.is_false(suppressions.matches(nil, "kw:*")) + assert.is_false(suppressions.matches("kw:viagra", nil)) + assert.is_false(suppressions.matches(42, "kw:*")) + end) + end) + + describe("scoping", function() + it("applies a global suppression everywhere", function() + install({ { flag = "kw:viagra", scope_type = "global" } }) + local patterns = suppressions.active_patterns("any-vhost", "any-endpoint") + assert.same({ "kw:viagra" }, patterns) + end) + + it("does not leak a vhost suppression to another vhost", function() + install({ { flag = "kw:viagra", scope_type = "vhost", scope_id = "pharmacy" } }) + assert.same({ "kw:viagra" }, suppressions.active_patterns("pharmacy", "contact")) + assert.same({}, suppressions.active_patterns("other-site", "contact")) + end) + + it("does not leak an endpoint suppression to another endpoint", function() + install({ { flag = "honeypot:filled", scope_type = "endpoint", scope_id = "signup" } }) + assert.same({ "honeypot:filled" }, suppressions.active_patterns("v", "signup")) + assert.same({}, suppressions.active_patterns("v", "contact")) + end) + + it("accumulates scopes rather than the narrower one replacing the broader", function() + install({ + { flag = "kw:viagra", scope_type = "global" }, + { flag = "kw:casino", scope_type = "vhost", scope_id = "shop" }, + { flag = "honeypot:filled", scope_type = "endpoint", scope_id = "signup" }, + }) + local patterns = suppressions.active_patterns("shop", "signup") + table.sort(patterns) + assert.same({ "honeypot:filled", "kw:casino", "kw:viagra" }, patterns) + end) + + it("skips a malformed entry rather than taking the request down", function() + -- This runs outside the pcall that wraps the mechanism, so an error + -- here does not degrade to a neutral result -- it propagates into + -- request handling. Anything written straight into the Redis hash + -- that is not a JSON object arrives here as a number or a string. + local cjson = require "cjson.safe" + ngx.shared.suppression_cache:set("suppressions", + cjson.encode({ 42, "a string", { flag = "kw:viagra", scope_type = "global" } })) + + local patterns + local ok = pcall(function() + patterns = suppressions.active_patterns("v", "e") + end) + assert.is_true(ok, "a malformed entry must not raise in the request path") + assert.same({ "kw:viagra" }, patterns, "the valid entry still applies") + end) + + it("ignores an unknown scope type instead of applying it", function() + install({ { flag = "kw:viagra", scope_type = "everywhere-ish" } }) + assert.same({}, suppressions.active_patterns("v", "e")) + end) + end) + + describe("applying to a defense result", function() + it("neutralises a node whose every flag was suppressed", function() + install({ { flag = "kw:viagra", scope_type = "global" } }) + local result = { score = 0, blocked = true, block_reason = "keyword_blocked", + flags = { "kw:viagra" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.is_false(out.blocked) + assert.equals(0, out.score) + assert.is_nil(out.block_reason) + assert.same({ "kw:viagra" }, removed) + end) + + it("leaves a node that also fired for an unsuppressed reason", function() + -- The property that stops suppression being a back door: casino was + -- not suppressed, so the block stands. + install({ { flag = "kw:viagra", scope_type = "global" } }) + local result = { score = 35, blocked = true, block_reason = "keyword_blocked", + flags = { "kw:viagra", "kw:casino" } } + local out = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked) + assert.equals(35, out.score) + assert.same({ "kw:casino" }, out.flags) + end) + + it("records what it removed, so a detection never just vanishes", function() + install({ { flag = "kw:*", scope_type = "global" } }) + local result = { score = 20, blocked = false, flags = { "kw:viagra", "fp_flag:bot" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.same({ "kw:viagra" }, removed) + assert.same({ "kw:viagra" }, out.details.suppressed) + assert.same({ "fp_flag:bot" }, out.flags) + end) + + it("is a no-op when nothing matches", function() + install({ { flag = "kw:viagra", scope_type = "global" } }) + local result = { score = 40, blocked = true, flags = { "fp_flag:bot" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked) + assert.equals(40, out.score) + assert.is_nil(removed) + end) + + it("is a no-op when no suppressions are configured", function() + local result = { score = 40, blocked = true, flags = { "kw:viagra" } } + local out, removed = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked) + assert.is_nil(removed) + end) + + it("leaves a flagless result alone", function() + install({ { flag = "kw:*", scope_type = "global" } }) + local result = { score = 50, blocked = true, flags = {} } + local out = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked, "a node with no flags has nothing to suppress") + assert.equals(50, out.score) + end) + + it("reports when it changed the verdict, and when it merely dropped a flag", function() + -- The caller logs these differently: a suppression that turns a block + -- into a pass is the answer to "why did this get through?", while one + -- that drops a flag from a node that was not blocking is routine. + install({ { flag = "kw:viagra", scope_type = "global" } }) + + local blocked = { score = 0, blocked = true, flags = { "kw:viagra" } } + local _, _, neutralised = suppressions.apply(blocked, "v", "e") + assert.is_true(neutralised) + + local scored = { score = 30, blocked = false, flags = { "kw:viagra" } } + local _, _, scored_neutralised = suppressions.apply(scored, "v", "e") + assert.is_true(scored_neutralised, "zeroing a contributing score is a changed verdict") + + local harmless = { score = 0, blocked = false, flags = { "kw:viagra" } } + local _, _, harmless_neutralised = suppressions.apply(harmless, "v", "e") + assert.is_false(harmless_neutralised, "a node that was not going to block either way") + + local partial = { score = 30, blocked = true, flags = { "kw:viagra", "kw:casino" } } + install({ { flag = "kw:viagra", scope_type = "global" } }) + local _, _, partial_neutralised = suppressions.apply(partial, "v", "e") + assert.is_false(partial_neutralised, "casino survived, so the verdict stands") + end) + + it("picks up a config change immediately despite caching the decode", function() + -- apply() runs several times per request, so the decode is cached on + -- the raw blob. A stale suppression is either traffic wrongly blocked + -- or wrongly allowed, so the cache must turn over the moment + -- redis_sync writes something new. + install({ { flag = "kw:viagra", scope_type = "global" } }) + assert.same({ "kw:viagra" }, suppressions.active_patterns("v", "e")) + + install({ { flag = "kw:casino", scope_type = "global" } }) + assert.same({ "kw:casino" }, suppressions.active_patterns("v", "e"), + "a new blob must not be served from the previous decode") + + install({}) + assert.same({}, suppressions.active_patterns("v", "e")) + end) + + it("survives junk in the cache rather than failing open or erroring", function() + ngx.shared.suppression_cache:set("suppressions", "not json at all") + local result = { score = 40, blocked = true, flags = { "kw:viagra" } } + local out = suppressions.apply(result, "v", "e") + assert.is_true(out.blocked, "unreadable config must not suppress anything") + end) + end) +end) diff --git a/scripts/check-helm-drift.py b/scripts/check-helm-drift.py new file mode 100755 index 0000000..c4172ae --- /dev/null +++ b/scripts/check-helm-drift.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +""" +Guard the seam between openresty/conf/nginx.conf and the Helm chart's own copy +of it in templates/openresty-configmap.yaml. + +Why this exists +--------------- +The chart does not mount the image's nginx.conf; it ships its own in a +ConfigMap. So every `env NAME;` declaration and every `lua_shared_dict` has to +be added in two places, and forgetting the second one fails silently and only +on Kubernetes: + + * A missing `env NAME;` makes os.getenv("NAME") return nil inside Lua, so a + value an operator sets in values.yaml is quietly ignored. HAPROXY_TIMEOUT + was in exactly this state -- set by the deployment, undeclared in the + ConfigMap, therefore dead. + + * A missing lua_shared_dict makes ngx.shared.NAME nil. Modules guard on that + and return early, so the feature does not crash, it just never does + anything. Shadow mode shipped in this state. + +Both are the "silently inert" class this codebase has been bitten by before, +and neither shows up in the compose stack, which is where testing happens. + +Exit codes: 0 in sync, 1 drifted. +""" + +import re +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +NGINX_CONF = ROOT / "openresty" / "conf" / "nginx.conf" +HELM_CONFIGMAP = ROOT / "helm" / "forms-waf" / "templates" / "openresty-configmap.yaml" + +# Case-sensitive on purpose but NOT uppercase-only: nginx.conf declares the +# lowercase proxy variables too (http_proxy et al), and an uppercase-only +# pattern reported "in sync" while three of them were missing from the chart. +ENV_RE = re.compile(r"^\s*env\s+([A-Za-z_][A-Za-z0-9_]*)\s*;", re.MULTILINE) +DICT_RE = re.compile(r"^\s*lua_shared_dict\s+([a-z_][a-z0-9_]*)\s", re.MULTILINE) + + +def extract(path, pattern): + if not path.exists(): + print(f"error: {path} not found", file=sys.stderr) + sys.exit(1) + return set(pattern.findall(path.read_text())) + + +def report(kind, missing_in_helm, missing_in_conf, consequence): + problems = 0 + if missing_in_helm: + problems += len(missing_in_helm) + print(f"\n {kind} declared in nginx.conf but MISSING from the Helm ConfigMap:") + for name in sorted(missing_in_helm): + print(f" - {name}") + print(f" {consequence}") + print(f" Fix: add it to {HELM_CONFIGMAP.relative_to(ROOT)}") + if missing_in_conf: + problems += len(missing_in_conf) + print(f"\n {kind} in the Helm ConfigMap but MISSING from nginx.conf:") + for name in sorted(missing_in_conf): + print(f" - {name}") + print(" The compose stack and the image will not have it.") + print(f" Fix: add it to {NGINX_CONF.relative_to(ROOT)}") + return problems + + +def main(): + conf_env = extract(NGINX_CONF, ENV_RE) + helm_env = extract(HELM_CONFIGMAP, ENV_RE) + conf_dicts = extract(NGINX_CONF, DICT_RE) + helm_dicts = extract(HELM_CONFIGMAP, DICT_RE) + + print("Helm / nginx.conf drift check") + print("=" * 30) + print(f" env declarations : {len(conf_env)} in nginx.conf, {len(helm_env)} in the chart") + print(f" shared dicts : {len(conf_dicts)} in nginx.conf, {len(helm_dicts)} in the chart") + + problems = 0 + problems += report( + "env declarations", + conf_env - helm_env, + helm_env - conf_env, + "Consequence: os.getenv() returns nil in Lua on Kubernetes, so the setting is ignored.", + ) + problems += report( + "lua_shared_dict", + conf_dicts - helm_dicts, + helm_dicts - conf_dicts, + "Consequence: ngx.shared. is nil on Kubernetes, so the feature is silently inert.", + ) + + print() + if problems: + print(f"FAILED: {problems} declaration(s) out of sync.") + return 1 + print("The chart's nginx.conf matches the image's.") + return 0 + + +if __name__ == "__main__": + sys.exit(main())