diff --git a/ApiCore-Backend/src/webhooks/webhook-delivery.ts b/ApiCore-Backend/src/webhooks/webhook-delivery.ts new file mode 100644 index 0000000..5e6d7a7 --- /dev/null +++ b/ApiCore-Backend/src/webhooks/webhook-delivery.ts @@ -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, + body: string, +): Promise { + 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); + }); +} diff --git a/ApiCore-Backend/src/webhooks/webhook-secret.ts b/ApiCore-Backend/src/webhooks/webhook-secret.ts new file mode 100644 index 0000000..9e3c2ec --- /dev/null +++ b/ApiCore-Backend/src/webhooks/webhook-secret.ts @@ -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; +} diff --git a/ApiCore-Backend/src/webhooks/webhook-url-guard.spec.ts b/ApiCore-Backend/src/webhooks/webhook-url-guard.spec.ts index 593c8d6..34593f6 100644 --- a/ApiCore-Backend/src/webhooks/webhook-url-guard.spec.ts +++ b/ApiCore-Backend/src/webhooks/webhook-url-guard.spec.ts @@ -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:pw@203.0.113.10/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(); + }); + }); }); diff --git a/ApiCore-Backend/src/webhooks/webhook-url-guard.ts b/ApiCore-Backend/src/webhooks/webhook-url-guard.ts index bb18783..228c8a0 100644 --- a/ApiCore-Backend/src/webhooks/webhook-url-guard.ts +++ b/ApiCore-Backend/src/webhooks/webhook-url-guard.ts @@ -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), + ); +} diff --git a/ApiCore-Backend/src/webhooks/webhooks.service.spec.ts b/ApiCore-Backend/src/webhooks/webhooks.service.spec.ts index 138fb7b..2899ade 100644 --- a/ApiCore-Backend/src/webhooks/webhooks.service.spec.ts +++ b/ApiCore-Backend/src/webhooks/webhooks.service.spec.ts @@ -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; +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; 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; 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', () => { diff --git a/ApiCore-Backend/src/webhooks/webhooks.service.ts b/ApiCore-Backend/src/webhooks/webhooks.service.ts index 32910c9..0732554 100644 --- a/ApiCore-Backend/src/webhooks/webhooks.service.ts +++ b/ApiCore-Backend/src/webhooks/webhooks.service.ts @@ -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 { 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 { 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 { 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( diff --git a/ApiCore-Backend/test/webhooks.e2e-spec.ts b/ApiCore-Backend/test/webhooks.e2e-spec.ts index fd1058d..cbb032c 100644 --- a/ApiCore-Backend/test/webhooks.e2e-spec.ts +++ b/ApiCore-Backend/test/webhooks.e2e-spec.ts @@ -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', () => { diff --git a/ApiCore-Frontend/src/app/(app)/settings/automation/create-webhook-dialog.tsx b/ApiCore-Frontend/src/app/(app)/settings/automation/create-webhook-dialog.tsx index 64282f5..ec77a71 100644 --- a/ApiCore-Frontend/src/app/(app)/settings/automation/create-webhook-dialog.tsx +++ b/ApiCore-Frontend/src/app/(app)/settings/automation/create-webhook-dialog.tsx @@ -91,6 +91,13 @@ export function CreateWebhookDialog() { X-ApiCore-Signature-Kopfzeile (HMAC-SHA256) jeder Auslieferung.

+

+ Signiert wird{" "} + Zeitstempel.Auslieferungs-ID.Body{" "} + aus X-ApiCore-Timestamp und{" "} + X-ApiCore-Delivery. Der Empfänger + sollte ältere als 5 Minuten und bereits gesehene IDs ablehnen. +

{createdSecret}