fix(security): normalize IPv6 in rate-limit keyGenerator (ADR-0021)
express-rate-limit v8 raises ERR_ERL_KEY_GEN_IPV6 at boot because the custom keyGenerator returned req.ip verbatim. For IPv6, that lets an attacker rotate through the host bits of their own subnet (~2^72 keys on a typical /56 residential allocation) and escape per-IP rate limiting entirely — useful as a brute-force protection it isn't, then. Wrap req.ip through the library's ipKeyGenerator helper before keying, which truncates IPv6 addresses to their /56 prefix and is a no-op for IPv4. Test coverage for IPv4 / session / SKIP_PATHS unchanged; new case asserts two addresses in the same /56 share a bucket and that distinct /56s remain isolated. Surfaced by the ADR-0030 dockerised dev mode validation. The BFF kept booting (v8's default error handler logs and continues for this validation), so the bypass was live in dev until now.
This commit is contained in:
@@ -153,6 +153,32 @@ describe('createRateLimitMiddleware', () => {
|
||||
await mw(makeReq({ ip: '10.0.0.11', path: '/api/x' }), res, next);
|
||||
expect(res.status).not.toHaveBeenCalledWith(429);
|
||||
});
|
||||
|
||||
it('keys IPv6 addresses by their /56 prefix so per-host rotation cannot bypass the bucket', async () => {
|
||||
const mw = createRateLimitMiddleware({ perMinute: 1, authPerMinute: 99 });
|
||||
const next = jest.fn() as unknown as NextFunction;
|
||||
|
||||
// Two addresses inside `2001:db8:abcd:0000::/56` must share a
|
||||
// bucket — otherwise an attacker swaps the host suffix on every
|
||||
// retry and the per-IP limit never bites. This is the bypass the
|
||||
// lib's `ERR_ERL_KEY_GEN_IPV6` boot-time validation refuses to
|
||||
// ship. (The lib v8 default mask is `/56`, a typical residential
|
||||
// ISP customer allocation.)
|
||||
let res = makeRes();
|
||||
await mw(makeReq({ ip: '2001:db8:abcd::1', path: '/api/x' }), res, next);
|
||||
expect(res.status).not.toHaveBeenCalledWith(429);
|
||||
|
||||
res = makeRes();
|
||||
await mw(makeReq({ ip: '2001:db8:abcd::ffff', path: '/api/x' }), res, next);
|
||||
expect(res.status).toHaveBeenCalledWith(429);
|
||||
|
||||
// Different `/56` (`2001:db8:abce::/56`) — independent bucket.
|
||||
// Confirms the truncation does not collapse all IPv6 traffic into
|
||||
// one global bucket.
|
||||
res = makeRes();
|
||||
await mw(makeReq({ ip: '2001:db8:abce::1', path: '/api/x' }), res, next);
|
||||
expect(res.status).not.toHaveBeenCalledWith(429);
|
||||
});
|
||||
});
|
||||
|
||||
function restore(name: string, value: string | undefined): void {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { NextFunction, Request, RequestHandler, Response } from 'express';
|
||||
import rateLimit, { type Options } from 'express-rate-limit';
|
||||
import rateLimit, { ipKeyGenerator, type Options } from 'express-rate-limit';
|
||||
import { errorResponse } from './structured-error.filter';
|
||||
|
||||
/**
|
||||
@@ -79,7 +79,14 @@ export function createRateLimitMiddleware(config: RateLimitConfig): RequestHandl
|
||||
if (hasSession && sessionId) {
|
||||
return `s:${sessionId}`;
|
||||
}
|
||||
return `ip:${req.ip ?? 'unknown'}`;
|
||||
// `ipKeyGenerator` normalises the address before keying — most
|
||||
// importantly, it truncates IPv6 to its `/56` prefix (the lib v8
|
||||
// default — a typical residential ISP customer allocation) so an
|
||||
// attacker can't rotate through the trailing bits of their own
|
||||
// subnet to escape the per-IP bucket. The lib raises
|
||||
// `ERR_ERL_KEY_GEN_IPV6` at boot if a custom keyGenerator returns
|
||||
// `req.ip` verbatim, exactly to prevent that bypass.
|
||||
return `ip:${ipKeyGenerator(req.ip ?? 'unknown')}`;
|
||||
},
|
||||
handler: (_req: Request, res: Response, _next: NextFunction, _optionsUsed: Options) => {
|
||||
res.status(429).json(errorResponse('rate_limited', 'Too many requests'));
|
||||
|
||||
Reference in New Issue
Block a user