fix(webhooks): close DNS rebinding, time out deliveries, sign with timestamp

- Deliveries connect through a lookup hook that checks the address actually dialled, so a host that resolves to a private address after registration (DNS rebinding) or a tampered row cannot reach internal services. Redirects are still not followed. - More blocked ranges (CGNAT/100.64/10 incl. cloud metadata, multicast, reserved, benchmarking, NAT64, 6to4, site-local, IPv4-mapped IPv6); only ports 80/443, no credentials in the URL, https in production. - 5 s timeout, response body discarded; dispatch no longer waits for receivers, so a hanging endpoint cannot stall the user's request. - Signature covers "timestamp.deliveryId.body" with X-ApiCore-Timestamp and X-ApiCore-Delivery headers, so receivers can reject replays. - Signing secrets are stored AES-256-GCM-encrypted; older plain rows still work. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
2026-10-01 02:17:40 +02:00
1 parent ebeb499a18
commit 79d20ee64e
@@ -0,0 +1,54 @@
+import * as http from 'node:http';
+import * as https from 'node:https';
+import { guardedLookup, isBlockedIp } from './webhook-url-guard';
+import * as net from 'node:net';
+
+/** A receiver gets this long to answer; a hanging one must not pile up. */
+export const WEBHOOK_TIMEOUT_MS = 5_000;
+
+export interface WebhookResponse {
+  status: number;
+}
+
+/**
+ * POSTs a webhook without following redirects, with a hard timeout, and
+ * through guardedLookup, so the connection can only ever go to a public
+ * address - whatever the host name resolves to by then. The response body
+ * is discarded unread.
+ */
+export function postWebhook(
+  rawUrl: string,
+  headers: Record<string, string>,
+  body: string,
+): Promise<WebhookResponse> {
+  return new Promise((resolve, reject) => {
+    const url = new URL(rawUrl);
+    const hostname = url.hostname.replace(/^\[|\]$/g, '');
+    if (net.isIP(hostname) && isBlockedIp(hostname)) {
+      reject(new Error(`Webhook target ${hostname} is a blocked address`));
+      return;
+    }
+    const client = url.protocol === 'https:' ? https : http;
+    const request = client.request(
+      url,
+      {
+        method: 'POST',
+        headers: {
+          ...headers,
+          'Content-Length': Buffer.byteLength(body).toString(),
+        },
+        lookup: guardedLookup as never,
+        timeout: WEBHOOK_TIMEOUT_MS,
+      },
+      (response) => {
+        response.resume();
+        resolve({ status: response.statusCode ?? 0 });
+      },
+    );
+    request.on('timeout', () =>
+      request.destroy(new Error('Webhook receiver did not answer in time')),
+    );
+    request.on('error', reject);
+    request.end(body);
+  });
+}
@@ -0,0 +1,19 @@
+import {
+  decryptMfaSecret,
+  encryptMfaSecret,
+} from '../auth/mfa/mfa-crypto.util';
+
+// Webhook signing secrets must stay reversible (the server signs with them),
+// so they are stored AES-256-GCM-encrypted like TOTP secrets. Rows written
+// before this carry the plain secret and no prefix - still readable.
+const PREFIX = 'enc:v1:';
+
+export function sealWebhookSecret(secret: string): string {
+  return `${PREFIX}${encryptMfaSecret(secret)}`;
+}
+
+export function openWebhookSecret(stored: string): string {
+  return stored.startsWith(PREFIX)
+    ? decryptMfaSecret(stored.slice(PREFIX.length))
+    : stored;
+}
@@ -1,5 +1,5 @@
 import { BadRequestException } from '@nestjs/common';
-import { assertPublicWebhookUrl } from './webhook-url-guard';
+import { assertPublicWebhookUrl, guardedLookup } from './webhook-url-guard';

 function lookupResolving(...addresses: string[]) {
  return () =>
@@ -124,4 +124,34 @@ describe('assertPublicWebhookUrl', () => {
      ),
    ).rejects.toBeInstanceOf(BadRequestException);
  });
