From 43191ba312fdf86549e07eb397843c3f317ff6eb Mon Sep 17 00:00:00 2001 From: cjkas Date: Tue, 27 Oct 2026 19:41:43 +0100 Subject: [PATCH 1/2] feat: sign tokens with a secret key, compare them in constant time Tokens were HMAC(serverId, credentials + client IP). serverId is public (mDNS, MQTT, /loginContext), so anyone who saw a token could test PIN guesses offline. The key is now 32 random bytes made on first boot in their own NVS namespace, which no settings save clears and no backup holds. It is made before WiFi starts, from the bootloader's entropy source. Tokens are compared with tokenMatches, which takes the same time whatever the guess, on the apikey header, the socket upgrade and /login. createAPIToken never freed its mbedtls context, so every token it made leaked: 563 bytes per login on the device, until it ran out of memory after about 150 logins. It now frees it: 0 bytes over 500 logins. Every token made before the upgrade is refused once. The web UI asks for login again, and Home Assistant 3.0 logs in again by itself. Closes #80 Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 4 +++ docs/KNOWN_ISSUES.md | 2 -- docs/SECURITY_PLAN.md | 11 ++++--- src/ApiToken.h | 26 +++++++++++++++ src/entryPoints/Web.cpp | 35 ++++++++++++++++++--- src/outputs/ProtoSocket.cpp | 3 +- test/e2e/test_auth.py | 22 +++++++++++++ test/test_apitoken/test_main.cpp | 54 ++++++++++++++++++++++++++++++++ 8 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 src/ApiToken.h create mode 100644 test/test_apitoken/test_main.cpp 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(); +} From 97d8a90da8367013720a7cb2c4589cdb9cc50ba9 Mon Sep 17 00:00:00 2001 From: cjkas Date: Tue, 27 Oct 2026 20:38:29 +0100 Subject: [PATCH 2/2] fix: review findings on the token key - Load the token key in Web::startup(), before either web server starts, so no token can be made with the all-zero key; log a failed pref.begin(). - createAPIToken checks mbedtls' return codes. On a failure it leaves the token empty, which tokenMatches never accepts, instead of hex from uninitialised stack. - API_TOKEN_SIZE in ApiToken.h replaces the five char[65] buffers; the HMAC's description moves to createAPIToken, with the None form. - handleLogin builds the candidate token once and compares it once. - e2e: the serverId test skips on `secured` like its neighbours, names its values, and says it assumes the device sees this PC's address. - KNOWN_ISSUES keeps #81, which #108 removes. Co-Authored-By: Claude Opus 5.5 --- docs/KNOWN_ISSUES.md | 1 + src/ApiToken.h | 10 +++--- src/DeviceOps.cpp | 2 +- src/entryPoints/Web.cpp | 58 ++++++++++++++++----------------- src/entryPoints/Web.h | 3 +- src/entryPoints/WebUiRoutes.cpp | 2 +- src/outputs/ProtoSocket.cpp | 3 +- test/e2e/test_auth.py | 30 ++++++++++------- 8 files changed, 59 insertions(+), 50 deletions(-) diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index 592edee..bb3c1cb 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -5,6 +5,7 @@ - [#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 +- [#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/src/ApiToken.h b/src/ApiToken.h index 13cfa7f..9e9fe26 100644 --- a/src/ApiToken.h +++ b/src/ApiToken.h @@ -3,14 +3,16 @@ #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. +// The token key's size: 32 random bytes the device makes on first boot and +// keeps in NVS. It never leaves the device. #define API_TOKEN_KEY_SIZE 32 +// A token's buffer: an HMAC-SHA256 in hex and its terminator. +#define API_TOKEN_SIZE 65 // 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. +// expected's length shows, and every token has the same one. An empty +// expected token, which is what a failed HMAC leaves, matches nothing. inline bool tokenMatches(const char *expected, const char *given) { if(!expected || !given || !expected[0]) return false; uint8_t diff = 0; diff --git a/src/DeviceOps.cpp b/src/DeviceOps.cpp index 0361008..f96f195 100644 --- a/src/DeviceOps.cpp +++ b/src/DeviceOps.cpp @@ -160,7 +160,7 @@ static void runSave(const proto_reply_to_t &to, settings_save_t *save) { const settings_section_t section = save->section; const uint16_t parts = settingsParts(section, req); bool reboot = false; - char token[65] = ""; + char token[API_TOKEN_SIZE] = ""; const security_types securityBefore = settings.Security.type; switch(section) { case settings_section_t::general: diff --git a/src/entryPoints/Web.cpp b/src/entryPoints/Web.cpp index 23c578b..5cbe89c 100644 --- a/src/entryPoints/Web.cpp +++ b/src/entryPoints/Web.cpp @@ -8,7 +8,6 @@ #include #include #include -#include "ApiToken.h" #include "ConfigSettings.h" #include "ConfigFile.h" #include "Utils.h" @@ -66,11 +65,13 @@ 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. +// Reads the token key, or makes one on first boot. Runs before either web +// server starts, so no token is made with an empty key, and before WiFi, so +// the hardware RNG takes its entropy from the bootloader's source: calling it +// once the radio is on would disturb WiFi. static void loadTokenKey() { Preferences pref; - pref.begin("TOKEN"); + if(!pref.begin("TOKEN")) ESP_LOGE(TAG, "Couldn't open the token key's NVS namespace: every token changes at the next boot"); if(pref.getBytes("key", tokenKey, sizeof(tokenKey)) != sizeof(tokenKey)) { bootloader_random_enable(); esp_fill_random(tokenKey, sizeof(tokenKey)); @@ -108,6 +109,7 @@ AsyncWebServer asyncServer(80); AsyncWebServer asyncApiServer(8081); void Web::startup() { ESP_LOGI(TAG, "Launching web server..."); + loadTokenKey(); asyncApiServer.begin(); ESP_LOGI(TAG, "Async API server started on port 8081"); } @@ -120,7 +122,7 @@ bool Web::isAuthenticated(AsyncWebServerRequest *request, bool cfg) { 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[65]; + 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())) { @@ -142,17 +144,24 @@ bool Web::createAPIPinToken(const IPAddress ipAddress, const char *pin, char *to bool Web::createAPIPasswordToken(const IPAddress ipAddress, const char *username, const char *password, char *token) { return this->createAPIToken((String(username) + ":" + String(password) + ":" + ipAddress.toString()).c_str(), token); } +// Writes HMAC-SHA256(token key, payload) in hex to token, which holds +// API_TOKEN_SIZE. The payload is "username:password:ip" under a password, +// "pin:ip" under a PIN and "ip" under None. When mbedtls fails (it allocates +// the HMAC context) token is left empty, which tokenMatches never accepts. 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, tokenKey, sizeof(tokenKey)); - mbedtls_md_hmac_update(&ctx, (const unsigned char *)payload, strlen(payload)); - mbedtls_md_hmac_finish(&ctx, hmacResult); + bool ok = mbedtls_md_setup(&ctx, mbedtls_md_info_from_type(MBEDTLS_MD_SHA256), 1) == 0 + && mbedtls_md_hmac_starts(&ctx, tokenKey, sizeof(tokenKey)) == 0 + && mbedtls_md_hmac_update(&ctx, (const unsigned char *)payload, strlen(payload)) == 0 + && mbedtls_md_hmac_finish(&ctx, hmacResult) == 0; mbedtls_md_free(&ctx); token[0] = '\0'; + if(!ok) { + ESP_LOGE(TAG, "Couldn't make a token"); + return false; + } for(int i = 0; i < sizeof(hmacResult); i++){ char str[3]; sprintf(str, "%02x", (int)hmacResult[i]); @@ -161,11 +170,9 @@ bool Web::createAPIToken(const char *payload, char *token) { return true; } bool Web::createAPIToken(const IPAddress ipAddress, char *token) { - String payload; - if(settings.Security.type == security_types::Password) createAPIPasswordToken(ipAddress, settings.Security.username, settings.Security.password, token); - else if(settings.Security.type == security_types::PinEntry) createAPIPinToken(ipAddress, settings.Security.pin, token); - else createAPIToken(ipAddress.toString().c_str(), token); - return true; + if(settings.Security.type == security_types::Password) return createAPIPasswordToken(ipAddress, settings.Security.username, settings.Security.password, token); + if(settings.Security.type == security_types::PinEntry) return createAPIPinToken(ipAddress, settings.Security.pin, token); + return createAPIToken(ipAddress.toString().c_str(), token); } // ===================================================== // Async API Handlers @@ -359,7 +366,7 @@ void Web::handleController(AsyncWebServerRequest *request) { } void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { if(request->method() == HTTP_OPTIONS) { request->send(200); return; } - char token[65]; + char token[API_TOKEN_SIZE]; memset(&token, 0x00, sizeof(token)); this->createAPIToken(request->client()->remoteIP(), token); if(settings.Security.type == security_types::None) { @@ -397,19 +404,11 @@ void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { if(!obj["password"].isNull()) strlcpy(password, obj["password"], sizeof(password)); if(!obj["pin"].isNull()) strlcpy(pin, obj["pin"], sizeof(pin)); } - bool success = false; - if(settings.Security.type == security_types::PinEntry) { - char ptok[65]; - memset(ptok, 0x00, sizeof(ptok)); - this->createAPIPinToken(request->client()->remoteIP(), pin, ptok); - 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); - success = tokenMatches(token, ptok); - } + // The token these credentials would get, against the stored ones'. + char ptok[API_TOKEN_SIZE] = ""; + if(settings.Security.type == security_types::PinEntry) this->createAPIPinToken(ip, pin, ptok); + else if(settings.Security.type == security_types::Password) this->createAPIPasswordToken(ip, username, password, ptok); + bool success = tokenMatches(token, ptok); if(success) { loginThrottle.succeeded(key); snprintf(g_async_content, sizeof(g_async_content), @@ -789,7 +788,6 @@ 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/entryPoints/Web.h b/src/entryPoints/Web.h index 8ab4adf..3317f5e 100644 --- a/src/entryPoints/Web.h +++ b/src/entryPoints/Web.h @@ -1,6 +1,7 @@ #include #include #include "Somfy.h" +#include "ApiToken.h" #ifndef webserver_h #define webserver_h @@ -10,7 +11,7 @@ class Web { void startup(); void begin(); void loop(); - // Auth helpers + // Auth helpers. token holds API_TOKEN_SIZE; false leaves it empty. bool createAPIToken(const IPAddress ipAddress, char *token); bool createAPIToken(const char *payload, char *token); bool createAPIPinToken(const IPAddress ipAddress, const char *pin, char *token); diff --git a/src/entryPoints/WebUiRoutes.cpp b/src/entryPoints/WebUiRoutes.cpp index b2683b5..4159fb4 100644 --- a/src/entryPoints/WebUiRoutes.cpp +++ b/src/entryPoints/WebUiRoutes.cpp @@ -601,7 +601,7 @@ void registerWebUiRoutes(AsyncWebServer &server) { JsonObject obj = json.as(); settings.Security.fromJSON(obj); settings.Security.save(); - char token[65]; + char token[API_TOKEN_SIZE]; webServer.createAPIToken(request->client()->remoteIP(), token); JsonDocument sdoc; JsonObject sobj = sdoc.to(); diff --git a/src/outputs/ProtoSocket.cpp b/src/outputs/ProtoSocket.cpp index 2324865..2ca117f 100644 --- a/src/outputs/ProtoSocket.cpp +++ b/src/outputs/ProtoSocket.cpp @@ -23,7 +23,6 @@ #include "Utils.h" #include "SocketEmitter.h" #include "../entryPoints/Web.h" -#include "ApiToken.h" #if HAS_ETHERNET #include #endif @@ -263,7 +262,7 @@ static bool authorized(AsyncWebServerRequest *request) { if(settings.Security.type == security_types::None) return true; const AsyncWebParameter *key = request->getParam("apikey"); if(!key || !request->client()) return false; - char token[65]; + char token[API_TOKEN_SIZE]; webServer.createAPIToken(request->client()->remoteIP(), token); return tokenMatches(token, key->value().c_str()); } diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 8997c83..82fa098 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -38,22 +38,30 @@ 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: +PASSWORD = 2 # the security type /loginContext reports for a password + + +def test_secured_refuses_a_token_keyed_with_the_server_id(request, host, secured, login_context, token): + """The token key used to be the public serverId, so anyone who saw a token could check PIN guesses against it. + + It works out the token the old way, for this PC's address as the route to + the device picks it. Through NAT or a VPN the device sees another address, + and the test passes without testing anything. + """ + if not secured: 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}" + my_ip = s.getsockname()[0] + if login_context["type"] == PASSWORD: + payload = f"{request.config.getoption('--username')}:{request.config.getoption('--password')}:{my_ip}" else: - payload = f"{request.config.getoption('--pin')}:{me}" - old = hmac.new(login_context["serverId"].encode(), payload.encode(), hashlib.sha256).hexdigest() + payload = f"{request.config.getoption('--pin')}:{my_ip}" + server_id_token = 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}) + is_device_token = server_id_token == token + assert not is_device_token, "the device's token is the one keyed with its serverId" + c = ProtoClient(host, params={"apikey": server_id_token}) assert c.status == 401