Skip to content

Commit 712422b

Browse files
vveerrggclaude
andcommitted
fix: JWT algorithm pinning, timing-safe comparisons, timestamp validation, challenge store
HIGH fixes: - Pin JWT to HS256 in both sign() and verify() (prevents algorithm confusion) - Constant-time API key comparison via crypto.timingSafeEqual (2 locations) - Event timestamp validation: reject >5min old or >60s future events - In-memory challenge store when Supabase unavailable (was completely bypassing challenge verification without it) - Browser fetches challenges from server instead of generating locally Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f3d4f73 commit 712422b

6 files changed

Lines changed: 108 additions & 11 deletions

File tree

src/browser/nostr-browser-auth.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,27 +26,42 @@ export interface NostrBrowserConfig {
2626
challengeTemplate?: string;
2727
/** Timeout in milliseconds for getPublicKey and signEvent operations */
2828
timeout?: number;
29+
/** Server URL for fetching challenges (e.g., 'https://auth.example.com') */
30+
serverUrl?: string;
2931
}
3032

3133
export class NostrBrowserAuth {
3234
private readonly kind: number;
3335
private readonly timeout: number;
3436
private readonly challengeTemplate: string;
37+
private readonly serverUrl: string;
3538

3639
constructor(config?: NostrBrowserConfig) {
3740
this.kind = config?.customKind || 22242;
3841
this.timeout = config?.timeout || 30000;
3942
this.challengeTemplate = config?.challengeTemplate || 'Sign this message to authenticate: %challenge%';
43+
this.serverUrl = config?.serverUrl || '';
4044
}
4145

4246
/**
43-
* Creates a challenge for authentication
47+
* Fetches a challenge from the server for authentication
48+
* @param {string} pubkey - The public key to request a challenge for
4449
* @returns {Promise<{challenge: string, timestamp: number}>}
50+
* @throws {Error} When the server URL is not configured or the request fails
4551
*/
46-
async createChallenge(): Promise<{ challenge: string; timestamp: number }> {
47-
const challenge = Array.from(crypto.getRandomValues(new Uint8Array(32)), b => b.toString(16).padStart(2, '0')).join('');
52+
async createChallenge(pubkey: string): Promise<{ challenge: string; timestamp: number }> {
53+
if (!this.serverUrl) {
54+
throw new Error('Server URL is required to fetch challenges. Set serverUrl in NostrBrowserConfig.');
55+
}
56+
57+
const response = await fetch(`${this.serverUrl}/challenge/${pubkey}`);
58+
if (!response.ok) {
59+
throw new Error('Failed to get challenge from server');
60+
}
61+
62+
const data = await response.json();
4863
const timestamp = Math.floor(Date.now() / 1000);
49-
return { challenge, timestamp };
64+
return { challenge: data.challenge, timestamp };
5065
}
5166

5267
/**
@@ -69,8 +84,8 @@ export class NostrBrowserAuth {
6984
// Step 1: Get public key and request read permission
7085
const pubkey = await window.nostr.getPublicKey();
7186

72-
// Step 2: Create a challenge
73-
const { challenge, timestamp } = await this.createChallenge();
87+
// Step 2: Fetch a challenge from the server
88+
const { challenge, timestamp } = await this.createChallenge(pubkey);
7489

7590
// Step 3: Request signature permission by asking user to sign the challenge
7691
const event = {

src/middleware/security.middleware.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Request, Response, NextFunction } from 'express';
22
import rateLimit from 'express-rate-limit';
3+
import crypto from 'crypto';
34
import { createLogger } from '../utils/logger.js';
45

56
const logger = createLogger('SecurityMiddleware');
@@ -9,7 +10,22 @@ export const validateApiKey = (req: Request, res: Response, next: NextFunction)
910
const apiKey = req.header('X-API-Key');
1011
const validApiKeys = process.env.API_KEYS?.split(',') || [];
1112

12-
if (!apiKey || !validApiKeys.includes(apiKey)) {
13+
if (!apiKey) {
14+
logger.warn(`Missing API key from IP: ${req.ip}`);
15+
return res.status(401).json({ error: 'Invalid API key' });
16+
}
17+
18+
const incomingHash = crypto.createHash('sha256').update(apiKey).digest();
19+
const isValid = validApiKeys.some(validKey => {
20+
const validHash = crypto.createHash('sha256').update(validKey).digest();
21+
try {
22+
return crypto.timingSafeEqual(incomingHash, validHash);
23+
} catch {
24+
return false;
25+
}
26+
});
27+
28+
if (!isValid) {
1329
logger.warn(`Invalid API key attempt from IP: ${req.ip}`);
1430
return res.status(401).json({ error: 'Invalid API key' });
1531
}

src/services/nostr.service.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ type SupabaseChallenge = {
2525
export class NostrService {
2626
private readonly config: NostrAuthConfig;
2727
private readonly supabase?: SupabaseClient;
28+
private challengeStore: Map<string, { challenge: string; pubkey: string; createdAt: number }> = new Map();
29+
private cleanupInterval?: ReturnType<typeof setInterval>;
2830

2931
constructor(config: NostrAuthConfig) {
3032
// Set default values for required properties
@@ -42,6 +44,25 @@ export class NostrService {
4244
if (config.supabaseUrl && config.supabaseKey) {
4345
this.supabase = createClient(config.supabaseUrl, config.supabaseKey);
4446
}
47+
48+
// Periodically clean up expired challenges from in-memory store (every 60 seconds)
49+
this.cleanupInterval = setInterval(() => {
50+
const now = Date.now();
51+
for (const [id, entry] of this.challengeStore) {
52+
if (now - entry.createdAt > this.config.eventTimeoutMs) {
53+
this.challengeStore.delete(id);
54+
}
55+
}
56+
}, 60000);
57+
}
58+
59+
/**
60+
* Stops the periodic cleanup interval (for graceful shutdown)
61+
*/
62+
destroy(): void {
63+
if (this.cleanupInterval) {
64+
clearInterval(this.cleanupInterval);
65+
}
4566
}
4667

4768
/**
@@ -67,6 +88,13 @@ export class NostrService {
6788
} catch (error) {
6889
logger.error('Failed to store challenge:', error);
6990
}
91+
} else {
92+
// Store in in-memory challenge store as fallback
93+
this.challengeStore.set(challenge.id, {
94+
challenge: challenge.challenge,
95+
pubkey,
96+
createdAt: Date.now()
97+
});
7098
}
7199

72100
return challenge.challenge;
@@ -106,6 +134,28 @@ export class NostrService {
106134
.from('challenges')
107135
.delete()
108136
.eq('id', data.id);
137+
} else {
138+
// Verify against in-memory challenge store
139+
let matchedId: string | null = null;
140+
141+
for (const [id, entry] of this.challengeStore) {
142+
if (entry.challenge === event.content && entry.pubkey === event.pubkey) {
143+
// Check if challenge has expired (5 min TTL)
144+
if (Date.now() - entry.createdAt > this.config.eventTimeoutMs) {
145+
this.challengeStore.delete(id);
146+
return { success: false, error: 'Challenge expired' };
147+
}
148+
matchedId = id;
149+
break;
150+
}
151+
}
152+
153+
if (!matchedId) {
154+
return { success: false, error: 'Challenge not found' };
155+
}
156+
157+
// Delete used challenge (single-use)
158+
this.challengeStore.delete(matchedId);
109159
}
110160

111161
return {

src/utils/api-key.utils.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* @security This module is critical for API security. Handle keys with care.
66
*/
77

8-
import { createHash, randomBytes } from 'crypto';
8+
import { createHash, randomBytes, timingSafeEqual } from 'crypto';
99
import { createLogger } from './logger.js';
1010

1111
const logger = createLogger('APIKeyUtils');
@@ -80,7 +80,11 @@ export function hashApiKey(apiKey: string): string {
8080
export function verifyApiKey(apiKey: string, hashedApiKey: string): boolean {
8181
try {
8282
const hash = hashApiKey(apiKey);
83-
return hash === hashedApiKey;
83+
try {
84+
return timingSafeEqual(Buffer.from(hash, 'hex'), Buffer.from(hashedApiKey, 'hex'));
85+
} catch {
86+
return false;
87+
}
8488
} catch (error) {
8589
logger.error('Error verifying API key:', { error: error instanceof Error ? error.message : String(error) });
8690
return false;

src/utils/jwt.utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ type JWTExpiresIn = `${number}h` | `${number}m` | `${number}s` | `${number}d`;
3131
*/
3232
export function generateJWT(pubkey: string, secret: string, expiresIn: JWTExpiresIn): string {
3333
try {
34-
return jwt.sign({ pubkey }, secret, { expiresIn });
34+
return jwt.sign({ pubkey }, secret, { expiresIn, algorithm: 'HS256' });
3535
} catch (error) {
3636
logger.error('Error generating JWT:', error);
3737
throw error;
@@ -54,7 +54,7 @@ export function generateJWT(pubkey: string, secret: string, expiresIn: JWTExpire
5454
*/
5555
export function verifyJWT(token: string, secret: string): { pubkey: string } {
5656
try {
57-
const decoded = jwt.verify(token, secret) as { pubkey: string };
57+
const decoded = jwt.verify(token, secret, { algorithms: ['HS256'] }) as { pubkey: string };
5858
return decoded;
5959
} catch (error) {
6060
logger.error('Error verifying JWT:', { error: error instanceof Error ? error.message : String(error) });

src/validators/event.validator.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,18 @@ export async function validateEvent(event: NostrEvent): Promise<VerificationResu
4848
return { success: false, error: 'Invalid signature' };
4949
}
5050

51+
// Validate timestamp to prevent replay attacks
52+
const now = Math.floor(Date.now() / 1000);
53+
if (!event.created_at || typeof event.created_at !== 'number') {
54+
return { success: false, error: 'Missing or invalid created_at timestamp' };
55+
}
56+
if (event.created_at < now - 300) {
57+
return { success: false, error: 'Event timestamp too old' };
58+
}
59+
if (event.created_at > now + 60) {
60+
return { success: false, error: 'Event timestamp too far in the future' };
61+
}
62+
5163
return { success: true, pubkey: event.pubkey };
5264
} catch (error) {
5365
logger.error('Event validation error:', { error: error instanceof Error ? error.message : String(error) });

0 commit comments

Comments
 (0)