security(relay): fix lockout DoS and DOM XSS in web client
Addresses two Major findings from PR review: - Auth lockout no longer griefable: the throttle key is namespaced per path and includes the client IP (web:<session>:<ip> vs desktop:<session>:<ip>), so a flood of bad web-client auths can neither lock other clients of the same session nor block the desktop from (re)authenticating. Per-socket failure close and brute-force lockout are preserved. - Relay web client XSS eliminated: app.html tab/macro rendering rebuilt with createElement/textContent (no macro/tab value reaches innerHTML); inline scripts/handlers externalized to /static/app.js and /static/login.js so the helmet CSP now uses script-src 'self' (dropped 'unsafe-inline' for scripts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -37,7 +37,8 @@ type DesktopMessage = AuthMessage | ApiResponseMessage | WsBroadcastMessage | Po
|
||||
export function handleDesktopConnection(
|
||||
socket: WebSocket,
|
||||
connectionManager: ConnectionManager,
|
||||
sessionManager: SessionManager
|
||||
sessionManager: SessionManager,
|
||||
clientIp: string
|
||||
): void {
|
||||
let authenticatedSessionId: string | null = null;
|
||||
// Track failed auth attempts on this individual socket.
|
||||
@@ -49,7 +50,7 @@ export function handleDesktopConnection(
|
||||
|
||||
switch (message.type) {
|
||||
case 'auth': {
|
||||
const failed = await handleAuth(socket, message, sessionManager, connectionManager, (sessionId) => {
|
||||
const failed = await handleAuth(socket, message, sessionManager, connectionManager, clientIp, (sessionId) => {
|
||||
authenticatedSessionId = sessionId;
|
||||
});
|
||||
if (failed) {
|
||||
@@ -110,6 +111,7 @@ async function handleAuth(
|
||||
message: AuthMessage,
|
||||
sessionManager: SessionManager,
|
||||
connectionManager: ConnectionManager,
|
||||
clientIp: string,
|
||||
setSessionId: (id: string) => void
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
@@ -117,8 +119,14 @@ async function handleAuth(
|
||||
let session;
|
||||
|
||||
if (sessionId) {
|
||||
// Reject early if this session id is currently locked out.
|
||||
const lockRemaining = getLockoutRemaining(sessionId);
|
||||
// Namespace the brute-force lockout key to the desktop keyspace and
|
||||
// include the client IP. This keeps the desktop's lockout state entirely
|
||||
// separate from the web-client keyspace, so a web-auth flood can never
|
||||
// lock the desktop out of (re)authenticating.
|
||||
const throttleKey = `desktop:${sessionId}:${clientIp}`;
|
||||
|
||||
// Reject early if this key is currently locked out.
|
||||
const lockRemaining = getLockoutRemaining(throttleKey);
|
||||
if (lockRemaining > 0) {
|
||||
socket.send(JSON.stringify({
|
||||
type: 'auth_response',
|
||||
@@ -132,7 +140,7 @@ async function handleAuth(
|
||||
// Validate existing session
|
||||
const valid = await sessionManager.validatePassword(sessionId, message.password);
|
||||
if (!valid) {
|
||||
recordFailure(sessionId);
|
||||
recordFailure(throttleKey);
|
||||
socket.send(JSON.stringify({
|
||||
type: 'auth_response',
|
||||
success: false,
|
||||
@@ -140,7 +148,7 @@ async function handleAuth(
|
||||
}));
|
||||
return true;
|
||||
}
|
||||
recordSuccess(sessionId);
|
||||
recordSuccess(throttleKey);
|
||||
session = sessionManager.getSession(sessionId);
|
||||
} else {
|
||||
// Create new session (may throw on limit / short password)
|
||||
|
||||
@@ -25,8 +25,13 @@ export function handleWebClientConnection(
|
||||
socket: WebSocket,
|
||||
sessionId: string,
|
||||
connectionManager: ConnectionManager,
|
||||
sessionManager: SessionManager
|
||||
sessionManager: SessionManager,
|
||||
clientIp: string
|
||||
): void {
|
||||
// Namespace the brute-force lockout key to the web keyspace and include the
|
||||
// client IP. This prevents one abusive IP (which knows the shareable session
|
||||
// id) from locking out other web clients or the desktop for this session.
|
||||
const throttleKey = `web:${sessionId}:${clientIp}`;
|
||||
// Check if session exists
|
||||
const session = sessionManager.getSession(sessionId);
|
||||
if (!session) {
|
||||
@@ -62,7 +67,7 @@ export function handleWebClientConnection(
|
||||
|
||||
switch (message.type) {
|
||||
case 'auth': {
|
||||
const failed = await handleAuth(socket, client, message, sessionId, sessionManager);
|
||||
const failed = await handleAuth(socket, client, message, sessionId, sessionManager, throttleKey);
|
||||
if (failed) {
|
||||
socketAuthFailures++;
|
||||
if (socketAuthFailures >= config.authMaxSocketFailures) {
|
||||
@@ -106,10 +111,11 @@ async function handleAuth(
|
||||
client: WebClientConnection,
|
||||
message: AuthMessage,
|
||||
sessionId: string,
|
||||
sessionManager: SessionManager
|
||||
sessionManager: SessionManager,
|
||||
throttleKey: string
|
||||
): Promise<boolean> {
|
||||
// Reject early if this session id is currently locked out.
|
||||
const lockRemaining = getLockoutRemaining(sessionId);
|
||||
// Reject early if this key is currently locked out.
|
||||
const lockRemaining = getLockoutRemaining(throttleKey);
|
||||
if (lockRemaining > 0) {
|
||||
socket.send(JSON.stringify({
|
||||
type: 'auth_response',
|
||||
@@ -123,7 +129,7 @@ async function handleAuth(
|
||||
const valid = await sessionManager.validatePassword(sessionId, message.password);
|
||||
|
||||
if (valid) {
|
||||
recordSuccess(sessionId);
|
||||
recordSuccess(throttleKey);
|
||||
client.authenticated = true;
|
||||
socket.send(JSON.stringify({
|
||||
type: 'auth_response',
|
||||
@@ -132,7 +138,7 @@ async function handleAuth(
|
||||
logger.debug(`Web client authenticated for session: ${sessionId}`);
|
||||
return false;
|
||||
} else {
|
||||
recordFailure(sessionId);
|
||||
recordFailure(throttleKey);
|
||||
socket.send(JSON.stringify({
|
||||
type: 'auth_response',
|
||||
success: false,
|
||||
|
||||
@@ -89,12 +89,15 @@ export function createServer() {
|
||||
|
||||
// Middleware
|
||||
app.use(helmet({
|
||||
// The bundled login/app pages rely on inline <script> and inline event
|
||||
// handlers, so script/style inline is permitted; images may be blobs.
|
||||
// Page scripts are served as external files (/static/app.js,
|
||||
// /static/login.js) with no inline handlers, so scripts are restricted to
|
||||
// 'self' (no 'unsafe-inline'). The pages still use inline <style> blocks,
|
||||
// so style-src keeps 'unsafe-inline'. Images may be data:/blob: (macro
|
||||
// images are loaded as blob object URLs) and WS connections need ws:/wss:.
|
||||
contentSecurityPolicy: {
|
||||
directives: {
|
||||
defaultSrc: ["'self'"],
|
||||
scriptSrc: ["'self'", "'unsafe-inline'"],
|
||||
scriptSrc: ["'self'"],
|
||||
styleSrc: ["'self'", "'unsafe-inline'"],
|
||||
imgSrc: ["'self'", 'data:', 'blob:'],
|
||||
connectSrc: ["'self'", 'ws:', 'wss:'],
|
||||
@@ -192,7 +195,7 @@ export function createServer() {
|
||||
// Desktop connection: /desktop
|
||||
if (pathname === '/desktop') {
|
||||
wss.handleUpgrade(request, socket, head, (ws) => {
|
||||
handleDesktopConnection(ws, connectionManager, sessionManager);
|
||||
handleDesktopConnection(ws, connectionManager, sessionManager, ip);
|
||||
});
|
||||
return;
|
||||
}
|
||||
@@ -202,7 +205,7 @@ export function createServer() {
|
||||
if (webClientMatch) {
|
||||
const sessionId = webClientMatch[1];
|
||||
wss.handleUpgrade(request, socket, head, (ws) => {
|
||||
handleWebClientConnection(ws, sessionId, connectionManager, sessionManager);
|
||||
handleWebClientConnection(ws, sessionId, connectionManager, sessionManager, ip);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
// Per-session-id auth brute-force lockout (in-memory).
|
||||
// Auth brute-force lockout (in-memory).
|
||||
//
|
||||
// Tracks failed authentication attempts keyed by session id. After
|
||||
// `authLockoutMaxAttempts` failures within `authLockoutWindowMs`, further
|
||||
// attempts are rejected until `authLockoutDurationMs` has elapsed.
|
||||
// Tracks failed authentication attempts keyed by an opaque, namespaced key
|
||||
// (e.g. `web:${sessionId}:${clientIp}` or `desktop:${sessionId}:${clientIp}`).
|
||||
// Namespacing by path and client IP keeps the web and desktop keyspaces
|
||||
// separate so one abusive web client cannot lock out other users or the
|
||||
// desktop for the same session. After `authLockoutMaxAttempts` failures within
|
||||
// `authLockoutWindowMs`, further attempts are rejected until
|
||||
// `authLockoutDurationMs` has elapsed.
|
||||
|
||||
import { config } from '../config';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user