From 3192d1a779523d99f4e8c2577df2376c4e27c768 Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Fri, 1 May 2026 09:04:28 -0400 Subject: [PATCH] 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 --- server/alerts/alert-server.ts | 38 +++----- server/package-lock.json | 156 ++++++++++++++++++++++++++++++ server/package.json | 2 + server/webrtc/signaling-server.ts | 46 ++++----- 4 files changed, 192 insertions(+), 50 deletions(-) diff --git a/server/alerts/alert-server.ts b/server/alerts/alert-server.ts index ad051da..1a3e350 100644 --- a/server/alerts/alert-server.ts +++ b/server/alerts/alert-server.ts @@ -2,6 +2,7 @@ import { WebSocketServer, WebSocket, Data } from 'ws'; import { randomBytes } from 'crypto'; import { IncomingMessage } from 'http'; import { EventEmitter } from 'events'; +import jwt from 'jsonwebtoken'; /** * WebSocket Alert Server for Real-Time Call Analysis @@ -77,32 +78,23 @@ const DEFAULT_CONFIG: AlertServerConfig = { shutdownTimeoutMs: 5000, }; -// ── JWT Helper (shared with signaling server) ──────────────────────────────── +// ── JWT Helper ─────────────────────────────────────────────────────────────── -function extractJwtFromQuery(url: string): string | null { - const match = url.match(/[?&]token=([^&]+)/); +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; -} - -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 crypto = require('crypto'); - const sigInput = `${parts[0]}.${parts[1]}`; - 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; } @@ -154,7 +146,7 @@ export class AlertServer extends EventEmitter { // JWT authentication if (this.config.enableAuth) { - const token = extractJwtFromQuery(info.req.url || '') || extractJwtFromHeader(info.req); + const token = extractJwt(info.req); if (!token) { cb(false, 401, 'Missing JWT token'); return; @@ -179,7 +171,7 @@ export class AlertServer extends EventEmitter { * 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 userId = payload?.sub || 'anonymous'; diff --git a/server/package-lock.json b/server/package-lock.json index e2e9bc4..93a4aad 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -5,10 +5,28 @@ "packages": { "": { "dependencies": { + "@types/jsonwebtoken": "^9.0.10", "@types/ws": "^8.18.1", + "jsonwebtoken": "^9.0.3", "ws": "^8.20.0" } }, + "node_modules/@types/jsonwebtoken": { + "version": "9.0.10", + "resolved": "https://registry.npmjs.org/@types/jsonwebtoken/-/jsonwebtoken-9.0.10.tgz", + "integrity": "sha512-asx5hIG9Qmf/1oStypjanR7iKTv0gXQ1Ov/jfrX6kS/EO0OFni8orbmGCn0672NHR3kXHwpAwR+B368ZGN/2rA==", + "license": "MIT", + "dependencies": { + "@types/ms": "*", + "@types/node": "*" + } + }, + "node_modules/@types/ms": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@types/ms/-/ms-2.1.0.tgz", + "integrity": "sha512-GsCCIZDE/p3i96vtEqx+7dBUGXrc7zeSK3wwPHIaRThS+9OhWIXRqzs4d6k1SVU8g91DrNRWxWUGhp5KXQb2VA==", + "license": "MIT" + }, "node_modules/@types/node": { "version": "25.6.0", "resolved": "https://registry.npmjs.org/@types/node/-/node-25.6.0.tgz", @@ -27,6 +45,144 @@ "@types/node": "*" } }, + "node_modules/buffer-equal-constant-time": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/buffer-equal-constant-time/-/buffer-equal-constant-time-1.0.1.tgz", + "integrity": "sha512-zRpUiDwd/xk6ADqPMATG8vc9VPrkck7T07OIx0gnjmJAnHnTVXNQG3vfvWNuiZIkwu9KrKdA1iJKfsfTVxE6NA==", + "license": "BSD-3-Clause" + }, + "node_modules/ecdsa-sig-formatter": { + "version": "1.0.11", + "resolved": "https://registry.npmjs.org/ecdsa-sig-formatter/-/ecdsa-sig-formatter-1.0.11.tgz", + "integrity": "sha512-nagl3RYrbNv6kQkeJIpt6NJZy8twLB/2vtz6yN9Z4vRKHN4/QZJIEbqohALSgwKdnksuY3k5Addp5lg8sVoVcQ==", + "license": "Apache-2.0", + "dependencies": { + "safe-buffer": "^5.0.1" + } + }, + "node_modules/jsonwebtoken": { + "version": "9.0.3", + "resolved": "https://registry.npmjs.org/jsonwebtoken/-/jsonwebtoken-9.0.3.tgz", + "integrity": "sha512-MT/xP0CrubFRNLNKvxJ2BYfy53Zkm++5bX9dtuPbqAeQpTVe0MQTFhao8+Cp//EmJp244xt6Drw/GVEGCUj40g==", + "license": "MIT", + "dependencies": { + "jws": "^4.0.1", + "lodash.includes": "^4.3.0", + "lodash.isboolean": "^3.0.3", + "lodash.isinteger": "^4.0.4", + "lodash.isnumber": "^3.0.3", + "lodash.isplainobject": "^4.0.6", + "lodash.isstring": "^4.0.1", + "lodash.once": "^4.0.0", + "ms": "^2.1.1", + "semver": "^7.5.4" + }, + "engines": { + "node": ">=12", + "npm": ">=6" + } + }, + "node_modules/jwa": { + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/jwa/-/jwa-2.0.1.tgz", + "integrity": "sha512-hRF04fqJIP8Abbkq5NKGN0Bbr3JxlQ+qhZufXVr0DvujKy93ZCbXZMHDL4EOtodSbCWxOqR8MS1tXA5hwqCXDg==", + "license": "MIT", + "dependencies": { + "buffer-equal-constant-time": "^1.0.1", + "ecdsa-sig-formatter": "1.0.11", + "safe-buffer": "^5.0.1" + } + }, + "node_modules/jws": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/jws/-/jws-4.0.1.tgz", + "integrity": "sha512-EKI/M/yqPncGUUh44xz0PxSidXFr/+r0pA70+gIYhjv+et7yxM+s29Y+VGDkovRofQem0fs7Uvf4+YmAdyRduA==", + "license": "MIT", + "dependencies": { + "jwa": "^2.0.1", + "safe-buffer": "^5.0.1" + } + }, + "node_modules/lodash.includes": { + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/lodash.includes/-/lodash.includes-4.3.0.tgz", + "integrity": "sha512-W3Bx6mdkRTGtlJISOvVD/lbqjTlPPUDTMnlXZFnVwi9NKJ6tiAk6LVdlhZMm17VZisqhKcgzpO5Wz91PCt5b0w==", + "license": "MIT" + }, + "node_modules/lodash.isboolean": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/lodash.isboolean/-/lodash.isboolean-3.0.3.tgz", + "integrity": "sha512-Bz5mupy2SVbPHURB98VAcw+aHh4vRV5IPNhILUCsOzRmsTmSQ17jIuqopAentWoehktxGd9e/hbIXq980/1QJg==", + "license": "MIT" + }, + "node_modules/lodash.isinteger": { + "version": "4.0.4", + "resolved": "https://registry.npmjs.org/lodash.isinteger/-/lodash.isinteger-4.0.4.tgz", + "integrity": "sha512-DBwtEWN2caHQ9/imiNeEA5ys1JoRtRfY3d7V9wkqtbycnAmTvRRmbHKDV4a0EYc678/dia0jrte4tjYwVBaZUA==", + "license": "MIT" + }, + "node_modules/lodash.isnumber": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/lodash.isnumber/-/lodash.isnumber-3.0.3.tgz", + "integrity": "sha512-QYqzpfwO3/CWf3XP+Z+tkQsfaLL/EnUlXWVkIk5FUPc4sBdTehEqZONuyRt2P67PXAk+NXmTBcc97zw9t1FQrw==", + "license": "MIT" + }, + "node_modules/lodash.isplainobject": { + "version": "4.0.6", + "resolved": "https://registry.npmjs.org/lodash.isplainobject/-/lodash.isplainobject-4.0.6.tgz", + "integrity": "sha512-oSXzaWypCMHkPC3NvBEaPHf0KsA5mvPrOPgQWDsbg8n7orZ290M0BmC/jgRZ4vcJ6DTAhjrsSYgdsW/F+MFOBA==", + "license": "MIT" + }, + "node_modules/lodash.isstring": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/lodash.isstring/-/lodash.isstring-4.0.1.tgz", + "integrity": "sha512-0wJxfxH1wgO3GrbuP+dTTk7op+6L41QCXbGINEmD+ny/G/eCqGzxyCsh7159S+mgDDcoarnBw6PC1PS5+wUGgw==", + "license": "MIT" + }, + "node_modules/lodash.once": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/lodash.once/-/lodash.once-4.1.1.tgz", + "integrity": "sha512-Sb487aTOCr9drQVL8pIxOzVhafOjZN9UU54hiN8PU3uAiSV7lx1yYNpbNmex2PK6dSJoNTSJUUswT651yww3Mg==", + "license": "MIT" + }, + "node_modules/ms": { + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", + "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", + "license": "MIT" + }, + "node_modules/safe-buffer": { + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.2.1.tgz", + "integrity": "sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "license": "MIT" + }, + "node_modules/semver": { + "version": "7.7.4", + "resolved": "https://registry.npmjs.org/semver/-/semver-7.7.4.tgz", + "integrity": "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA==", + "license": "ISC", + "bin": { + "semver": "bin/semver.js" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/undici-types": { "version": "7.19.2", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.19.2.tgz", diff --git a/server/package.json b/server/package.json index 7f96898..a0106d5 100644 --- a/server/package.json +++ b/server/package.json @@ -1,6 +1,8 @@ { "dependencies": { + "@types/jsonwebtoken": "^9.0.10", "@types/ws": "^8.18.1", + "jsonwebtoken": "^9.0.3", "ws": "^8.20.0" } } diff --git a/server/webrtc/signaling-server.ts b/server/webrtc/signaling-server.ts index 12d47fa..b25ceab 100644 --- a/server/webrtc/signaling-server.ts +++ b/server/webrtc/signaling-server.ts @@ -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 || '';