fix(auth): stop a caller working through the user code space

A user code is eight characters from a twenty-five character alphabet,
which is a large space but a fixed one, and the endpoint that checked
them had no opinion about how often you asked. That is the guessing
attack RFC 8628 section 5.2 asks implementations to limit, and nothing
here did.

Wrong codes are now counted per caller address over a rolling window,
and the endpoint stops answering once the budget is gone. Getting a code
right is not charged for, so somebody who mistypes once and then succeeds
is not walking towards a lockout. A caller whose address cannot be
established shares one bucket with every other such caller, which makes
stripping the headers that say where you are buy a smaller budget rather
than an unlimited one.

The counter lives in the general-purpose store and is approximate. The
number that decides this is whether somebody is working through the code
space, and a handful either way does not change that answer.

packages/auth/src/issuer.ts

@@ -397,6 +397,31 @@ export interface IssuerInput<
 	 * store can make each transition a single operation.
 	 */
 	deviceStore?: DeviceStore;
+	/**
+	 * How hard a caller may guess at user codes before `/device` stops
+	 * answering them.
+	 *
+	 * A user code is short so that a person can read it off one screen and type
+	 * it into another, and short means guessable given enough tries. RFC 8628
+	 * §5.2 asks for a limit on the verification endpoint for exactly this
+	 * reason. Counted per caller address over a rolling window; a caller who
+	 * gets one right is not charged for it.
+	 */
+	deviceVerification?: {
+		/** Wrong codes allowed per window. @default 10 */
+		guessLimit?: number;
+		/** Length of the window, in seconds. @default 600 */
+		guessWindow?: number;
+		/**
+		 * Which caller a guess is charged to.
+		 *
+		 * Defaults to the usual forwarded-address headers. Returning undefined
+		 * puts the request in one shared bucket, which is the right answer for
+		 * a caller whose address cannot be established: it means stripping the
+		 * headers buys a smaller budget rather than an unlimited one.
+		 */
+		address?(req: Request): string | undefined;
+	};
 	/**
 	 * Whether a client may start a device authorization grant.
 	 *
@@ -529,6 +554,15 @@ export function issuer<
 	const ttlDevice = input.ttl?.device ?? 60 * 10;
 	const deviceInterval = input.ttl?.deviceInterval ?? 5;
 	const deviceStore = input.deviceStore ?? MemoryDeviceStore();
+	const deviceGuessLimit = input.deviceVerification?.guessLimit ?? 10;
+	const deviceGuessWindow = input.deviceVerification?.guessWindow ?? 600;
+	const deviceAddress =
+		input.deviceVerification?.address ??
+		((req: Request) =>
+			req.headers.get('cf-connecting-ip') ??
+			req.headers.get('x-forwarded-for')?.split(',')[0]?.trim() ??
+			req.headers.get('x-real-ip') ??
+			undefined);
 	if (input.theme) {
 		setTheme(input.theme);
 	}
@@ -769,6 +803,44 @@ export function issuer<
 		subject: DeviceGrantSubject;
 	}
 
+	/**
+	 * How many user codes this caller has got wrong lately.
+	 *
+	 * Kept in the general-purpose store rather than with the grants, because it
+	 * is a counter and not a grant, and because being approximate is fine here:
+	 * the number that matters is whether somebody is working through the code
+	 * space, and a handful either way does not change the answer. A caller
+	 * spread across several addresses gets a budget per address, which is what
+	 * makes the limit worth having rather than a way to lock one person out.
+	 */
+	async function chargeGuess(req: Request): Promise<boolean> {
+		const who = deviceAddress(req) ?? 'unknown';
+		const key = ['oauth:device:guess', who];
+		const now = Date.now();
+		const bucket = await Storage.get<{ count: number; resetAt: number }>(storage!, key);
+		const next =
+			bucket && bucket.resetAt > now
+				? { count: bucket.count + 1, resetAt: bucket.resetAt }
+				: { count: 1, resetAt: now + deviceGuessWindow * 1000 };
+		await Storage.set(
+			storage!,
+			key,
+			next,
+			Math.max(1, Math.ceil((next.resetAt - now) / 1000))
+		);
+		return next.count <= deviceGuessLimit;
+	}
+
+	async function guessesLeft(req: Request): Promise<boolean> {
+		const who = deviceAddress(req) ?? 'unknown';
+		const bucket = await Storage.get<{ count: number; resetAt: number }>(storage!, [
+			'oauth:device:guess',
+			who
+		]);
+		if (!bucket || bucket.resetAt <= Date.now()) return true;
+		return bucket.count < deviceGuessLimit;
+	}
+
 	/**
 	 * The code as stored, from the code as a person typed it.
 	 *
@@ -1390,8 +1462,16 @@ export function issuer<
 			);
 		}
 
+		if (!(await guessesLeft(c.req.raw))) {
+			return c.text('Too many codes tried. Wait a while and start again from the app.', 429);
+		}
+
 		const found = await deviceStore.byUserCode(canonicalUserCode(raw));
 		if (!found || found.status !== 'pending' || found.expires <= Date.now()) {
+			// Charged only when the code was wrong. Getting one right costs
+			// nothing, so a person mistyping once and then succeeding is not
+			// walking towards a lockout.
+			await chargeGuess(c.req.raw);
 			return c.text('That code is not valid any more. Ask the app for a new one.', 400);
 		}
 

packages/auth/test/device.test.ts

@@ -21,6 +21,7 @@ const auth = issuer({
 	subjects,
 	allow: async () => true,
 	allowDeviceClient: async (clientID) => clientID !== 'banned',
+	deviceVerification: { guessLimit: 3, guessWindow: 60 },
 	providers: {
 		dummy: {
 			type: 'dummy',
@@ -266,7 +267,11 @@ describe('approval', () => {
 	});
 
 	test('an unknown user code does not start a provider flow', async () => {
-		const response = await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZZ`);
+		// Its own address, so the budget it spends is its own — the shared
+		// bucket for callers with no address is asserted on further down.
+		const response = await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZZ`, {
+			headers: { 'cf-connecting-ip': '198.51.100.9' }
+		});
 		expect(response.status).toBe(400);
 	});
 
@@ -411,3 +416,73 @@ describe('when both halves move at once', () => {
 		expect([a, b].filter(Boolean)).toHaveLength(1);
 	});
 });
+
+/**
+ * Working through the code space, and what stops it.
+ *
+ * A user code is eight characters from an alphabet of twenty-five, so guessing
+ * one is not cheap — but it is a fixed cost, and the endpoint that checks them
+ * had no opinion about how often you asked. RFC 8628 §5.2 asks for one.
+ */
+describe('guessing at user codes', () => {
+	/** A caller with an address of its own, so budgets do not run together. */
+	function from(address: string) {
+		return (userCode: string) =>
+			auth.request(`${ORIGIN}/device?user_code=${encodeURIComponent(userCode)}`, {
+				headers: { 'cf-connecting-ip': address }
+			});
+	}
+
+	test('a caller runs out of tries', async () => {
+		const tries = from('198.51.100.1');
+
+		expect((await tries('ZZZZZZZZ')).status).toBe(400);
+		expect((await tries('ZZZZZZZY')).status).toBe(400);
+		expect((await tries('ZZZZZZZX')).status).toBe(400);
+		expect((await tries('ZZZZZZZW')).status).toBe(429);
+	});
+
+	test('one caller running out does not lock out another', async () => {
+		const noisy = from('198.51.100.2');
+		for (let i = 0; i < 4; i++) await noisy(`ZZZZZZZ${'ABCD'[i]}`);
+		expect((await noisy('ZZZZZZZZ')).status).toBe(429);
+
+		const grant = await started();
+		const quiet = await auth.request(
+			`${ORIGIN}/device?user_code=${encodeURIComponent(grant.user_code)}`,
+			{ headers: { 'cf-connecting-ip': '198.51.100.3' } }
+		);
+		expect(quiet.status).toBe(302);
+	});
+
+	test('getting one right is not charged for', async () => {
+		const address = '198.51.100.4';
+		const tries = from(address);
+		expect((await tries('ZZZZZZZZ')).status).toBe(400);
+		expect((await tries('ZZZZZZZY')).status).toBe(400);
+
+		// Two wrong out of a budget of three. A correct code in between must
+		// not be what tips the next wrong one over.
+		const grant = await started();
+		const right = await auth.request(
+			`${ORIGIN}/device?user_code=${encodeURIComponent(grant.user_code)}`,
+			{ headers: { 'cf-connecting-ip': address } }
+		);
+		expect(right.status).toBe(302);
+
+		expect((await tries('ZZZZZZZX')).status).toBe(400);
+		expect((await tries('ZZZZZZZW')).status).toBe(429);
+	});
+
+	// A caller who strips the headers that say where they are lands in one
+	// shared bucket. That is deliberate: it makes hiding cost a smaller budget
+	// rather than buying an unlimited one.
+	test('a caller with no address still has a budget', async () => {
+		for (let i = 0; i < 3; i++) {
+			expect((await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZ${'ABC'[i]}`)).status).toBe(
+				400
+			);
+		}
+		expect((await auth.request(`${ORIGIN}/device?user_code=ZZZZZZZD`)).status).toBe(429);
+	});
+});