From b0c02a3f08430ea21c5bae8d6ecf4dec94234927 Mon Sep 17 00:00:00 2001 From: Ugur Cekmez Date: Wed, 26 Aug 2026 21:28:43 +0300 Subject: [PATCH 1/2] =?UTF-8?q?fix(spec):=20make=20the=20=C2=A75.3=20verif?= =?UTF-8?q?ication=20example=20reject=20truncated=20signatures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The normative webhook-verification example in SPECIFICATION.md §5.3 called `timingSafeEqual` on two buffers without checking their lengths. Node throws `RangeError: Input buffers must have the same byte length` when they differ, so an attacker-supplied truncated `webhook-signature` header produced an unhandled exception — surfacing as HTTP 500 rather than an authentication failure. The same example also parsed a single signature token, so it rejected valid traffic for the whole duration of any `delivery_secret` rotation, during which publishers send multiple space-delimited signatures. The reference libraries (`@eep-dev/signer`, `eep-signer`) already handle both cases correctly. Only the specification — which is what implementers copy from — taught the unsafe version. Changes: - Rewrite the §5.3 example: parse every space-delimited token, guard on buffer length before the constant-time compare, and enforce the 60s timestamp tolerance the surrounding text already mandates but the example omitted entirely. - Promote both behaviours to explicit receiving-platform MUSTs, and point readers at `@eep-dev/signer` rather than a hand-rolled compare. - Add conformance fixture `signature/truncated-signature` (a strict prefix of a valid signature) with the manifest entry and the TypeScript + Python harness cases. - Add regression tests asserting "returns false, does not throw" for truncated and over-long signatures in `@eep-dev/signer`, `eep-signer` and the compliance-cli verifier. No wire-format change: this corrects prose and example code to match the behaviour the reference implementations already ship. Refs: EEP audit 2026-08 finding A8 Signed-off-by: Ugur Cekmez --- docs/current/SPECIFICATION.md | 41 +++++++++++++++---- .../compliance-cli/src/helpers.test.ts | 21 ++++++++++ packages/@eep-dev/signer/src/index.test.ts | 26 ++++++++++++ .../eep-signer-python/tests/test_signer.py | 36 ++++++++++++++++ tests/conformance-fixtures.test.ts | 8 ++++ tests/conformance-fixtures/manifest.json | 41 +++++++++++-------- .../signature/truncated-signature/README.md | 12 ++++++ .../signature/truncated-signature/body.txt | 1 + .../truncated-signature/expected.json | 4 ++ .../truncated-signature/headers.json | 6 +++ .../signature/truncated-signature/now.txt | 1 + .../signature/truncated-signature/secret.txt | 1 + tests/cross-impl/test_conformance_fixtures.py | 8 ++++ 13 files changed, 182 insertions(+), 24 deletions(-) create mode 100644 tests/conformance-fixtures/signature/truncated-signature/README.md create mode 100644 tests/conformance-fixtures/signature/truncated-signature/body.txt create mode 100644 tests/conformance-fixtures/signature/truncated-signature/expected.json create mode 100644 tests/conformance-fixtures/signature/truncated-signature/headers.json create mode 100644 tests/conformance-fixtures/signature/truncated-signature/now.txt create mode 100644 tests/conformance-fixtures/signature/truncated-signature/secret.txt diff --git a/docs/current/SPECIFICATION.md b/docs/current/SPECIFICATION.md index ee5d58b..fe6a7b5 100644 --- a/docs/current/SPECIFICATION.md +++ b/docs/current/SPECIFICATION.md @@ -361,13 +361,17 @@ The key is the `delivery_secret` established at subscription time. **Receiving platforms MUST:** 1. Verify the signature using `crypto.timingSafeEqual()` (or equivalent constant-time comparison) to prevent timing attacks. -2. Reject requests where the `webhook-timestamp` is more than **60 seconds** in the past or future to prevent replay attacks. -3. Return HTTP `200` within 10 seconds, or the publisher will treat the delivery as failed. +2. Compare buffers of equal length only. `crypto.timingSafeEqual()` **throws** when its arguments differ in length, so implementations MUST check the length first and return a verification failure — never propagate the exception. A caller that omits this check turns an attacker-supplied truncated `webhook-signature` into an unhandled error rather than an authentication failure. +3. Accept a `webhook-signature` header carrying **multiple** space-delimited signatures (`v1, v1,`) and treat the request as verified if **any** of them matches. Publishers send more than one signature while rotating a `delivery_secret`; a receiver that parses only the first value rejects valid traffic for the duration of every rotation. +4. Reject requests where the `webhook-timestamp` is more than **60 seconds** in the past or future to prevent replay attacks. +5. Return HTTP `200` within 10 seconds, or the publisher will treat the delivery as failed. **Example verification (Node.js):** ```typescript import { createHmac, timingSafeEqual } from 'crypto'; +const TOLERANCE_SECONDS = 60; + function verifyWebhook( rawBody: string, webhookId: string, @@ -375,16 +379,37 @@ function verifyWebhook( webhookSignature: string, secret: string ): boolean { + // 1. Reject stale or future-dated deliveries (replay protection). + const timestamp = Number.parseInt(webhookTimestamp, 10); + if (!Number.isFinite(timestamp)) return false; + const age = Math.floor(Date.now() / 1000) - timestamp; + if (Math.abs(age) > TOLERANCE_SECONDS) return false; + + // 2. Compute the expected signature over the RAW body bytes. const signedContent = `${webhookId}.${webhookTimestamp}.${rawBody}`; - const expected = createHmac('sha256', secret) - .update(signedContent) - .digest('base64'); - - const incoming = webhookSignature.replace('v1,', ''); - return timingSafeEqual(Buffer.from(expected), Buffer.from(incoming)); + const expected = Buffer.from( + `v1,${createHmac('sha256', secret).update(signedContent, 'utf8').digest('base64')}` + ); + + // 3. Compare against every offered signature. The length guard is + // required: timingSafeEqual throws a RangeError on a length + // mismatch, which would surface as a 500 instead of a 401. + for (const candidate of webhookSignature.split(' ')) { + const incoming = Buffer.from(candidate); + if (incoming.length === expected.length && timingSafeEqual(incoming, expected)) { + return true; + } + } + + return false; } ``` +> The reference implementation of this algorithm is +> [`@eep-dev/signer`](https://www.npmjs.com/package/@eep-dev/signer) (`EEPSigner.verify`) +> and its Python sibling `eep-signer`. Prefer importing it over +> re-implementing the comparison by hand. + ### 5.4 Retry policy (exponential backoff) If a webhook delivery fails (non-2xx response or timeout), the publisher MUST retry with exponential backoff: diff --git a/packages/@eep-dev/compliance-cli/src/helpers.test.ts b/packages/@eep-dev/compliance-cli/src/helpers.test.ts index 9c852db..065fb8d 100644 --- a/packages/@eep-dev/compliance-cli/src/helpers.test.ts +++ b/packages/@eep-dev/compliance-cli/src/helpers.test.ts @@ -440,5 +440,26 @@ describe('@eep-dev/compliance-cli helpers', () => { expect(result.valid).toBe(fx.expected.valid); expect(result.reason).toBe('ok_via_multi_signature'); }); + + // A truncated token is attacker-controlled input whose only purpose + // is to reach `timingSafeEqual` with mismatched buffer lengths. The + // verifier MUST return a failure rather than throw — a propagated + // RangeError surfaces as HTTP 500 instead of 401. See + // SPECIFICATION.md §5.3, receiving-platform requirement 2. + it('matches fixture: signature/truncated-signature → invalid, and does not throw', () => { + const fx = loadFixture('truncated-signature'); + const call = () => + verifyWebhookSignature({ + webhookId: fx.headers['webhook-id'], + timestamp: fx.headers['webhook-timestamp'], + rawBody: fx.body, + secret: fx.secret, + signatureHeader: fx.headers['webhook-signature'], + }); + expect(call).not.toThrow(); + const result = call(); + expect(result.valid).toBe(fx.expected.valid); + expect(result.reason).toBe('signature_mismatch'); + }); }); }); diff --git a/packages/@eep-dev/signer/src/index.test.ts b/packages/@eep-dev/signer/src/index.test.ts index b10082d..3ffd224 100644 --- a/packages/@eep-dev/signer/src/index.test.ts +++ b/packages/@eep-dev/signer/src/index.test.ts @@ -221,5 +221,31 @@ describe('@eep-dev/signer', () => { const shortSig = 'v1,YQ=='; expect(signer.verify(WEBHOOK_ID, now, shortSig, BODY)).toBe(false); }); + + // Regression guard for SPECIFICATION.md §5.3 requirement 2: a + // truncated prefix of the *correct* signature is the input that + // makes an unguarded `timingSafeEqual` raise RangeError. Verifying + // it MUST return false, never throw — otherwise attacker-controlled + // bytes turn a 401 into a 500. Mirrors the conformance fixture + // `tests/conformance-fixtures/signature/truncated-signature`. + it('should return false, not throw, for a truncated valid signature', () => { + const signer = new EEPSigner(SECRET); + const now = Math.floor(Date.now() / 1000).toString(); + const real = signer.sign(WEBHOOK_ID, now, BODY); + for (const cut of [4, 10, 20, real.length - 1]) { + const truncated = real.slice(0, cut); + expect(() => signer.verify(WEBHOOK_ID, now, truncated, BODY)).not.toThrow(); + expect(signer.verify(WEBHOOK_ID, now, truncated, BODY)).toBe(false); + } + }); + + // Same guarantee for a signature that is LONGER than expected. + it('should return false, not throw, for an over-long signature', () => { + const signer = new EEPSigner(SECRET); + const now = Math.floor(Date.now() / 1000).toString(); + const padded = signer.sign(WEBHOOK_ID, now, BODY) + 'AAAA'; + expect(() => signer.verify(WEBHOOK_ID, now, padded, BODY)).not.toThrow(); + expect(signer.verify(WEBHOOK_ID, now, padded, BODY)).toBe(false); + }); }); }); diff --git a/packages/eep-signer-python/tests/test_signer.py b/packages/eep-signer-python/tests/test_signer.py index d6de098..989fa03 100644 --- a/packages/eep-signer-python/tests/test_signer.py +++ b/packages/eep-signer-python/tests/test_signer.py @@ -99,3 +99,39 @@ def test_invalid_secret_length_returns_false(self): "webhook-signature": "v1,fake", } assert verify_eep_webhook(BODY, headers, "short") is False + + +class TestTruncatedSignature: + """SPECIFICATION.md §5.3 requirement 2: a length-mismatched signature + MUST produce a verification failure, never an exception. + + A truncated prefix of the *correct* signature is the attacker-controlled + input that makes an unguarded constant-time comparison raise instead of + returning False — turning an authentication failure (401) into a server + error (500). Mirrors the conformance fixture + ``tests/conformance-fixtures/signature/truncated-signature``. + """ + + def test_truncated_signature_returns_false(self): + signer = EEPSigner(SECRET) + ts = str(int(time.time())) + real = signer.sign(WEBHOOK_ID, ts, BODY) + for cut in (4, 10, 20, len(real) - 1): + assert signer.verify(WEBHOOK_ID, ts, real[:cut], BODY) is False + + def test_overlong_signature_returns_false(self): + signer = EEPSigner(SECRET) + ts = str(int(time.time())) + padded = signer.sign(WEBHOOK_ID, ts, BODY) + "AAAA" + assert signer.verify(WEBHOOK_ID, ts, padded, BODY) is False + + def test_truncated_signature_via_convenience_helper(self): + signer = EEPSigner(SECRET) + ts = str(int(time.time())) + real = signer.sign(WEBHOOK_ID, ts, BODY) + headers = { + "webhook-id": WEBHOOK_ID, + "webhook-timestamp": ts, + "webhook-signature": real[:20], + } + assert verify_eep_webhook(BODY, headers, SECRET) is False diff --git a/tests/conformance-fixtures.test.ts b/tests/conformance-fixtures.test.ts index 55eeeb1..781d05d 100644 --- a/tests/conformance-fixtures.test.ts +++ b/tests/conformance-fixtures.test.ts @@ -167,6 +167,14 @@ describe.each(signedBundles)('signed-bundle fixture: $id', (entry) => { // Recorded sig was produced with a different secret. The // recomputation with the verifier's secret MUST NOT match. expect(expectedSig).not.toBe(recomputed); + } else if (entry.id === 'signature-truncated-signature') { + // The recorded token is a strict prefix of the real signature. + // It MUST be shorter, because the whole point of the fixture is + // to exercise the length guard that keeps timingSafeEqual from + // throwing RangeError on attacker-controlled input. + expect(expectedSig).not.toBe(recomputed); + expect(expectedSig.length).toBeLessThan(recomputed.length); + expect(recomputed.startsWith(expectedSig)).toBe(true); } else if (entry.id === 'signature-multi-header') { // The header is "FAKE REAL". The real one MUST match recompute. const tokens = expectedSig.split(' '); diff --git a/tests/conformance-fixtures/manifest.json b/tests/conformance-fixtures/manifest.json index 5e34018..a37df9d 100644 --- a/tests/conformance-fixtures/manifest.json +++ b/tests/conformance-fixtures/manifest.json @@ -9,7 +9,7 @@ "id": "discovery-well-known-eep-valid", "category": "discovery", "tier": "Core", - "spec_section": "§4 Discovery", + "spec_section": "\u00a74 Discovery", "schema": "schemas/v0.1/eep-manifest.json", "input": "discovery/well-known-eep-valid.input.json", "expected": "discovery/well-known-eep-valid.expected.json", @@ -20,7 +20,7 @@ "id": "discovery-well-known-eep-missing-version", "category": "discovery", "tier": "Core", - "spec_section": "§4 Discovery", + "spec_section": "\u00a74 Discovery", "schema": "schemas/v0.1/eep-manifest.json", "input": "discovery/well-known-eep-missing-version.input.json", "expected": "discovery/well-known-eep-missing-version.expected.json", @@ -31,7 +31,7 @@ "id": "envelope-valid-cloudevents", "category": "envelope", "tier": "Core", - "spec_section": "§5 Event envelope", + "spec_section": "\u00a75 Event envelope", "schema": "schemas/v0.1/event.envelope.json", "input": "envelope/valid-cloudevents.input.json", "expected": "envelope/valid-cloudevents.expected.json", @@ -42,7 +42,7 @@ "id": "envelope-invalid-missing-source", "category": "envelope", "tier": "Core", - "spec_section": "§5 Event envelope", + "spec_section": "\u00a75 Event envelope", "schema": "schemas/v0.1/event.envelope.json", "input": "envelope/invalid-missing-source.input.json", "expected": "envelope/invalid-missing-source.expected.json", @@ -53,7 +53,7 @@ "id": "signature-valid-fresh", "category": "signature", "tier": "Standard", - "spec_section": "§5.3 Signing / Standard Webhooks", + "spec_section": "\u00a75.3 Signing / Standard Webhooks", "path": "signature/valid-fresh-signature", "shape": "signed-bundle", "asserts_valid": true @@ -62,7 +62,7 @@ "id": "signature-expired-timestamp", "category": "signature", "tier": "Standard", - "spec_section": "§5.3 Signing / Replay window", + "spec_section": "\u00a75.3 Signing / Replay window", "path": "signature/expired-timestamp", "shape": "signed-bundle", "asserts_valid": false @@ -71,7 +71,7 @@ "id": "signature-wrong-secret", "category": "signature", "tier": "Standard", - "spec_section": "§5.3 Signing", + "spec_section": "\u00a75.3 Signing", "path": "signature/wrong-secret", "shape": "signed-bundle", "asserts_valid": false @@ -80,7 +80,7 @@ "id": "signature-short-secret-rejected", "category": "signature", "tier": "Standard", - "spec_section": "§5.3 Signing — secret minimum length", + "spec_section": "\u00a75.3 Signing \u2014 secret minimum length", "path": "signature/short-secret-rejected", "shape": "signed-bundle", "asserts_valid": false @@ -89,16 +89,25 @@ "id": "signature-multi-header", "category": "signature", "tier": "Standard", - "spec_section": "§5.3 Signing — multi-value header", + "spec_section": "\u00a75.3 Signing \u2014 multi-value header", "path": "signature/multi-signature-header", "shape": "signed-bundle", "asserts_valid": true }, + { + "id": "signature-truncated-signature", + "category": "signature", + "tier": "Standard", + "spec_section": "\u00a75.3 Signing / constant-time comparison", + "path": "signature/truncated-signature", + "shape": "signed-bundle", + "asserts_valid": false + }, { "id": "gates-402-payment-required", "category": "gates", "tier": "Standard", - "spec_section": "§7 Gates and access control", + "spec_section": "\u00a77 Gates and access control", "schema": "schemas/v0.1/gate.402-response.json", "input": "gates/402-payment-required.input.json", "expected": "gates/402-payment-required.expected.json", @@ -109,7 +118,7 @@ "id": "gates-403-tier-mismatch", "category": "gates", "tier": "Standard", - "spec_section": "§7 Gates and access control", + "spec_section": "\u00a77 Gates and access control", "schema": "schemas/v0.1/gate.403-response.json", "input": "gates/403-tier-mismatch.input.json", "expected": "gates/403-tier-mismatch.expected.json", @@ -120,7 +129,7 @@ "id": "gates-429-rate-limited", "category": "gates", "tier": "Core", - "spec_section": "§9 Rate limiting", + "spec_section": "\u00a79 Rate limiting", "schema": "schemas/v0.1/gate.429-response.json", "input": "gates/429-rate-limited.input.json", "expected": "gates/429-rate-limited.expected.json", @@ -131,7 +140,7 @@ "id": "gates-451-unavailable-for-legal-reasons", "category": "gates", "tier": "Standard", - "spec_section": "§7 Gates / legal restrictions", + "spec_section": "\u00a77 Gates / legal restrictions", "schema": "schemas/v0.1/gate.451-response.json", "input": "gates/451-unavailable-for-legal-reasons.input.json", "expected": "gates/451-unavailable-for-legal-reasons.expected.json", @@ -142,7 +151,7 @@ "id": "subscription-valid-webhook-request", "category": "subscription", "tier": "Core", - "spec_section": "§6 Subscription lifecycle", + "spec_section": "\u00a76 Subscription lifecycle", "schema": "schemas/v0.1/subscription.request.json", "input": "subscription/valid-webhook-request.input.json", "expected": "subscription/valid-webhook-request.expected.json", @@ -153,7 +162,7 @@ "id": "subscription-ssrf-private-ip-rejected", "category": "subscription", "tier": "Standard", - "spec_section": "§11 Security — SSRF", + "spec_section": "\u00a711 Security \u2014 SSRF", "input": "subscription/ssrf-private-ip-rejected.input.json", "expected": "subscription/ssrf-private-ip-rejected.expected.json", "shape": "json-pair", @@ -163,7 +172,7 @@ "id": "discovery-crosswalk-host-bundle", "category": "discovery", "tier": "Informative", - "spec_section": "§12 Discovery (informative crosswalk)", + "spec_section": "\u00a712 Discovery (informative crosswalk)", "path": "discovery/crosswalk-host", "expected": "discovery/crosswalk-host/expected.json", "shape": "bundle", diff --git a/tests/conformance-fixtures/signature/truncated-signature/README.md b/tests/conformance-fixtures/signature/truncated-signature/README.md new file mode 100644 index 0000000..f65cb11 --- /dev/null +++ b/tests/conformance-fixtures/signature/truncated-signature/README.md @@ -0,0 +1,12 @@ +Signature header carries a truncated token, shorter than a real +HMAC-SHA256/base64 signature. A verifier MUST return a verification +failure (valid=false). + +It MUST NOT propagate an exception: Node's `crypto.timingSafeEqual()` +throws `RangeError` when its two buffers differ in length, so an +implementation that compares without a length guard turns this +attacker-controlled input into an unhandled error (HTTP 500) instead +of an authentication failure (HTTP 401). See SPECIFICATION.md §5.3, +receiving-platform requirement 2. + +The full, correct signature for this bundle is `v1,fsFPyN/nacY2QTb0lQLeMbEr6YmtCCmdKBJf5hrH+q8=`. diff --git a/tests/conformance-fixtures/signature/truncated-signature/body.txt b/tests/conformance-fixtures/signature/truncated-signature/body.txt new file mode 100644 index 0000000..3ea1134 --- /dev/null +++ b/tests/conformance-fixtures/signature/truncated-signature/body.txt @@ -0,0 +1 @@ +{"specversion":"1.0","type":"com.example.entity.updated","source":"did:web:test.eep.dev:u:alice","id":"evt-5","time":"2026-05-09T12:00:00Z","data":{}} \ No newline at end of file diff --git a/tests/conformance-fixtures/signature/truncated-signature/expected.json b/tests/conformance-fixtures/signature/truncated-signature/expected.json new file mode 100644 index 0000000..5178cbf --- /dev/null +++ b/tests/conformance-fixtures/signature/truncated-signature/expected.json @@ -0,0 +1,4 @@ +{ + "valid": false, + "reason": "signature_length_mismatch_must_not_throw" +} diff --git a/tests/conformance-fixtures/signature/truncated-signature/headers.json b/tests/conformance-fixtures/signature/truncated-signature/headers.json new file mode 100644 index 0000000..0819a89 --- /dev/null +++ b/tests/conformance-fixtures/signature/truncated-signature/headers.json @@ -0,0 +1,6 @@ +{ + "webhook-id": "msg_01HN3QK7GXFIXTURE0005", + "webhook-timestamp": "1778414400", + "webhook-signature": "v1,fsFPyN/nacY2QTb0lQLe", + "content-type": "application/json" +} diff --git a/tests/conformance-fixtures/signature/truncated-signature/now.txt b/tests/conformance-fixtures/signature/truncated-signature/now.txt new file mode 100644 index 0000000..73547c1 --- /dev/null +++ b/tests/conformance-fixtures/signature/truncated-signature/now.txt @@ -0,0 +1 @@ +1778414400 diff --git a/tests/conformance-fixtures/signature/truncated-signature/secret.txt b/tests/conformance-fixtures/signature/truncated-signature/secret.txt new file mode 100644 index 0000000..8de1da6 --- /dev/null +++ b/tests/conformance-fixtures/signature/truncated-signature/secret.txt @@ -0,0 +1 @@ +super-secret-test-key-1234 diff --git a/tests/cross-impl/test_conformance_fixtures.py b/tests/cross-impl/test_conformance_fixtures.py index 4d32092..af80d4c 100644 --- a/tests/cross-impl/test_conformance_fixtures.py +++ b/tests/cross-impl/test_conformance_fixtures.py @@ -89,6 +89,14 @@ def test_signed_bundle_round_trips(entry: dict) -> None: if entry["id"] == "signature-wrong-secret": assert recorded_sig != recomputed + elif entry["id"] == "signature-truncated-signature": + # The recorded token is a strict prefix of the real signature. It + # MUST be shorter — the fixture exists to exercise the length guard + # that keeps a constant-time comparison from raising on + # attacker-controlled input. + assert recorded_sig != recomputed + assert len(recorded_sig) < len(recomputed) + assert recomputed.startswith(recorded_sig) elif entry["id"] == "signature-multi-header": tokens = recorded_sig.split(" ") assert len(tokens) >= 2 From 0f404c00db0b86329db31cdb10e8e0562049dec1 Mon Sep 17 00:00:00 2001 From: Ugur Cekmez Date: Wed, 26 Aug 2026 21:46:47 +0300 Subject: [PATCH 2/2] ci: run the test matrix on every pull request, not only PRs into main `pull_request.branches` was limited to `main`, so a PR based on another branch got no checks whatsoever. That is exactly the situation where a reviewer most needs them: a stacked change is only reviewable in isolation if CI has actually run against it. Stacking PRs is the practical way to ship a series of related changes without resolving the same conflicts N times, and it should not cost the series its test coverage. Pushes to `main` are unaffected. Signed-off-by: Ugur Cekmez --- .github/workflows/test.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c37f7b1..dc6d30a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -4,7 +4,11 @@ on: push: branches: [main] pull_request: - branches: [main] + # Every pull request is tested, not only those targeting `main`. + # Stacked PRs (where one change is based on the branch of another so the + # diffs stay reviewable) previously got no checks at all, which is + # precisely when a reviewer most wants them. + branches: ['**'] jobs: dependency-policy: