Merge pull request #110 from cjkas/feat/token-key

PR B: secret token key, constant-time token compare, HMAC leak fix

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

docs/KNOWN_ISSUES.md

@@ -5,7 +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
 - [#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

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.
+- **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.
-- **Comparison:** constant-time, used for HTTP and the websocket handshake alike.
+  - 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=<token>` 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=<token>` 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
 

src/ApiToken.h

@@ -0,0 +1,28 @@
+#ifndef apitoken_h
+#define apitoken_h
+#include <stddef.h>
+#include <stdint.h>
+
+// 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.  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;
+  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

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:

src/entryPoints/Web.cpp

@@ -5,6 +5,9 @@
 #include <esp_timer.h>
 #include "esp_log.h"
 #include "mbedtls/md.h"
+#include <Preferences.h>
+#include <esp_random.h>
+#include <bootloader_random.h>
 #include "ConfigSettings.h"
 #include "ConfigFile.h"
 #include "Utils.h"
@@ -59,6 +62,26 @@ 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 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;
+  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));
+    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) {
@@ -86,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");
 }
@@ -98,10 +122,10 @@ bool Web::isAuthenticated(AsyncWebServerRequest *request, bool cfg) {
   else if(!cfg && (settings.Security.permissions & static_cast<uint8_t>(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(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;
     }
@@ -120,15 +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);
+    bool ok = mbedtls_md_setup(&ctx, mbedtls_md_info_from_type(MBEDTLS_MD_SHA256), 1) == 0
-    mbedtls_md_hmac_starts(&ctx, (const unsigned char *)settings.serverId, strlen(settings.serverId));
+      && mbedtls_md_hmac_starts(&ctx, tokenKey, sizeof(tokenKey)) == 0
-    mbedtls_md_hmac_update(&ctx, (const unsigned char *)payload, strlen(payload)); 
+      && mbedtls_md_hmac_update(&ctx, (const unsigned char *)payload, strlen(payload)) == 0
-    mbedtls_md_hmac_finish(&ctx, hmacResult);
+      && 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]);
@@ -137,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) return createAPIPasswordToken(ipAddress, settings.Security.username, settings.Security.password, token);
-    if(settings.Security.type == security_types::Password) createAPIPasswordToken(ipAddress, settings.Security.username, settings.Security.password, token);
+    if(settings.Security.type == security_types::PinEntry) return createAPIPinToken(ipAddress, settings.Security.pin, token);
-    else if(settings.Security.type == security_types::PinEntry) createAPIPinToken(ipAddress, settings.Security.pin, token);
+    return createAPIToken(ipAddress.toString().c_str(), token);
-    else createAPIToken(ipAddress.toString().c_str(), token);
-    return true;
 }
 // =====================================================
 // Async API Handlers
@@ -335,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) {
@@ -373,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;
+  // The token these credentials would get, against the stored ones'.
-  if(settings.Security.type == security_types::PinEntry) {
+  char ptok[API_TOKEN_SIZE] = "";
-    char ptok[65];
+  if(settings.Security.type == security_types::PinEntry) this->createAPIPinToken(ip, pin, ptok);
-    memset(ptok, 0x00, sizeof(ptok));
+  else if(settings.Security.type == security_types::Password) this->createAPIPasswordToken(ip, username, password, ptok);
-    this->createAPIPinToken(request->client()->remoteIP(), pin, ptok);
+  bool success = tokenMatches(token, ptok);
-    if(String(ptok) == String(token)) success = true;
-  }
-  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;
-  }
   if(success) {
     loginThrottle.succeeded(key);
     snprintf(g_async_content, sizeof(g_async_content),

src/entryPoints/Web.h

@@ -1,6 +1,7 @@
 #include <ESPAsyncWebServer.h>
 #include <AsyncJson.h>
 #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);

src/entryPoints/WebUiRoutes.cpp

@@ -601,7 +601,7 @@ void registerWebUiRoutes(AsyncWebServer &server) {
       JsonObject obj = json.as<JsonObject>();
       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<JsonObject>();

src/outputs/ProtoSocket.cpp

@@ -262,9 +262,9 @@ 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 key->value() == token;
+  return tokenMatches(token, key->value().c_str());
 }
 static void onConnect(AsyncWebSocketClient *client, AsyncWebServerRequest *request) {
   const uint32_t id = client->id();

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,33 @@ def test_secured_refuses_a_wrong_token(host, secured):
     assert c.status == 401
 
 
+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))
+        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')}:{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.
+    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
+
+
 def test_secured_accepts_the_token(host, secured, token):
     if not secured:
         pytest.skip("the device's security is None")

test/test_apitoken/test_main.cpp

@@ -0,0 +1,54 @@
+#include <unity.h>
+#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();
+}