fix(webhooks): unify signing scheme with timestamp binding and replay-window enforcement

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).

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=<hex hmac of the raw body>` |
+| `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.

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

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,
 };

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);
 

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,
+};