diff --git a/GLOSSARY.md b/GLOSSARY.md index 85564f0..f2bba7e 100644 --- a/GLOSSARY.md +++ b/GLOSSARY.md @@ -148,6 +148,10 @@ _Avoid_: Secure Config Only (removed) 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) +**Token key** +The secret a token is made with: random bytes the device makes on first boot and never sends anywhere. Changing it refuses every token. +_Avoid_: API key, server key + **Login throttle** How the device slows down guessing: a client address with too many failed logins is refused for a while, and each refusal is a *lockout*, longer than the one before. _Avoid_: login backoff, rate limit diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index 1472703..592edee 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -5,8 +5,6 @@ - [#77](https://github.com/cjkas/ESPSomfy-RTS/issues/77) Port 80 routes don't check login - [#78](https://github.com/cjkas/ESPSomfy-RTS/issues/78) CORS allows any origin - [#79](https://github.com/cjkas/ESPSomfy-RTS/issues/79) Websocket has no auth -- [#80](https://github.com/cjkas/ESPSomfy-RTS/issues/80) API token key is public -- [#81](https://github.com/cjkas/ESPSomfy-RTS/issues/81) `/login` has no rate limit - [#82](https://github.com/cjkas/ESPSomfy-RTS/issues/82) A saved WiFi passphrase or MQTT password can't be cleared from the UI - [#83](https://github.com/cjkas/ESPSomfy-RTS/issues/83) Installing "Main" from the update dialog always fails - [#84](https://github.com/cjkas/ESPSomfy-RTS/issues/84) Transmitting a frame stops `loop()` for about 365 ms diff --git a/docs/SECURITY_PLAN.md b/docs/SECURITY_PLAN.md index 72de503..008d9c9 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:** agreed, nothing built yet. +**Status:** steps 1 to 3, PR B and PR C built; step 4 next. **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 @@ -50,8 +50,9 @@ ### Token key (PR B) - The token keeps its shape, `HMAC(key, PIN-or-password + client IP)`: stateless, and it survives reboots. Only the key changes. -- **Key:** 32 random bytes from `esp_fill_random`, generated on first boot in their own NVS namespace. Never sent anywhere, never in a backup, replaced only when NVS is erased. There is no rotate button: changing the PIN or password already changes every token. -- **Comparison:** constant-time, used for HTTP and the websocket handshake alike. +- **Key:** 32 random bytes from `esp_fill_random`, generated on first boot in their own NVS namespace (`TOKEN`). Never sent anywhere, never in a backup, replaced only when NVS is erased. There is no rotate button: changing the PIN or password already changes every token. + - The key is made in `Web::begin`, before WiFi starts, so the RNG takes its entropy from the bootloader's source (`bootloader_random_enable`) rather than the radio. +- **Comparison:** constant-time (`tokenMatches` in [src/ApiToken.h](../src/ApiToken.h)), used for HTTP, the websocket handshake and `/login` alike. - **Migration:** none needed. The browser logs in again when its token is refused, and HA logs in again at runtime. ### Login throttle (PR C) @@ -71,7 +72,7 @@ - Today HA logs in only during the config flow, on a temporary API object, and sends `apikey: ""` at runtime. It only works with security None or Secure Config Only. - **Setup:** the config flow reads the device info and security mode from `/loginContext`, then logs in with `/login`. - **Login:** at `async_setup_entry` it logs in with the PIN or password stored in `entry.data`. On a refusal it logs in again once and retries; if that fails, it raises HA's reauth flow. -- **Socket:** it connects to `ws://host/ws?apikey=` and speaks protocol 2. Against firmware that sends no `hello`, it falls back to the old HTTP API on 8081 and events on 8080. +- **Socket:** it connects to `ws://host/ws?apikey=` and speaks protocol 2. Against firmware whose `/ws` answers 404 or serves a web page, it falls back to the old HTTP API on 8081 and events on 8080. ## Order @@ -97,7 +98,7 @@ The rules go into small pure helpers with native tests, following [src/Secrets.h - the Origin/Host match - the login throttle's schedule -The e2e suite (`test/e2e`) checks, when given credentials, that a socket without a token is refused, that the HTTP routes answer 401, and that a token works. It never changes the device's security settings itself. +The e2e suite (`test/e2e`) checks, when given credentials, that a socket without a token is refused, that the HTTP routes answer 401, that a token keyed with `serverId` is refused, and that a token works. It never changes the device's security settings itself. ## Not in this plan diff --git a/src/ApiToken.h b/src/ApiToken.h new file mode 100644 index 0000000..13cfa7f --- /dev/null +++ b/src/ApiToken.h @@ -0,0 +1,26 @@ +#ifndef apitoken_h +#define apitoken_h +#include +#include + +// A token is HMAC-SHA256(token key, PIN or username:password + ":" + client +// IP) in hex. The token key is 32 random bytes the device makes on first boot +// and keeps in NVS: it never leaves the device. +#define API_TOKEN_KEY_SIZE 32 + +// True when given is the expected token. It takes the same time whatever +// given holds, so the time doesn't tell how much of a guess was right. Only +// expected's length shows, and every token has the same one. +inline bool tokenMatches(const char *expected, const char *given) { + if(!expected || !given || !expected[0]) return false; + uint8_t diff = 0; + for(size_t i = 0; expected[i]; i++) { + diff |= (uint8_t)(expected[i] ^ *given); + // A shorter given compares its terminator with the rest of expected. + if(*given) given++; + } + // A longer given doesn't match either. + diff |= (uint8_t)*given; + return diff == 0; +} +#endif diff --git a/src/entryPoints/Web.cpp b/src/entryPoints/Web.cpp index f75adb8..23c578b 100644 --- a/src/entryPoints/Web.cpp +++ b/src/entryPoints/Web.cpp @@ -5,6 +5,10 @@ #include #include "esp_log.h" #include "mbedtls/md.h" +#include +#include +#include +#include "ApiToken.h" #include "ConfigSettings.h" #include "ConfigFile.h" #include "Utils.h" @@ -59,6 +63,24 @@ static uint32_t loginKey(const IPAddress &ip) { return (uint32_t)ip; } // The throttle's clock, in ms, which doesn't wrap as millis() does. static uint64_t loginNow() { return (uint64_t)esp_timer_get_time() / 1000; } +// The tokens' HMAC key. It has its own NVS namespace, which no settings +// save clears and no backup holds, so only erasing NVS replaces it. +static uint8_t tokenKey[API_TOKEN_KEY_SIZE]; +// Reads the token key, or makes one on first boot. Runs before the radio is +// on, so the hardware RNG gets its entropy from the bootloader's source. +static void loadTokenKey() { + Preferences pref; + pref.begin("TOKEN"); + if(pref.getBytes("key", tokenKey, sizeof(tokenKey)) != sizeof(tokenKey)) { + bootloader_random_enable(); + esp_fill_random(tokenKey, sizeof(tokenKey)); + bootloader_random_disable(); + if(pref.putBytes("key", tokenKey, sizeof(tokenKey)) == sizeof(tokenKey)) ESP_LOGI(TAG, "Made a new token key"); + else ESP_LOGE(TAG, "Couldn't store the new token key: every token changes at the next boot"); + } + pref.end(); +} + // Hands an item to loop(), which owns the shades. When it can't be the // request is answered with the reason and false is returned. bool queueLoopItem(AsyncWebServerRequest *request, const loop_item_t &item) { @@ -101,7 +123,7 @@ bool Web::isAuthenticated(AsyncWebServerRequest *request, bool cfg) { char token[65]; memset(token, 0x00, sizeof(token)); this->createAPIToken(request->client()->remoteIP(), token); - if(String(token) != request->getHeader("apikey")->value()) { + if(!tokenMatches(token, request->getHeader("apikey")->value().c_str())) { request->send(401, _encoding_text, "Unauthorized API Key"); return false; } @@ -124,10 +146,12 @@ bool Web::createAPIToken(const char *payload, char *token) { byte hmacResult[32]; mbedtls_md_context_t ctx; mbedtls_md_type_t md_type = MBEDTLS_MD_SHA256; + mbedtls_md_init(&ctx); mbedtls_md_setup(&ctx, mbedtls_md_info_from_type(md_type), 1); - mbedtls_md_hmac_starts(&ctx, (const unsigned char *)settings.serverId, strlen(settings.serverId)); - mbedtls_md_hmac_update(&ctx, (const unsigned char *)payload, strlen(payload)); + mbedtls_md_hmac_starts(&ctx, tokenKey, sizeof(tokenKey)); + mbedtls_md_hmac_update(&ctx, (const unsigned char *)payload, strlen(payload)); mbedtls_md_hmac_finish(&ctx, hmacResult); + mbedtls_md_free(&ctx); token[0] = '\0'; for(int i = 0; i < sizeof(hmacResult); i++){ char str[3]; @@ -378,13 +402,13 @@ void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { char ptok[65]; memset(ptok, 0x00, sizeof(ptok)); this->createAPIPinToken(request->client()->remoteIP(), pin, ptok); - if(String(ptok) == String(token)) success = true; + success = tokenMatches(token, ptok); } else if(settings.Security.type == security_types::Password) { char ptok[65]; memset(ptok, 0x00, sizeof(ptok)); this->createAPIPasswordToken(request->client()->remoteIP(), username, password, ptok); - if(String(ptok) == String(token)) success = true; + success = tokenMatches(token, ptok); } if(success) { loginThrottle.succeeded(key); @@ -765,6 +789,7 @@ static void timeRequest(AsyncWebServerRequest *request, ArMiddlewareNext next, c void Web::begin() { ESP_LOGI(TAG, "Creating Web MicroServices..."); + loadTokenKey(); asyncApiServer.addMiddleware([](AsyncWebServerRequest *r, ArMiddlewareNext next) { timeRequest(r, next, "api"); }); asyncServer.addMiddleware([](AsyncWebServerRequest *r, ArMiddlewareNext next) { timeRequest(r, next, "web"); }); // Async API Server (port 8081) diff --git a/src/outputs/ProtoSocket.cpp b/src/outputs/ProtoSocket.cpp index 34c68e0..2324865 100644 --- a/src/outputs/ProtoSocket.cpp +++ b/src/outputs/ProtoSocket.cpp @@ -23,6 +23,7 @@ #include "Utils.h" #include "SocketEmitter.h" #include "../entryPoints/Web.h" +#include "ApiToken.h" #if HAS_ETHERNET #include #endif @@ -264,7 +265,7 @@ static bool authorized(AsyncWebServerRequest *request) { if(!key || !request->client()) return false; char token[65]; webServer.createAPIToken(request->client()->remoteIP(), token); - return key->value() == token; + return tokenMatches(token, key->value().c_str()); } static void onConnect(AsyncWebSocketClient *client, AsyncWebServerRequest *request) { const uint32_t id = client->id(); diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 06744a1..8997c83 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -3,7 +3,10 @@ The suite never changes the device's security itself, so it tests the mode it finds: pass --password or --pin for a Secured device. """ +import hashlib +import hmac import json +import socket import time import urllib.error import urllib.request @@ -35,6 +38,25 @@ def test_secured_refuses_a_wrong_token(host, secured): assert c.status == 401 +def test_secured_refuses_a_token_keyed_with_the_server_id(request, host, login_context, token): + """The token key used to be the public serverId, so anyone who saw a token could check PIN guesses against it.""" + if token is None: + pytest.skip("the device's security is None") + with socket.socket(socket.AF_INET, socket.SOCK_DGRAM) as s: + s.connect((host, 80)) + me = s.getsockname()[0] + if login_context["type"] == 2: + payload = f"{request.config.getoption('--username')}:{request.config.getoption('--password')}:{me}" + else: + payload = f"{request.config.getoption('--pin')}:{me}" + old = hmac.new(login_context["serverId"].encode(), payload.encode(), hashlib.sha256).hexdigest() + # A bool, so a failure doesn't print the tokens. + same = old == token + assert not same, "the device's token is the one keyed with its serverId" + c = ProtoClient(host, params={"apikey": old}) + assert c.status == 401 + + def test_secured_accepts_the_token(host, secured, token): if not secured: pytest.skip("the device's security is None") diff --git a/test/test_apitoken/test_main.cpp b/test/test_apitoken/test_main.cpp new file mode 100644 index 0000000..275c549 --- /dev/null +++ b/test/test_apitoken/test_main.cpp @@ -0,0 +1,54 @@ +#include +#include "ApiToken.h" + +void setUp() {} +void tearDown() {} + +static const char TOKEN[] = "3f9a0c1e5b7d2468ace013579bdf2468ace013579bdf2468ace013579bdf2468"; + +/********************************************************************* + * Comparing tokens + ********************************************************************/ +static void test_the_same_token_matches() { + TEST_ASSERT_TRUE(tokenMatches(TOKEN, "3f9a0c1e5b7d2468ace013579bdf2468ace013579bdf2468ace013579bdf2468")); +} +static void test_a_token_differing_in_the_first_character_does_not_match() { + TEST_ASSERT_FALSE(tokenMatches(TOKEN, "4f9a0c1e5b7d2468ace013579bdf2468ace013579bdf2468ace013579bdf2468")); +} +static void test_a_token_differing_in_the_last_character_does_not_match() { + TEST_ASSERT_FALSE(tokenMatches(TOKEN, "3f9a0c1e5b7d2468ace013579bdf2468ace013579bdf2468ace013579bdf2469")); +} +static void test_a_prefix_of_the_token_does_not_match() { + TEST_ASSERT_FALSE(tokenMatches(TOKEN, "3f9a0c1e5b7d2468ace013579bdf2468ace013579bdf2468ace013579bdf246")); + TEST_ASSERT_FALSE(tokenMatches(TOKEN, "3f9a")); +} +static void test_the_token_with_more_after_it_does_not_match() { + TEST_ASSERT_FALSE(tokenMatches(TOKEN, "3f9a0c1e5b7d2468ace013579bdf2468ace013579bdf2468ace013579bdf24680")); +} +static void test_an_empty_token_does_not_match() { + TEST_ASSERT_FALSE(tokenMatches(TOKEN, "")); +} +static void test_a_missing_token_does_not_match() { + TEST_ASSERT_FALSE(tokenMatches(TOKEN, nullptr)); +} +static void test_nothing_matches_an_empty_expected_token() { + TEST_ASSERT_FALSE(tokenMatches("", "")); + TEST_ASSERT_FALSE(tokenMatches(nullptr, TOKEN)); +} +static void test_case_matters() { + TEST_ASSERT_FALSE(tokenMatches(TOKEN, "3F9A0C1E5B7D2468ACE013579BDF2468ACE013579BDF2468ACE013579BDF2468")); +} + +int main(int, char **) { + UNITY_BEGIN(); + RUN_TEST(test_the_same_token_matches); + RUN_TEST(test_a_token_differing_in_the_first_character_does_not_match); + RUN_TEST(test_a_token_differing_in_the_last_character_does_not_match); + RUN_TEST(test_a_prefix_of_the_token_does_not_match); + RUN_TEST(test_the_token_with_more_after_it_does_not_match); + RUN_TEST(test_an_empty_token_does_not_match); + RUN_TEST(test_a_missing_token_does_not_match); + RUN_TEST(test_nothing_matches_an_empty_expected_token); + RUN_TEST(test_case_matters); + return UNITY_END(); +}