Update clear key logic, delete keys after prevRotationDate + KEY_LIFE…

…SPAN (48H), except last uploaded key

src/errors.ts

@@ -136,3 +136,10 @@ export class BadTokenKeyRequestedError extends HTTPError {
 		this.code = BadTokenKeyRequestedError.CODE;
 	}
 }
+
+export class PrevRotationTimeError extends Error {
+	constructor(message: string) {
+		super(message);
+		this.name = 'PrevRotationTimeError';
+	}
+}

src/index.ts

@@ -25,7 +25,7 @@ import { hexEncode } from './utils/hex';
 import { DIRECTORY_CACHE_REQUEST, clearDirectoryCache, getDirectoryCache } from './cache';
 const { BlindRSAMode, Issuer, TokenRequest } = publicVerif;
 
-import { shouldRotateKey } from './utils/keyRotation';
+import { getPrevRotationTime, shouldRotateKey, shouldClearKey } from './utils/keyRotation';
 
 const keyToTokenKeyID = async (key: Uint8Array): Promise<number> => {
 	const hash = await crypto.subtle.digest('SHA-256', key);
@@ -38,8 +38,6 @@ interface StorageMetadata extends Record<string, string> {
 	tokenKeyID: string;
 }
 
-const KEY_LIFESPAN = 48 * 60 * 60 * 1000;
-
 export const handleTokenRequest = async (ctx: Context, request: Request) => {
 	ctx.metrics.issuanceRequestTotal.inc({ version: ctx.env.VERSION_METADATA.id ?? RELEASE });
 	const contentType = request.headers.get('content-type');
@@ -231,31 +229,40 @@ export const handleRotateKey = async (ctx: Context, _request?: Request) => {
 
 const handleClearKey = async (ctx: Context, _request?: Request) => {
 	ctx.metrics.keyClearTotal.inc();
-
-	const now = Date.now();
+	const now = new Date();
 
 	const keys = await ctx.bucket.ISSUANCE_KEYS.list({ shouldUseCache: false });
 
+	if (keys.objects.length === 0) {
+		return new Response('No keys to clear', { status: 201 });
+	}
+
+	// iterating twice over keys, because we need to know the latest upload key time to enforce key clearance
+	// Find the latest key based on the upload time
 	let latestKey = keys.objects[0];
+	for (const key of keys.objects) {
+		if (new Date(key.uploaded).getTime() > new Date(latestKey.uploaded).getTime()) {
+			latestKey = key;
+		}
+	}
+
+	const effectivePrevRotationTime = getPrevRotationTime(new Date(latestKey.uploaded), ctx);
+
+	if (effectivePrevRotationTime == null) {
+		return new Response('Failed to determine previous rotation time', { status: 500 });
+	}
+
 	const toDelete: Set<string> = new Set();
 
 	for (const key of keys.objects) {
-		const keyUploadTime = new Date(key.uploaded).getTime();
-		const keyExpirationTime = keyUploadTime + KEY_LIFESPAN;
-
-		if (keyExpirationTime < now) {
+		const keyUploadTime = new Date(key.uploaded);
+		if (shouldClearKey(keyUploadTime, now, effectivePrevRotationTime)) {
 			toDelete.add(key.key);
 		}
-
-		if (latestKey.uploaded < key.uploaded) {
-			latestKey = key;
-		}
 	}
 
 	// Ensure the latest key is never deleted
-	if (toDelete.has(latestKey.key)) {
-		toDelete.delete(latestKey.key);
-	}
+	toDelete.delete(latestKey.key);
 
 	if (toDelete.size === 0) {
 		return new Response('No keys to clear', { status: 201 });

src/utils/keyRotation.ts

@@ -1,17 +1,45 @@
 import { Bindings } from '../bindings';
 import cronParser from 'cron-parser';
+import { Context } from '../context';
+import { PrevRotationTimeError } from '../errors';
+
+interface CronParseResult {
+	prevTime?: number;
+	nextTime?: number;
+	match: boolean;
+}
+
+const KEY_LIFESPAN = 48 * 60 * 60 * 1000;
+
+export function getPrevRotationTime(mostRecentKeyUploadTime: Date, ctx: Context): number {
+	let effectivePrevTime: number;
+	if (ctx.env.ROTATION_CRON_STRING) {
+		const { prevTime } = matchCronTime(ctx.env.ROTATION_CRON_STRING, mostRecentKeyUploadTime);
+
+		if (prevTime === undefined) {
+			console.error('Failed to determine previous rotation time for key');
+			throw new PrevRotationTimeError('Failed to determine previous rotation time');
+		}
+
+		effectivePrevTime = Math.max(prevTime, mostRecentKeyUploadTime.getTime());
+	} else {
+		effectivePrevTime = mostRecentKeyUploadTime.getTime();
+	}
+	return effectivePrevTime;
+}
 
 export function shouldRotateKey(date: Date, env: Bindings): boolean {
 	const utcDate = new Date(date.toISOString());
+	return env.ROTATION_CRON_STRING ? matchCronTime(env.ROTATION_CRON_STRING, utcDate).match : false;
+}
 
-	if (env.ROTATION_CRON_STRING) {
-		const result = matchCronTime(env.ROTATION_CRON_STRING, utcDate);
-		return result;
-	}
-	return false;
+export function shouldClearKey(keyUploadTime: Date, now: Date, effectivePrevTime: number): boolean {
+	const keyExpirationTime = keyUploadTime.getTime() + KEY_LIFESPAN;
+	const rotationBasedExpirationTime = effectivePrevTime + KEY_LIFESPAN;
+	return now.getTime() >= Math.max(keyExpirationTime, rotationBasedExpirationTime);
 }
 
-function matchCronTime(cronString: string, date: Date): boolean {
+export function matchCronTime(cronString: string, date: Date): CronParseResult {
 	// Set seconds and milliseconds to 0 to truncate to the nearest minute
 	// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/setUTCSeconds#parameters
 	date.setUTCSeconds(0, 0);
@@ -25,12 +53,18 @@ function matchCronTime(cronString: string, date: Date): boolean {
 	try {
 		interval = cronParser.parseExpression(cronString, options);
 	} catch (error) {
-		return false;
+		console.error('Error parsing cron string', error);
+		return { match: false };
 	}
 
 	const prevDate = interval.prev().toDate();
 	const nextDate = interval.next().toDate();
 
 	const result = date.getTime() === prevDate.getTime() || date.getTime() === nextDate.getTime();
-	return result;
+
+	return {
+		prevTime: prevDate.getTime(),
+		nextTime: nextDate.getTime(),
+		match: result,
+	};
 }

test/index.test.ts

@@ -17,7 +17,7 @@ import {
 	util,
 } from '@cloudflare/privacypass-ts';
 import { getDirectoryCache } from '../src/cache';
-import { shouldRotateKey } from '../src/utils/keyRotation';
+import { shouldRotateKey, shouldClearKey } from '../src/utils/keyRotation';
 const { TokenRequest } = publicVerif;
 
 const sampleURL = 'http://localhost';
@@ -356,3 +356,72 @@ describe('key rotation', () => {
 		expect(shouldRotateKey(date, ctx.env)).toBe(true);
 	});
 });
+
+describe('shouldClearKey Function', () => {
+	it('should not clear key when neither expiration time has passed', () => {
+		const keyUploadTime = new Date('2023-10-01T12:00:00Z');
+		const now = new Date('2023-10-02T11:59:59Z');
+		const effectivePrevTime = new Date('2023-09-30T00:00:00Z').getTime();
+
+		const result = shouldClearKey(keyUploadTime, now, effectivePrevTime);
+		expect(result).toBe(false);
+	});
+
+	it('should not clear key when per-key expiration has passed but rotation-based expiration has not', () => {
+		const keyUploadTime = new Date('2023-09-29T12:00:00Z');
+		const now = new Date('2023-10-01T12:00:01Z');
+		const effectivePrevTime = new Date('2023-10-03T00:00:00Z').getTime();
+
+		const result = shouldClearKey(keyUploadTime, now, effectivePrevTime);
+		expect(result).toBe(false);
+	});
+
+	it('should not clear key when rotation-based expiration has passed but per-key expiration has not', () => {
+		const keyUploadTime = new Date('2023-10-03T12:00:00Z');
+		const now = new Date('2023-10-05T11:59:59Z');
+		const effectivePrevTime = new Date('2023-10-01T00:00:00Z').getTime();
+
+		const result = shouldClearKey(keyUploadTime, now, effectivePrevTime);
+		expect(result).toBe(false);
+	});
+
+	it('should clear key when both expiration times have passed', () => {
+		const keyUploadTime = new Date('2023-09-29T12:00:00Z');
+		const now = new Date('2023-10-05T12:00:01Z');
+		const effectivePrevTime = new Date('2023-09-30T00:00:00Z').getTime();
+
+		const result = shouldClearKey(keyUploadTime, now, effectivePrevTime);
+		expect(result).toBe(true);
+	});
+
+	it('should clear key when current time equals the maximum expiration time', () => {
+		const keyUploadTime = new Date('2023-10-01T12:00:00Z');
+		const now = new Date('2023-10-03T12:00:00Z');
+		const effectivePrevTime = new Date('2023-10-01T12:00:00Z').getTime();
+
+		const result = shouldClearKey(keyUploadTime, now, effectivePrevTime);
+		expect(result).toBe(true);
+	});
+});
+
+describe('getPrevRotationTime Function', () => {
+	it.todo(
+		'should return the maximum of prevTime and mostRecentKeyUploadTime when ROTATION_CRON_STRING is defined'
+	);
+
+	it.todo('should return mostRecentKeyUploadTime when ROTATION_CRON_STRING is not defined');
+
+	it.todo('should return null when prevTime cannot be determined');
+
+	it.todo('should return mostRecentKeyUploadTime when prevTime is earlier than key upload time');
+});
+
+describe('handleClearKey Function', () => {
+	it.todo('should not clear any keys when none are expired');
+
+	it.todo('should clear expired keys and preserve the latest key');
+
+	it.todo('should handle failure to determine previous rotation time gracefully');
+
+	it.todo('should not delete keys if the latest key is expired but should be preserved');
+});