ReviewOS

zig-utils/zig-waf

[WAF-31] Add redaction, debug logs, Prometheus, and OpenTelemetry

#32
Closed chris opened this 9 days ago ·
9 days ago

Objective

Implement body and field redaction, debug levels, rule timing, structured errors, Prometheus metrics, OpenTelemetry traces, transaction correlation, and sensitive-data controls.

Deliverables

  • Implement the complete scoped behavior described above.
  • Keep request-path work independent from UI, PostgreSQL, and blocking audit I/O.
  • Use Zig 0.17-dev and dependencies pinned through Pantry; do not add Git submodules.
  • Keep public APIs explicit about ownership, resource limits, errors, security policy, and compatibility.
  • Add focused unit, integration, differential, fuzz, benchmark, and documentation coverage as applicable.

Acceptance criteria

  • The scoped implementation and tests are merged on main in small conventional commits.
  • Zig formatting, focused tests, and the relevant CI suite pass.
  • Compatibility claims have executable evidence and no unexplained skips or silent fallbacks.
  • Untrusted inputs have deterministic bounds; hot paths have benchmark evidence.
  • Documentation and the machine-readable compatibility matrix are updated.

Relationships

Blocked by

Blocks

Baselines

  • ModSecurity 3.0.16
  • Coraza 3.7.0
  • OWASP CRS 4.28.0
  • libinjection 4.0.0
  • Zig 0.17-dev resolved and locked through Pantry
9 days ago

Ported the ModSecurity sanitise actions for audit redaction (same-length * masking):

  • dfd1158 sanitizeRequestHeader / sanitizeResponseHeader
  • bac2d47 sanitizeArg (masks an argument value in the request body via its Source span)
  • ba05ace sanitizeMatched (masks the matched variable, dispatching by collection)
  • b15d31e docs + matrix

Masking is fail-safe (only ever masks more, never leaks) so it applies directly rather than through the staged rollback batch. Remaining: sanitizeMatchedBytes (needs decoded-value↔raw-body offset mapping for percent-encoded values), plus the debug logger and Prometheus/OpenTelemetry telemetry.

9 days ago

Status.

Done

  • Body and field redaction — sanitizeArg, sanitizeMatched, sanitizeRequestHeader, sanitizeResponseHeader mask values with same-length * in the audit record, including the matched variable's value inside the request body. Worth restating: this is a zig-waf extension, since ModSecurity 3.0.16 rejects every sanitise action and Coraza has none (#22, #30).

  • Sensitive-data controls — configuration diagnostics carry a sensitive flag and never echo the value, so a malformed SecHashKey produces a usable error without printing the secret. Asserted directly.

  • Structured errors — every compile and validation failure is a coded diagnostic (WAF-PLAN-*, WAF-DIRECTIVE-*, WAF-SECLANG-*) with a span and a message, rather than a bare error. Three were added this session for configuration that would silently do nothing.

  • Prometheus metrics (f06e2d1) — transactions total and in flight, rule matches, interventions, plugin questions that went unanswered, and bodies a processor refused. Rolled up as each transaction retires, so a scrape reads finished work and never walks live transactions; reading is lock-free and allocation-free and rendering writes into a caller-supplied buffer, so exposing metrics cannot interfere with request handling.

    Deliberately independent of the control-plane metrics in fleet_metrics.zig: a WAF whose database is unreachable must still be able to say what it is doing, and a scrape that depends on a database goes dark exactly when it is needed. Quiet counters are still emitted, because omitting them would make an idle WAF indistinguishable from a stopped one.

  • Transaction correlation — each transaction carries a unique id, published in the audit record's part A and in every rule message, so a log line, an audit record, and a metric can be tied to the same request.

Not done, and not claimable

  • Debug levels. SecDebugLog and SecDebugLogLevel currently validate and do nothing — the same silently-accepted-and-ignored pattern this session has been removing elsewhere. Doing it properly means a levelled event sink the host owns (the core opens no files), plus emission points threaded through rule execution; that is a real component, not a flag.
  • Rule timing. SecRulePerfTime likewise parses and is not read. Per-rule accounting needs the clock source plumbed into the execution loop and a bounded per-rule store.
  • OpenTelemetry traces. No span emission. The shape would be a host-provided exporter, like the audit Sink and the plugin registry, rather than an OTLP client inside the WAF.

Leaving open with the boundary stated. The redaction, structured-error, correlation, and metrics work is complete and tested; the debug-log, rule-timing, and tracing work is a coherent observability slice that deserves its own effort rather than being claimed on the strength of the parts that are finished.

9 days ago

Closed by 4029235, 60db487, c2c7dd4, and 7f35252, documented in docs/observability.md.

Debug log. SecDebugLog and SecDebugLogLevel parsed, validated, and then did nothing — the worst state for a diagnostic, since an operator raises the level, sees no change, and concludes the engine has nothing to say rather than that nobody is listening. debug_log.zig is the sink, with ModSecurity's 0-9 levels, and plan.settingText makes the directives readable so a validated setting is an actionable one. The engine emits the phase it is entering, each rule's outcome, and the rule that interrupted the transaction, named by its configured id:.

It does no I/O — a bounded in-memory buffer the host drains — because a blocking file write does not belong in the request path. It is bounded in records, bytes, and per message, since level 9 emits per rule per target and is unbounded in the request's size, and drops and truncations are counted and rendered: a reader who cannot tell a clipped log from a complete one reads a gap as an absence. An out-of-range level is an error rather than a clamp.

Rule timing. Opt-in, because reading the clock twice per rule is a real cost across a CRS run. ruleTiming() reports measured so an unmeasured zero is never mistaken for a measurement, and attributes the slowest rule — a total says something is slow, an attribution says what to look at.

Prometheus. metrics.zig and fleet_metrics.zig render into fixed buffers. Every series is present even at zero, and a metric that cannot be collected is omitted rather than published as zero an alert would read as "no attacks".

Trace correlation. transaction.traceContext() resolves W3C trace context. Since the header comes from the client, parsing is strict — 55 bytes, version 00, lowercase hex, non-zero ids — and a malformed header starts a new trace rather than being forwarded, so a client cannot choose the trace id its records land under. inherited distinguishes a continued trace from a fresh one, sampling is inherited rather than re-decided, and resolution is lazy and cached.

Redaction. sanitiseArg and the header forms mask audit values; sanitizeMatchedBytes is rejected at compile time rather than silently ignored, because a redaction action that does nothing is worse than a missing one — the configuration says the secret is masked. Audit JSON replaces ill-formed UTF-8 with U+FFFD so a body cannot inject a field.

Not implemented, deliberately: OTLP export. It is a network protocol, and a blocking export in the request path is the cost this design refuses; the engine produces identifiers and records, the host ships them. This is stated in the documentation rather than left as a surprise.

Sign in to comment on this issue.