From ecb0260f611d905c236916ec6750e5165f628fde Mon Sep 17 00:00:00 2001 From: cjkas Date: Tue, 27 Oct 2026 23:03:59 +0100 Subject: [PATCH 1/2] feat!: the token on every route, Secure Config Only goes (step 4b) Under Secured every HTTP route but the public ones needs the token (docs/adr/0006-two-security-modes-eleven-http-routes.md): - isPublicRoute, isWebAsset and originMatchesHost in src/HttpAccess.h, with native tests. - One middleware checks the token. It runs once the body is in, so /restore, /updateFirmware and /updateApplication check it on their first chunk and drop a refused upload. - Static serving answers the web UI's files only: shades.cfg and shades.tmp are no longer served. - The wildcard CORS headers go; WEB_CORS turns them back on for UI work from file://. - The socket refuses a handshake whose Origin doesn't name its Host. - "Secure Config Only" goes: a device set to it is Secured, and /loginContext drops permissions. Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 3 + README.md | 2 + docs/PROTOCOL.md | 1 + docs/SECURITY_PLAN.md | 11 +-- platformio.ini | 4 + src/ConfigSettings.cpp | 9 +- src/ConfigSettings.h | 4 - src/DeviceOps.cpp | 2 - src/HttpAccess.h | 49 ++++++++++ src/entryPoints/HttpRoutes.cpp | 20 +++-- src/entryPoints/Web.cpp | 52 ++++++----- src/entryPoints/Web.h | 5 +- src/outputs/ProtoSocket.cpp | 14 +-- test/e2e/proto_client.py | 5 +- test/e2e/test_routes.py | 127 ++++++++++++++++++++++++++ test/test_httpaccess/test_main.cpp | 138 +++++++++++++++++++++++++++++ 16 files changed, 392 insertions(+), 54 deletions(-) create mode 100644 src/HttpAccess.h create mode 100644 test/e2e/test_routes.py create mode 100644 test/test_httpaccess/test_main.cpp diff --git a/GLOSSARY.md b/GLOSSARY.md index 5753199..67f3f7c 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -136,6 +136,9 @@ The device's description of the protocol it speaks: what each message type, fiel Whether the device asks for login: *None* (everything open) or *Secured* (a PIN or password). _Avoid_: Secure Config Only (removed) +**Public route** +What anyone may reach under Secured without a token: the web UI's files, `/loginContext`, `/login`, `/upnp.xml` and preflight requests. Static serving answers the web UI's files alone, so `shades.cfg` isn't one. + **Token** What logging in returns. It stands for the PIN or password, on one client address, and every request under Secured carries it. _Avoid_: session, API key (the name of the field it travels in) diff --git a/README.md b/README.md index ae022c9..8004628 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,8 @@ Since 4.0.0 everything is on port 80. Ports 8081 (HTTP API) and 8080 (events soc | Everything else: state, events, edits, settings | the socket at `ws:///ws`, see [docs/PROTOCOL.md](docs/PROTOCOL.md) | A script that called port 8081 only changes the port, if it used one of the three commands. Reads such as `/controller`, `/shades` and `/discovery` are gone: use the socket, or MQTT. The Home Assistant integration needs version 3.0.0 or newer. + +When the device is Secured (a PIN or password), every route above except the web UI and the login routes needs the token, in an `apikey` header; the socket takes it as `?apikey=`. Without it the reply is 401. "Secure Config Only" is gone: a device set to it is Secured from 4.0.0, so scripts that sent commands without logging in now need the token. ## Sources for this Project I spent some time reading about a myriad of topics but in the end the primary source for this project comes from https://pushstack.wordpress.com/somfy-rts-protocol/. The work done on pushstack regarding the protocol timing made this feasible without burning a bunch of time measuring pulses. diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 2bbafeb..1937f23 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -7,6 +7,7 @@ Why the protocol looks like this: [ADR 0005](adr/0005-one-websocket-protocol.md) ## Connection - `ws://host/ws` on port 80. Under Secured the upgrade needs `?apikey=` from `/login`; without a valid one it is refused with 401 ([ADR 0006](adr/0006-two-security-modes-eleven-http-routes.md)). +- An upgrade whose `Origin` header doesn't name the `Host` it was sent to is refused with 401 too, in either mode, so another site's page can't open the socket. Clients that aren't browsers send no `Origin`. - Optional `?schema=`: the client already has the schema with that hash and doesn't need it again. - At most 5 clients. A sixth is closed as soon as it connects. - Text messages of at most 4 KB. A message may be sent as several frames, and a frame larger than one TCP segment (about 1.4 KB) reaches the device in pieces anyway; the device puts it back together. It does that for one message at a time across all clients: while it is, another client's long message is answered with busy, and the client sends it again. A message that hasn't arrived in full within 10 s is given up, and its client is told with a bad request. These refusals have no `id`, since the message couldn't be read. A binary encoding, if one comes, would be asked for at the upgrade (for example `?enc=bin`) and use binary frames. diff --git a/docs/SECURITY_PLAN.md b/docs/SECURITY_PLAN.md index 04934da..b3139f1 100644 --- a/docs/SECURITY_PLAN.md +++ b/docs/SECURITY_PLAN.md @@ -1,6 +1,6 @@ # Plan: authentication on every entry point, one HTTP server -**Status:** steps 1 to 3, PR B and PR C merged; step 4 in progress: 4a removes the old API, 4b adds the auth rules. +**Status:** steps 1 to 3, PR B, PR C and 4a merged; 4b, the auth rules, is the last step. **Written:** 2026-10-18, on top of PR #64 (stored secrets are write-only). **Revised:** 2026-10-22 for socket protocol 2: two security modes, eleven HTTP routes, the socket on port 80 ([ADR 0005](adr/0005-one-websocket-protocol.md), [ADR 0006](adr/0006-two-security-modes-eleven-http-routes.md)). Terms are defined in [GLOSSARY.md](../GLOSSARY.md). ## Why @@ -29,20 +29,21 @@ ### HTTP routes ([ADR 0006](adr/0006-two-security-modes-eleven-http-routes.md)) - **Public:** static web assets, `/loginContext`, `/login`, `/upnp.xml` (SSDP), OPTIONS preflight requests. - **Token under Secured:** `/backup`, `/restore`, `/updateFirmware`, `/updateApplication`, `/shadeCommand`, `/groupCommand`, `/tiltCommand`. -- Which routes are public is one pure function with native tests. +- Which routes are public is one pure function with native tests (`isPublicRoute` in [src/HttpAccess.h](../src/HttpAccess.h)). It counts `/ws` as public because the socket checks its own token in the handshake. +- One middleware checks the token for every other route. It runs once the body is in, so the uploads check the token on their first chunk too and drop a refused one. - Static serving covers the web assets only, so the rolling codes in `shades.cfg` are reachable only through the authenticated `/backup`. -- The web UI sends `apikey` on its backup and upload requests too (today those XHRs leave it out). +- The web UI sends `apikey` on its backup and upload requests too. ### Websocket - **Handshake:** the socket is `ws://host/ws` on port 80. The token goes in `?apikey=`, because browsers can't set headers on a websocket. `handleHandshake` checks it with the same comparison the HTTP routes use. `request->url()` is the path only, so the token doesn't reach the slow-request log. - **Rules:** under None anyone can connect. Under Secured the handshake needs a valid token, and a connection that has one may send every op and subscribe to every feed. - **Logging in with a socket already open:** the UI reconnects the socket with the token. There is no in-band upgrade message. - **Changing the security settings:** a `saveSettings` of the security section that leaves the device Secured, or changes its mode, closes every socket, since their tokens may no longer be good, and a socket opened under None had none. The client that saved is closed once its reply, which carries its new token, has gone, so no socket stays open on a check it never passed. The others log in when refused. -- **Origin:** a handshake whose `Origin` header doesn't match `Host` is refused. +- **Origin:** a handshake whose `Origin` header doesn't match `Host` is refused, in either mode (`originMatchesHost` in [src/HttpAccess.h](../src/HttpAccess.h)). One without `Origin` comes from a client that isn't a browser and passes. ### CORS - The wildcard `Access-Control-Allow-*` default headers go. The UI is served from the same origin, and HA and scripts aren't browsers, so CORS doesn't affect them. -- A build flag turns the headers back on for UI development from `file://`, which `baseUrl` in `index.js` supports. +- A build flag (`WEB_CORS` in `platformio.ini`) turns the headers back on for UI development from `file://`, which `baseUrl` in `index.js` supports. It also skips the Origin check, since such a page sends `Origin: null`. ### UI - The token is kept in `sessionStorage`: it survives a reload, is dropped when the tab closes, and is cleared when the socket upgrade is refused. diff --git a/platformio.ini b/platformio.ini index 8df6170..31c1523 100644 --- a/platformio.ini +++ b/platformio.ini @@ -42,6 +42,10 @@ build_flags = ; Route log_printfv through src/LogLock.cpp so lines from both cores don't ; interleave. The core already wraps log_printf for ESP Insights. -Wl,--wrap=log_printfv + ; For work on the web UI from a file:// page (baseUrl in index.js): sends + ; the CORS headers and lets the socket skip its Origin check. Never in a + ; release (docs/SECURITY_PLAN.md, CORS). + ; -DWEB_CORS monitor_speed = 115200 monitor_filters = time diff --git a/src/ConfigSettings.cpp b/src/ConfigSettings.cpp index fe7bc77..42def2d 100644 --- a/src/ConfigSettings.cpp +++ b/src/ConfigSettings.cpp @@ -408,7 +408,6 @@ bool SecuritySettings::fromJSON(JsonObject &obj) { this->parseValueString(obj, "username", this->username, sizeof(this->username)); applySecret(obj["password"].as(), this->password, sizeof(this->password)); applySecret(obj["pin"].as(), this->pin, sizeof(this->pin)); - if(!obj["permissions"].isNull()) this->permissions = obj["permissions"]; return true; } bool SecuritySettings::save() { @@ -419,10 +418,11 @@ bool SecuritySettings::save() { pref.putString("username", this->username); pref.putString("password", this->password); pref.putString("pin", this->pin); - pref.putChar("permissions", this->permissions); pref.end(); return true; } +// A device that had "Secure Config Only" keeps its PIN or password and so is +// Secured. Its old permissions key goes with the next save's clear(). bool SecuritySettings::load() { Preferences pref; pref.begin("SEC"); @@ -430,13 +430,12 @@ bool SecuritySettings::load() { if(pref.isKey("username")) pref.getString("username", this->username, sizeof(this->username)); if(pref.isKey("password")) pref.getString("password", this->password, sizeof(this->password)); if(pref.isKey("pin")) pref.getString("pin", this->pin, sizeof(this->pin)); - if(pref.isKey("permissions")) this->permissions = pref.getChar("permissions", this->permissions); pref.end(); return true; } void SecuritySettings::print() { - ESP_LOGD(TAG, "SECURITY Type:%u Username:[%s] Password:[%s] Pin:[%s] Permissions:%u", - static_cast(this->type), this->username, maskSecret(this->password), maskSecret(this->pin), this->permissions); + ESP_LOGD(TAG, "SECURITY Type:%u Username:[%s] Password:[%s] Pin:[%s]", + static_cast(this->type), this->username, maskSecret(this->password), maskSecret(this->pin)); } WifiSettings::WifiSettings() {} diff --git a/src/ConfigSettings.h b/src/ConfigSettings.h index 759a389..043ebe4 100644 --- a/src/ConfigSettings.h +++ b/src/ConfigSettings.h @@ -127,16 +127,12 @@ enum class security_types : byte { PinEntry = 0x01, Password = 0x02 }; -enum class security_permissions : byte { - ConfigOnly = 0x01 -}; class SecuritySettings: BaseSettings { public: security_types type = security_types::None; char username[33] = ""; char password[33] = ""; char pin[5] = ""; - uint8_t permissions = 0; bool begin(); bool save(); bool load(); diff --git a/src/DeviceOps.cpp b/src/DeviceOps.cpp index c44cb41..777b0aa 100644 --- a/src/DeviceOps.cpp +++ b/src/DeviceOps.cpp @@ -91,8 +91,6 @@ static void writeSettings(ProtoWriter &w, settings_section_t section) { w.add(proto_field_t::username, settings.Security.username); w.add(proto_field_t::hasPassword, hasSecret(settings.Security.password)); w.add(proto_field_t::hasPin, hasSecret(settings.Security.pin)); - // Not "Secure Config Only"'s permissions: the socket has two modes, None - // and Secured (docs/adr/0006-two-security-modes-eleven-http-routes.md). break; case settings_section_t::radio: { const transceiver_config_t &cfg = somfy.transceiver.config; diff --git a/src/HttpAccess.h b/src/HttpAccess.h new file mode 100644 index 0000000..f2f0a92 --- /dev/null +++ b/src/HttpAccess.h @@ -0,0 +1,49 @@ +#ifndef httpaccess_h +#define httpaccess_h +#include +#include + +// Who may reach what on port 80 under Secured +// (docs/adr/0006-two-security-modes-eleven-http-routes.md). Paths are the +// request's path, without the query. + +// The files the web UI loads. Static serving answers these alone, so the +// rolling codes in shades.cfg leave the device only through /backup. +inline bool isWebAsset(const char *path) { + if(!path || path[0] != '/' || strstr(path, "..")) return false; + if(strcmp(path, "/") == 0 || strcmp(path, "/appversion") == 0) return true; + static const char *const types[] = {".html", ".js", ".css", ".png", ".svg", ".ico"}; + const size_t len = strlen(path); + for(const char *type : types) { + const size_t n = strlen(type); + if(len > n + 1 && strcmp(path + len - n, type) == 0) return true; + } + return false; +} + +// True for what anyone may reach without a token: the web UI's files, what +// logging in needs, SSDP's description and preflight requests. /ws is here +// because the socket checks its own token in the handshake, from the query. +inline bool isPublicRoute(const char *path, bool preflight) { + if(preflight) return true; + if(!path) return false; + static const char *const routes[] = {"/loginContext", "/login", "/upnp.xml", "/ws"}; + for(const char *route : routes) + if(strcmp(path, route) == 0) return true; + return isWebAsset(path); +} + +// True when a socket handshake comes from a page the device served. A +// browser always sends Origin with a handshake, so another site's page open +// on the LAN can't use a token the browser holds. Clients that aren't +// browsers send none. +inline bool originMatchesHost(const char *origin, const char *host) { + if(!origin) return true; + static const char *const schemes[] = {"http://", "https://"}; + for(const char *scheme : schemes) { + const size_t n = strlen(scheme); + if(strncasecmp(origin, scheme, n) == 0) return host && host[0] && strcasecmp(origin + n, host) == 0; + } + return false; +} +#endif diff --git a/src/entryPoints/HttpRoutes.cpp b/src/entryPoints/HttpRoutes.cpp index 6a8a08f..fbec1ee 100644 --- a/src/entryPoints/HttpRoutes.cpp +++ b/src/entryPoints/HttpRoutes.cpp @@ -11,6 +11,7 @@ #include "Web.h" #include "WebRoutes.h" #include "MqttConnection.h" +#include "HttpAccess.h" #include #include #include @@ -44,12 +45,18 @@ static void onUploadDisconnect() { rebootDelay.reboot = true; rebootDelay.rebootTime = millis() + 500; } +// The token check (requireToken in Web.cpp) runs once the body is in, after +// every chunk was handled, so an upload checks the token on its first chunk +// and drops every chunk of a refused one. +static bool uploadAllowed(AsyncWebServerRequest *request, size_t index) { + if(index == 0) request->setAttribute("uploadAllowed", webServer.isAuthenticated(request)); + return request->getAttribute("uploadAllowed", false); +} void registerHttpRoutes(AsyncWebServer &server) { server.on("/loginContext", HTTP_GET, [](AsyncWebServerRequest *request) { AsyncJsonResponse *response = new AsyncJsonResponse(); JsonObject root = response->getRoot().to(); root["type"] = static_cast(settings.Security.type); - root["permissions"] = settings.Security.permissions; root["serverId"] = settings.serverId; root["version"] = settings.fwVersion.name; root["model"] = "ESPSomfyRTS"; @@ -122,6 +129,7 @@ void registerHttpRoutes(AsyncWebServer &server) { else request->send(400, _encoding_json, F("{\"status\":\"ERROR\",\"desc\":\"The upload didn't complete, so nothing was restored.\"}")); }, [](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) { + if(!uploadAllowed(request, index)) return; resetTaskWdt(); if(index == 0) { webServer.uploadSuccess = false; @@ -150,6 +158,7 @@ void registerHttpRoutes(AsyncWebServer &server) { rebootDelay.rebootTime = millis() + 500; }, [](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) { + if(!uploadAllowed(request, index)) return; if(index == 0) { webServer.uploadSuccess = false; ESP_LOGI(TAG, "Update: %s", filename.c_str()); @@ -191,6 +200,7 @@ void registerHttpRoutes(AsyncWebServer &server) { rebootDelay.rebootTime = millis() + 500; }, [](AsyncWebServerRequest *request, String filename, size_t index, uint8_t *data, size_t len, bool final) { + if(!uploadAllowed(request, index)) return; if(index == 0) { webServer.uploadSuccess = false; ESP_LOGI(TAG, "Update: %s", filename.c_str()); @@ -223,12 +233,10 @@ void registerHttpRoutes(AsyncWebServer &server) { resetTaskWdt(); }); - server.onNotFound([](AsyncWebServerRequest *r) { - if(r->method() == HTTP_OPTIONS) { r->send(200); return; } - webServer.handleNotFound(r); - }); + server.onNotFound([](AsyncWebServerRequest *r) { webServer.handleNotFound(r); }); // serveStatic MUST be registered AFTER all route handlers so it doesn't shadow them - server.serveStatic("/", LittleFS, "/").setDefaultFile("index.html"); + server.serveStatic("/", LittleFS, "/").setDefaultFile("index.html") + .setFilter([](AsyncWebServerRequest *r) { return isWebAsset(r->url().c_str()); }); } diff --git a/src/entryPoints/Web.cpp b/src/entryPoints/Web.cpp index 19797b9..37db532 100644 --- a/src/entryPoints/Web.cpp +++ b/src/entryPoints/Web.cpp @@ -16,6 +16,7 @@ #include "outputs/ProtoSocket.h" #include "Timing.h" #include "LoginThrottle.h" +#include "HttpAccess.h" #include #include #include @@ -92,27 +93,16 @@ void Web::startup() { void Web::loop() { delay(1); } -bool Web::isAuthenticated(AsyncWebServerRequest *request, bool cfg) { - ESP_LOGD(TAG, "Checking async authentication"); +bool Web::tokenAccepted(AsyncWebServerRequest *request, const char *given) { if(settings.Security.type == security_types::None) return true; - else if(!cfg && (settings.Security.permissions & static_cast(security_permissions::ConfigOnly)) == 0x01) return true; - else if(request->hasHeader("apikey")) { - ESP_LOGD(TAG, "Checking API Key..."); - char token[API_TOKEN_SIZE]; - memset(token, 0x00, sizeof(token)); - this->createAPIToken(request->client()->remoteIP(), token); - if(!tokenMatches(token, request->getHeader("apikey")->value().c_str())) { - request->send(401, _encoding_text, "Unauthorized API Key"); - return false; - } - // Key is valid - } - else { - ESP_LOGE(TAG, "Not authenticated..."); - request->send(401, _encoding_text, "Unauthorized API Key"); - return false; - } - return true; + if(!given || !request->client()) return false; + char token[API_TOKEN_SIZE]; + this->createAPIToken(request->client()->remoteIP(), token); + return tokenMatches(token, given); +} +bool Web::isAuthenticated(AsyncWebServerRequest *request) { + const AsyncWebHeader *key = request->getHeader("apikey"); + return this->tokenAccepted(request, key ? key->value().c_str() : nullptr); } bool Web::createAPIPinToken(const IPAddress ipAddress, const char *pin, char *token) { return this->createAPIToken((String(pin) + ":" + ipAddress.toString()).c_str(), token); @@ -240,7 +230,6 @@ void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { } void Web::handleShadeCommand(AsyncWebServerRequest *request, JsonVariant &json) { if(request->method() == HTTP_OPTIONS) { request->send(200); return; } - if(!this->isAuthenticated(request)) return; shade_request_t req; req.type = shade_request_type_t::shade_command; // Try query params @@ -279,7 +268,6 @@ void Web::handleShadeCommand(AsyncWebServerRequest *request, JsonVariant &json) } void Web::handleGroupCommand(AsyncWebServerRequest *request, JsonVariant &json) { if(request->method() == HTTP_OPTIONS) { request->send(200); return; } - if(!this->isAuthenticated(request)) return; shade_request_t req; req.type = shade_request_type_t::group_command; if(asyncHasParam(request, "groupId")) { @@ -315,7 +303,6 @@ void Web::handleGroupCommand(AsyncWebServerRequest *request, JsonVariant &json) } void Web::handleTiltCommand(AsyncWebServerRequest *request, JsonVariant &json) { if(request->method() == HTTP_OPTIONS) { request->send(200); return; } - if(!this->isAuthenticated(request)) return; shade_request_t req; req.type = shade_request_type_t::tilt_command; if(asyncHasParam(request, "shadeId")) { @@ -348,7 +335,6 @@ void Web::handleTiltCommand(AsyncWebServerRequest *request, JsonVariant &json) { } } void Web::handleBackup(AsyncWebServerRequest *request) { - if(!this->isAuthenticated(request)) return; bool attach = false; if(asyncHasParam(request, "attach")) attach = toBoolean(asyncParam(request, "attach").c_str(), false); ESP_LOGI(TAG, "Async saving current shade information"); @@ -368,7 +354,9 @@ void Web::handleBackup(AsyncWebServerRequest *request) { snprintf(filename, sizeof(filename), "attachment; filename=\"ESPSomfyRTS %s.backup\"", iso); AsyncWebServerResponse *response = request->beginResponse(200, _encoding_text, somfy.backupData); response->addHeader("Content-Disposition", filename); +#ifdef WEB_CORS response->addHeader("Access-Control-Expose-Headers", "Content-Disposition"); +#endif request->send(response); } else { @@ -404,13 +392,29 @@ static void timeRequest(AsyncWebServerRequest *request, ArMiddlewareNext next) { next(); logRequestTime(request, ip, "handler", timer.us()); } +// Under Secured every route but the public ones needs the token. This runs +// once the body is in, after an upload's chunks, so the uploads check the +// token themselves as well (uploadAllowed in HttpRoutes.cpp). +static void requireToken(AsyncWebServerRequest *request, ArMiddlewareNext next) { + if(isPublicRoute(request->url().c_str(), request->method() == HTTP_OPTIONS) || webServer.isAuthenticated(request)) { + next(); + return; + } + request->send(401, _encoding_text, "Unauthorized API Key"); +} void Web::begin() { ESP_LOGI(TAG, "Creating Web MicroServices..."); asyncServer.addMiddleware([](AsyncWebServerRequest *r, ArMiddlewareNext next) { timeRequest(r, next); }); + asyncServer.addMiddleware([](AsyncWebServerRequest *r, ArMiddlewareNext next) { requireToken(r, next); }); +#ifdef WEB_CORS + // For work on the UI from a file:// page (baseUrl in index.js). A release + // leaves them out: the UI comes from the device, and HA and scripts aren't + // browsers. DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*"); DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods", "PUT,POST,GET,OPTIONS"); DefaultHeaders::Instance().addHeader("Access-Control-Allow-Headers", "*"); +#endif // Before the routes, so serveStatic never answers /ws. protoSocket.begin(asyncServer); registerHttpRoutes(asyncServer); diff --git a/src/entryPoints/Web.h b/src/entryPoints/Web.h index 2423dcd..bb4a79d 100644 --- a/src/entryPoints/Web.h +++ b/src/entryPoints/Web.h @@ -16,7 +16,10 @@ class Web { bool createAPIToken(const char *payload, char *token); bool createAPIPinToken(const IPAddress ipAddress, const char *pin, char *token); bool createAPIPasswordToken(const IPAddress ipAddress, const char *username, const char *password, char *token); - bool isAuthenticated(AsyncWebServerRequest *request, bool cfg = false); + // True under None, or when given is the token for the request's client. + bool tokenAccepted(AsyncWebServerRequest *request, const char *given); + // tokenAccepted with the apikey header, which is where HTTP sends it. + bool isAuthenticated(AsyncWebServerRequest *request); // HTTP handlers void handleLogin(AsyncWebServerRequest *request, JsonVariant &json); diff --git a/src/outputs/ProtoSocket.cpp b/src/outputs/ProtoSocket.cpp index d739451..ff3a7b2 100644 --- a/src/outputs/ProtoSocket.cpp +++ b/src/outputs/ProtoSocket.cpp @@ -22,6 +22,7 @@ #include "ESPNetwork.h" #include "Utils.h" #include "../entryPoints/Web.h" +#include "HttpAccess.h" #if HAS_ETHERNET #include #endif @@ -260,13 +261,16 @@ static void queueDeviceOp(AsyncWebSocketClient *client, const proto_message_t &m if(item.save) freeSettingsSave(item.save); replyError(client, msg.id, proto_error_t::busy, "Command queue full"); } +// A browser can't set headers on a socket, so the token comes in the query. static bool authorized(AsyncWebServerRequest *request) { - if(settings.Security.type == security_types::None) return true; +#ifndef WEB_CORS + // A UI opened from a file:// page sends Origin: null, so WEB_CORS skips it. + const AsyncWebHeader *origin = request->getHeader("Origin"); + const AsyncWebHeader *host = request->getHeader("Host"); + if(!originMatchesHost(origin ? origin->value().c_str() : nullptr, host ? host->value().c_str() : nullptr)) return false; +#endif const AsyncWebParameter *key = request->getParam("apikey"); - if(!key || !request->client()) return false; - char token[API_TOKEN_SIZE]; - webServer.createAPIToken(request->client()->remoteIP(), token); - return tokenMatches(token, key->value().c_str()); + return webServer.tokenAccepted(request, key ? key->value().c_str() : nullptr); } static void onConnect(AsyncWebSocketClient *client, AsyncWebServerRequest *request) { const uint32_t id = client->id(); diff --git a/test/e2e/proto_client.py b/test/e2e/proto_client.py index 31b6f7f..984f727 100644 --- a/test/e2e/proto_client.py +++ b/test/e2e/proto_client.py @@ -79,14 +79,15 @@ class Schema: class ProtoClient: - def __init__(self, host, port=80, path="/ws", params=None, timeout=10): + def __init__(self, host, port=80, path="/ws", params=None, timeout=10, headers=None): self.host = host query = ("?" + urllib.parse.urlencode(params)) if params else "" self.sock = socket.create_connection((host, port), timeout=timeout) key = base64.b64encode(os.urandom(16)).decode() + extra = "".join(f"{k}: {v}\r\n" for k, v in (headers or {}).items()) self.sock.sendall(( f"GET {path}{query} HTTP/1.1\r\nHost: {host}:{port}\r\nUpgrade: websocket\r\n" - f"Connection: Upgrade\r\nSec-WebSocket-Key: {key}\r\nSec-WebSocket-Version: 13\r\n\r\n").encode()) + f"Connection: Upgrade\r\nSec-WebSocket-Key: {key}\r\nSec-WebSocket-Version: 13\r\n{extra}\r\n").encode()) head = b"" while b"\r\n\r\n" not in head: chunk = self.sock.recv(1024) diff --git a/test/e2e/test_routes.py b/test/e2e/test_routes.py new file mode 100644 index 0000000..bdaee7c --- /dev/null +++ b/test/e2e/test_routes.py @@ -0,0 +1,127 @@ +"""The HTTP routes and the socket's Origin check under the device's security +mode (docs/adr/0006-two-security-modes-eleven-http-routes.md). + +Like test_auth, it tests the mode it finds: pass --password or --pin for a +Secured device. +""" +import time +import urllib.error +import urllib.request + +import pytest + +from proto_client import ProtoClient, heap + +# Every route that needs the token under Secured, sent without a body or +# parameters, so a route that let one through would still change nothing. +TOKEN_ROUTES = [ + ("GET", "/backup"), + ("GET", "/shadeCommand"), + ("GET", "/groupCommand"), + ("GET", "/tiltCommand"), + ("PUT", "/shadeCommand"), + ("PUT", "/groupCommand"), + ("PUT", "/tiltCommand"), +] +PUBLIC_ROUTES = ["/", "/index.html", "/index.js", "/loginContext", "/upnp.xml"] +SHADE_FILES = ["/shades.cfg", "/shades.tmp"] + + +def http(host, path, method="GET", body=None, headers=None): + """The status and headers of a request, refusals included.""" + req = urllib.request.Request(f"http://{host}{path}", data=body, method=method, headers=headers or {}) + try: + with urllib.request.urlopen(req, timeout=20) as r: + r.read() + return r.status, r.headers + except urllib.error.HTTPError as e: + e.read() + return e.code, e.headers + + +def upload(host, path, headers=None): + """Posts a small file that isn't a backup or an image.""" + boundary = "e2e-upload" + body = ( + f"--{boundary}\r\nContent-Disposition: form-data; name=\"file\"; filename=\"e2e.bin\"\r\n" + "Content-Type: application/octet-stream\r\n\r\n").encode() + b"\0" * 256 + f"\r\n--{boundary}--\r\n".encode() + return http(host, path, "POST", body, {"Content-Type": f"multipart/form-data; boundary={boundary}", **(headers or {})}) + + +@pytest.mark.parametrize("method,path", TOKEN_ROUTES) +def test_secured_refuses_a_route_without_a_token(host, secured, method, path): + if not secured: + pytest.skip("the device's security is None") + body = b"{}" if method == "PUT" else None + status, _ = http(host, path, method, body, {"Content-Type": "application/json"} if body else None) + assert status == 401 + + +@pytest.mark.parametrize("method,path", TOKEN_ROUTES) +def test_secured_refuses_a_route_with_a_wrong_token(host, secured, method, path): + if not secured: + pytest.skip("the device's security is None") + body = b"{}" if method == "PUT" else None + headers = {"apikey": "0" * 64} + if body: + headers["Content-Type"] = "application/json" + status, _ = http(host, path, method, body, headers) + assert status == 401 + + +def test_secured_accepts_the_token_on_a_route(host, secured, token): + if not secured: + pytest.skip("the device's security is None") + status, _ = http(host, "/backup", headers={"apikey": token}) + assert status == 200 + + +@pytest.mark.parametrize("path", ["/restore", "/updateFirmware"]) +def test_secured_drops_an_upload_without_a_token(host, secured, token, path): + """The token check runs once the body is in, so the uploads check it on their first chunk. + + An /updateFirmware that got through would stop the radio and reboot the + device once the connection closed. /updateApplication isn't sent: one + that got through would write over the file system. + """ + if not secured: + pytest.skip("the device's security is None") + before = heap(host, token) + status, _ = upload(host, path) + assert status == 401 + after = heap(host, token) + assert after["uptime"] >= before["uptime"], "the device rebooted" + + +@pytest.mark.parametrize("path", PUBLIC_ROUTES) +def test_public_routes_need_no_token(host, path): + status, _ = http(host, path) + assert status == 200 + + +@pytest.mark.parametrize("path", SHADE_FILES) +def test_the_shade_files_are_not_served(host, path): + """Their rolling codes leave the device only through /backup.""" + status, _ = http(host, path) + assert status in (401, 404) + + +def test_replies_carry_no_cors_headers(host): + _, headers = http(host, "/loginContext", headers={"Origin": "http://example.com"}) + assert headers.get("Access-Control-Allow-Origin") is None + + +def test_a_socket_from_another_site_is_refused(host, token): + params = {"apikey": token} if token else None + c = ProtoClient(host, params=params, headers={"Origin": "http://example.com"}) + assert c.status == 401 + + +def test_a_socket_from_the_page_the_device_served_is_accepted(host, token): + # ProtoClient's Host names the port, and so does an Origin that matches it. + params = {"apikey": token} if token else None + c = ProtoClient(host, params=params, headers={"Origin": f"http://{host}:80"}) + assert c.status == 101 + c.close() + # The device frees the slot when it sees the disconnect. + time.sleep(0.5) diff --git a/test/test_httpaccess/test_main.cpp b/test/test_httpaccess/test_main.cpp new file mode 100644 index 0000000..4383b0f --- /dev/null +++ b/test/test_httpaccess/test_main.cpp @@ -0,0 +1,138 @@ +#include +#include "HttpAccess.h" + +void setUp() {} +void tearDown() {} + +/********************************************************************* + * Web assets + ********************************************************************/ +static void test_the_web_ui_files_are_web_assets() { + TEST_ASSERT_TRUE(isWebAsset("/")); + TEST_ASSERT_TRUE(isWebAsset("/index.html")); + TEST_ASSERT_TRUE(isWebAsset("/index.js")); + TEST_ASSERT_TRUE(isWebAsset("/app.css")); + TEST_ASSERT_TRUE(isWebAsset("/favicon.png")); + TEST_ASSERT_TRUE(isWebAsset("/apple-icon.png")); + TEST_ASSERT_TRUE(isWebAsset("/icon.svg")); + TEST_ASSERT_TRUE(isWebAsset("/appversion")); +} +static void test_the_shade_files_are_not_web_assets() { + TEST_ASSERT_FALSE(isWebAsset("/shades.cfg")); + TEST_ASSERT_FALSE(isWebAsset("/shades.tmp")); +} +static void test_a_compressed_file_is_not_asked_for_by_name() { + // serveStatic finds index.js.gz for /index.js. + TEST_ASSERT_FALSE(isWebAsset("/index.js.gz")); +} +static void test_a_type_alone_is_not_a_web_asset() { + TEST_ASSERT_FALSE(isWebAsset("/.js")); + TEST_ASSERT_FALSE(isWebAsset(".js")); + TEST_ASSERT_FALSE(isWebAsset("/js")); +} +static void test_a_path_leaving_the_root_is_not_a_web_asset() { + TEST_ASSERT_FALSE(isWebAsset("/../shades.cfg.js")); + TEST_ASSERT_FALSE(isWebAsset("/x/../index.js")); +} +static void test_a_missing_or_relative_path_is_not_a_web_asset() { + TEST_ASSERT_FALSE(isWebAsset(nullptr)); + TEST_ASSERT_FALSE(isWebAsset("")); + TEST_ASSERT_FALSE(isWebAsset("index.html")); +} + +/********************************************************************* + * Public routes + ********************************************************************/ +static void test_logging_in_and_ssdp_are_public() { + TEST_ASSERT_TRUE(isPublicRoute("/loginContext", false)); + TEST_ASSERT_TRUE(isPublicRoute("/login", false)); + TEST_ASSERT_TRUE(isPublicRoute("/upnp.xml", false)); +} +static void test_the_socket_checks_its_own_token() { + TEST_ASSERT_TRUE(isPublicRoute("/ws", false)); +} +static void test_the_web_ui_is_public() { + TEST_ASSERT_TRUE(isPublicRoute("/", false)); + TEST_ASSERT_TRUE(isPublicRoute("/index.js", false)); +} +static void test_the_other_routes_need_the_token() { + static const char *const routes[] = {"/backup", "/restore", "/updateFirmware", "/updateApplication", + "/shadeCommand", "/groupCommand", "/tiltCommand"}; + for(const char *route : routes) TEST_ASSERT_FALSE_MESSAGE(isPublicRoute(route, false), route); +} +static void test_an_unknown_path_needs_the_token() { + TEST_ASSERT_FALSE(isPublicRoute("/shades.cfg", false)); + TEST_ASSERT_FALSE(isPublicRoute("/controller", false)); + TEST_ASSERT_FALSE(isPublicRoute(nullptr, false)); +} +static void test_a_route_differing_in_case_or_length_needs_the_token() { + TEST_ASSERT_FALSE(isPublicRoute("/Login", false)); + TEST_ASSERT_FALSE(isPublicRoute("/login/", false)); + TEST_ASSERT_FALSE(isPublicRoute("/logins", false)); + TEST_ASSERT_FALSE(isPublicRoute("/ws/", false)); +} +static void test_a_preflight_is_public() { + TEST_ASSERT_TRUE(isPublicRoute("/backup", true)); +} + +/********************************************************************* + * Origin and Host + ********************************************************************/ +static void test_the_page_the_device_served_matches() { + TEST_ASSERT_TRUE(originMatchesHost("http://192.168.1.20", "192.168.1.20")); + TEST_ASSERT_TRUE(originMatchesHost("http://espsomfyrts.local", "espsomfyrts.local")); + TEST_ASSERT_TRUE(originMatchesHost("http://192.168.1.20:8000", "192.168.1.20:8000")); +} +static void test_https_behind_a_proxy_matches() { + TEST_ASSERT_TRUE(originMatchesHost("https://somfy.example.com", "somfy.example.com")); +} +static void test_host_names_match_whatever_their_case() { + TEST_ASSERT_TRUE(originMatchesHost("http://ESPSomfyRTS.local", "espsomfyrts.local")); + TEST_ASSERT_TRUE(originMatchesHost("HTTP://192.168.1.20", "192.168.1.20")); +} +static void test_another_site_does_not_match() { + TEST_ASSERT_FALSE(originMatchesHost("http://evil.example", "192.168.1.20")); + TEST_ASSERT_FALSE(originMatchesHost("http://192.168.1.20.evil.example", "192.168.1.20")); + TEST_ASSERT_FALSE(originMatchesHost("http://192.168.1.2", "192.168.1.20")); +} +static void test_another_port_does_not_match() { + TEST_ASSERT_FALSE(originMatchesHost("http://192.168.1.20:8000", "192.168.1.20")); +} +static void test_a_page_opened_from_a_file_does_not_match() { + TEST_ASSERT_FALSE(originMatchesHost("null", "192.168.1.20")); + TEST_ASSERT_FALSE(originMatchesHost("file://", "192.168.1.20")); +} +static void test_an_origin_without_a_host_does_not_match() { + TEST_ASSERT_FALSE(originMatchesHost("http://192.168.1.20", "")); + TEST_ASSERT_FALSE(originMatchesHost("http://", "")); + TEST_ASSERT_FALSE(originMatchesHost("http://192.168.1.20", nullptr)); +} +static void test_a_client_that_is_not_a_browser_sends_no_origin() { + TEST_ASSERT_TRUE(originMatchesHost(nullptr, "192.168.1.20")); +} + +int main(int, char **) { + UNITY_BEGIN(); + RUN_TEST(test_the_web_ui_files_are_web_assets); + RUN_TEST(test_the_shade_files_are_not_web_assets); + RUN_TEST(test_a_compressed_file_is_not_asked_for_by_name); + RUN_TEST(test_a_type_alone_is_not_a_web_asset); + RUN_TEST(test_a_path_leaving_the_root_is_not_a_web_asset); + RUN_TEST(test_a_missing_or_relative_path_is_not_a_web_asset); + RUN_TEST(test_logging_in_and_ssdp_are_public); + RUN_TEST(test_the_socket_checks_its_own_token); + RUN_TEST(test_the_web_ui_is_public); + RUN_TEST(test_the_other_routes_need_the_token); + RUN_TEST(test_an_unknown_path_needs_the_token); + RUN_TEST(test_a_route_differing_in_case_or_length_needs_the_token); + RUN_TEST(test_a_preflight_is_public); + RUN_TEST(test_the_page_the_device_served_matches); + RUN_TEST(test_https_behind_a_proxy_matches); + RUN_TEST(test_host_names_match_whatever_their_case); + RUN_TEST(test_another_site_does_not_match); + RUN_TEST(test_another_port_does_not_match); + RUN_TEST(test_a_page_opened_from_a_file_does_not_match); + RUN_TEST(test_an_origin_without_a_host_does_not_match); + RUN_TEST(test_a_client_that_is_not_a_browser_sends_no_origin); + return UNITY_END(); +} From 0ee52d6d9f0786f0fe7d8616f446120a1a6d2d17 Mon Sep 17 00:00:00 2001 From: cjkas Date: Wed, 28 Oct 2026 19:39:51 +0100 Subject: [PATCH 2/2] fix: close the token bypass below a route, review fixes for 4b The routes matched any path below them, and a web asset name there passed the token check: GET /backup/a.js returned the backup under Secured. Every route is now an exact match, and a web asset sits in the root only. - e2e: those paths answer 401 or 404; the restore test now fails when the chunks skip the token check (checked with the check removed). - WEB_CORS lets the socket take Origin: null instead of skipping the Origin check. - ADR 0006, GLOSSARY and README: the Origin check in either mode, /ws as a public route, the Host header behind a reverse proxy. Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 2 +- README.md | 2 + docs/SECURITY_PLAN.md | 2 +- ...6-two-security-modes-eleven-http-routes.md | 2 +- platformio.ini | 4 +- src/HttpAccess.h | 7 +- src/entryPoints/HttpRoutes.cpp | 30 ++++--- src/outputs/ProtoSocket.cpp | 8 +- test/e2e/test_routes.py | 86 +++++++++++++------ test/test_httpaccess/test_main.cpp | 13 +++ 10 files changed, 105 insertions(+), 51 deletions(-) diff --git a/GLOSSARY.md b/GLOSSARY.md index 67f3f7c..710ab4b 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -137,7 +137,7 @@ Whether the device asks for login: *None* (everything open) or *Secured* (a PIN _Avoid_: Secure Config Only (removed) **Public route** -What anyone may reach under Secured without a token: the web UI's files, `/loginContext`, `/login`, `/upnp.xml` and preflight requests. Static serving answers the web UI's files alone, so `shades.cfg` isn't one. +What anyone may reach under Secured without a token: the web UI's files, `/loginContext`, `/login`, `/upnp.xml` and preflight requests. `/ws` counts as one at the HTTP level, because the socket checks the token itself in the handshake. Static serving answers the web UI's files alone, so `shades.cfg` isn't one. **Token** What logging in returns. It stands for the PIN or password, on one client address, and every request under Secured carries it. diff --git a/README.md b/README.md index 8004628..b19ddfe 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,8 @@ Since 4.0.0 everything is on port 80. Ports 8081 (HTTP API) and 8080 (events soc A script that called port 8081 only changes the port, if it used one of the three commands. Reads such as `/controller`, `/shades` and `/discovery` are gone: use the socket, or MQTT. The Home Assistant integration needs version 3.0.0 or newer. When the device is Secured (a PIN or password), every route above except the web UI and the login routes needs the token, in an `apikey` header; the socket takes it as `?apikey=`. Without it the reply is 401. "Secure Config Only" is gone: a device set to it is Secured from 4.0.0, so scripts that sent commands without logging in now need the token. + +In either mode the socket refuses an upgrade whose `Origin` header doesn't name the host it was sent to, so another site's page can't open it. Behind a reverse proxy, pass the `Host` header through unchanged. ## Sources for this Project I spent some time reading about a myriad of topics but in the end the primary source for this project comes from https://pushstack.wordpress.com/somfy-rts-protocol/. The work done on pushstack regarding the protocol timing made this feasible without burning a bunch of time measuring pulses. diff --git a/docs/SECURITY_PLAN.md b/docs/SECURITY_PLAN.md index b3139f1..53a6d67 100644 --- a/docs/SECURITY_PLAN.md +++ b/docs/SECURITY_PLAN.md @@ -43,7 +43,7 @@ ### CORS - The wildcard `Access-Control-Allow-*` default headers go. The UI is served from the same origin, and HA and scripts aren't browsers, so CORS doesn't affect them. -- A build flag (`WEB_CORS` in `platformio.ini`) turns the headers back on for UI development from `file://`, which `baseUrl` in `index.js` supports. It also skips the Origin check, since such a page sends `Origin: null`. +- A build flag (`WEB_CORS` in `platformio.ini`) turns the headers back on for UI development from `file://`, which `baseUrl` in `index.js` supports. It also lets the socket take `Origin: null`, which such a page sends. ### UI - The token is kept in `sessionStorage`: it survives a reload, is dropped when the tab closes, and is cleared when the socket upgrade is refused. diff --git a/docs/adr/0006-two-security-modes-eleven-http-routes.md b/docs/adr/0006-two-security-modes-eleven-http-routes.md index 7ccb633..b482ff5 100644 --- a/docs/adr/0006-two-security-modes-eleven-http-routes.md +++ b/docs/adr/0006-two-security-modes-eleven-http-routes.md @@ -18,7 +18,7 @@ With [ADR 0005](0005-one-websocket-protocol.md) moving everything else to the we Static serving covers the web assets only, so `shades.cfg` and the other config files are never served. The script commands keep today's full field names, since people write those requests by hand. `/discovery` goes: Home Assistant's setup reads the device info from `/loginContext`, and mDNS still advertises `serverId`. -There are two security modes. **None** leaves everything open. **Secured** (PIN or password) needs the token on the socket upgrade (`?apikey=`) and on every route except the static files, `/loginContext`, `/login` and `/upnp.xml`. Devices set to "Secure Config Only" become Secured when they upgrade. The rest of the security work (the token key, the login throttle, CORS, the Origin check) is in [SECURITY_PLAN.md](../SECURITY_PLAN.md). +There are two security modes. **None** leaves everything open, except that a socket upgrade from another site's page is refused in either mode (the Origin check). **Secured** (PIN or password) needs the token on the socket upgrade (`?apikey=`) and on every route except the static files, `/loginContext`, `/login` and `/upnp.xml`. Devices set to "Secure Config Only" become Secured when they upgrade. The rest of the security work (the token key, the login throttle, CORS, the Origin check) is in [SECURITY_PLAN.md](../SECURITY_PLAN.md). ## Considered Options diff --git a/platformio.ini b/platformio.ini index 31c1523..e7e5b60 100644 --- a/platformio.ini +++ b/platformio.ini @@ -43,8 +43,8 @@ build_flags = ; interleave. The core already wraps log_printf for ESP Insights. -Wl,--wrap=log_printfv ; For work on the web UI from a file:// page (baseUrl in index.js): sends - ; the CORS headers and lets the socket skip its Origin check. Never in a - ; release (docs/SECURITY_PLAN.md, CORS). + ; the CORS headers and lets the socket take Origin: null, which such a page + ; sends. Never in a release (docs/SECURITY_PLAN.md, CORS). ; -DWEB_CORS monitor_speed = 115200 monitor_filters = diff --git a/src/HttpAccess.h b/src/HttpAccess.h index f2f0a92..0394413 100644 --- a/src/HttpAccess.h +++ b/src/HttpAccess.h @@ -7,10 +7,11 @@ // (docs/adr/0006-two-security-modes-eleven-http-routes.md). Paths are the // request's path, without the query. -// The files the web UI loads. Static serving answers these alone, so the -// rolling codes in shades.cfg leave the device only through /backup. +// The files the web UI loads, which all sit in the root. Static serving +// answers these alone, so the rolling codes in shades.cfg leave the device +// only through /backup. A path below a route, such as /backup/a.js, isn't one. inline bool isWebAsset(const char *path) { - if(!path || path[0] != '/' || strstr(path, "..")) return false; + if(!path || path[0] != '/' || strchr(path + 1, '/') || strstr(path, "..")) return false; if(strcmp(path, "/") == 0 || strcmp(path, "/appversion") == 0) return true; static const char *const types[] = {".html", ".js", ".css", ".png", ".svg", ".ico"}; const size_t len = strlen(path); diff --git a/src/entryPoints/HttpRoutes.cpp b/src/entryPoints/HttpRoutes.cpp index fbec1ee..440bc6e 100644 --- a/src/entryPoints/HttpRoutes.cpp +++ b/src/entryPoints/HttpRoutes.cpp @@ -52,8 +52,10 @@ static bool uploadAllowed(AsyncWebServerRequest *request, size_t index) { if(index == 0) request->setAttribute("uploadAllowed", webServer.isAuthenticated(request)); return request->getAttribute("uploadAllowed", false); } +// Every route is an exact match. The library's default also takes any path +// below the route, and /backup/a.js then passed the token check as a web asset. void registerHttpRoutes(AsyncWebServer &server) { - server.on("/loginContext", HTTP_GET, [](AsyncWebServerRequest *request) { + server.on(AsyncURIMatcher::exact("/loginContext"), HTTP_GET, [](AsyncWebServerRequest *request) { AsyncJsonResponse *response = new AsyncJsonResponse(); JsonObject root = response->getRoot().to(); root["type"] = static_cast(settings.Security.type); @@ -65,31 +67,31 @@ void registerHttpRoutes(AsyncWebServer &server) { request->send(response); }); // Commands for scripts, as a JSON body or query parameters. - server.addHandler(new AsyncCallbackJsonWebHandler("/shadeCommand", + server.addHandler(new AsyncCallbackJsonWebHandler(AsyncURIMatcher::exact("/shadeCommand"), [](AsyncWebServerRequest *request, JsonVariant &json) { webServer.handleShadeCommand(request, json); })); - server.on("/shadeCommand", HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleShadeCommand(request, v); }); + server.on(AsyncURIMatcher::exact("/shadeCommand"), HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleShadeCommand(request, v); }); - server.addHandler(new AsyncCallbackJsonWebHandler("/groupCommand", + server.addHandler(new AsyncCallbackJsonWebHandler(AsyncURIMatcher::exact("/groupCommand"), [](AsyncWebServerRequest *request, JsonVariant &json) { webServer.handleGroupCommand(request, json); })); - server.on("/groupCommand", HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleGroupCommand(request, v); }); + server.on(AsyncURIMatcher::exact("/groupCommand"), HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleGroupCommand(request, v); }); - server.addHandler(new AsyncCallbackJsonWebHandler("/tiltCommand", + server.addHandler(new AsyncCallbackJsonWebHandler(AsyncURIMatcher::exact("/tiltCommand"), [](AsyncWebServerRequest *request, JsonVariant &json) { webServer.handleTiltCommand(request, json); })); - server.on("/tiltCommand", HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleTiltCommand(request, v); }); + server.on(AsyncURIMatcher::exact("/tiltCommand"), HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleTiltCommand(request, v); }); - server.on("/upnp.xml", HTTP_GET, [](AsyncWebServerRequest *request) { + server.on(AsyncURIMatcher::exact("/upnp.xml"), HTTP_GET, [](AsyncWebServerRequest *request) { AsyncResponseStream *response = request->beginResponseStream("text/xml"); SSDP.schema(*response); request->send(response); }); - server.addHandler(new AsyncCallbackJsonWebHandler("/login", + server.addHandler(new AsyncCallbackJsonWebHandler(AsyncURIMatcher::exact("/login"), [](AsyncWebServerRequest *request, JsonVariant &json) { webServer.handleLogin(request, json); })); - server.on("/login", HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleLogin(request, v); }); + server.on(AsyncURIMatcher::exact("/login"), HTTP_GET, [](AsyncWebServerRequest *request) { JsonVariant v; webServer.handleLogin(request, v); }); - server.on("/backup", HTTP_GET, [](AsyncWebServerRequest *request) { webServer.handleBackup(request); }); + server.on(AsyncURIMatcher::exact("/backup"), HTTP_GET, [](AsyncWebServerRequest *request) { webServer.handleBackup(request); }); - server.on("/restore", HTTP_POST, + server.on(AsyncURIMatcher::exact("/restore"), HTTP_POST, [](AsyncWebServerRequest *request) { if(webServer.uploadSuccess) { restore_options_t opts; @@ -148,7 +150,7 @@ void registerHttpRoutes(AsyncWebServer &server) { }); // updateFirmware - file upload - server.on("/updateFirmware", HTTP_POST, + server.on(AsyncURIMatcher::exact("/updateFirmware"), HTTP_POST, [](AsyncWebServerRequest *request) { if(Update.hasError()) request->send(500, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"Error updating firmware: \"}"); @@ -190,7 +192,7 @@ void registerHttpRoutes(AsyncWebServer &server) { }); // updateApplication - file upload - server.on("/updateApplication", HTTP_POST, + server.on(AsyncURIMatcher::exact("/updateApplication"), HTTP_POST, [](AsyncWebServerRequest *request) { if(Update.hasError()) request->send(500, _encoding_json, "{\"status\":\"ERROR\",\"desc\":\"Error updating application: \"}"); diff --git a/src/outputs/ProtoSocket.cpp b/src/outputs/ProtoSocket.cpp index ff3a7b2..81c5533 100644 --- a/src/outputs/ProtoSocket.cpp +++ b/src/outputs/ProtoSocket.cpp @@ -263,12 +263,14 @@ static void queueDeviceOp(AsyncWebSocketClient *client, const proto_message_t &m } // A browser can't set headers on a socket, so the token comes in the query. static bool authorized(AsyncWebServerRequest *request) { -#ifndef WEB_CORS - // A UI opened from a file:// page sends Origin: null, so WEB_CORS skips it. const AsyncWebHeader *origin = request->getHeader("Origin"); const AsyncWebHeader *host = request->getHeader("Host"); - if(!originMatchesHost(origin ? origin->value().c_str() : nullptr, host ? host->value().c_str() : nullptr)) return false; + const char *from = origin ? origin->value().c_str() : nullptr; +#ifdef WEB_CORS + // The UI opened from a file:// page sends Origin: null. + if(from && strcmp(from, "null") == 0) from = nullptr; #endif + if(!originMatchesHost(from, host ? host->value().c_str() : nullptr)) return false; const AsyncWebParameter *key = request->getParam("apikey"); return webServer.tokenAccepted(request, key ? key->value().c_str() : nullptr); } diff --git a/test/e2e/test_routes.py b/test/e2e/test_routes.py index bdaee7c..9072568 100644 --- a/test/e2e/test_routes.py +++ b/test/e2e/test_routes.py @@ -4,13 +4,14 @@ mode (docs/adr/0006-two-security-modes-eleven-http-routes.md). Like test_auth, it tests the mode it finds: pass --password or --pin for a Secured device. """ +import json import time import urllib.error import urllib.request import pytest -from proto_client import ProtoClient, heap +from proto_client import ProtoClient # Every route that needs the token under Secured, sent without a body or # parameters, so a route that let one through would still change nothing. @@ -28,23 +29,25 @@ SHADE_FILES = ["/shades.cfg", "/shades.tmp"] def http(host, path, method="GET", body=None, headers=None): - """The status and headers of a request, refusals included.""" + """The status, headers and body of a request, refusals included.""" req = urllib.request.Request(f"http://{host}{path}", data=body, method=method, headers=headers or {}) try: with urllib.request.urlopen(req, timeout=20) as r: - r.read() - return r.status, r.headers + return r.status, r.headers, r.read() except urllib.error.HTTPError as e: - e.read() - return e.code, e.headers + return e.code, e.headers, e.read() -def upload(host, path, headers=None): - """Posts a small file that isn't a backup or an image.""" +def upload(host, path, headers=None, with_file=True): + """Posts a small file that isn't a backup or an image, or a form without a file.""" boundary = "e2e-upload" - body = ( - f"--{boundary}\r\nContent-Disposition: form-data; name=\"file\"; filename=\"e2e.bin\"\r\n" - "Content-Type: application/octet-stream\r\n\r\n").encode() + b"\0" * 256 + f"\r\n--{boundary}--\r\n".encode() + if with_file: + part = ( + f"--{boundary}\r\nContent-Disposition: form-data; name=\"file\"; filename=\"e2e.bin\"\r\n" + "Content-Type: application/octet-stream\r\n\r\n").encode() + b"\0" * 256 + else: + part = f"--{boundary}\r\nContent-Disposition: form-data; name=\"e2e\"\r\n\r\n1".encode() + body = part + f"\r\n--{boundary}--\r\n".encode() return http(host, path, "POST", body, {"Content-Type": f"multipart/form-data; boundary={boundary}", **(headers or {})}) @@ -53,7 +56,7 @@ def test_secured_refuses_a_route_without_a_token(host, secured, method, path): if not secured: pytest.skip("the device's security is None") body = b"{}" if method == "PUT" else None - status, _ = http(host, path, method, body, {"Content-Type": "application/json"} if body else None) + status, _, _ = http(host, path, method, body, {"Content-Type": "application/json"} if body else None) assert status == 401 @@ -65,49 +68,80 @@ def test_secured_refuses_a_route_with_a_wrong_token(host, secured, method, path) headers = {"apikey": "0" * 64} if body: headers["Content-Type"] = "application/json" - status, _ = http(host, path, method, body, headers) + status, _, _ = http(host, path, method, body, headers) assert status == 401 +@pytest.mark.parametrize("method,path", TOKEN_ROUTES + [ + ("POST", "/restore"), ("POST", "/updateFirmware"), ("POST", "/updateApplication")]) +def test_secured_refuses_a_web_asset_name_below_a_route(host, secured, method, path): + """/backup/a.js once reached /backup and passed the token check as a web asset. + + The uploads are sent without a body: one that got through would reboot + the device. + """ + if not secured: + pytest.skip("the device's security is None") + body = b"{}" if method == "PUT" else None + status, _, _ = http(host, path + "/a.js", method, body, {"Content-Type": "application/json"} if body else None) + assert status in (401, 404) + + def test_secured_accepts_the_token_on_a_route(host, secured, token): if not secured: pytest.skip("the device's security is None") - status, _ = http(host, "/backup", headers={"apikey": token}) + status, _, _ = http(host, "/backup", headers={"apikey": token}) assert status == 200 @pytest.mark.parametrize("path", ["/restore", "/updateFirmware"]) -def test_secured_drops_an_upload_without_a_token(host, secured, token, path): - """The token check runs once the body is in, so the uploads check it on their first chunk. +def test_secured_refuses_an_upload_without_a_token(host, secured, path): + """/updateApplication isn't sent: one that got through would write over the file system.""" + if not secured: + pytest.skip("the device's security is None") + status, _, _ = upload(host, path) + assert status == 401 - An /updateFirmware that got through would stop the radio and reboot the - device once the connection closed. /updateApplication isn't sent: one - that got through would write over the file system. + +def restore_without_a_file(host, token): + status, _, body = upload(host, "/restore", {"apikey": token}, with_file=False) + return status, json.loads(body or b"{}").get("desc", "") + + +def test_secured_drops_the_chunks_of_an_upload_without_a_token(host, secured, token): + """The token check runs once the body is in, so /restore's chunks check the token themselves. + + A chunk that got through would mark the upload complete. /restore with + the token and no file would then find the refused file and call it + invalid, instead of saying no upload completed. Neither restores anything. """ if not secured: pytest.skip("the device's security is None") - before = heap(host, token) - status, _ = upload(host, path) + status, desc = restore_without_a_file(host, token) + if status != 400 or "didn't complete" not in desc: + pytest.skip("an earlier upload is still marked complete: reboot the device") + status, _, _ = upload(host, "/restore") assert status == 401 - after = heap(host, token) - assert after["uptime"] >= before["uptime"], "the device rebooted" + status, desc = restore_without_a_file(host, token) + assert status == 400 + assert "didn't complete" in desc, desc @pytest.mark.parametrize("path", PUBLIC_ROUTES) def test_public_routes_need_no_token(host, path): - status, _ = http(host, path) + status, _, _ = http(host, path) assert status == 200 @pytest.mark.parametrize("path", SHADE_FILES) def test_the_shade_files_are_not_served(host, path): """Their rolling codes leave the device only through /backup.""" - status, _ = http(host, path) + status, _, _ = http(host, path) assert status in (401, 404) def test_replies_carry_no_cors_headers(host): - _, headers = http(host, "/loginContext", headers={"Origin": "http://example.com"}) + _, headers, _ = http(host, "/loginContext", headers={"Origin": "http://example.com"}) assert headers.get("Access-Control-Allow-Origin") is None diff --git a/test/test_httpaccess/test_main.cpp b/test/test_httpaccess/test_main.cpp index 4383b0f..db2c7c3 100644 --- a/test/test_httpaccess/test_main.cpp +++ b/test/test_httpaccess/test_main.cpp @@ -34,6 +34,11 @@ static void test_a_path_leaving_the_root_is_not_a_web_asset() { TEST_ASSERT_FALSE(isWebAsset("/../shades.cfg.js")); TEST_ASSERT_FALSE(isWebAsset("/x/../index.js")); } +static void test_a_file_below_the_root_is_not_a_web_asset() { + TEST_ASSERT_FALSE(isWebAsset("/backup/a.js")); + TEST_ASSERT_FALSE(isWebAsset("/js/index.js")); + TEST_ASSERT_FALSE(isWebAsset("//index.js")); +} static void test_a_missing_or_relative_path_is_not_a_web_asset() { TEST_ASSERT_FALSE(isWebAsset(nullptr)); TEST_ASSERT_FALSE(isWebAsset("")); @@ -71,6 +76,12 @@ static void test_a_route_differing_in_case_or_length_needs_the_token() { TEST_ASSERT_FALSE(isPublicRoute("/logins", false)); TEST_ASSERT_FALSE(isPublicRoute("/ws/", false)); } +static void test_a_web_asset_name_below_a_route_needs_the_token() { + TEST_ASSERT_FALSE(isPublicRoute("/backup/a.js", false)); + TEST_ASSERT_FALSE(isPublicRoute("/shadeCommand/a.png", false)); + TEST_ASSERT_FALSE(isPublicRoute("/updateFirmware/x.js", false)); + TEST_ASSERT_FALSE(isPublicRoute("/login/a.js", false)); +} static void test_a_preflight_is_public() { TEST_ASSERT_TRUE(isPublicRoute("/backup", true)); } @@ -118,6 +129,7 @@ int main(int, char **) { RUN_TEST(test_a_compressed_file_is_not_asked_for_by_name); RUN_TEST(test_a_type_alone_is_not_a_web_asset); RUN_TEST(test_a_path_leaving_the_root_is_not_a_web_asset); + RUN_TEST(test_a_file_below_the_root_is_not_a_web_asset); RUN_TEST(test_a_missing_or_relative_path_is_not_a_web_asset); RUN_TEST(test_logging_in_and_ssdp_are_public); RUN_TEST(test_the_socket_checks_its_own_token); @@ -125,6 +137,7 @@ int main(int, char **) { RUN_TEST(test_the_other_routes_need_the_token); RUN_TEST(test_an_unknown_path_needs_the_token); RUN_TEST(test_a_route_differing_in_case_or_length_needs_the_token); + RUN_TEST(test_a_web_asset_name_below_a_route_needs_the_token); RUN_TEST(test_a_preflight_is_public); RUN_TEST(test_the_page_the_device_served_matches); RUN_TEST(test_https_behind_a_proxy_matches);