Fix JWT security issues in signaling and alert servers (FRE-4497)
- Replace custom JWT parser with jsonwebtoken library (timing-safe HMAC) - Prefer Authorization header over URL query for token extraction - Add jsonwebtoken + @types/jsonwebtoken to server dependencies Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { WebSocketServer, WebSocket, Data } from 'ws';
|
||||
import { randomBytes } from 'crypto';
|
||||
import { IncomingMessage } from 'http';
|
||||
import jwt from 'jsonwebtoken';
|
||||
|
||||
/**
|
||||
* WebRTC Signaling Server
|
||||
@@ -73,36 +74,27 @@ function validateMessage(raw: unknown): raw is SignalingMessage {
|
||||
return true;
|
||||
}
|
||||
|
||||
// ── JWT Helper (lightweight, no external dep) ────────────────────────────────
|
||||
// ── JWT Helper ───────────────────────────────────────────────────────────────
|
||||
|
||||
function extractJwtFromQuery(url: string): string | null {
|
||||
const match = url.match(/[?&]token=([^&]+)/);
|
||||
/**
|
||||
* Extract JWT from Authorization header (preferred) or URL query (fallback).
|
||||
* Header-first avoids token exposure in access logs.
|
||||
*/
|
||||
function extractJwt(req: IncomingMessage): string | null {
|
||||
const auth = req.headers['authorization'];
|
||||
if (auth?.startsWith('Bearer ')) return auth.slice(7);
|
||||
const match = req.url?.match(/[?&]token=([^&]+)/);
|
||||
return match ? decodeURIComponent(match[1]) : null;
|
||||
}
|
||||
|
||||
function extractJwtFromHeader(req: IncomingMessage): string | null {
|
||||
const auth = req.headers['authorization'];
|
||||
return auth?.startsWith('Bearer ') ? auth.slice(7) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimal JWT verification (HS256). In production, use jsonwebtoken.
|
||||
* Returns decoded payload or null on failure.
|
||||
*/
|
||||
function verifyJwt(token: string, secret: string): { sub: string; exp: number } | null {
|
||||
function verifyJwt(token: string, secret: string): { sub: string; exp?: number } | null {
|
||||
try {
|
||||
const parts = token.split('.');
|
||||
if (parts.length !== 3) return null;
|
||||
const header = JSON.parse(Buffer.from(parts[0], 'base64url').toString());
|
||||
if (header.alg !== 'HS256') return null;
|
||||
const payload = JSON.parse(Buffer.from(parts[1], 'base64url').toString());
|
||||
if (!payload.sub || typeof payload.sub !== 'string') return null;
|
||||
if (payload.exp && Date.now() / 1000 > payload.exp) return null;
|
||||
const sigInput = `${parts[0]}.${parts[1]}`;
|
||||
const crypto = require('crypto');
|
||||
const expected = crypto.createHmac('sha256', secret).update(sigInput).digest('base64url');
|
||||
if (expected !== parts[2]) return null;
|
||||
return { sub: payload.sub, exp: payload.exp || 0 };
|
||||
const decoded = jwt.verify(token, secret, { algorithms: ['HS256'] });
|
||||
if (typeof decoded !== 'object' || !decoded.sub) return null;
|
||||
return {
|
||||
sub: String(decoded.sub),
|
||||
exp: decoded.exp ? Number(decoded.exp) : undefined,
|
||||
};
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
@@ -145,7 +137,7 @@ export class SignalingServer {
|
||||
}
|
||||
|
||||
// JWT authentication
|
||||
const token = extractJwtFromQuery(info.req.url || '') || extractJwtFromHeader(info.req);
|
||||
const token = extractJwt(info.req);
|
||||
if (!token) {
|
||||
cb(false, 401, 'Missing JWT token');
|
||||
return;
|
||||
@@ -164,7 +156,7 @@ export class SignalingServer {
|
||||
* Handle new WebSocket connection
|
||||
*/
|
||||
private handleConnection(ws: WebSocket, req: IncomingMessage) {
|
||||
const token = extractJwtFromQuery(req.url || '') || extractJwtFromHeader(req);
|
||||
const token = extractJwt(req);
|
||||
const payload = token ? verifyJwt(token, this.config.jwtSecret) : null;
|
||||
const authenticatedUserId = payload?.sub || '';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user