+
+  it('rejects non-standard ports and embedded credentials', async () => {
+    await expect(
+      assertPublicWebhookUrl('https://203.0.113.10:8080/hook'),
+    ).rejects.toBeInstanceOf(BadRequestException);
+    await expect(
+      assertPublicWebhookUrl('https://user:[email protected]/hook'),
+    ).rejects.toBeInstanceOf(BadRequestException);
+  });
+
+  it('rejects CGNAT, IPv4-mapped and NAT64 targets', async () => {
+    for (const url of [
+      'http://100.100.100.200/latest/meta-data',
+      'http://[::ffff:127.0.0.1]/hook',
+      'http://[64:ff9b::a9fe:a9fe]/hook',
+    ]) {
+      await expect(assertPublicWebhookUrl(url)).rejects.toBeInstanceOf(
+        BadRequestException,
+      );
+    }
+  });
+});
+
+describe('guardedLookup', () => {
+  it('refuses to connect to a name that resolves to a private address', (done) => {
+    guardedLookup('localhost', { all: true }, (error) => {
+      expect(error?.code).toBe('EBLOCKEDADDRESS');
+      done();
+    });
+  });
 });
@@ -11,16 +11,33 @@ blockedRanges.addSubnet('192.168.0.0', 16, 'ipv4'); // RFC 1918
 blockedRanges.addSubnet('127.0.0.0', 8, 'ipv4'); // loopback
 blockedRanges.addSubnet('169.254.0.0', 16, 'ipv4'); // link-local, incl. cloud metadata
 blockedRanges.addSubnet('0.0.0.0', 8, 'ipv4'); // "this network"
+blockedRanges.addSubnet('100.64.0.0', 10, 'ipv4'); // CGNAT, incl. some cloud metadata
+blockedRanges.addSubnet('192.0.0.0', 24, 'ipv4'); // IETF protocol assignments
+blockedRanges.addSubnet('198.18.0.0', 15, 'ipv4'); // benchmarking
+blockedRanges.addSubnet('224.0.0.0', 4, 'ipv4'); // multicast
+blockedRanges.addSubnet('240.0.0.0', 4, 'ipv4'); // reserved + broadcast
 blockedRanges.addAddress('::1', 'ipv6'); // loopback
 blockedRanges.addAddress('::', 'ipv6'); // unspecified
 blockedRanges.addSubnet('fe80::', 10, 'ipv6'); // link-local
 blockedRanges.addSubnet('fc00::', 7, 'ipv6'); // unique local
+blockedRanges.addSubnet('fec0::', 10, 'ipv6'); // site-local (deprecated)
+blockedRanges.addSubnet('ff00::', 8, 'ipv6'); // multicast
+blockedRanges.addSubnet('64:ff9b::', 96, 'ipv6'); // NAT64 - embeds an IPv4 target
+blockedRanges.addSubnet('2002::', 16, 'ipv6'); // 6to4 - embeds an IPv4 target

