diff --git a/macropad-relay/DEPLOY.md b/macropad-relay/DEPLOY.md index e809bd6..c2b6011 100644 --- a/macropad-relay/DEPLOY.md +++ b/macropad-relay/DEPLOY.md @@ -80,6 +80,19 @@ Set these in your container configuration: - `DATA_DIR` - Data storage path (default: ./data) - `NODE_ENV` - production or development - `LOG_LEVEL` - info, debug, error +- `ALLOWED_ORIGINS` - Comma-separated list of allowed browser origins for the + WebSocket Origin check (e.g. `https://macropad.example.com`). **Set this to + your public origin(s) when running behind a reverse proxy** (see note below). + +> **Reverse proxy note (`ALLOWED_ORIGINS`):** When `ALLOWED_ORIGINS` is unset, +> the WebSocket upgrade Origin check falls back to comparing the browser's +> `Origin` host against the raw `Host` header seen by the app. Behind a reverse +> proxy the internal `Host` (e.g. `localhost:3000`) often differs from the +> public origin the browser sends (e.g. `https://macropad.example.com`), so +> legitimate browser WebSocket upgrades get **rejected**. Set `ALLOWED_ORIGINS` +> to your public origin(s) to fix this. (Forwarding the public host via +> `proxy_set_header Host $host;`, as in the Nginx example below, also helps, but +> setting `ALLOWED_ORIGINS` explicitly is the reliable fix.) ## Test It Works diff --git a/macropad-relay/public/app.js b/macropad-relay/public/app.js index 6e61335..8cb6f02 100644 --- a/macropad-relay/public/app.js +++ b/macropad-relay/public/app.js @@ -10,6 +10,13 @@ class MacroPadApp { this.desktopConnected = false; this.wsAuthenticated = false; + // Reconnect backoff. Reset only after a successful AUTH (not on + // socket open): a server that accepts the socket and then closes it + // would otherwise reset the backoff on every open and defeat it. + this.baseReconnectDelay = 3000; + this.maxReconnectDelay = 30000; + this.reconnectDelay = this.baseReconnectDelay; + // Get session ID from URL const pathMatch = window.location.pathname.match(/^\/([a-zA-Z0-9]+)/); this.sessionId = pathMatch ? pathMatch[1] : null; @@ -117,7 +124,12 @@ class MacroPadApp { this.ws.onclose = () => { this.wsAuthenticated = false; this.updateConnectionStatus(false); - setTimeout(() => this.setupWebSocket(), 3000); + // Reconnect using the current backoff, then grow it (capped) for + // the next attempt. The backoff is only reset on a successful + // auth response, so accept-then-close cycles keep backing off. + const delay = this.reconnectDelay; + this.reconnectDelay = Math.min(this.reconnectDelay * 2, this.maxReconnectDelay); + setTimeout(() => this.setupWebSocket(), delay); }; this.ws.onerror = () => this.updateConnectionStatus(false); @@ -133,6 +145,9 @@ class MacroPadApp { case 'auth_response': if (data.success) { this.wsAuthenticated = true; + // Reset the reconnect backoff only after a confirmed, + // successful auth (not merely on socket open). + this.reconnectDelay = this.baseReconnectDelay; this.updateConnectionStatus(this.desktopConnected); } else { this.handleAuthError(); diff --git a/macropad-relay/src/handlers/apiProxy.ts b/macropad-relay/src/handlers/apiProxy.ts index fa2f522..db8a070 100644 --- a/macropad-relay/src/handlers/apiProxy.ts +++ b/macropad-relay/src/handlers/apiProxy.ts @@ -12,26 +12,22 @@ export function createApiProxy( return async (req: Request, res: Response, next: NextFunction) => { const sessionId = req.params.sessionId; - // Check session exists - const session = sessionManager.getSession(sessionId); - if (!session) { - return res.status(404).json({ error: 'Session not found' }); - } - // Credentials must be supplied via the header (never the URL/query, // which leaks into logs, history and referrers). const password = req.headers['x-macropad-password'] as string; - if (!password) { - return res.status(401).json({ error: 'Password required' }); + // Uniform auth failure. A missing session, a missing password, and a + // wrong password all return the SAME status + generic message so a + // caller cannot infer whether a given session id exists (no + // enumeration oracle). validatePassword() safely returns false for an + // unknown session id. + if (!password || !(await sessionManager.validatePassword(sessionId, password))) { + return res.status(401).json({ error: 'Authentication failed' }); } - const valid = await sessionManager.validatePassword(sessionId, password); - if (!valid) { - return res.status(401).json({ error: 'Invalid password' }); - } - - // Check desktop is connected + // Only AFTER successful auth do we reveal desktop connectivity, so the + // 503 "not connected" signal cannot itself be used to probe which + // session ids exist. const desktop = connectionManager.getDesktopBySessionId(sessionId); if (!desktop) { return res.status(503).json({ error: 'Desktop not connected' }); diff --git a/macropad-relay/src/handlers/webClientHandler.ts b/macropad-relay/src/handlers/webClientHandler.ts index fd94bb5..bfedd71 100644 --- a/macropad-relay/src/handlers/webClientHandler.ts +++ b/macropad-relay/src/handlers/webClientHandler.ts @@ -32,31 +32,19 @@ export function handleWebClientConnection( // 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) { - socket.send(JSON.stringify({ - type: 'error', - error: 'Session not found' - })); - socket.close(); - return; - } - // Add client (not authenticated yet) + // Do NOT reveal whether the session exists. A non-existent session id and + // an existing one must be indistinguishable to an unauthenticated client so + // session ids cannot be enumerated: we always run the same handshake and let + // the (uniform) auth response be the only signal. validatePassword() safely + // returns false for an unknown session id, and desktop connectivity is only + // disclosed AFTER a successful auth (see the 'auth' case below). const client = connectionManager.addWebClient(sessionId, socket, false); // Track failed auth attempts on this individual socket. let socketAuthFailures = 0; - // Check if desktop is connected - const desktop = connectionManager.getDesktopBySessionId(sessionId); - socket.send(JSON.stringify({ - type: 'desktop_status', - status: desktop ? 'connected' : 'disconnected' - })); - - // Request authentication + // Request authentication. socket.send(JSON.stringify({ type: 'auth_required' })); @@ -76,6 +64,12 @@ export function handleWebClientConnection( } } else { socketAuthFailures = 0; + // Authenticated: only now do we disclose desktop connectivity. + const desktop = connectionManager.getDesktopBySessionId(sessionId); + socket.send(JSON.stringify({ + type: 'desktop_status', + status: desktop ? 'connected' : 'disconnected' + })); } break; } @@ -142,7 +136,7 @@ async function handleAuth( socket.send(JSON.stringify({ type: 'auth_response', success: false, - error: 'Invalid password' + error: 'Authentication failed' })); return true; } diff --git a/macropad-relay/src/services/SessionManager.ts b/macropad-relay/src/services/SessionManager.ts index 2505140..378cf84 100644 --- a/macropad-relay/src/services/SessionManager.ts +++ b/macropad-relay/src/services/SessionManager.ts @@ -136,6 +136,21 @@ export class SessionManager { } while (this.sessions.has(id)); const passwordHash = await bcrypt.hash(password, config.bcryptRounds); + + // Re-check the cap AFTER the (awaited) hash. bcrypt.hash is the only + // suspension point between the first cap check and the set() below, so + // concurrent createSession calls could each have passed the earlier check + // and then filled the map while we were hashing. Re-checking here — with + // no await between this check and the synchronous set() — makes the + // check-and-insert atomic under Node's single-threaded model, so the cap + // cannot be overshot. + if (this.sessions.size >= config.maxSessions) { + this.pruneExpired(); + if (this.sessions.size >= config.maxSessions) { + throw new Error('Session limit reached'); + } + } + const now = new Date().toISOString(); const session: Session = { diff --git a/macropad-relay/src/utils/authThrottle.ts b/macropad-relay/src/utils/authThrottle.ts index e298278..94d97f3 100644 --- a/macropad-relay/src/utils/authThrottle.ts +++ b/macropad-relay/src/utils/authThrottle.ts @@ -33,10 +33,12 @@ export function getLockoutRemaining(key: string): number { } /** - * Record a failed auth attempt. Returns the lockout remaining (ms) after - * this failure is applied (0 if still not locked). + * Record a failed auth attempt. Locks the key out for + * `authLockoutDurationMs` once `authLockoutMaxAttempts` failures accumulate + * within `authLockoutWindowMs`. Callers query the resulting lockout via + * getLockoutRemaining(), so this returns nothing. */ -export function recordFailure(key: string): number { +export function recordFailure(key: string): void { const now = Date.now(); let rec = records.get(key); @@ -51,8 +53,6 @@ export function recordFailure(key: string): number { if (rec.count >= config.authLockoutMaxAttempts) { rec.lockedUntil = now + config.authLockoutDurationMs; } - - return rec.lockedUntil > now ? rec.lockedUntil - now : 0; } /** diff --git a/macropad-relay/src/utils/idGenerator.ts b/macropad-relay/src/utils/idGenerator.ts index dd8fa26..2db916b 100644 --- a/macropad-relay/src/utils/idGenerator.ts +++ b/macropad-relay/src/utils/idGenerator.ts @@ -18,7 +18,11 @@ export function generateSessionId(length: number = 12): string { let result = ''; while (result.length < length) { - // Over-fetch a little to reduce the number of syscalls when rejecting. + // Fetch exactly `length` random bytes per pass. Bytes at or above the + // rejection threshold are skipped to avoid modulo bias, so a pass may + // yield fewer than `length` characters; the outer while then runs another + // pass until we have enough. (No over-fetching — each pass draws exactly + // `length` bytes.) const bytes = randomBytes(length); for (let i = 0; i < bytes.length && result.length < length; i++) { const byte = bytes[i];