diff --git a/CHANGELOG.md b/CHANGELOG.md index 576be97..1271b7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,4 +9,19 @@ tooling is wired up, entries below are added manually per release. ## Unreleased +- **BREAKING** — `fix(webhooks)`: unified the webhook signing scheme and bound + signatures to a timestamp (#97). Deliveries now sign + `` `${timestamp}.${rawBody}` `` instead of the raw body alone and carry two + new headers, `X-SmartDrop-Timestamp` and `X-SmartDrop-Signature-Version: 2`. + Signatures are accepted only while `|now - timestamp|` is within + `WEBHOOK_SIGNATURE_MAX_AGE_SECONDS` (new, default 300), checked + symmetrically so future-dated timestamps are rejected too; previously a + captured payload stayed replayable indefinitely. + + This is a breaking change to a published contract rather than to an HTTP + route, so it does not ship under a new `/api/v2` path. **v1 signatures are no + longer emitted**, and any subscriber verifying an HMAC of the raw body alone + will begin rejecting deliveries. Migration guidance and a working v2 + verification snippet are in the [webhook signing section of the + README](README.md#verifying-the-signature-nodejs). - Added this changelog (#216). diff --git a/README.md b/README.md index 0903e81..5c209ba 100644 --- a/README.md +++ b/README.md @@ -253,6 +253,7 @@ The application reads configurations from the `.env` file at the root. | `WEBHOOK_RETRY_BASE_MS` | Base backoff between retries (ms) | 30000 | No | | `WEBHOOK_RETRY_FACTOR` | Exponential backoff multiplier | 2 | No | | `WEBHOOK_TIMEOUT_MS` | HTTP timeout per delivery attempt | 5000 | No | +| `WEBHOOK_SIGNATURE_MAX_AGE_SECONDS` | Replay window for delivery signatures (s) | 300 | No | | `WEBHOOK_RETRY_POLL_MS` | Retry worker poll interval | 5000 | No | | `WEBHOOK_RETRY_BATCH` | Max retries processed per tick | 25 | No | | `WEBHOOK_RATELIMIT_WINDOW` | Mgmt rate-limit window (s) | 60 | No | @@ -801,7 +802,9 @@ Every delivery is a JSON POST with the following headers: | `User-Agent` | `SmartDrop-Webhooks/1.0` | | `X-SmartDrop-Event` | event type (e.g. `pool.assets_locked`) | | `X-SmartDrop-Delivery` | unique delivery id (`dlv_…`) | -| `X-SmartDrop-Signature` | `sha256=` | +| `X-SmartDrop-Signature` | `sha256=` + hex HMAC of `{timestamp}.{rawBody}` | +| `X-SmartDrop-Timestamp` | epoch milliseconds this attempt was signed at | +| `X-SmartDrop-Signature-Version` | signing scheme version, currently `2` | Body: ```json @@ -813,17 +816,45 @@ Body: } ``` +### Signing algorithm + +The signed message is the timestamp, a literal `.`, and the raw request body: + +``` +signature = "sha256=" + HMAC_SHA256(secret, `${timestamp}.${rawBody}`) +``` + +where `timestamp` is the value sent in `X-SmartDrop-Timestamp`, verbatim. The +timestamp is *inside* the MAC rather than merely alongside it, so a captured +delivery cannot be re-dated to keep it valid. Combined with the freshness +check below, that bounds how long an intercepted payload stays replayable. + ### Verifying the signature (Node.js) ```js const crypto = require('crypto'); +// Must match the sender's WEBHOOK_SIGNATURE_MAX_AGE_SECONDS (default 300). +const MAX_AGE_SECONDS = 300; + function verifySmartDrop(req, secret) { const provided = req.header('X-SmartDrop-Signature') || ''; + const timestamp = (req.header('X-SmartDrop-Timestamp') || '').trim(); + + // Epoch milliseconds, digits only. Do not use Number() alone to validate: + // Number('') and Number(null) are both 0, a valid-looking 1970 timestamp. + if (!/^\d+$/.test(timestamp)) return false; + + // Reject BOTH stale and future-dated timestamps. A one-directional + // `now - timestamp > maxAge` check accepts anything dated forward, which + // hands the holder a signature that never expires. + if (Math.abs(Date.now() - Number(timestamp)) > MAX_AGE_SECONDS * 1000) return false; + const expected = 'sha256=' + crypto .createHmac('sha256', secret) - .update(req.rawBody) // verify against the RAW body, not re-stringified JSON + .update(`${timestamp}.${req.rawBody}`) // RAW body, not re-stringified JSON .digest('hex'); + const a = Buffer.from(provided); const b = Buffer.from(expected); return a.length === b.length && crypto.timingSafeEqual(a, b); @@ -832,6 +863,49 @@ function verifySmartDrop(req, secret) { Express tip: capture the raw body via `express.json({ verify: (req, _res, buf) => { req.rawBody = buf.toString(); } })` so the HMAC matches byte-for-byte. +This snippet is executed verbatim against real signed output in +`test/webhookSignature.test.js` — it is working code, not illustrative +pseudocode, and the test fails if this block and the implementation drift +apart. + +Reject the delivery if verification fails. Retries are expected: a delivery +that is retried after backoff is re-signed at the moment of each attempt, so +every attempt arrives with its own fresh timestamp and must be verified on +its own terms. Do not cache a signature or timestamp across attempts. + +### Signature scheme v2 — breaking change + +**Signature scheme v2 replaces v1 for all deliveries. v1 signatures are no +longer emitted.** Per the [API Versioning](#api-versioning) policy this is not +an HTTP route change, so it does not ship under a new `/api/v2` path — but it +*is* a breaking change to a published contract, and is documented here and in +[`CHANGELOG.md`](CHANGELOG.md) with the same migration guidance a deprecated +endpoint would carry. + +| | v1 (removed) | v2 (current) | +|---|---|---| +| Signed message | `rawBody` | `` `${timestamp}.${rawBody}` `` | +| Freshness | none — signatures never expired | `±WEBHOOK_SIGNATURE_MAX_AGE_SECONDS` | +| Headers | `X-SmartDrop-Signature` | adds `X-SmartDrop-Timestamp`, `X-SmartDrop-Signature-Version` | + +**What breaks:** any verifier that HMACs the raw body alone. Because v1 +recomputation no longer matches, every correctly-implemented v1 subscriber +begins rejecting deliveries as soon as v2 ships. + +**To migrate:** replace your verification function with the v2 snippet above. +Two things beyond the message format matter: + +1. **Check the timestamp symmetrically.** `Math.abs(now - timestamp)`, not + `now - timestamp`. A one-directional check leaves future-dated timestamps + permanently valid, which reintroduces the replay window this change closes. +2. **Keep your clock in sync.** Verification compares your clock to ours, so + drift beyond the max-age window rejects otherwise-valid deliveries. Run NTP, + or widen `MAX_AGE_SECONDS` on your side if you cannot. + +`X-SmartDrop-Signature-Version` is sent so you can branch on the scheme +explicitly; treat an unrecognised value as a delivery you cannot verify and +reject it. + ### Retry & failure semantics - Up to `WEBHOOK_MAX_ATTEMPTS` (default 3) total attempts per event. diff --git a/src/config.js b/src/config.js index 8d7d32f..4668d0b 100644 --- a/src/config.js +++ b/src/config.js @@ -251,6 +251,12 @@ module.exports = { retryBaseMs: parseInt(process.env.WEBHOOK_RETRY_BASE_MS, 10) || 30000, retryFactor: parseFloat(process.env.WEBHOOK_RETRY_FACTOR) || 2, timeoutMs: parseInt(process.env.WEBHOOK_TIMEOUT_MS, 10) || 5000, + // Replay window for outgoing delivery signatures (#97). A signature is + // only accepted while |now - X-SmartDrop-Timestamp| is within this many + // seconds, so a captured payload stops being replayable once it expires. + // Applied symmetrically, which also bounds how far a clock-skewed (or + // forged) future-dated timestamp can push the window forward. + signatureMaxAgeSeconds: parseInt(process.env.WEBHOOK_SIGNATURE_MAX_AGE_SECONDS, 10) || 300, // retryPollMs/retryBatchSize: #128 considered retuning these once // backoffMs() gained jitter (a wider spread of nextRetryAt values could // argue for a shorter poll interval and/or smaller batch, since due diff --git a/src/services/webhook.js b/src/services/webhook.js index cec4dd4..511195f 100644 --- a/src/services/webhook.js +++ b/src/services/webhook.js @@ -1,41 +1,23 @@ -const crypto = require('crypto'); const axios = require('axios'); const logger = require('../logger'); +const signature = require('./webhookSignature'); const DEFAULT_TIMEOUT_MS = 10000; -function payloadBody(payload) { - return typeof payload === 'string' ? payload : JSON.stringify(payload); -} - -function signPayload(secret, payload, timestamp = Date.now()) { - const body = payloadBody(payload); - return crypto - .createHmac('sha256', secret) - .update(`${timestamp}.${body}`) - .digest('hex'); -} - +/** + * Headers for an alert delivery. + * + * The signing scheme itself lives entirely in `webhookSignature` — this used + * to carry a second, subtly different HMAC implementation, which is how the + * alert path and the dispatcher path drifted apart in the first place (#97). + */ function buildSignatureHeaders(secret, payload, timestamp = Date.now()) { - const signature = signPayload(secret, payload, timestamp); return { 'Content-Type': 'application/json', - 'X-SmartDrop-Signature': `sha256=${signature}`, - 'X-SmartDrop-Timestamp': String(timestamp), + ...signature.signatureHeaders(secret, payload, timestamp), }; } -function verifySignature(secret, payload, signatureHeader, timestamp) { - if (!signatureHeader || !timestamp || !signatureHeader.startsWith('sha256=')) { - return false; - } - - const expected = Buffer.from(signPayload(secret, payload, timestamp), 'hex'); - const actual = Buffer.from(signatureHeader.slice('sha256='.length), 'hex'); - - return expected.length === actual.length && crypto.timingSafeEqual(expected, actual); -} - async function sendSignedRequest(webhookUrl, secret, payload, options = {}) { const timestamp = options.timestamp || Date.now(); const headers = buildSignatureHeaders(secret, payload, timestamp); @@ -111,6 +93,4 @@ module.exports = { deliver, probeReachability, sendSignedRequest, - signPayload, - verifySignature, }; diff --git a/src/services/webhookDispatcher.js b/src/services/webhookDispatcher.js index 7bf0b7a..3b5770e 100644 --- a/src/services/webhookDispatcher.js +++ b/src/services/webhookDispatcher.js @@ -124,13 +124,22 @@ function shouldRetry(responseStatus, networkError) { return false; } +/** + * Builds the headers for one delivery attempt. + * + * `timestamp` is the instant *this individual attempt* is being sent at — + * not the instant the event was dispatched. It is threaded in explicitly + * rather than read from the clock in here so the freshness guarantee is + * visible at the call site; see attempt() for why that distinction matters. + */ +function buildHeaders(secret, body, eventType, deliveryId, requestId, timestamp) { function buildHeaders(secret, body, eventType, deliveryId, requestId, sequence) { const headers = { 'Content-Type': 'application/json', 'User-Agent': USER_AGENT, 'X-SmartDrop-Event': eventType, 'X-SmartDrop-Delivery': deliveryId, - 'X-SmartDrop-Signature': signature.sign(secret, body), + ...signature.signatureHeaders(secret, body, timestamp), }; if (sequence != null) headers['X-SmartDrop-Sequence'] = String(sequence); // Lets receivers correlate a delivery with the API request that caused @@ -207,6 +216,16 @@ async function attempt(deliveryId, sequence) { occurred_at: delivery.created_at, }; const body = JSON.stringify(payload); + // Signed fresh for THIS attempt, immediately before the request goes + // out. A retry can be delivered long after the event was dispatched — + // backoff alone compounds across attempts, and a delivery can sit in + // the retry queue behind a backlog on top of that — so a timestamp + // captured once at dispatch time would arrive already outside the + // replay window and be rejected by the subscriber on arrival, breaking + // retries entirely. Re-reading the clock per attempt is what keeps a + // legitimately late retry legitimately signed (#97). + const timestamp = Date.now(); + const headers = buildHeaders(webhook.secret, body, delivery.event_type, delivery.id, delivery.request_id, timestamp); const seq = sequence ?? delivery.sequence; const headers = buildHeaders(webhook.secret, body, delivery.event_type, delivery.id, delivery.request_id, seq); diff --git a/src/services/webhookSignature.js b/src/services/webhookSignature.js index 867ab8c..024ab5e 100644 --- a/src/services/webhookSignature.js +++ b/src/services/webhookSignature.js @@ -1,36 +1,136 @@ 'use strict'; const crypto = require('crypto'); +const config = require('../config'); const SIGNATURE_PREFIX = 'sha256='; -function sign(secret, body) { +// Bumped from the unversioned v1 scheme (HMAC over the raw body alone) to v2 +// (HMAC over `${timestamp}.${body}`) by #97. v1 signatures carried nothing +// that expired, so a captured delivery stayed replayable forever. Sent on +// every delivery so subscribers can tell the two apart on the wire. +const SIGNATURE_VERSION = '2'; + +const SIGNATURE_HEADER = 'X-SmartDrop-Signature'; +const TIMESTAMP_HEADER = 'X-SmartDrop-Timestamp'; +const VERSION_HEADER = 'X-SmartDrop-Signature-Version'; + +function payloadBody(body) { + return typeof body === 'string' ? body : JSON.stringify(body); +} + +/** + * Parses a timestamp that arrived over the wire (so: almost certainly a + * string) into epoch milliseconds, or null when it is not a usable value. + * + * Deliberately stricter than `Number()`, which coerces a surprising number + * of junk values into something that looks like a valid instant: + * `Number('') === 0`, `Number([]) === 0`, `Number(null) === 0`, and + * `Number(true) === 1`. Each of those would sail through a plain + * `Number.isNaN` guard and then be compared against the replay window as if + * it were 1970, so they must be rejected by shape rather than by value. + */ +function parseTimestamp(value) { + if (typeof value !== 'string' && typeof value !== 'number') return null; + const digits = String(value).trim(); + // Digits only: rejects '', 'abc', '-1', '12.5', '1e3' and '0x10'. + if (!/^\d+$/.test(digits)) return null; + const millis = Number(digits); + return Number.isSafeInteger(millis) ? millis : null; +} + +/** + * Signs `body` for delivery at `timestamp`, returning the value of the + * X-SmartDrop-Signature header. + * + * The timestamp is inside the MAC, not merely alongside it: a captured + * delivery cannot be re-dated without invalidating the signature, which is + * what makes the replay window on the verify side meaningful. + */ +function sign(secret, body, timestamp = Date.now()) { if (typeof secret !== 'string' || secret.length === 0) { throw new Error('signature secret must be a non-empty string'); } - const payload = typeof body === 'string' ? body : JSON.stringify(body); - const digest = crypto.createHmac('sha256', secret).update(payload).digest('hex'); + const signedAt = parseTimestamp(timestamp); + if (signedAt === null) { + throw new Error('signature timestamp must be epoch milliseconds'); + } + const digest = crypto + .createHmac('sha256', secret) + .update(`${signedAt}.${payloadBody(body)}`) + .digest('hex'); return `${SIGNATURE_PREFIX}${digest}`; } -function verify(secret, body, providedSignature) { - if (typeof providedSignature !== 'string' || !providedSignature.startsWith(SIGNATURE_PREFIX)) { +/** + * Verifies a delivery signature against the timestamp it was signed with. + * + * Returns false rather than throwing for every rejection reason, so callers + * can treat it as a plain predicate. Rejects when: + * - the timestamp header is missing or not epoch milliseconds; + * - the timestamp is outside the replay window, measured symmetrically: + * a future-dated timestamp is as invalid as a stale one, otherwise an + * attacker could hand us a timestamp years ahead and hold a signature + * that never expires; + * - the recomputed MAC does not match, compared in constant time. + */ +function verify(secret, body, signatureHeader, timestampHeader, options = {}) { + const maxAgeSeconds = options.maxAgeSeconds ?? config.webhooks.signatureMaxAgeSeconds; + + if (typeof signatureHeader !== 'string' || !signatureHeader.startsWith(SIGNATURE_PREFIX)) { return false; } + + const signedAt = parseTimestamp(timestampHeader); + if (signedAt === null) return false; + if (Math.abs(Date.now() - signedAt) > maxAgeSeconds * 1000) return false; + let expected; try { - expected = sign(secret, body); + expected = sign(secret, body, signedAt); } catch { return false; } + const a = Buffer.from(expected); - const b = Buffer.from(providedSignature); + const b = Buffer.from(signatureHeader); if (a.length !== b.length) return false; return crypto.timingSafeEqual(a, b); } +/** + * Builds the three signature headers that every outgoing delivery carries. + * + * Resolving the timestamp once here — rather than letting the caller sign + * with one value and stamp the header from another — is what keeps the + * header and the MAC from ever drifting apart. Callers that need extra + * headers (event type, delivery id, …) spread this into their own set so + * there is exactly one place the signing scheme is defined. + */ +function signatureHeaders(secret, body, timestamp = Date.now()) { + const signedAt = parseTimestamp(timestamp); + if (signedAt === null) { + throw new Error('signature timestamp must be epoch milliseconds'); + } + return { + [SIGNATURE_HEADER]: sign(secret, body, signedAt), + [TIMESTAMP_HEADER]: String(signedAt), + [VERSION_HEADER]: SIGNATURE_VERSION, + }; +} + function generateSecret(bytes = 32) { return `whsec_${crypto.randomBytes(bytes).toString('hex')}`; } -module.exports = { sign, verify, generateSecret, SIGNATURE_PREFIX }; +module.exports = { + sign, + verify, + signatureHeaders, + generateSecret, + SIGNATURE_PREFIX, + SIGNATURE_VERSION, + SIGNATURE_HEADER, + TIMESTAMP_HEADER, + VERSION_HEADER, +}; diff --git a/test/webhookDispatcher.test.js b/test/webhookDispatcher.test.js index 6d193d6..6d5fe0f 100644 --- a/test/webhookDispatcher.test.js +++ b/test/webhookDispatcher.test.js @@ -56,7 +56,12 @@ describe('dispatcher delivery success', () => { expect(parsed.event).toBe('pool.assets_locked'); expect(parsed.event_id).toBe('evt_1'); expect(parsed.data).toEqual({ pool_id: 'p1' }); - expect(opts.headers['X-SmartDrop-Signature']).toBe(signature.sign(w.secret, body)); + // The signature is bound to the timestamp that shipped alongside it, so + // it can only be recomputed using that same header value. + const sentAt = opts.headers['X-SmartDrop-Timestamp']; + expect(sentAt).toMatch(/^\d+$/); + expect(opts.headers['X-SmartDrop-Signature']).toBe(signature.sign(w.secret, body, Number(sentAt))); + expect(opts.headers['X-SmartDrop-Signature-Version']).toBe('2'); expect(opts.headers['X-SmartDrop-Event']).toBe('pool.assets_locked'); }); }); @@ -521,3 +526,105 @@ describe('request id propagation into webhook deliveries (issue #250)', () => { expect(opts.headers['X-Request-Id']).toBeUndefined(); }); }); + +describe('dispatcher signature freshness across retries (#97)', () => { + const MAX_AGE_MS = 300 * 1000; + const T0 = 1_700_000_000_000; + + test('a retry delivered past the replay window signs fresh at attempt time', async () => { + // The regression this whole change turns on. Backoff can push a retry + // past WEBHOOK_SIGNATURE_MAX_AGE_SECONDS from the original dispatch. If + // the timestamp were computed once in dispatch() and reused, the retry + // would go out stamped with an instant that is already outside the + // window and the subscriber would reject it on arrival — a "fix" that + // silently breaks retries. + const clock = jest.spyOn(Date, 'now').mockReturnValue(T0); + try { + const w = await createWebhook(); + mockAxiosPost.mockResolvedValueOnce({ status: 503 }); + + const [{ delivery }] = await dispatcher.dispatch({ + event_type: 'pool.assets_locked', + event_id: 'evt_retry_freshness', + }); + expect(delivery.status).toBe('pending'); + + const [, firstBody, firstOpts] = mockAxiosPost.mock.calls[0]; + const firstTimestamp = firstOpts.headers['X-SmartDrop-Timestamp']; + expect(Number(firstTimestamp)).toBe(T0); + + // Jump to when the retry actually becomes due, then well past the + // replay window. next_retry_at is read back off the delivery record + // rather than recomputed from the backoff formula, because backoffMs + // applies equal jitter (delay lands anywhere in [det/2, det)) — a + // hardcoded delay would make this test flaky. + const dueAt = new Date(delivery.next_retry_at).getTime(); + const retryAt = Math.max(dueAt, T0 + MAX_AGE_MS) + 60_000; + expect(retryAt - T0).toBeGreaterThan(MAX_AGE_MS); + clock.mockReturnValue(retryAt); + + // Sanity-check that the window really did elapse: the signature from + // the first attempt is now stale. Without this the test could pass + // trivially by never crossing the boundary at all. + expect(signature.verify( + w.secret, firstBody, firstOpts.headers['X-SmartDrop-Signature'], firstTimestamp + )).toBe(false); + + mockAxiosPost.mockResolvedValueOnce({ status: 200 }); + const retried = await dispatcher.attempt(delivery.id); + expect(retried.status).toBe('success'); + expect(retried.attempts).toBe(2); + + const [, retryBody, retryOpts] = mockAxiosPost.mock.calls[1]; + const retryTimestamp = retryOpts.headers['X-SmartDrop-Timestamp']; + + // Fresh clock reading for this attempt, not the dispatch-time one. + expect(retryTimestamp).not.toBe(firstTimestamp); + expect(Number(retryTimestamp)).toBe(retryAt); + + // The payload is unchanged between attempts, so the timestamp is the + // only thing that moved — and the retry verifies as currently valid. + expect(retryBody).toBe(firstBody); + expect(signature.verify( + w.secret, retryBody, retryOpts.headers['X-SmartDrop-Signature'], retryTimestamp + )).toBe(true); + } finally { + clock.mockRestore(); + } + }); + + test('each target in one dispatch signs with its own timestamp', async () => { + // The other half of the requirement: no timestamp shared across the + // targets of a single dispatch either. The clock advances on every + // reading, so two targets cannot coincidentally agree. + let tick = 0; + const clock = jest.spyOn(Date, 'now').mockImplementation(() => T0 + (tick += 1)); + try { + const a = await createWebhook({ url: 'https://a.com' }); + const b = await createWebhook({ url: 'https://b.com' }); + mockAxiosPost.mockResolvedValue({ status: 200 }); + + await dispatcher.dispatch({ + event_type: 'pool.assets_locked', + event_id: 'evt_per_target_ts', + }); + + expect(mockAxiosPost).toHaveBeenCalledTimes(2); + const sent = mockAxiosPost.mock.calls.map(([url, body, opts]) => ({ + secret: url === 'https://a.com' ? a.secret : b.secret, + body, + signature: opts.headers['X-SmartDrop-Signature'], + timestamp: opts.headers['X-SmartDrop-Timestamp'], + })); + + expect(sent[0].timestamp).not.toBe(sent[1].timestamp); + for (const s of sent) { + expect(signature.verify(s.secret, s.body, s.signature, s.timestamp)).toBe(true); + } + // Each signature is bound to its own timestamp: swapping them fails. + expect(signature.verify(sent[0].secret, sent[0].body, sent[0].signature, sent[1].timestamp)).toBe(false); + } finally { + clock.mockRestore(); + } + }); +}); diff --git a/test/webhookSignature.test.js b/test/webhookSignature.test.js index a599f50..75cccfc 100644 --- a/test/webhookSignature.test.js +++ b/test/webhookSignature.test.js @@ -2,89 +2,245 @@ const http = require('http'); const signature = require('../src/services/webhookSignature'); -const { - buildSignatureHeaders, - sendSignedRequest, - signPayload, - verifySignature, -} = require('../src/services/webhook'); +const { buildSignatureHeaders, sendSignedRequest } = require('../src/services/webhook'); + +const MAX_AGE_MS = 300 * 1000; + +// A fixed instant to sign/verify against, so every freshness assertion is +// exact rather than "whatever the clock did between the two calls". +const NOW = 1_700_000_000_000; + +function freezeClock(at = NOW) { + return jest.spyOn(Date, 'now').mockReturnValue(at); +} describe('webhook signature', () => { const secret = 'whsec_test_supersecret_value'; const body = JSON.stringify({ event: 'pool.assets_locked', amount: 42 }); test('sign produces a sha256= prefixed hex string', () => { - const sig = signature.sign(secret, body); + const sig = signature.sign(secret, body, NOW); expect(sig).toMatch(/^sha256=[0-9a-f]{64}$/); }); - test('verify returns true for matching body and signature', () => { - const sig = signature.sign(secret, body); - expect(signature.verify(secret, body, sig)).toBe(true); + test('sign binds the timestamp into the MAC', () => { + // The whole point of #97: the same body signed a second apart must not + // produce the same signature, or the timestamp is decorative and a + // captured delivery can simply be re-dated. + expect(signature.sign(secret, body, NOW)).not.toBe(signature.sign(secret, body, NOW + 1000)); + }); + + test('verify returns true for a signature checked at the moment it was signed', () => { + const clock = freezeClock(); + try { + expect(signature.verify(secret, body, signature.sign(secret, body, NOW), NOW)).toBe(true); + } finally { + clock.mockRestore(); + } }); test('verify returns false when body is tampered', () => { - const sig = signature.sign(secret, body); - const tampered = body.replace('42', '43'); - expect(signature.verify(secret, tampered, sig)).toBe(false); + const clock = freezeClock(); + try { + const sig = signature.sign(secret, body, NOW); + expect(signature.verify(secret, body.replace('42', '43'), sig, NOW)).toBe(false); + } finally { + clock.mockRestore(); + } }); test('verify returns false when signature is tampered', () => { - const sig = signature.sign(secret, body); - const tampered = sig.replace(/.$/, sig.endsWith('a') ? 'b' : 'a'); - expect(signature.verify(secret, body, tampered)).toBe(false); + const clock = freezeClock(); + try { + const sig = signature.sign(secret, body, NOW); + const tampered = sig.replace(/.$/, sig.endsWith('a') ? 'b' : 'a'); + expect(signature.verify(secret, body, tampered, NOW)).toBe(false); + } finally { + clock.mockRestore(); + } }); test('verify returns false when signature lacks the prefix', () => { - const sig = signature.sign(secret, body).replace('sha256=', ''); - expect(signature.verify(secret, body, sig)).toBe(false); + const clock = freezeClock(); + try { + const sig = signature.sign(secret, body, NOW).replace('sha256=', ''); + expect(signature.verify(secret, body, sig, NOW)).toBe(false); + } finally { + clock.mockRestore(); + } + }); + + test('verify returns false for a wrong secret', () => { + const clock = freezeClock(); + try { + const sig = signature.sign(secret, body, NOW); + expect(signature.verify('other_secret_value', body, sig, NOW)).toBe(false); + } finally { + clock.mockRestore(); + } + }); + + test('verify returns false when the timestamp is re-dated after signing', () => { + // Presenting a valid signature alongside a different (still fresh) + // timestamp must fail — otherwise a captured delivery could be kept + // alive indefinitely by simply advancing the header. + const clock = freezeClock(); + try { + const sig = signature.sign(secret, body, NOW); + expect(signature.verify(secret, body, sig, NOW + 1000)).toBe(false); + } finally { + clock.mockRestore(); + } + }); + + test('verify returns false once the signature is older than the replay window', () => { + const signedAt = NOW; + const sig = signature.sign(secret, body, signedAt); + const clock = freezeClock(signedAt + MAX_AGE_MS + 1000); + try { + expect(signature.verify(secret, body, sig, signedAt)).toBe(false); + } finally { + clock.mockRestore(); + } + }); + + test('verify returns false for a timestamp dated into the future', () => { + // The skew check is symmetric. A one-directional `now - ts > maxAge` + // check would accept this, handing the holder a signature that never + // expires. + const signedAt = NOW + MAX_AGE_MS + 1000; + const sig = signature.sign(secret, body, signedAt); + const clock = freezeClock(NOW); + try { + expect(signature.verify(secret, body, sig, signedAt)).toBe(false); + } finally { + clock.mockRestore(); + } + }); + + test('the replay window is inclusive at its edge in both directions', () => { + // Pins the comparison as `> maxAge` rather than `>=`, and pins it + // symmetrically, so an off-by-one cannot silently widen or narrow the + // window in either direction. + const clock = freezeClock(); + try { + const at = (offset) => { + const sig = signature.sign(secret, body, NOW + offset); + return signature.verify(secret, body, sig, NOW + offset); + }; + expect(at(-MAX_AGE_MS)).toBe(true); + expect(at(-MAX_AGE_MS - 1)).toBe(false); + expect(at(MAX_AGE_MS)).toBe(true); + expect(at(MAX_AGE_MS + 1)).toBe(false); + } finally { + clock.mockRestore(); + } }); - test('verify returns false for empty/wrong secret', () => { - const sig = signature.sign(secret, body); - expect(signature.verify('other_secret_value', body, sig)).toBe(false); + test('verify rejects malformed timestamps without throwing', () => { + const clock = freezeClock(); + try { + const sig = signature.sign(secret, body, NOW); + // Everything here would survive a naive `Number()` + `isNaN` guard: + // '', null, [] and false all coerce to 0, and true coerces to 1. + const malformed = ['', ' ', 'abc', null, undefined, '-1', '12.5', '1e3', '0x10', [], {}, true, false, NaN, Infinity]; + for (const value of malformed) { + expect(signature.verify(secret, body, sig, value)).toBe(false); + } + } finally { + clock.mockRestore(); + } + }); + + test('verify rejects alternate numeric encodings of a valid instant', () => { + // Each of these coerces to exactly NOW under `Number()`, so loose + // parsing would accept them and recompute a matching MAC. A subscriber + // running the documented verifier builds `${rawHeader}.${body}` from the + // header *as received*, so it would compute a different MAC and reject. + // Requiring the timestamp to be the same digits we signed keeps our + // verifier and the published one from disagreeing. + const clock = freezeClock(); + try { + const sig = signature.sign(secret, body, NOW); + for (const encoding of ['1.7e12', '+1700000000000', '0x18BCFE56800', '1700000000000.0']) { + expect(Number(encoding)).toBe(NOW); + expect(signature.verify(secret, body, sig, encoding)).toBe(false); + } + } finally { + clock.mockRestore(); + } + }); + + test('verify honours an explicit maxAgeSeconds override', () => { + const signedAt = NOW; + const sig = signature.sign(secret, body, signedAt); + const clock = freezeClock(signedAt + 600 * 1000); + try { + expect(signature.verify(secret, body, sig, signedAt, { maxAgeSeconds: 300 })).toBe(false); + expect(signature.verify(secret, body, sig, signedAt, { maxAgeSeconds: 900 })).toBe(true); + } finally { + clock.mockRestore(); + } }); test('generateSecret produces a whsec_-prefixed token', () => { - const s = signature.generateSecret(); - expect(s).toMatch(/^whsec_[0-9a-f]{64}$/); + expect(signature.generateSecret()).toMatch(/^whsec_[0-9a-f]{64}$/); }); test('sign accepts objects by stringifying them', () => { const obj = { a: 1, b: 'two' }; - const sigFromObj = signature.sign(secret, obj); - const sigFromStr = signature.sign(secret, JSON.stringify(obj)); - expect(sigFromObj).toBe(sigFromStr); + expect(signature.sign(secret, obj, NOW)).toBe(signature.sign(secret, JSON.stringify(obj), NOW)); }); -}); -describe('webhook signatures', () => { - test('signs and verifies payloads with timestamped HMAC-SHA256', () => { - const payload = { event: 'airdrop.completed', airdrop_id: 'drop-1' }; - const timestamp = 1782345600000; - const signature = `sha256=${signPayload('whsec_testsecret', payload, timestamp)}`; + test('signatureHeaders emits all three headers from a single resolved timestamp', () => { + const headers = signature.signatureHeaders(secret, body, NOW); + + expect(headers['X-SmartDrop-Signature']).toBe(signature.sign(secret, body, NOW)); + expect(headers['X-SmartDrop-Timestamp']).toBe(String(NOW)); + expect(headers['X-SmartDrop-Signature-Version']).toBe('2'); + }); - expect(verifySignature('whsec_testsecret', payload, signature, timestamp)).toBe(true); - expect(verifySignature('wrong_secret', payload, signature, timestamp)).toBe(false); + test('signatureHeaders defaults to a timestamp its own signature agrees with', () => { + // Guards the mismatch this helper exists to prevent: signing with one + // clock reading and stamping the header from a second one. + const headers = signature.signatureHeaders(secret, body); + expect( + signature.verify(secret, body, headers['X-SmartDrop-Signature'], headers['X-SmartDrop-Timestamp']) + ).toBe(true); }); +}); - test('builds SmartDrop signature and timestamp headers', () => { - const headers = buildSignatureHeaders('whsec_testsecret', { event: 'ping' }, 1782345600000); +describe('webhook alert deliveries share the one signing scheme', () => { + const secret = 'whsec_testsecret'; - expect(headers['X-SmartDrop-Signature']).toMatch(/^sha256=[a-f0-9]{64}$/); - expect(headers['X-SmartDrop-Timestamp']).toBe('1782345600000'); + test('buildSignatureHeaders produces headers the canonical verifier accepts', () => { + // Cross-module agreement: the alert path (webhook.js) and the canonical + // module must not drift apart again. This fails if webhook.js ever + // regrows its own HMAC. + const payload = { event: 'airdrop.completed', airdrop_id: 'drop-1' }; + const headers = buildSignatureHeaders(secret, payload, NOW); + const clock = freezeClock(); + try { + expect(headers['Content-Type']).toBe('application/json'); + expect(headers['X-SmartDrop-Signature-Version']).toBe('2'); + expect( + signature.verify(secret, payload, headers['X-SmartDrop-Signature'], headers['X-SmartDrop-Timestamp']) + ).toBe(true); + expect( + signature.verify('wrong_secret', payload, headers['X-SmartDrop-Signature'], headers['X-SmartDrop-Timestamp']) + ).toBe(false); + } finally { + clock.mockRestore(); + } }); - test('mock HTTP server receives signed request', async () => { + test('mock HTTP server receives a signed request that verifies off the wire', async () => { let captured = null; const server = http.createServer((req, res) => { const chunks = []; req.on('data', (chunk) => chunks.push(chunk)); req.on('end', () => { - captured = { - headers: req.headers, - body: Buffer.concat(chunks).toString('utf8'), - }; + captured = { headers: req.headers, body: Buffer.concat(chunks).toString('utf8') }; res.statusCode = 204; res.end(); }); @@ -95,16 +251,15 @@ describe('webhook signatures', () => { try { const payload = { event: 'ping', timestamp: '2026-06-25T00:00:00.000Z' }; - const result = await sendSignedRequest( - `http://127.0.0.1:${port}/hook`, - 'whsec_testsecret', - payload - ); + const result = await sendSignedRequest(`http://127.0.0.1:${port}/hook`, 'whsec_testsecret', payload); expect(result).toMatchObject({ ok: true, status: 204 }); expect(captured.headers['x-smartdrop-signature']).toMatch(/^sha256=[a-f0-9]{64}$/); - expect(captured.headers['x-smartdrop-timestamp']).toBeDefined(); - expect(verifySignature( + expect(captured.headers['x-smartdrop-timestamp']).toMatch(/^\d+$/); + expect(captured.headers['x-smartdrop-signature-version']).toBe('2'); + // Verified against the RAW bytes that arrived, not a re-stringified + // object — the same thing a subscriber does. + expect(signature.verify( 'whsec_testsecret', captured.body, captured.headers['x-smartdrop-signature'], @@ -115,3 +270,77 @@ describe('webhook signatures', () => { } }); }); + +describe("the README's documented verifier", () => { + const secret = 'whsec_readme_example_secret'; + const rawBody = JSON.stringify({ event: 'pool.assets_locked', data: { pool_id: 'p1' } }); + + // Extracted from README.md rather than copied into this file: a copy would + // let the published snippet rot silently, which is the exact failure this + // test exists to prevent. Subscribers paste this code; it has to work. + function loadDocumentedVerifier() { + const readme = require('fs').readFileSync(require('path').join(__dirname, '..', 'README.md'), 'utf8'); + const section = readme.split('### Verifying the signature (Node.js)')[1]; + expect(section).toBeDefined(); + const snippet = section.match(/```js\n([\s\S]*?)```/); + expect(snippet).not.toBeNull(); + // eslint-disable-next-line no-new-func + return new Function('require', `${snippet[1]}\nreturn verifySmartDrop;`)(require); + } + + function requestFrom(headers, body) { + return { rawBody: body, header: (name) => headers[name] }; + } + + test('accepts a delivery signed by this codebase', () => { + const verifySmartDrop = loadDocumentedVerifier(); + const headers = signature.signatureHeaders(secret, rawBody); + + expect(verifySmartDrop(requestFrom(headers, rawBody), secret)).toBe(true); + }); + + test('rejects a tampered body, a wrong secret, and a re-dated timestamp', () => { + const verifySmartDrop = loadDocumentedVerifier(); + const headers = signature.signatureHeaders(secret, rawBody); + + expect(verifySmartDrop(requestFrom(headers, rawBody.replace('p1', 'p2')), secret)).toBe(false); + expect(verifySmartDrop(requestFrom(headers, rawBody), 'whsec_wrong')).toBe(false); + expect(verifySmartDrop( + requestFrom({ ...headers, 'X-SmartDrop-Timestamp': String(Number(headers['X-SmartDrop-Timestamp']) + 1) }, rawBody), + secret + )).toBe(false); + }); + + test('closes the replay window in both directions, like the implementation', () => { + const verifySmartDrop = loadDocumentedVerifier(); + const clock = freezeClock(); + try { + const check = (offset) => { + const headers = signature.signatureHeaders(secret, rawBody, NOW + offset); + const documented = verifySmartDrop(requestFrom(headers, rawBody), secret); + // The published verifier and this codebase must agree on every + // verdict, or subscribers reject deliveries we consider valid. + expect(documented).toBe( + signature.verify(secret, rawBody, headers['X-SmartDrop-Signature'], headers['X-SmartDrop-Timestamp']) + ); + return documented; + }; + expect(check(0)).toBe(true); + expect(check(-MAX_AGE_MS)).toBe(true); + expect(check(-MAX_AGE_MS - 1)).toBe(false); // stale + expect(check(MAX_AGE_MS)).toBe(true); + expect(check(MAX_AGE_MS + 1)).toBe(false); // future-dated + } finally { + clock.mockRestore(); + } + }); + + test('rejects malformed timestamps without throwing', () => { + const verifySmartDrop = loadDocumentedVerifier(); + const headers = signature.signatureHeaders(secret, rawBody); + + for (const value of ['', ' ', 'abc', undefined, '-1', '12.5', '1.7e12', '+1700000000000']) { + expect(verifySmartDrop(requestFrom({ ...headers, 'X-SmartDrop-Timestamp': value }, rawBody), secret)).toBe(false); + } + }); +});