-function isBlockedIp(ip: string): boolean {
+// Only the standard web ports: anything else is a port scan in disguise.
+const ALLOWED_PORTS = new Set(['', '80', '443']);
+
+export function isBlockedIp(ip: string): boolean {
  const version = net.isIP(ip);
  if (version === 4) return blockedRanges.check(ip, 'ipv4');
-  if (version === 6) return blockedRanges.check(ip, 'ipv6');
-  return false;
+  if (version === 6) {
+    // IPv4-mapped (::ffff:a.b.c.d) is checked against the IPv4 rules.
+    const mapped = /^::ffff:(\d+\.\d+\.\d+\.\d+)$/i.exec(ip);
+    if (mapped) return blockedRanges.check(mapped[1], 'ipv4');
+    return blockedRanges.check(ip, 'ipv6');
+  }
+  return true;
 }

 interface LookupAddress {
@@ -42,10 +59,9 @@ const defaultLookup: LookupFn = (hostname) =>
 * `net.BlockList`. Plain hostnames are resolved via DNS and every returned
 * address is checked too, so an attacker-controlled domain pointed at an
 * internal IP is rejected rather than silently passing the literal-only
- * check. This still does not defend against DNS rebinding (the same
- * hostname resolving to a different, private address only later, at actual
- * delivery time) — closing that would require re-resolving on every
- * delivery, which the caller intentionally does not do (see deliver()).
+ * check. DNS rebinding (the name resolving to a private address only
+ * later) is closed at delivery time: webhook-delivery.ts connects through
+ * guardedLookup, which checks the address actually dialled.
 */
 export async function assertPublicWebhookUrl(
  rawUrl: string,
@@ -61,6 +77,15 @@ export async function assertPublicWebhookUrl(
  if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') {
    throw new BadRequestException('Webhook URL must use http or https');
  }
+  if (process.env.NODE_ENV === 'production' && parsed.protocol !== 'https:') {
+    throw new BadRequestException('Webhook URL must use https');
+  }
+  if (!ALLOWED_PORTS.has(parsed.port)) {
+    throw new BadRequestException('Webhook URL must use port 80 or 443');
+  }
+  if (parsed.username || parsed.password) {
+    throw new BadRequestException('Webhook URL must not contain credentials');
+  }

  const hostname = parsed.hostname.toLowerCase().replace(/^\[|\]$/g, '');
  if (BLOCKED_HOSTNAMES.has(hostname)) {
@@ -91,3 +116,47 @@ export async function assertPublicWebhookUrl(
    );
  }
 }
+
+type LookupCallback = (
+  error: NodeJS.ErrnoException | null,
+  address: string | LookupAddress[],
+  family?: number,
+) => void;
+
+/**
+ * `lookup` for http(s).request: resolves like the default, then refuses to
+ * hand over any private, loopback or link-local address. Because it runs
+ * for the connection that is actually made, a name that turned private after
+ * registration (DNS rebinding) cannot reach internal hosts.
+ */
+export function guardedLookup(
+  hostname: string,
+  options: { all?: boolean; family?: number } | number,
+  callback: LookupCallback,
+): void {
+  const family = typeof options === 'number' ? options : options.family;
+  const wantsAll = typeof options === 'object' && options.all === true;
+  dns
+    .lookup(hostname, { all: true, family: family ?? 0 })
+    .then((addresses) => {
+      if (
+        addresses.length === 0 ||
+        addresses.some((record) => isBlockedIp(record.address))
+      ) {
+        const error: NodeJS.ErrnoException = new Error(
+          `Webhook host ${hostname} resolves to a blocked address`,
+        );
+        error.code = 'EBLOCKEDADDRESS';
+        callback(error, wantsAll ? [] : '', 0);
+        return;
+      }
+      if (wantsAll) {
+        callback(null, addresses);
+      } else {
+        callback(null, addresses[0].address, addresses[0].family);
+      }
+    })
+    .catch((error: NodeJS.ErrnoException) =>
+      callback(error, wantsAll ? [] : '', 0),
+    );
+}
@@ -3,6 +3,13 @@ import { BadRequestException, NotFoundException } from '@nestjs/common';
 import { WebhooksService } from './webhooks.service';
 import { PrismaService } from '../prisma/prisma.service';
 import { AuditLogService } from '../audit/audit-log.service';
+import { postWebhook } from './webhook-delivery';
+import { openWebhookSecret, sealWebhookSecret } from './webhook-secret';
+
+jest.mock('./webhook-delivery', () => ({ postWebhook: jest.fn() }));
+
+const postWebhookMock = postWebhook as jest.MockedFunction<typeof postWebhook>;
+const flushDeliveries = () => new Promise((resolve) => setImmediate(resolve));

 describe('WebhooksService', () => {
  let service: WebhooksService;
@@ -16,9 +23,11 @@ describe('WebhooksService', () => {
    };
  };
  let auditLog: { record: jest.Mock };
-  let fetchMock: jest.SpiedFunction<typeof fetch>;

  beforeEach(() => {
+    process.env.JWT_ACCESS_SECRET = 'unit-test-jwt-access-secret-0123456789';
+    postWebhookMock.mockReset();
+    postWebhookMock.mockResolvedValue({ status: 200 });
    prisma = {
      webhookSubscription: {
        findMany: jest.fn().mockResolvedValue([]),
@@ -33,9 +42,6 @@ describe('WebhooksService', () => {
      prisma as unknown as PrismaService,
      auditLog as unknown as AuditLogService,
    );
-    fetchMock = jest
-      .spyOn(global, 'fetch')
-      .mockResolvedValue(new Response(null, { status: 200 }));
  });

  it('create generates a random secret and stores it', async () => {
@@ -53,8 +59,20 @@ describe('WebhooksService', () => {

    expect(subscription.secret).toHaveLength(64);
    const [[createArg]] = prisma.webhookSubscription.create.mock.calls as [
-      [{ data: { url: string; events: string[]; createdByUserId: string } }],
+      [
+        {
+          data: {
+            url: string;
+            events: string[];
+            createdByUserId: string;
+            secret: string;
+          };
+        },
+      ],
    ];
+    // Stored encrypted, shown in plain only in this one response.
+    expect(createArg.data.secret).not.toContain(subscription.secret);
+    expect(openWebhookSecret(createArg.data.secret)).toBe(subscription.secret);
    expect(createArg.data.url).toBe('https://203.0.113.10/hook');
    expect(createArg.data.events).toEqual(['colony.created']);
    expect(createArg.data.createdByUserId).toBe('user-1');
@@ -75,33 +93,43 @@ describe('WebhooksService', () => {
    });
  });

-  it('dispatch signs the payload with HMAC-SHA256 and posts it with event headers', async () => {
+  it('dispatch signs timestamp, delivery id and payload with HMAC-SHA256', async () => {
    const subscription = {
      id: 'wh-1',
      url: 'https://example.test/hook',
-      secret: 'topsecret',
+      secret: sealWebhookSecret('topsecret'),
      active: true,
    };
    prisma.webhookSubscription.findMany.mockResolvedValueOnce([subscription]);

    await service.dispatch('colony.created', { id: 'colony-1' });
+    await flushDeliveries();

-    expect(fetchMock).toHaveBeenCalledTimes(1);
-    const [url, options] = fetchMock.mock.calls[0] as [
-      string,
-      { method: string; headers: Record<string, string>; body: string },
-    ];
+    expect(postWebhookMock).toHaveBeenCalledTimes(1);
+    const [url, headers, body] = postWebhookMock.mock.calls[0];
    expect(url).toBe('https://example.test/hook');
-    expect(options.method).toBe('POST');
-    expect(options.headers['X-ApiCore-Event']).toBe('colony.created');
+    expect(headers['X-ApiCore-Event']).toBe('colony.created');
+    expect(headers['X-ApiCore-Delivery']).toMatch(/^[0-9a-f-]{36}$/);
+    expect(Number(headers['X-ApiCore-Timestamp'])).toBeGreaterThan(0);

    const expectedSignature = crypto
-      .createHmac('sha256', subscription.secret)
-      .update(options.body)
+      .createHmac('sha256', 'topsecret')
+      .update(
+        `${headers['X-ApiCore-Timestamp']}.${headers['X-ApiCore-Delivery']}.${body}`,
+      )
      .digest('hex');
-    expect(options.headers['X-ApiCore-Signature']).toBe(
-      `sha256=${expectedSignature}`,
-    );
+    expect(headers['X-ApiCore-Signature']).toBe(`sha256=${expectedSignature}`);
+  });
+
+  it('dispatch returns before a slow receiver has answered', async () => {
+    prisma.webhookSubscription.findMany.mockResolvedValueOnce([
+      { id: 'wh-1', url: 'https://example.test/hook', secret: 's' },
+    ]);
+    postWebhookMock.mockReturnValueOnce(new Promise(() => undefined));
+
+    await expect(
+      service.dispatch('colony.created', { id: 'colony-1' }),
+    ).resolves.toBeUndefined();
  });

  it('dispatch does not throw when delivery fails', async () => {
@@ -113,11 +141,12 @@ describe('WebhooksService', () => {
        active: true,
      },
    ]);
-    fetchMock.mockRejectedValueOnce(new Error('network down'));
+    postWebhookMock.mockRejectedValueOnce(new Error('network down'));

    await expect(
      service.dispatch('colony.created', { id: 'colony-1' }),
    ).resolves.toBeUndefined();
+    await flushDeliveries();
  });

  describe('update', () => {
@@ -7,6 +7,8 @@ import { CreateWebhookDto } from './dto/create-webhook.dto';
 import { UpdateWebhookDto } from './dto/update-webhook.dto';
 import { WebhookEvent } from './webhook-events';
 import { assertPublicWebhookUrl } from './webhook-url-guard';
+import { postWebhook } from './webhook-delivery';
+import { openWebhookSecret, sealWebhookSecret } from './webhook-secret';

 const SECRET_BYTES = 32;

@@ -40,14 +42,16 @@ export class WebhooksService {
  ): Promise<WebhookSubscription> {
    await assertPublicWebhookUrl(dto.url);
    const secret = crypto.randomBytes(SECRET_BYTES).toString('hex');
-    const subscription = await this.prisma.webhookSubscription.create({
+    const stored = await this.prisma.webhookSubscription.create({
      data: {
        url: dto.url,
        events: dto.events,
-        secret,
+        secret: sealWebhookSecret(secret),
        createdByUserId: actingUserId,
      },
    });
+    // The plain secret leaves the server exactly once, in this response.
+    const subscription = { ...stored, secret };
    await this.auditLog.record({
      entityType: 'WebhookSubscription',
      entityId: subscription.id,
@@ -79,10 +83,14 @@ export class WebhooksService {
    if (dto.url) {
      await assertPublicWebhookUrl(dto.url);
    }
-    const subscription = await this.prisma.webhookSubscription.update({
+    const updated = await this.prisma.webhookSubscription.update({
      where: { id },
      data: { url: dto.url, events: dto.events, active: dto.active },
    });
+    const subscription = {
+      ...updated,
+      secret: openWebhookSecret(updated.secret),
+    };
    await this.auditLog.record({
      entityType: 'WebhookSubscription',
      entityId: id,
@@ -123,15 +131,18 @@ export class WebhooksService {
    return redacted;
  }

+  /**
+   * Starts delivery and returns without waiting for the receivers: a slow
+   * or hanging endpoint must never hold up the request that caused the
+   * event. Failures are logged by deliver().
+   */
  async dispatch(event: WebhookEvent, payload: unknown): Promise<void> {
    const subscriptions = await this.prisma.webhookSubscription.findMany({
      where: { active: true, events: { has: event } },
    });
-    await Promise.all(
-      subscriptions.map((subscription) =>
-        this.deliver(subscription, event, payload),
-      ),
-    );
+    for (const subscription of subscriptions) {
+      void this.deliver(subscription, event, payload);
+    }
  }

  private async deliver(
@@ -140,25 +151,30 @@ export class WebhooksService {
    payload: unknown,
  ): Promise<void> {
    const body = JSON.stringify({ event, data: payload });
-    const signature = crypto
-      .createHmac('sha256', subscription.secret)
-      .update(body)
-      .digest('hex');
+    // Signed together with a timestamp and a unique delivery id, so a
+    // receiver can reject a captured request replayed later
+    // ("t.id.body", the same idea as Stripe's signatures).
+    const deliveryId = crypto.randomUUID();
+    const timestamp = Math.floor(Date.now() / 1000).toString();
    try {
-      await fetch(subscription.url, {
-        method: 'POST',
-        headers: {
+      const signature = crypto
+        .createHmac('sha256', openWebhookSecret(subscription.secret))
+        .update(`${timestamp}.${deliveryId}.${body}`)
+        .digest('hex');
+      // No redirects, 5 s timeout, connect-time address check - see
+      // postWebhook().
+      await postWebhook(
+        subscription.url,
+        {
          'Content-Type': 'application/json',
+          'User-Agent': 'ApiCore-Webhooks/1',
          'X-ApiCore-Event': event,
+          'X-ApiCore-Delivery': deliveryId,
+          'X-ApiCore-Timestamp': timestamp,
          'X-ApiCore-Signature': `sha256=${signature}`,
        },
        body,
-        // A redirect target is unvalidated (assertPublicWebhookUrl only
-        // checked the registered URL), so it must not be auto-followed —
-        // otherwise a subscription pointed at an attacker-controlled public
-        // URL could redirect delivery into internal infrastructure.
-        redirect: 'manual',
-      });
+      );
    } catch (error: unknown) {
      const message = error instanceof Error ? error.message : String(error);
      this.logger.warn(
@@ -152,10 +152,11 @@ describe('Webhooks (e2e)', () => {
    expect(listed?.secret).toBeUndefined();
  });

-  it('delivers an HMAC-signed POST to the subscribed URL when task.created fires', async () => {
-    // Delivery is tested against a real local receiver, so the subscription
-    // is inserted directly (the create() SSRF guard blocks loopback URLs by
-    // design; the guard itself is covered separately above).
+  it('never delivers to a private address, even from a row that bypassed the create guard', async () => {
+    // Stands in for DNS rebinding or a tampered row: the stored URL points
+    // at a loopback receiver. The connection-time check must refuse it, and
+    // the request that fired the event must not wait for the receiver.
+    // Signing is covered by webhooks.service.spec.ts.
    const subscription = await prisma.webhookSubscription.create({
      data: {
        url: receiverUrl,
@@ -166,30 +167,14 @@ describe('Webhooks (e2e)', () => {
    });
    deliverySubscriptionId = subscription.id;

-    const taskResponse = await request(app.getHttpServer())
+    await request(app.getHttpServer())
      .post('/api/v1/tasks')
      .set('Authorization', `Bearer ${adminAccessToken}`)
      .send({ title: `Webhook-Test ${Date.now()}` })
      .expect(201);
+    await new Promise((resolve) => setTimeout(resolve, 300));

-    expect(receivedRequests).toHaveLength(1);
-    const delivery = receivedRequests[0];
-    expect(delivery.headers['x-apicore-event']).toBe('task.created');
-
-    const expectedSignature = crypto
-      .createHmac('sha256', subscription.secret)
-      .update(delivery.body)
-      .digest('hex');
-    expect(delivery.headers['x-apicore-signature']).toBe(
-      `sha256=${expectedSignature}`,
-    );
-
-    const payload = JSON.parse(delivery.body) as {
-      event: string;
-      data: { id: string };
-    };
-    expect(payload.event).toBe('task.created');
-    expect(payload.data.id).toBe((taskResponse.body as { id: string }).id);
+    expect(receivedRequests).toHaveLength(0);
  });

  it('lets an admin delete a webhook subscription', () => {
@@ -91,6 +91,13 @@ export function CreateWebhookDialog() {
              <code className="rounded bg-muted px-1 py-0.5">X-ApiCore-Signature</code>-Kopfzeile
              (HMAC-SHA256) jeder Auslieferung.
            </p>
+            <p className="text-sm text-muted-foreground">
+              Signiert wird{" "}
+              <code className="rounded bg-muted px-1 py-0.5">Zeitstempel.Auslieferungs-ID.Body</code>{" "}
+              aus <code className="rounded bg-muted px-1 py-0.5">X-ApiCore-Timestamp</code> und{" "}
+              <code className="rounded bg-muted px-1 py-0.5">X-ApiCore-Delivery</code>. Der Empfänger
+              sollte ältere als 5 Minuten und bereits gesehene IDs ablehnen.
+            </p>
            <div className="flex items-center gap-2 rounded-lg border bg-muted/50 p-3">
              <code className="flex-1 overflow-x-auto text-xs">{createdSecret}</code>
              <Button type="button" variant="outline" size="sm" onClick={() => void copySecret()}>