From ef6d70840b696d5ad6ab23abdf1be038b785ee3b Mon Sep 17 00:00:00 2001 From: Rita Agafonova <36133358+rita-aga@users.noreply.github.com> Date: Tue, 7 Jul 2026 02:03:16 -0700 Subject: [PATCH] fix(paw-ingest): verify webhook HMAC-SHA256 and fail closed (ARN-168) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/triggers/webhook/{route_key}` endpoint is public, so the request signature is the only thing authenticating an inbound webhook. The validate_webhook WASM integration only checked that the signature *header was present* — it never computed or compared HMAC-SHA256(secret, body) — and returned hmac_verified:"true" whenever the header existed, dispatching the payload regardless. Any body with a bogus X-Hub-Signature-256 was accepted and routed into the scout/SRE/heal/patrol agents (Class B). Fix (mirrors the kernel counterpart for ARN-171): - Compute HMAC-SHA256(secret, raw_payload) and constant-time compare it (subtle::ConstantTimeEq, not ==) against the signature header. - Accept an optional `sha256=` prefix, case-insensitively; header lookup is case-insensitive. - Resolve the signing secret from the route: literal value or {secret:KEY} template resolved from the host secret store. - Fail closed: a route that declares a secret but has a missing, unresolvable, or mismatched signature transitions the WebhookEvent to ValidationFailed and is never routed/processed. Routes with no secret keep the existing "skipped" carve-out. Verification logic is factored into pure, host-free helpers (classify_secret, extract_header, verify_signature, signature_matches) covered by a red→green unit suite: a forged signature (previously accepted as "true") and a tampered body / wrong secret / missing header are now rejected, a correctly signed payload is accepted. Adds hmac, sha2, hex, subtle (pure-Rust, compile for wasm32-unknown-unknown). See os-apps/paw-ingest/adrs/001-webhook-hmac-verification.md. Co-Authored-By: Claude Opus 4.8 --- .../adrs/001-webhook-hmac-verification.md | 78 ++++ .../wasm/validate_webhook/Cargo.toml | 4 + .../wasm/validate_webhook/src/lib.rs | 332 +++++++++++++++++- 3 files changed, 397 insertions(+), 17 deletions(-) create mode 100644 os-apps/paw-ingest/adrs/001-webhook-hmac-verification.md diff --git a/os-apps/paw-ingest/adrs/001-webhook-hmac-verification.md b/os-apps/paw-ingest/adrs/001-webhook-hmac-verification.md new file mode 100644 index 000000000..15867b074 --- /dev/null +++ b/os-apps/paw-ingest/adrs/001-webhook-hmac-verification.md @@ -0,0 +1,78 @@ +# ADR-001: Webhook HMAC Verification + +**Status:** Accepted +**Scope:** wasm-integration (validate_webhook) +**Author:** TemperPaw maintainers +**Date:** 2026-07-07 +**Tracking:** ARN-168 (Class B, epic ARN-165); mirrors the kernel fix ARN-171 (temper PR #340 / ADR-0156) + +## Context + +`/triggers/webhook/{route_key}` is a public endpoint — it is exempted from bearer +auth (`crates/temperpaw/src/auth.rs`, `is_public_path`) because external senders +(GitHub, Datadog, …) cannot present a Temper credential. The signature over the +request body is therefore the *only* thing that authenticates an inbound webhook. + +The trigger creates one `WebhookEvent` and dispatches `Received`, which fires the +`validate_webhook` WASM integration. That module was supposed to verify the HMAC +signature, but the implementation only checked that the signature *header was +present* — it never computed or compared `HMAC-SHA256(secret, body)`. It returned +`hmac_verified: "true"` whenever the header existed and always transitioned to +`Validated`, so the pipeline dispatched the payload regardless. + +Exploit: `POST /triggers/webhook/github` with header +`X-Hub-Signature-256: sha256=anything` was recorded as `hmac_verified: true` and +routed into the scout/SRE/heal/patrol agents — allowing spoofed GitHub/Datadog +events, injected agent instructions, and resource abuse. The module's own doc +comment admitted it: "Full cryptographic verification can be added later." This +is an instance of systemic **Class B** (unauthenticated ingress). + +## Decision + +`validate_webhook` now performs real signature verification and fails closed. + +- **Compute and compare.** When a route declares a `webhook_secret`, compute + `HMAC-SHA256(secret, raw_payload)` and compare it against the signature header + using a **constant-time** comparison (`subtle::ConstantTimeEq`), not `==`, to + avoid a timing side channel on the digest. +- **Signature format.** The provided value is trimmed and lower-cased, an + optional `sha256=` prefix (as GitHub sends) is stripped, then the remaining hex + digest is compared. Bare-hex signatures (no prefix) are also accepted. +- **Secret resolution.** `webhook_secret` may be a literal value or a + `{secret:KEY}` template resolved from the host secret store via + `ctx.get_secret(KEY)`. An empty or unresolvable secret is treated as *not + resolvable*. +- **Fail closed.** If a route declares a secret and the request has no signature + header, an unresolvable secret, or a mismatched signature, the module + transitions the `WebhookEvent` to `ValidationFailed` (→ `Rejected`) with a + `validation_error`. The payload is never routed or processed. +- **No secret configured → skipped.** A route with an empty `webhook_secret` has + opted out of signature verification; behaviour is unchanged (`hmac_verified: + "skipped"`, transitions to `Validated`). This matches the kernel counterpart, + which leaves authenticity for such routes to the Cedar gate rather than + rejecting outright, and avoids breaking routes that intentionally carry no + secret. + +The header a route's signature is read from is selected by `source_type` +(`github` → `x-hub-signature-256`, `datadog` → `x-datadog-signature`, default +`x-hub-signature-256`). Header lookup is case-insensitive. + +## Consequences + +### Positive +- Spoofed webhooks with forged or absent signatures are rejected before any agent + is invoked, closing the Class B ingress on the TemperPaw side. +- The signing secret is resolved from the secret store, not trusted from a header. +- Constant-time comparison removes a timing side channel on the digest. +- Verification logic is factored into pure, host-free functions + (`classify_secret`, `extract_header`, `verify_signature`, `signature_matches`) + that are unit-tested (red exploit test → green), keeping `run()` a thin shell. + +### Negative +- Routes that had a `webhook_secret` set but relied on the old no-op now require + the sender to produce a correct `HMAC-SHA256` signature; a misconfigured secret + will start rejecting traffic (the intended fail-closed behaviour). Operators + must ensure the stored secret matches the sender's signing key. +- Adds `hmac`, `sha2`, `hex`, and `subtle` as build dependencies of the module + (all pure-Rust, `no_std`-friendly, and confirmed to compile for + `wasm32-unknown-unknown`). diff --git a/os-apps/paw-ingest/wasm/validate_webhook/Cargo.toml b/os-apps/paw-ingest/wasm/validate_webhook/Cargo.toml index d9ab34396..691bf54d1 100644 --- a/os-apps/paw-ingest/wasm/validate_webhook/Cargo.toml +++ b/os-apps/paw-ingest/wasm/validate_webhook/Cargo.toml @@ -10,3 +10,7 @@ crate-type = ["cdylib"] [dependencies] temper-wasm-sdk = { git = "https://github.com/nerdsane/temper.git", rev = "1a57fb0c9cfc935135550b5d88fbd49afcfabb9e" } +hmac = { version = "0.12", default-features = false } +sha2 = { version = "0.10", default-features = false } +hex = { version = "0.4", default-features = false, features = ["alloc"] } +subtle = { version = "2", default-features = false } diff --git a/os-apps/paw-ingest/wasm/validate_webhook/src/lib.rs b/os-apps/paw-ingest/wasm/validate_webhook/src/lib.rs index 09d9d73a9..2db805c36 100644 --- a/os-apps/paw-ingest/wasm/validate_webhook/src/lib.rs +++ b/os-apps/paw-ingest/wasm/validate_webhook/src/lib.rs @@ -1,8 +1,12 @@ //! Validate Webhook — WASM module for validating incoming webhook payloads. //! //! Triggered by WebhookEvent.Received action. Looks up the WebhookRoute by -//! route_key, verifies HMAC signature if a secret is configured, and transitions -//! to Validated or ValidationFailed. +//! route_key and, when the route declares a signing secret, verifies the +//! request's `HMAC-SHA256(secret, raw_body)` signature before letting the +//! payload proceed. Fails closed — a route with a configured secret and a +//! missing, unresolvable, or mismatched signature transitions to +//! ValidationFailed and is never dispatched (ARN-168 / Class B). Routes +//! without a secret skip verification and transition to Validated. //! //! Build: `cargo build --target wasm32-unknown-unknown --release` @@ -96,11 +100,42 @@ pub extern "C" fn run(_ctx_ptr: i32, _ctx_len: i32) -> i32 { .and_then(|v| v.as_str()) .unwrap_or(""); - // HMAC verification (simplified — check header presence if secret is configured) + // HMAC verification (ARN-168): compute HMAC-SHA256(secret, raw_body) and + // constant-time compare it against the signature header. Fail closed — + // if a route declares a secret we must be able to resolve it, find the + // signature header, and match it, or the payload is rejected outright. let hmac_verified = if webhook_secret.is_empty() { + // No secret configured on this route: nothing to verify against. "skipped" } else { - verify_signature_header(raw_headers, source_type) + match resolve_webhook_secret(&ctx, webhook_secret) { + None => { + ctx.log( + "warn", + "validate_webhook: signing secret configured but unresolvable — rejecting", + ); + set_success_result("ValidationFailed", &json!({ + "validation_error": + "webhook signing secret is configured but could not be resolved" + })); + return Ok(()); + } + Some(secret) => { + match verify_signature(&secret, raw_headers, source_type, raw_payload) { + VerifyOutcome::Verified => "true", + VerifyOutcome::Rejected(reason) => { + ctx.log( + "warn", + &format!("validate_webhook: signature rejected — {reason}"), + ); + set_success_result("ValidationFailed", &json!({ + "validation_error": reason + })); + return Ok(()); + } + } + } + } }; ctx.log( @@ -154,25 +189,288 @@ fn urlencoded(s: &str) -> String { .replace('=', "%3D") } -/// Simplified HMAC signature check: verify the expected signature header exists -/// in the raw headers JSON. Full cryptographic verification can be added later. -fn verify_signature_header(raw_headers: &str, source_type: &str) -> &'static str { - let header_name = match source_type { +/// Outcome of verifying a webhook signature against a configured secret. +enum VerifyOutcome { + /// The signature matched `HMAC-SHA256(secret, raw_body)`. + Verified, + /// The request must be rejected (fail closed). Carries a human-readable + /// reason recorded as the WebhookEvent `validation_error`. + Rejected(String), +} + +/// How a route's configured `webhook_secret` value should be resolved. +enum SecretSource<'a> { + /// No secret is configured (empty or malformed template). + None, + /// A literal secret value stored directly on the route. + Literal(&'a str), + /// A `{secret:KEY}` template — `KEY` is resolved from the host secret store. + Template(&'a str), +} + +/// Map a webhook source type to the header that carries its HMAC signature. +fn signature_header_name(source_type: &str) -> &'static str { + match source_type { "datadog" => "x-datadog-signature", "github" => "x-hub-signature-256", _ => "x-hub-signature-256", - }; + } +} - // raw_headers is a JSON object string - let parsed: Result = serde_json::from_str(raw_headers); - match parsed { - Ok(headers_obj) => { - if headers_obj.get(header_name).is_some() { - "true" +/// Classify a route's configured secret value without touching the host. +/// +/// Kept pure so the parsing rules are unit-testable; the host is only consulted +/// (for `{secret:KEY}` templates) by [`resolve_webhook_secret`]. +fn classify_secret(configured: &str) -> SecretSource<'_> { + let configured = configured.trim(); + if configured.is_empty() { + return SecretSource::None; + } + if let Some(rest) = configured.strip_prefix("{secret:") { + return match rest.strip_suffix('}').map(str::trim) { + Some(key) if !key.is_empty() => SecretSource::Template(key), + _ => SecretSource::None, + }; + } + SecretSource::Literal(configured) +} + +/// Resolve the webhook signing secret for a route. +/// +/// Supports `{secret:KEY}` templates (resolved from the host secret store) and +/// literal secret values. Returns `None` (fail closed) when the value is empty, +/// the template is malformed, or the resolved secret is empty/unresolved. +fn resolve_webhook_secret(ctx: &Context, configured: &str) -> Option { + match classify_secret(configured) { + SecretSource::None => None, + SecretSource::Literal(value) => Some(value.to_string()), + SecretSource::Template(key) => { + let resolved = ctx.get_secret(key).ok()?; + let resolved = resolved.trim().to_string(); + if resolved.is_empty() || resolved.contains("{secret:") { + None } else { - "false" + Some(resolved) } } - Err(_) => "false", + } +} + +/// Extract a header value from the `raw_headers` JSON object. +/// +/// HTTP header names are case-insensitive, and the webhook trigger serializes +/// them lowercase, so an exact match is tried first and then a case-insensitive +/// scan. +fn extract_header(raw_headers: &str, header_name: &str) -> Option { + let parsed: Value = serde_json::from_str(raw_headers).ok()?; + let obj = parsed.as_object()?; + if let Some(value) = obj.get(header_name).and_then(|v| v.as_str()) { + return Some(value.to_string()); + } + obj.iter() + .find(|(k, _)| k.eq_ignore_ascii_case(header_name)) + .and_then(|(_, v)| v.as_str()) + .map(|s| s.to_string()) +} + +/// Verify the signature header against `HMAC-SHA256(secret, raw_payload)`. +/// +/// Fails closed: a missing signature header or a mismatched signature is +/// rejected. The `secret` must already be resolved (non-empty). +fn verify_signature( + secret: &str, + raw_headers: &str, + source_type: &str, + raw_payload: &str, +) -> VerifyOutcome { + let header_name = signature_header_name(source_type); + let Some(provided) = extract_header(raw_headers, header_name) else { + return VerifyOutcome::Rejected(format!( + "missing webhook signature header '{header_name}'" + )); + }; + if signature_matches(secret, raw_payload.as_bytes(), &provided) { + VerifyOutcome::Verified + } else { + VerifyOutcome::Rejected("webhook signature verification failed".to_string()) + } +} + +/// Constant-time comparison of a provided signature against the expected +/// `HMAC-SHA256(secret, body)`. +/// +/// Accepts an optional `sha256=` prefix and is case-insensitive over the hex +/// digest. Uses [`subtle::ConstantTimeEq`] rather than `==` to avoid a timing +/// side channel on the digest. +fn signature_matches(secret: &str, body: &[u8], provided: &str) -> bool { + use hmac::{Hmac, Mac}; + use sha2::Sha256; + use subtle::ConstantTimeEq; + + let Ok(mut mac) = Hmac::::new_from_slice(secret.as_bytes()) else { + return false; + }; + mac.update(body); + let expected_hex = hex::encode(mac.finalize().into_bytes()); + + // Normalize (lowercase) so the `sha256=` prefix matches case-insensitively, + // then strip it before comparing the hex digest. + let normalized = provided.trim().to_ascii_lowercase(); + let provided_hex = normalized + .strip_prefix("sha256=") + .unwrap_or(normalized.as_str()) + .trim(); + + provided_hex + .as_bytes() + .ct_eq(expected_hex.as_bytes()) + .into() +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Compute the reference `HMAC-SHA256(secret, body)` hex digest. + fn sign(secret: &str, body: &str) -> String { + use hmac::{Hmac, Mac}; + use sha2::Sha256; + let mut mac = Hmac::::new_from_slice(secret.as_bytes()).unwrap(); + mac.update(body.as_bytes()); + hex::encode(mac.finalize().into_bytes()) + } + + fn github_headers(signature: &str) -> String { + json!({ + "x-hub-signature-256": signature, + "content-type": "application/json", + }) + .to_string() + } + + #[test] + fn correctly_signed_payload_is_accepted() { + let secret = "s3cr3t"; + let body = r#"{"action":"opened"}"#; + let headers = github_headers(&format!("sha256={}", sign(secret, body))); + assert!(matches!( + verify_signature(secret, &headers, "github", body), + VerifyOutcome::Verified + )); + } + + /// ARN-168 exploit: a present-but-forged signature must be rejected. + /// Against the old code this was accepted as `"true"` on header presence. + #[test] + fn forged_signature_is_rejected() { + let secret = "s3cr3t"; + let body = r#"{"action":"opened"}"#; + let headers = github_headers( + "sha256=deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + ); + assert!(matches!( + verify_signature(secret, &headers, "github", body), + VerifyOutcome::Rejected(_) + )); + } + + #[test] + fn missing_signature_header_is_rejected() { + let secret = "s3cr3t"; + let body = r#"{"action":"opened"}"#; + let headers = json!({ "content-type": "application/json" }).to_string(); + assert!(matches!( + verify_signature(secret, &headers, "github", body), + VerifyOutcome::Rejected(_) + )); + } + + /// A signature valid for one body must not validate a different body — + /// proves the HMAC covers the payload, not merely header presence. + #[test] + fn tampered_body_is_rejected() { + let secret = "s3cr3t"; + let signed_body = r#"{"action":"opened"}"#; + let attacker_body = r#"{"action":"deleted"}"#; + let headers = github_headers(&format!("sha256={}", sign(secret, signed_body))); + assert!(matches!( + verify_signature(secret, &headers, "github", attacker_body), + VerifyOutcome::Rejected(_) + )); + } + + #[test] + fn wrong_secret_is_rejected() { + let body = r#"{"action":"opened"}"#; + let headers = github_headers(&format!("sha256={}", sign("attacker-guess", body))); + assert!(matches!( + verify_signature("real-secret", &headers, "github", body), + VerifyOutcome::Rejected(_) + )); + } + + #[test] + fn bare_hex_signature_without_prefix_is_accepted() { + // Sources that send a bare hex digest (no `sha256=` prefix) also verify. + let secret = "s3cr3t"; + let body = "payload"; + let headers = json!({ "x-datadog-signature": sign(secret, body) }).to_string(); + assert!(matches!( + verify_signature(secret, &headers, "datadog", body), + VerifyOutcome::Verified + )); + } + + #[test] + fn uppercase_prefixed_signature_is_accepted() { + let secret = "s3cr3t"; + let body = "payload"; + let headers = github_headers(&format!("SHA256={}", sign(secret, body).to_uppercase())); + assert!(matches!( + verify_signature(secret, &headers, "github", body), + VerifyOutcome::Verified + )); + } + + #[test] + fn header_lookup_is_case_insensitive() { + let secret = "s3cr3t"; + let body = "payload"; + let headers = json!({ + "X-Hub-Signature-256": format!("sha256={}", sign(secret, body)), + }) + .to_string(); + assert!(matches!( + verify_signature(secret, &headers, "github", body), + VerifyOutcome::Verified + )); + } + + #[test] + fn signature_matches_only_the_correct_digest() { + let secret = "key"; + let body = b"body-bytes"; + let good = sign(secret, "body-bytes"); + assert!(signature_matches(secret, body, &good)); + assert!(signature_matches(secret, body, &format!("sha256={good}"))); + assert!(!signature_matches(secret, body, "sha256=00")); + assert!(!signature_matches(secret, body, "")); + assert!(!signature_matches(secret, body, "not-hex")); + } + + #[test] + fn classify_secret_variants() { + assert!(matches!(classify_secret(""), SecretSource::None)); + assert!(matches!(classify_secret(" "), SecretSource::None)); + assert!(matches!(classify_secret("{secret:}"), SecretSource::None)); + assert!(matches!(classify_secret("{secret: }"), SecretSource::None)); + assert!(matches!( + classify_secret("literal-secret"), + SecretSource::Literal("literal-secret") + )); + match classify_secret("{secret:GITHUB_WEBHOOK}") { + SecretSource::Template(key) => assert_eq!(key, "GITHUB_WEBHOOK"), + _ => panic!("expected a {{secret:KEY}} template"), + } } }