From 4bd4c374fde459b0655637e88da244af732c354c Mon Sep 17 00:00:00 2001 From: Mike Teehan Date: Tue, 18 Aug 2026 13:36:55 +0000 Subject: [PATCH] fix: send WS JWT as bare subprotocol name; CSSOM inline styles; open mgmt services on public zone - websocket.js passes the raw JWT as the Sec-WebSocket-Protocol subprotocol (no 'Bearer ' prefix): subprotocol names must be valid RFC 6455 tokens, and the space in 'Bearer ' made the browser reject the constructor with a SyntaxError. - daemon accepts a JWT-shaped subprotocol plus the legacy 'Bearer ' form via _extract_ws_token; unit tests in tests/test_ws_auth.py. - vdom.js applies inline styles through el.style (CSSOM) instead of setAttribute, which the management-domain CSP (no 'unsafe-inline') blocks. - install.sh opens http/https/ssh on the public zone alongside WAN setup. - docs (hoover.md, security.md) updated to match. --- daemon/server.py | 55 +++++++++++++++++++++++++------- docs/hoover.md | 2 +- docs/security.md | 2 +- scripts/install.sh | 5 +++ tests/test_ws_auth.py | 47 +++++++++++++++++++++++++++ webui/static/hoover/vdom.js | 17 ++++++++-- webui/static/hoover/websocket.js | 8 +++-- 7 files changed, 118 insertions(+), 18 deletions(-) create mode 100644 tests/test_ws_auth.py diff --git a/daemon/server.py b/daemon/server.py index 5432488..0645361 100644 --- a/daemon/server.py +++ b/daemon/server.py @@ -9,6 +9,7 @@ import hashlib import json import logging import os +import re import signal import time from collections.abc import Callable @@ -359,6 +360,38 @@ def create_app() -> web.Application: return app +# JWTs are dot-joined base64url segments — a subset of the RFC 6455 token +# character set. Browsers therefore carry the access token as the +# Sec-WebSocket-Protocol subprotocol name itself (see websocket.js); the +# "Bearer " form is not a valid subprotocol (space is not a token +# character) and is rejected by the WebSocket constructor. +_JWT_SUBPROTOCOL_RE = re.compile(r"^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$") + + +def _extract_ws_token(subprotocols: list[str]) -> tuple[str | None, str | None]: + """Extract the auth JWT from parsed Sec-WebSocket-Protocol names. + + Accepts the raw JWT as a subprotocol name (bundled client) or the + legacy "Bearer " form (non-browser clients that can send spaces). + + Args: + subprotocols: Parsed subprotocol names (already stripped/split). + + Returns: + ``(token, matched_subprotocol)`` or ``(None, None)`` if no usable + auth subprotocol was found. + """ + for proto in subprotocols: + if _JWT_SUBPROTOCOL_RE.fullmatch(proto): + return proto, proto + for proto in subprotocols: + if proto.startswith("Bearer "): + token = proto[7:].strip() + if token: + return token, proto + return None, None + + # WebSocket subscribers _ws_subscribers: set[web.WebSocketResponse] = set() _ws_tasks: set[asyncio.Task[None]] = set() @@ -374,10 +407,12 @@ async def _handle_ws(request: web.Request) -> web.Response: updated subsystem versions. Clients disconnect to unsubscribe. Authentication: JWT access token passed via: - 1. WebSocket subprotocol header (Sec-WebSocket-Protocol: "Bearer ") - — the bundled client path. - 2. X-Auth-Token header — fallback for custom nginx setups that inject it - (not set by the bundled nginx config). + 1. WebSocket subprotocol header — the bundled client sends the raw JWT + as the subprotocol name (Sec-WebSocket-Protocol must carry a valid + RFC 6455 token; a JWT is one, "Bearer " is not). + 2. "Bearer " subprotocol — legacy form for non-browser clients. + 3. X-Auth-Token header — fallback for custom nginx setups that inject + it (not set by the bundled nginx config). """ from aiohttp import hdrs @@ -386,16 +421,12 @@ async def _handle_ws(request: web.Request) -> web.Response: token_param = None matched_proto = None - # Prefer subprotocol header (client JS sends "Bearer "). - # Sec-WebSocket-Protocol is a comma-separated list; parse it the same - # way aiohttp's own handshake does (Request has no subprotocol helper). + # Prefer the subprotocol header. Sec-WebSocket-Protocol is a + # comma-separated list; parse it the same way aiohttp's own handshake + # does (Request has no subprotocol helper). protocol_header = request.headers.get(hdrs.SEC_WEBSOCKET_PROTOCOL, "") subprotocols = [p.strip() for p in protocol_header.split(",") if p.strip()] - for proto in subprotocols: - if proto.startswith("Bearer "): - token_param = proto[7:] - matched_proto = proto - break + token_param, matched_proto = _extract_ws_token(subprotocols) if token_param is None: token_param = request.headers.get("X-Auth-Token") diff --git a/docs/hoover.md b/docs/hoover.md index 9a7241c..b0274d1 100644 --- a/docs/hoover.md +++ b/docs/hoover.md @@ -551,7 +551,7 @@ Link({ path: '/zones', class: 'active', children: ['Zones'] }) Start the WebSocket connection to the daemon at `ws:///ws` (auto-detects `wss:` for HTTPS). Auto-reconnects with exponential backoff (max 15s). -The JWT is read from the auth model and sent in the WebSocket subprotocol header (`Bearer `). With no token, no socket is created (the daemon 401s unauthenticated WS connections). After 3 consecutive close failures a token refresh is triggered through the auth model; reconnection branches on the model's token state (`getAuthToken()`), never on the refresh promise. +The JWT is read from the auth model and sent as the WebSocket subprotocol name (`Sec-WebSocket-Protocol`) — the token is sent as-is, without a `Bearer ` prefix, because subprotocol names must be valid RFC 6455 tokens and a JWT (base64url + `.`) is one, while the space in `Bearer ` is not (the browser rejects the whole constructor with a SyntaxError). With no token, no socket is created (the daemon 401s unauthenticated WS connections). After 3 consecutive close failures a token refresh is triggered through the auth model; reconnection branches on the model's token state (`getAuthToken()`), never on the refresh promise. ### `disconnect()` diff --git a/docs/security.md b/docs/security.md index fa1f74c..e13555b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -104,7 +104,7 @@ Token theft protection: - Token blacklist prevents reuse after logout or password change - XSS mitigations: CSP headers, `X-XSS-Protection` header on management domain -**WebSocket session binding limitation**: WebSocket connections skip `session_id` validation. Browsers cannot send custom headers during the WebSocket handshake — the bundled nginx config passes the token via the `Sec-WebSocket-Protocol` subprotocol header (a custom nginx setup may instead inject it as `X-Auth-Token`). This means a stolen access token can be used to open WebSocket connections for the full 15-minute TTL without session verification. Short-lived TTL and management domain CSP headers mitigate this risk. +**WebSocket session binding limitation**: WebSocket connections skip `session_id` validation. Browsers cannot send custom headers during the WebSocket handshake — the bundled client passes the raw JWT as the `Sec-WebSocket-Protocol` subprotocol name (a JWT is a valid RFC 6455 token; the `Bearer ` prefix is not, so it cannot be used) (a custom nginx setup may instead inject it as `X-Auth-Token`). This means a stolen access token can be used to open WebSocket connections for the full 15-minute TTL without session verification. Short-lived TTL and management domain CSP headers mitigate this risk. ### WebAuthn Security diff --git a/scripts/install.sh b/scripts/install.sh index d757ffb..e1e7561 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -416,6 +416,11 @@ else "$(jq -n --arg zone "public" --arg iface "$WAN_IFACE" \ '{zone: $zone, interfaces: [$iface]}')" \ "WAN interface assigned to public zone" + + # Open management services (HTTP, HTTPS, SSH) on the public/WAN zone + _daemon_post "/firewall/zones/services" \ + "$(jq -n '{zone: "public", services: ["http", "https", "ssh"]}')" \ + "Management services opened on public zone (http, https, ssh)" fi if [[ -n "$LAN_IFACES" ]]; then diff --git a/tests/test_ws_auth.py b/tests/test_ws_auth.py new file mode 100644 index 0000000..1cc9b4a --- /dev/null +++ b/tests/test_ws_auth.py @@ -0,0 +1,47 @@ +"""Tests for WebSocket subprotocol token extraction (daemon.server).""" + +import pytest + +from daemon.server import _extract_ws_token + +# Realistic-looking access token (base64url header.payload.signature). +TOKEN = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJhZG1pbiJ9.Y3WNO0CTxb4dlcVCPjUnQi" + + +class TestExtractWsToken: + def test_raw_jwt_subprotocol(self): + """Bundled client path: the JWT is sent as the subprotocol name itself.""" + assert _extract_ws_token([TOKEN]) == (TOKEN, TOKEN) + + def test_raw_jwt_among_other_subprotocols(self): + assert _extract_ws_token(["vacuum-wall", TOKEN]) == (TOKEN, TOKEN) + + @pytest.mark.parametrize( + "raw", + ["none", "vacuum-wall", "a.b", "a.b.c.d"], + ) + def test_no_auth_subprotocol(self, raw): + assert _extract_ws_token([raw]) == (None, None) + + def test_empty_list(self): + assert _extract_ws_token([]) == (None, None) + + def test_bearer_prefix_form(self): + """Legacy non-browser form: 'Bearer ' subprotocol.""" + assert _extract_ws_token([f"Bearer {TOKEN}"]) == (TOKEN, f"Bearer {TOKEN}") + + def test_bearer_with_non_jwt_token(self): + assert _extract_ws_token(["Bearer abc123"]) == ("abc123", "Bearer abc123") + + def test_bearer_without_token_ignored(self): + assert _extract_ws_token(["Bearer ", "Bearer"]) == (None, None) + + def test_jwt_like_name_preferred_over_bearer(self): + """A JWT-shaped subprotocol wins even when listed after a Bearer one.""" + result = _extract_ws_token([f"Bearer {TOKEN}", TOKEN]) + assert result == (TOKEN, TOKEN) + + def test_subprotocol_with_space_rejected(self): + """A space is not an RFC 6455 token character — 'Bearer ' must + go through the Bearer branch, never the JWT-shape branch.""" + assert _extract_ws_token(["a b.c.d"]) == (None, None) diff --git a/webui/static/hoover/vdom.js b/webui/static/hoover/vdom.js index 7d52cae..6cb7306 100644 --- a/webui/static/hoover/vdom.js +++ b/webui/static/hoover/vdom.js @@ -134,8 +134,21 @@ export function setProp(el, key, value) { el.className = Object.keys(value).filter(k => value[k]).join(' '); return; } - if (key === 'style' && typeof value === 'object' && value !== null) { - for (const [sk, sv] of Object.entries(value)) el.style[sk] = sv; + if (key === 'style') { + // Inline styles go through the CSSOM (el.style), never through + // setAttribute('style', ...) — the latter applies an inline style + // attribute, which the management-domain CSP (style-src 'self', + // no 'unsafe-inline') blocks. + if (value == null || value === false) { + el.removeAttribute('style'); + return; + } + if (typeof value === 'object') { + el.style.cssText = ''; // clear stale props from a previous style + for (const [sk, sv] of Object.entries(value)) el.style[sk] = sv; + } else { + el.style.cssText = String(value); + } return; } diff --git a/webui/static/hoover/websocket.js b/webui/static/hoover/websocket.js index 6b0fdf8..edc530a 100644 --- a/webui/static/hoover/websocket.js +++ b/webui/static/hoover/websocket.js @@ -34,8 +34,12 @@ function _wsUrl() { } /** Attempt a WebSocket connection. - * Passes the JWT in the WebSocket subprotocol header (Sec-WebSocket-Protocol) + * Passes the JWT as the WebSocket subprotocol name (Sec-WebSocket-Protocol) * instead of a query parameter, keeping it out of logs and browser history. + * The token is sent as-is, WITHOUT a "Bearer " prefix: subprotocol names + * must be valid RFC 6455 tokens and a JWT (base64url + dots) is one, but + * the space in "Bearer " is not a token character — the browser + * rejects the whole constructor with a SyntaxError. * No token: no socket is created — the daemon 401s unauthenticated WS * connections and connect() only runs while authenticated. */ @@ -46,7 +50,7 @@ function _wsConnect() { const token = getAuthToken(); if (!token) return; - _wsConn = new WebSocket(_wsUrl(), ['Bearer ' + token]); + _wsConn = new WebSocket(_wsUrl(), [token]); _wsConn.onopen = () => { _wsReconnectMs = 0;