From 986135b074b80b49a555e5072a115df50db16c6e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 25 Oct 2026 12:12:02 +0000 Subject: [PATCH] fix(gateway): require a pairing token on WebSocket upgrade libwebsockets 4.3 delivers HTTP_CONFIRM_UPGRADE on the HTTP protocol, so the check in ws_callback never ran. A client that offered the registered "ws" subprotocol was accepted with no token and could inject WebChat messages. The dashboard offered only bearer., which is not a registered protocol, so its handshake never completed. Check the token on the HTTP confirm callback and on FILTER_PROTOCOL_CONNECTION. Browsers offer both "ws" and bearer.. --- CHANGELOG.md | 3 + README.md | 2 +- docs/ARCHITECTURE.md | 2 +- docs/SECURITY.md | 2 +- src/gateway/http_lws.c | 127 +++++++++++++++++++++++++++----------- tests/test_gateway_http.c | 117 +++++++++++++++++++++++++++++++++++ web/js/app.js | 5 +- 7 files changed, 216 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0252119..fb81f86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] +### Security +- WebSocket upgrade requires a pairing token. libwebsockets 4.3 delivers `LWS_CALLBACK_HTTP_CONFIRM_UPGRADE` on the HTTP protocol, so the check on the `ws` callback never ran and `Sec-WebSocket-Protocol: ws` was accepted with no token (the socket can inject WebChat messages into the agent). The dashboard offered only `bearer.`, which is not a registered protocol, so that handshake never completed. Clients offer `ws` and `bearer.`, or send `Authorization: Bearer` with subprotocol `ws`. + ## [1.0.1] - 2026-10-18 ### Fixed diff --git a/README.md b/README.md index 1afa7a3..245d595 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for PR workflow, coding standards, and th **Jetson install (v1.0):** `./scripts/install.sh`, `./scripts/build_llama_jetson.sh`, `./scripts/download_model.sh phi3` — details in [`docs/HARDWARE_JETSON.md`](docs/HARDWARE_JETSON.md) and [`docs/LOCAL_INFERENCE.md`](docs/LOCAL_INFERENCE.md). -**WebSocket auth (breaking vs early gateway builds):** browsers cannot send `Authorization` on WebSocket; use subprotocol `bearer.` when opening `/ws` (see `web/js/app.js`). +**WebSocket auth:** browsers cannot send `Authorization` on WebSocket. Offer the registered subprotocol `ws` together with `bearer.` (see `web/js/app.js`). `bearer.` alone is not a protocol libwebsockets will select, and `ws` alone is rejected without a valid pairing token. Non-browser clients may send `Authorization: Bearer ` with subprotocol `ws`. **Debug (macOS):** Symbols in `tests-dSYM/`. Use `lldb build/test_agent` then `settings set target.debug-file-search-path tests-dSYM`. Old `.dSYM` in repo root? Run `make clean-root-dsym`. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index bd52b26..0652417 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -86,7 +86,7 @@ Convention: one primary `.c` + `.h` per module; new tools go in `src/tools/` (not `Authorization` header). +- **WebSocket:** browsers offer subprotocols `ws` and `bearer.` (`ws` is the registered protocol; the pairing token is read from the list). Non-browser clients may send `Authorization: Bearer` with subprotocol `ws`. - **Hardware API:** `/api/hardware/*` — board info, GPIO snapshot, tegrastats on Jetson; sensor/camera panels stubbed “Coming in v1.2”. - **Rate limits:** per-IP on `/asap`; per-token on camera snapshot (1/sec) when enabled. - **Signed manifest:** `GET /.well-known/asap/manifest.json` builds and signs the JSON synchronously in the HTTP handler (single-threaded today; not safe for concurrent manifest builds without locking). diff --git a/docs/SECURITY.md b/docs/SECURITY.md index c6ac5e1..6497270 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -300,7 +300,7 @@ This section summarizes Jetson Orin Nano Super / JetPack 6.2.x concerns that do |------|------------------|----------------------------| | Sandbox isolation | [`src/sandbox/sandbox.c`](../src/sandbox/sandbox.c), [`src/sandbox/sandbox_landlock.c`](../src/sandbox/sandbox_landlock.c) | user ns + `unshare` + PID-1 fork, Landlock workspace bound, cgroups v2, fail-closed isolation | | Command policy | [`src/sandbox/allowlist.c`](../src/sandbox/allowlist.c) | Blocklist (incl. Jetson GPU + Argus), workspace path containment (DiD) | -| Gateway auth | [`src/gateway/http_lws.c`](../src/gateway/http_lws.c) | `requires_auth()` Bearer gate for `/api/*` | +| Gateway auth | [`src/gateway/http_lws.c`](../src/gateway/http_lws.c) | `requires_auth()` Bearer gate for `/api/*`; WebSocket upgrade checks the pairing token on HTTP `CONFIRM_UPGRADE` and ws `FILTER_PROTOCOL_CONNECTION` | | Hardware HTTP API | [`src/gateway/routes_hardware.c`](../src/gateway/routes_hardware.c) | Read-only GET handlers, camera POST deferred stub | | Rate limits | [`src/gateway/rate_limit.c`](../src/gateway/rate_limit.c) | Per-IP `/asap` RPM (64-slot table); reuses expired windows; **fail-closed** (429) when table is full and no slot expired | | Camera spawn | [`src/hardware/hardware_camera.c`](../src/hardware/hardware_camera.c) | argv-only `execvp`, validated resolution/type/path | diff --git a/src/gateway/http_lws.c b/src/gateway/http_lws.c index 9794007..27daaab 100644 --- a/src/gateway/http_lws.c +++ b/src/gateway/http_lws.c @@ -129,43 +129,91 @@ const char *http_request_bearer_token(struct lws *wsi, char *buf, size_t buf_siz return buf + 7; } -/** WebSocket upgrade: Bearer from Authorization (or custom header); browser may send bearer. subprotocol. */ +static int copy_token_text(const char *src, size_t len, char *token_out, size_t token_size) +{ + if (!src || len == 0 || !token_out || token_size == 0) + return -1; + /* A truncated token must not be checked: validation is an exact match. */ + if (len >= token_size) + return -1; + memcpy(token_out, src, len); + token_out[len] = '\0'; + return token_out[0] != '\0' ? 0 : -1; +} + +/** `bearer.` may sit anywhere in the comma-separated subprotocol list. */ +static int token_from_bearer_list(const char *list, int list_len, + char *token_out, size_t token_size) +{ + const char *p; + const char *end; + + if (!list || list_len <= 0) + return -1; + p = list; + end = list + list_len; + while (p < end) { + const char *comma; + const char *stop; + size_t len; + + while (p < end && (*p == ' ' || *p == '\t' || *p == ',')) + p++; + if (p >= end) + break; + comma = memchr(p, ',', (size_t)(end - p)); + stop = comma ? comma : end; + while (stop > p && (stop[-1] == ' ' || stop[-1] == '\t')) + stop--; + len = (size_t)(stop - p); + if (len > 7 && memcmp(p, "bearer.", 7) == 0) + return copy_token_text(p + 7, len - 7, token_out, token_size); + p = comma ? comma + 1 : end; + } + return -1; +} + +/** + * Pairing token for a WebSocket upgrade. + * Browsers cannot set Authorization, so they offer `ws` (the registered + * protocol) and `bearer.` in Sec-WebSocket-Protocol. The token is + * not only a prefix: lws selects the first name it knows (`ws`). + */ static int ws_copy_upgrade_token(struct lws *wsi, char *token_out, size_t token_size) { - char auth_buf[256]; + char hdr[512]; const char *token; - if (!wsi || !token_out || token_size == 0) return -1; + int n; + + if (!wsi || !token_out || token_size == 0) + return -1; token_out[0] = '\0'; - token = http_request_bearer_token(wsi, auth_buf, sizeof(auth_buf)); - if (!token) { - int n = lws_hdr_custom_copy(wsi, auth_buf, sizeof(auth_buf), "authorization", 13); - if (n > 7 && strncmp(auth_buf, "Bearer ", 7) == 0) - token = auth_buf + 7; - } - if (token && token[0] != '\0') { - size_t len = strlen(token); - if (len >= token_size) len = token_size - 1; - memcpy(token_out, token, len); - token_out[len] = '\0'; + token = http_request_bearer_token(wsi, hdr, sizeof(hdr)); + if (token && token[0] != '\0') + return copy_token_text(token, strlen(token), token_out, token_size); + n = lws_hdr_copy(wsi, hdr, (int)sizeof(hdr), WSI_TOKEN_PROTOCOL); + if (n <= 0) + n = lws_hdr_custom_copy(wsi, hdr, (int)sizeof(hdr), + "sec-websocket-protocol", 22); + if (n > 0 && token_from_bearer_list(hdr, n, token_out, token_size) == 0) return 0; - } - { - char proto[512]; - int n = lws_hdr_copy(wsi, proto, sizeof(proto), WSI_TOKEN_COLON_PROTOCOL); - if (n <= 0) - n = lws_hdr_custom_copy(wsi, proto, sizeof(proto), "sec-websocket-protocol", 22); - if (n > 7 && strncmp(proto, "bearer.", 7) == 0) { - const char *sub = proto + 7; - size_t len = (size_t)(n - 7); - if (len >= token_size) len = token_size - 1; - memcpy(token_out, sub, len); - token_out[len] = '\0'; - return token_out[0] != '\0' ? 0 : -1; - } - } return -1; } +/** 1 when this upgrade carries a stored pairing token. */ +static int ws_upgrade_allowed(struct lws *wsi) +{ + http_server_ctx_t *ctx; + char token_buf[256]; + + ctx = http_ctx_get(); + if (!ctx || !ctx->auth) + return 0; + if (ws_copy_upgrade_token(wsi, token_buf, sizeof(token_buf)) != 0) + return 0; + return auth_validate_token(ctx->auth, token_buf) ? 1 : 0; +} + static int is_static_path(const char *uri, int uri_len, int method) { if (method != HTTP_GET) return 0; @@ -395,6 +443,13 @@ int http_callback(struct lws *wsi, enum lws_callback_reasons reason, void *user, } return 0; } + case LWS_CALLBACK_HTTP_CONFIRM_UPGRADE: + /* lws 4.3 calls this on the HTTP protocol (protocols[0]) before + * it binds "ws". The same case on ws_callback never runs, so an + * upgrade of Sec-WebSocket-Protocol: ws was accepted with no token. */ + if (!ws_upgrade_allowed(wsi)) + return -1; + return 0; case LWS_CALLBACK_CLOSED_HTTP: { http_conn_t *conn = lws_wsi_user(wsi); if (conn) { @@ -434,18 +489,16 @@ static void ws_on_writable(struct lws *wsi, int conn_id) int ws_callback(struct lws *wsi, enum lws_callback_reasons reason, void *user, void *in, size_t len) { - http_server_ctx_t *ctx = http_ctx_get(); (void)user; switch (reason) { - case LWS_CALLBACK_HTTP_CONFIRM_UPGRADE: { - char token_buf[256] = {0}; - if (!ctx || !ctx->auth) return -1; - if (ws_copy_upgrade_token(wsi, token_buf, sizeof(token_buf)) != 0) - return -1; - if (!auth_validate_token(ctx->auth, token_buf)) + case LWS_CALLBACK_HTTP_CONFIRM_UPGRADE: + case LWS_CALLBACK_FILTER_PROTOCOL_CONNECTION: + /* FILTER runs on the ws protocol after lws selects it, still + * before the 101. CONFIRM on this callback is unused on lws 4.3 + * (it is delivered to http_callback) and stays as a backstop. */ + if (!ws_upgrade_allowed(wsi)) return -1; return 0; - } case LWS_CALLBACK_ESTABLISHED: { int conn_id = ws_next_conn_id(); lws_set_wsi_user(wsi, (void *)(intptr_t)conn_id); diff --git a/tests/test_gateway_http.c b/tests/test_gateway_http.c index f2b6aab..5511d21 100644 --- a/tests/test_gateway_http.c +++ b/tests/test_gateway_http.c @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -1614,6 +1615,118 @@ static int test_api_hardware_camera_deferred(const char *token) return 0; } +/* HTTP status from a /ws upgrade, or 0 when the peer closes with no status line. */ +static int ws_upgrade_read_status(int fd) +{ + struct pollfd pfd; + char resp[256]; + size_t got; + ssize_t n; + + pfd.fd = fd; + pfd.events = POLLIN; + got = 0; + resp[0] = '\0'; + while (got + 1 < sizeof(resp)) { + if (poll(&pfd, 1, 2000) <= 0) + break; + n = recv(fd, resp + got, sizeof(resp) - 1 - got, 0); + if (n <= 0) + break; + got += (size_t)n; + resp[got] = '\0'; + if (memchr(resp, '\n', got) != NULL) + break; + } + if (strncmp(resp, "HTTP/1.1 ", 9) != 0 && strncmp(resp, "HTTP/1.0 ", 9) != 0) + return 0; + return atoi(resp + 9); +} + +static int ws_upgrade_append(char *req, size_t cap, int used, const char *line) +{ + int n; + + if (used < 0 || (size_t)used >= cap) + return -1; + n = snprintf(req + used, cap - (size_t)used, "%s", line); + if (n < 0 || (size_t)used + (size_t)n >= cap) + return -1; + return used + n; +} + +static int ws_upgrade_http_status(int port, const char *protocol, const char *bearer) +{ + int fd; + int used; + struct sockaddr_in addr; + char req[768]; + char line[320]; + + fd = socket(AF_INET, SOCK_STREAM, 0); + if (fd < 0) + return -1; + memset(&addr, 0, sizeof(addr)); + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + addr.sin_port = htons((uint16_t)port); + if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) != 0) { + close(fd); + return -1; + } + used = snprintf(req, sizeof(req), + "GET /ws HTTP/1.1\r\n" + "Host: 127.0.0.1:%d\r\n" + "Upgrade: websocket\r\n" + "Connection: Upgrade\r\n" + "Sec-WebSocket-Version: 13\r\n" + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n", + port); + if (protocol) { + snprintf(line, sizeof(line), "Sec-WebSocket-Protocol: %s\r\n", protocol); + used = ws_upgrade_append(req, sizeof(req), used, line); + } + if (bearer) { + snprintf(line, sizeof(line), "Authorization: Bearer %s\r\n", bearer); + used = ws_upgrade_append(req, sizeof(req), used, line); + } + used = ws_upgrade_append(req, sizeof(req), used, "\r\n"); + if (used < 0 || send(fd, req, (size_t)used, 0) != used) { + close(fd); + return -1; + } + used = ws_upgrade_read_status(fd); + close(fd); + return used; +} + +static int test_ws_upgrade_requires_token(int port, const char *token) +{ + char proto[160]; + int status; + + status = ws_upgrade_http_status(port, "ws", NULL); + ASSERT(status != 101); + status = ws_upgrade_http_status(port, NULL, NULL); + ASSERT(status != 101); + snprintf(proto, sizeof(proto), "bearer.%s", token); + status = ws_upgrade_http_status(port, proto, NULL); + ASSERT(status != 101); + snprintf(proto, sizeof(proto), "ws, bearer.%s", token); + status = ws_upgrade_http_status(port, proto, NULL); + ASSERT(status == 101); + snprintf(proto, sizeof(proto), "bearer.%s, ws", token); + status = ws_upgrade_http_status(port, proto, NULL); + ASSERT(status == 101); + status = ws_upgrade_http_status(port, "ws, bearer.not-the-token", NULL); + ASSERT(status != 101); + status = ws_upgrade_http_status(port, "ws", token); + ASSERT(status == 101); + status = ws_upgrade_http_status(port, "ws", "not-the-token"); + ASSERT(status != 101); + return 0; +} + int main(int argc, char **argv) { (void)argc; @@ -1820,6 +1933,10 @@ int main(int argc, char **argv) fprintf(stderr, "test_api_hardware_camera_deferred failed\n"); failed++; } + if (test_ws_upgrade_requires_token(port, token) != 0) { + fprintf(stderr, "test_ws_upgrade_requires_token failed\n"); + failed++; + } } if (test_shutdown_does_not_crash(pid, token, &shutdown_reaped) != 0) { fprintf(stderr, "test_shutdown_does_not_crash failed\n"); diff --git a/web/js/app.js b/web/js/app.js index 03aea9c..d08a61f 100644 --- a/web/js/app.js +++ b/web/js/app.js @@ -33,8 +33,9 @@ const token = getToken(); if (!token) return null; const proto = location.protocol === 'https:' ? 'wss:' : 'ws:'; - /* Browsers cannot set Authorization on WebSocket; server accepts bearer. subprotocol. */ - const ws = new WebSocket(proto + '//' + location.host + '/ws', ['bearer.' + token]); + /* Browsers cannot set Authorization. lws completes the handshake only for + the registered "ws" protocol; bearer. carries the pairing token. */ + const ws = new WebSocket(proto + '//' + location.host + '/ws', ['ws', 'bearer.' + token]); ws.onmessage = e => { try { const d = JSON.parse(e.data); onMessage(d); } catch (_) {} }; return ws; }