From bab83313eea35bc9794578ee247f2a82ce5580ed Mon Sep 17 00:00:00 2001 From: cjkas Date: Mon, 26 Oct 2026 22:08:30 +0100 Subject: [PATCH 1/3] feat: throttle failed logins per client IP PR C of SECURITY_PLAN.md: after 5 failed /login attempts an IP is refused for 30 s, and each failure after that refuses it for twice as long, up to 15 min. A success resets it, as does an hour without a failure. - LoginThrottle.h holds the schedule and 8 IPs, with the time passed in so native tests cover it. While all 8 hold recent failures, every other IP shares one more slot, so many addresses don't each get five free tries. - A refused IP gets 429 with Retry-After before its credentials are checked, so a right guess during a lockout reveals nothing. Home Assistant retries a 429 instead of asking for new credentials. - The web UI's login panel shows a 401's or 429's message, where it popped a service error. - test/e2e --lockout checks it on a Secured device and waits it out. Co-Authored-By: Claude Opus 5.5 --- data-src/index.js | 5 +- docs/SECURITY_PLAN.md | 4 + src/LoginThrottle.h | 102 ++++++++++++++++++ src/entryPoints/Web.cpp | 20 ++++ test/e2e/README.md | 1 + test/e2e/conftest.py | 1 + test/e2e/test_auth.py | 45 ++++++++ test/test_loginthrottle/test_main.cpp | 149 ++++++++++++++++++++++++++ 8 files changed, 326 insertions(+), 1 deletion(-) create mode 100644 src/LoginThrottle.h create mode 100644 test/test_loginthrottle/test_main.cpp diff --git a/data-src/index.js b/data-src/index.js index 7bbb3c4..be301b9 100644 --- a/data-src/index.js +++ b/data-src/index.js @@ -1187,7 +1187,10 @@ class Security { } sec.pin = pin; putJSONSync('/login', sec, (err, log) => { - if (err) ui.serviceError(err); + // Wrong credentials (401) or too many of them (429) say why in + // the panel, as an old 200 with success false did. + if (err && (err.htmlError === 401 || err.htmlError === 429) && err.msg) msg.innerHTML = err.msg; + else if (err) ui.serviceError(err); else { console.log(log); if (log.success) { diff --git a/docs/SECURITY_PLAN.md b/docs/SECURITY_PLAN.md index 1becb58..9853364 100644 --- a/docs/SECURITY_PLAN.md +++ b/docs/SECURITY_PLAN.md @@ -56,6 +56,10 @@ ### Login throttling (PR C) - Per client IP: after 5 failed `/login` attempts, refuse that IP for 30 s, doubling each time up to 15 min. A success resets it. + - After the first lockout, each failure starts the next one, so an IP gets one try per lockout rather than five. + - An hour without a failure forgets the IP. + - A refused IP gets 429 with `Retry-After`, before its credentials are checked. Home Assistant tries again later instead of asking for new credentials. + - The device keeps 8 IPs ([src/LoginThrottle.h](../src/LoginThrottle.h)). While all of them hold recent failures, every other IP shares one more slot. That way many addresses don't each get five free tries, but an unknown IP may have to wait during such an attack. - Only `/login` is throttled. After PR B a token is a 256-bit HMAC with a secret key, so guessing one through `apikey` is hopeless. ### One server on port 80 ([ADR 0006](adr/0006-two-security-modes-eleven-http-routes.md)) diff --git a/src/LoginThrottle.h b/src/LoginThrottle.h new file mode 100644 index 0000000..7224a27 --- /dev/null +++ b/src/LoginThrottle.h @@ -0,0 +1,102 @@ +#ifndef loginthrottle_h +#define loginthrottle_h +#include + +// Failed /login attempts per client IP (SECURITY_PLAN.md, PR C). The first +// LOGIN_FREE_FAILURES failures cost nothing, and the last of them locks the +// IP out for LOGIN_FIRST_LOCKOUT_MS. After that every failure locks it out +// again, for twice as long as the lockout before, up to LOGIN_MAX_LOCKOUT_MS. +// A success forgets the IP, and so does LOGIN_FORGET_MS without a failure. +// +// It keeps LOGIN_THROTTLE_SLOTS IPs. Once each holds a recent failure, every +// other IP shares one more slot, so an attacker with many addresses can't +// get the free failures on each: while the table is full, an IP it doesn't +// hold may have to wait even with the right credentials. +// +// The time is passed in (millis()), so native tests cover it. Only +// async_tcp's task calls it: both web servers run their handlers there. + +#define LOGIN_FREE_FAILURES 5 +#define LOGIN_FIRST_LOCKOUT_MS 30000UL +#define LOGIN_MAX_LOCKOUT_MS 900000UL +#define LOGIN_FORGET_MS 3600000UL +#define LOGIN_THROTTLE_SLOTS 8 + +struct login_attempts_t { + bool used = false; + uint32_t ip = 0; + uint16_t failures = 0; + uint32_t lastFailure = 0; // millis() of the last failure + uint32_t lockoutMs = 0; // the lockout that failure began, 0 for none +}; + +class LoginThrottle { + public: + // How long ip must wait before it may try again, in ms; 0 when it may now. + uint32_t retryAfter(uint32_t ip, uint32_t now) const { + const login_attempts_t *a = this->find(ip, now); + return a ? remaining(*a, now) : 0; + } + // A wrong PIN or password from ip. Returns the lockout it begins, in ms, + // or 0 while ip still has free failures. + uint32_t failed(uint32_t ip, uint32_t now) { + login_attempts_t *a = this->claim(ip, now); + if(a->failures < UINT16_MAX) a->failures++; + a->lastFailure = now; + a->lockoutMs = lockoutFor(a->failures); + return a->lockoutMs; + } + // The right credentials from ip. The shared slot is left as it is: one + // user's success says nothing about the other addresses in it. + void succeeded(uint32_t ip) { + for(auto &a : this->slots) + if(a.used && a.ip == ip) a = login_attempts_t(); + } + // The lockout the failures-th failure in a row begins. + static uint32_t lockoutFor(uint16_t failures) { + if(failures < LOGIN_FREE_FAILURES) return 0; + uint32_t ms = LOGIN_FIRST_LOCKOUT_MS; + for(uint16_t i = LOGIN_FREE_FAILURES; i < failures && ms < LOGIN_MAX_LOCKOUT_MS; i++) ms *= 2; + return ms < LOGIN_MAX_LOCKOUT_MS ? ms : LOGIN_MAX_LOCKOUT_MS; + } + private: + login_attempts_t slots[LOGIN_THROTTLE_SLOTS]; + login_attempts_t shared; + + static uint32_t remaining(const login_attempts_t &a, uint32_t now) { + if(!a.used || a.lockoutMs == 0) return 0; + uint32_t elapsed = now - a.lastFailure; + return elapsed < a.lockoutMs ? a.lockoutMs - elapsed : 0; + } + // Unused, or its last failure is old enough to forget. + static bool vacant(const login_attempts_t &a, uint32_t now) { + return !a.used || now - a.lastFailure >= LOGIN_FORGET_MS; + } + // ip's record: its own slot, nothing while a slot is vacant, else the + // shared one. + const login_attempts_t *find(uint32_t ip, uint32_t now) const { + bool anyVacant = false; + for(const auto &a : this->slots) { + if(vacant(a, now)) anyVacant = true; + else if(a.ip == ip) return &a; + } + if(anyVacant || vacant(this->shared, now)) return nullptr; + return &this->shared; + } + // ip's record to count a failure in, taking a vacant slot when it has none. + login_attempts_t *claim(uint32_t ip, uint32_t now) { + login_attempts_t *free = nullptr; + for(auto &a : this->slots) { + if(vacant(a, now)) { if(!free) free = &a; } + else if(a.ip == ip) return &a; + } + login_attempts_t *a = free ? free : &this->shared; + if(vacant(*a, now)) { + *a = login_attempts_t(); + a->used = true; + a->ip = ip; + } + return a; + } +}; +#endif diff --git a/src/entryPoints/Web.cpp b/src/entryPoints/Web.cpp index 786fb85..8475f7b 100644 --- a/src/entryPoints/Web.cpp +++ b/src/entryPoints/Web.cpp @@ -20,6 +20,7 @@ #include "MqttConnection.h" #include "ESPNetwork.h" #include "Timing.h" +#include "LoginThrottle.h" #include #include #include @@ -49,6 +50,9 @@ const char _encoding_json[] = "application/json"; static const char *TAG = "Web"; +// Failed logins per client IP, on both web servers. +static LoginThrottle loginThrottle; + // 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) { @@ -335,6 +339,19 @@ void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { request->send(200, _encoding_json, g_async_content); return; } + // A refused IP isn't told whether these credentials were right. + IPAddress ip = request->client()->remoteIP(); + uint32_t wait = loginThrottle.retryAfter((uint32_t)ip, millis()); + if(wait) { + uint32_t secs = (wait + 999) / 1000; + snprintf(g_async_content, sizeof(g_async_content), + "{\"type\":%u,\"msg\":\"Too many failed logins. Try again in %lu s.\",\"success\":false,\"retryAfter\":%lu}", + static_cast(settings.Security.type), (unsigned long)secs, (unsigned long)secs); + AsyncWebServerResponse *response = request->beginResponse(429, _encoding_json, g_async_content); + response->addHeader("Retry-After", String(secs)); + request->send(response); + return; + } char username[33] = ""; char password[33] = ""; char pin[5] = ""; @@ -363,12 +380,15 @@ void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { if(String(ptok) == String(token)) success = true; } if(success) { + loginThrottle.succeeded((uint32_t)ip); snprintf(g_async_content, sizeof(g_async_content), "{\"type\":%u,\"apiKey\":\"%s\",\"msg\":\"Success\",\"success\":true}", static_cast(settings.Security.type), token); request->send(200, _encoding_json, g_async_content); } else { + uint32_t lockout = loginThrottle.failed((uint32_t)ip, millis()); + if(lockout) ESP_LOGW(TAG, "Failed login from %s: refused for %lu s", ip.toString().c_str(), (unsigned long)(lockout / 1000)); snprintf(g_async_content, sizeof(g_async_content), "{\"type\":%u,\"msg\":\"Invalid credentials\",\"success\":false}", static_cast(settings.Security.type)); diff --git a/test/e2e/README.md b/test/e2e/README.md index a42e530..9795d4a 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -13,6 +13,7 @@ Runs against a real device on your network, by hand, never in CI. It checks sock | `--radio --shade ` | also send real commands: that shade moves a little and back | | `--edit` | also add a room and delete it again | | `--reboot` | also reboot the device and wait for it to come back | +| `--lockout` | on a Secured device, also fail `/login` until this PC is refused, then wait out the 30 s lockout | | `--record` | save hello, the sync and the schema to `test/fixtures/proto2/`, for the web UI and Home Assistant tests | A plain run sends nothing on air, changes no setting (it saves only values the device already has) and doesn't reboot the device. The command tests send ops that are refused before they reach the radio, and one `setPositions` that sets a shade to the position the sync gave: that drops the fraction of a percent the shade's believed position may have had. The edit tests send edits that are refused, and one shade sort order set to the order it has, which saves the shades again unchanged. The settings tests send saves that are refused, and one that saves the NTP server and time zone the device already has. The frequency scan test pauses the radio's sending for a second or two, as a scan does. It does fill every socket slot for a few seconds, and replays a page load. diff --git a/test/e2e/conftest.py b/test/e2e/conftest.py index 46dcf5a..23e564b 100644 --- a/test/e2e/conftest.py +++ b/test/e2e/conftest.py @@ -19,6 +19,7 @@ def pytest_addoption(parser): parser.addoption("--shade", type=int, default=None, help="with --radio: the id of the shade the commands may move") parser.addoption("--edit", action="store_true", help="also run the edits that change the shade model, then undo them") parser.addoption("--reboot", action="store_true", help="also run the test that reboots the device") + parser.addoption("--lockout", action="store_true", help="also run the test that locks this PC out of /login for 30 s") @pytest.fixture(scope="session") diff --git a/test/e2e/test_auth.py b/test/e2e/test_auth.py index 41b8f00..06744a1 100644 --- a/test/e2e/test_auth.py +++ b/test/e2e/test_auth.py @@ -3,6 +3,11 @@ 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 json +import time +import urllib.error +import urllib.request + import pytest from proto_client import ProtoClient @@ -36,3 +41,43 @@ def test_secured_accepts_the_token(host, secured, token): c = ProtoClient(host, params={"apikey": token}) assert c.status == 101 c.close() + + +def login_status(host, body): + """/login's status and reply, refusals included.""" + req = urllib.request.Request( + f"http://{host}/login", + data=json.dumps(body).encode(), + method="PUT", + headers={"Content-Type": "application/json"}, + ) + try: + with urllib.request.urlopen(req, timeout=20) as r: + return r.status, json.loads(r.read()), r.headers + except urllib.error.HTTPError as e: + return e.code, json.loads(e.read() or b"{}"), e.headers + + +def test_failed_logins_lock_this_pc_out(request, host, secured, token): + """Five wrong logins refuse this IP for 30 s, even with the right ones (SECURITY_PLAN.md, PR C).""" + if not request.config.getoption("--lockout"): + pytest.skip("locks this PC out of /login for 30 s: pass --lockout") + if not secured: + pytest.skip("the device's security is None") + password = request.config.getoption("--password") + pin = request.config.getoption("--pin") + right = {"username": request.config.getoption("--username"), "password": password or "", "pin": pin or ""} + wrong = {**right, "password": (password or "") + "x", "pin": "0000" if pin != "0000" else "1111"} + # The token fixture's login started this PC with a clean count. + for _ in range(5): + status, reply, _ = login_status(host, wrong) + assert status == 401 and reply["success"] is False + status, reply, headers = login_status(host, right) + assert status == 429, "the right credentials got through a lockout" + wait = int(headers["Retry-After"]) + assert 25 <= wait <= 30 + assert reply["retryAfter"] == wait + time.sleep(wait + 1) + # Over: the right credentials work, and reset the count. + status, reply, _ = login_status(host, right) + assert status == 200 and reply["apiKey"] == token diff --git a/test/test_loginthrottle/test_main.cpp b/test/test_loginthrottle/test_main.cpp new file mode 100644 index 0000000..5115fca --- /dev/null +++ b/test/test_loginthrottle/test_main.cpp @@ -0,0 +1,149 @@ +#include +#include "LoginThrottle.h" + +void setUp() {} +void tearDown() {} + +static const uint32_t IP_A = 0x0A01A8C0; // 192.168.1.10 +static const uint32_t IP_B = 0x0B01A8C0; // 192.168.1.11 + +// Fails ip n times at now, as fast as the throttle lets it. +static void failTimes(LoginThrottle &t, uint32_t ip, int n, uint32_t now) { + for(int i = 0; i < n; i++) t.failed(ip, now); +} + +/********************************************************************* + * The schedule + ********************************************************************/ +static void test_the_first_failures_are_free() { + for(uint16_t f = 0; f < LOGIN_FREE_FAILURES; f++) + TEST_ASSERT_EQUAL_UINT32(0, LoginThrottle::lockoutFor(f)); +} +static void test_the_fifth_failure_locks_out_for_30s() { + TEST_ASSERT_EQUAL_UINT32(30000, LoginThrottle::lockoutFor(5)); +} +static void test_each_later_failure_doubles_up_to_15_minutes() { + TEST_ASSERT_EQUAL_UINT32(60000, LoginThrottle::lockoutFor(6)); + TEST_ASSERT_EQUAL_UINT32(120000, LoginThrottle::lockoutFor(7)); + TEST_ASSERT_EQUAL_UINT32(240000, LoginThrottle::lockoutFor(8)); + TEST_ASSERT_EQUAL_UINT32(480000, LoginThrottle::lockoutFor(9)); + TEST_ASSERT_EQUAL_UINT32(900000, LoginThrottle::lockoutFor(10)); + TEST_ASSERT_EQUAL_UINT32(900000, LoginThrottle::lockoutFor(11)); + TEST_ASSERT_EQUAL_UINT32(900000, LoginThrottle::lockoutFor(UINT16_MAX)); +} + +/********************************************************************* + * One IP + ********************************************************************/ +static void test_an_ip_with_no_failures_may_try() { + LoginThrottle t; + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, 1000)); +} +static void test_four_failures_cost_nothing() { + LoginThrottle t; + for(int i = 0; i < 4; i++) TEST_ASSERT_EQUAL_UINT32(0, t.failed(IP_A, 1000)); + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, 1000)); +} +static void test_the_fifth_failure_refuses_the_ip_for_30s() { + LoginThrottle t; + failTimes(t, IP_A, 4, 1000); + TEST_ASSERT_EQUAL_UINT32(30000, t.failed(IP_A, 1000)); + TEST_ASSERT_EQUAL_UINT32(30000, t.retryAfter(IP_A, 1000)); + TEST_ASSERT_EQUAL_UINT32(1, t.retryAfter(IP_A, 30999)); + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, 31000)); +} +static void test_after_a_lockout_the_next_failure_locks_out_twice_as_long() { + LoginThrottle t; + failTimes(t, IP_A, 5, 0); + TEST_ASSERT_EQUAL_UINT32(60000, t.failed(IP_A, 30000)); + TEST_ASSERT_EQUAL_UINT32(60000, t.retryAfter(IP_A, 30000)); + TEST_ASSERT_EQUAL_UINT32(120000, t.failed(IP_A, 90000)); +} +static void test_a_success_resets_the_ip() { + LoginThrottle t; + failTimes(t, IP_A, 6, 0); + t.succeeded(IP_A); + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, 0)); + // Its free failures are back. + for(int i = 0; i < 4; i++) TEST_ASSERT_EQUAL_UINT32(0, t.failed(IP_A, 0)); + TEST_ASSERT_EQUAL_UINT32(30000, t.failed(IP_A, 0)); +} +static void test_an_hour_without_a_failure_forgets_the_ip() { + LoginThrottle t; + failTimes(t, IP_A, 7, 0); + // Still counted just before the hour: the next failure doubles. + TEST_ASSERT_EQUAL_UINT32(240000, t.failed(IP_A, LOGIN_FORGET_MS - 1)); + uint32_t later = 2 * LOGIN_FORGET_MS; + TEST_ASSERT_EQUAL_UINT32(0, t.failed(IP_A, later)); +} +static void test_millis_wrapping_keeps_the_lockout() { + LoginThrottle t; + uint32_t now = UINT32_MAX - 10000; + failTimes(t, IP_A, 5, now); + TEST_ASSERT_EQUAL_UINT32(10000, t.retryAfter(IP_A, now + 20000)); + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, now + 30000)); +} + +/********************************************************************* + * Several IPs + ********************************************************************/ +static void test_one_ip_locked_out_leaves_the_others_alone() { + LoginThrottle t; + failTimes(t, IP_A, 5, 0); + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_B, 0)); + TEST_ASSERT_EQUAL_UINT32(0, t.failed(IP_B, 0)); +} +static void test_a_success_resets_only_its_own_ip() { + LoginThrottle t; + failTimes(t, IP_A, 5, 0); + t.succeeded(IP_B); + TEST_ASSERT_EQUAL_UINT32(30000, t.retryAfter(IP_A, 0)); +} +static void test_with_the_table_full_other_ips_share_one_slot() { + LoginThrottle t; + for(uint32_t i = 0; i < LOGIN_THROTTLE_SLOTS; i++) failTimes(t, 100 + i, 1, 0); + // New addresses count against each other: five between them lock all out. + for(uint32_t i = 0; i < 4; i++) TEST_ASSERT_EQUAL_UINT32(0, t.failed(200 + i, 0)); + TEST_ASSERT_EQUAL_UINT32(30000, t.failed(204, 0)); + TEST_ASSERT_EQUAL_UINT32(30000, t.retryAfter(205, 0)); + // The IPs that have a slot keep their own count. + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(100, 0)); +} +static void test_a_forgotten_slot_is_taken_again() { + LoginThrottle t; + for(uint32_t i = 0; i < LOGIN_THROTTLE_SLOTS; i++) failTimes(t, 100 + i, 5, 0); + // An hour later every slot is vacant: a new IP has its own free failures. + uint32_t later = LOGIN_FORGET_MS; + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, later)); + failTimes(t, IP_A, 4, later); + TEST_ASSERT_EQUAL_UINT32(30000, t.failed(IP_A, later)); + // Taken by IP_A, not shared: another new IP isn't refused. + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_B, later)); +} +static void test_a_success_doesnt_clear_the_shared_slot() { + LoginThrottle t; + for(uint32_t i = 0; i < LOGIN_THROTTLE_SLOTS; i++) failTimes(t, 100 + i, 1, 0); + failTimes(t, 200, 5, 0); + t.succeeded(201); + TEST_ASSERT_EQUAL_UINT32(30000, t.retryAfter(202, 0)); +} + +int main(int argc, char **argv) { + UNITY_BEGIN(); + RUN_TEST(test_the_first_failures_are_free); + RUN_TEST(test_the_fifth_failure_locks_out_for_30s); + RUN_TEST(test_each_later_failure_doubles_up_to_15_minutes); + RUN_TEST(test_an_ip_with_no_failures_may_try); + RUN_TEST(test_four_failures_cost_nothing); + RUN_TEST(test_the_fifth_failure_refuses_the_ip_for_30s); + RUN_TEST(test_after_a_lockout_the_next_failure_locks_out_twice_as_long); + RUN_TEST(test_a_success_resets_the_ip); + RUN_TEST(test_an_hour_without_a_failure_forgets_the_ip); + RUN_TEST(test_millis_wrapping_keeps_the_lockout); + RUN_TEST(test_one_ip_locked_out_leaves_the_others_alone); + RUN_TEST(test_a_success_resets_only_its_own_ip); + RUN_TEST(test_with_the_table_full_other_ips_share_one_slot); + RUN_TEST(test_a_forgotten_slot_is_taken_again); + RUN_TEST(test_a_success_doesnt_clear_the_shared_slot); + return UNITY_END(); +} From ff7517e10f14beb3fa574227417a59ea04518ff8 Mon Sep 17 00:00:00 2001 From: cjkas Date: Tue, 27 Oct 2026 18:44:39 +0100 Subject: [PATCH 2/3] fix: review findings on the login throttle - The throttle runs on a 64-bit clock in ms (esp_timer_get_time()), so a slot left alone for 49.7 days isn't brought back by millis() wrapping round to its old failure. - Web.cpp turns the client into the throttle's key in one place, and says that an IPv6 client would share key 0; names are clearer (retryMs, vacantSlot). - The shared slot's test now succeeds from the IP counted in it, so a success that cleared the shared slot would fail it. - GLOSSARY.md names it the login throttle, with lockouts, and SECURITY_PLAN.md and ADR 0006 use that name. Co-Authored-By: Claude Opus 5.5 --- GLOSSARY.md | 4 +++ docs/SECURITY_PLAN.md | 6 ++-- ...6-two-security-modes-eleven-http-routes.md | 2 +- src/LoginThrottle.h | 30 ++++++++++--------- src/entryPoints/Web.cpp | 17 +++++++---- test/test_loginthrottle/test_main.cpp | 24 ++++++++++----- 6 files changed, 53 insertions(+), 30 deletions(-) diff --git a/GLOSSARY.md b/GLOSSARY.md index 5fa4f78..85564f0 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) +**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 + ## Domain modules **Domain module** diff --git a/docs/SECURITY_PLAN.md b/docs/SECURITY_PLAN.md index 9853364..72de503 100644 --- a/docs/SECURITY_PLAN.md +++ b/docs/SECURITY_PLAN.md @@ -54,7 +54,7 @@ - **Comparison:** constant-time, used for HTTP and the websocket handshake alike. - **Migration:** none needed. The browser logs in again when its token is refused, and HA logs in again at runtime. -### Login throttling (PR C) +### Login throttle (PR C) - Per client IP: after 5 failed `/login` attempts, refuse that IP for 30 s, doubling each time up to 15 min. A success resets it. - After the first lockout, each failure starts the next one, so an IP gets one try per lockout rather than five. - An hour without a failure forgets the IP. @@ -87,7 +87,7 @@ Protocol 2 lands as one PR per step, and `main` keeps working throughout because | 3 | HA | protocol 2 with the old-firmware fallback; `/loginContext` setup; runtime login and reauth | | 4 | firmware | removals: the old routes, ports 8081 and 8080, `/discovery`, ModelSnapshot, "Secure Config Only"; static serving limited to web assets; CORS headers; Origin check | -PR B (token key) and PR C (login backoff) are separate firmware PRs that can land at any point. The GitHub update was removed ahead of the rest ([ADR 0007](adr/0007-no-github-update.md)), as protocol 2 didn't fit beside it. +PR B (token key) and PR C (login throttle) are separate firmware PRs that can land at any point. The GitHub update was removed ahead of the rest ([ADR 0007](adr/0007-no-github-update.md)), as protocol 2 didn't fit beside it. ## Tests @@ -95,7 +95,7 @@ The rules go into small pure helpers with native tests, following [src/Secrets.h - which routes are public - the token comparison - the Origin/Host match -- the backoff schedule +- 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. 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 cff72c5..7ccb633 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 backoff, CORS, the Origin check) is in [SECURITY_PLAN.md](../SECURITY_PLAN.md). +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). ## Considered Options diff --git a/src/LoginThrottle.h b/src/LoginThrottle.h index 7224a27..b1aeb51 100644 --- a/src/LoginThrottle.h +++ b/src/LoginThrottle.h @@ -13,8 +13,10 @@ // get the free failures on each: while the table is full, an IP it doesn't // hold may have to wait even with the right credentials. // -// The time is passed in (millis()), so native tests cover it. Only -// async_tcp's task calls it: both web servers run their handlers there. +// The time is passed in, in ms on a 64-bit clock (esp_timer_get_time() / +// 1000), so native tests cover it and a slot left alone for 49.7 days isn't +// woken by millis() wrapping. Only async_tcp's task calls it: both web +// servers run their handlers there. #define LOGIN_FREE_FAILURES 5 #define LOGIN_FIRST_LOCKOUT_MS 30000UL @@ -26,20 +28,20 @@ struct login_attempts_t { bool used = false; uint32_t ip = 0; uint16_t failures = 0; - uint32_t lastFailure = 0; // millis() of the last failure + uint64_t lastFailure = 0; // the time of the last failure uint32_t lockoutMs = 0; // the lockout that failure began, 0 for none }; class LoginThrottle { public: // How long ip must wait before it may try again, in ms; 0 when it may now. - uint32_t retryAfter(uint32_t ip, uint32_t now) const { + uint32_t retryAfter(uint32_t ip, uint64_t now) const { const login_attempts_t *a = this->find(ip, now); return a ? remaining(*a, now) : 0; } // A wrong PIN or password from ip. Returns the lockout it begins, in ms, // or 0 while ip still has free failures. - uint32_t failed(uint32_t ip, uint32_t now) { + uint32_t failed(uint32_t ip, uint64_t now) { login_attempts_t *a = this->claim(ip, now); if(a->failures < UINT16_MAX) a->failures++; a->lastFailure = now; @@ -63,18 +65,18 @@ class LoginThrottle { login_attempts_t slots[LOGIN_THROTTLE_SLOTS]; login_attempts_t shared; - static uint32_t remaining(const login_attempts_t &a, uint32_t now) { + static uint32_t remaining(const login_attempts_t &a, uint64_t now) { if(!a.used || a.lockoutMs == 0) return 0; - uint32_t elapsed = now - a.lastFailure; - return elapsed < a.lockoutMs ? a.lockoutMs - elapsed : 0; + uint64_t elapsed = now - a.lastFailure; + return elapsed < a.lockoutMs ? (uint32_t)(a.lockoutMs - elapsed) : 0; } // Unused, or its last failure is old enough to forget. - static bool vacant(const login_attempts_t &a, uint32_t now) { + static bool vacant(const login_attempts_t &a, uint64_t now) { return !a.used || now - a.lastFailure >= LOGIN_FORGET_MS; } // ip's record: its own slot, nothing while a slot is vacant, else the // shared one. - const login_attempts_t *find(uint32_t ip, uint32_t now) const { + const login_attempts_t *find(uint32_t ip, uint64_t now) const { bool anyVacant = false; for(const auto &a : this->slots) { if(vacant(a, now)) anyVacant = true; @@ -84,13 +86,13 @@ class LoginThrottle { return &this->shared; } // ip's record to count a failure in, taking a vacant slot when it has none. - login_attempts_t *claim(uint32_t ip, uint32_t now) { - login_attempts_t *free = nullptr; + login_attempts_t *claim(uint32_t ip, uint64_t now) { + login_attempts_t *vacantSlot = nullptr; for(auto &a : this->slots) { - if(vacant(a, now)) { if(!free) free = &a; } + if(vacant(a, now)) { if(!vacantSlot) vacantSlot = &a; } else if(a.ip == ip) return &a; } - login_attempts_t *a = free ? free : &this->shared; + login_attempts_t *a = vacantSlot ? vacantSlot : &this->shared; if(vacant(*a, now)) { *a = login_attempts_t(); a->used = true; diff --git a/src/entryPoints/Web.cpp b/src/entryPoints/Web.cpp index 8475f7b..f75adb8 100644 --- a/src/entryPoints/Web.cpp +++ b/src/entryPoints/Web.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include "esp_log.h" #include "mbedtls/md.h" #include "ConfigSettings.h" @@ -52,6 +53,11 @@ static const char *TAG = "Web"; // Failed logins per client IP, on both web servers. static LoginThrottle loginThrottle; +// The throttle's key for a client: its IPv4 address. The firmware doesn't +// enable IPv6; an IPv6 client would share key 0 with every other one. +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; } // 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. @@ -341,9 +347,10 @@ void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { } // A refused IP isn't told whether these credentials were right. IPAddress ip = request->client()->remoteIP(); - uint32_t wait = loginThrottle.retryAfter((uint32_t)ip, millis()); - if(wait) { - uint32_t secs = (wait + 999) / 1000; + uint32_t key = loginKey(ip); + uint32_t retryMs = loginThrottle.retryAfter(key, loginNow()); + if(retryMs) { + uint32_t secs = (retryMs + 999) / 1000; snprintf(g_async_content, sizeof(g_async_content), "{\"type\":%u,\"msg\":\"Too many failed logins. Try again in %lu s.\",\"success\":false,\"retryAfter\":%lu}", static_cast(settings.Security.type), (unsigned long)secs, (unsigned long)secs); @@ -380,14 +387,14 @@ void Web::handleLogin(AsyncWebServerRequest *request, JsonVariant &json) { if(String(ptok) == String(token)) success = true; } if(success) { - loginThrottle.succeeded((uint32_t)ip); + loginThrottle.succeeded(key); snprintf(g_async_content, sizeof(g_async_content), "{\"type\":%u,\"apiKey\":\"%s\",\"msg\":\"Success\",\"success\":true}", static_cast(settings.Security.type), token); request->send(200, _encoding_json, g_async_content); } else { - uint32_t lockout = loginThrottle.failed((uint32_t)ip, millis()); + uint32_t lockout = loginThrottle.failed(key, loginNow()); if(lockout) ESP_LOGW(TAG, "Failed login from %s: refused for %lu s", ip.toString().c_str(), (unsigned long)(lockout / 1000)); snprintf(g_async_content, sizeof(g_async_content), "{\"type\":%u,\"msg\":\"Invalid credentials\",\"success\":false}", diff --git a/test/test_loginthrottle/test_main.cpp b/test/test_loginthrottle/test_main.cpp index 5115fca..95bf725 100644 --- a/test/test_loginthrottle/test_main.cpp +++ b/test/test_loginthrottle/test_main.cpp @@ -8,7 +8,7 @@ static const uint32_t IP_A = 0x0A01A8C0; // 192.168.1.10 static const uint32_t IP_B = 0x0B01A8C0; // 192.168.1.11 // Fails ip n times at now, as fast as the throttle lets it. -static void failTimes(LoginThrottle &t, uint32_t ip, int n, uint32_t now) { +static void failTimes(LoginThrottle &t, uint32_t ip, int n, uint64_t now) { for(int i = 0; i < n; i++) t.failed(ip, now); } @@ -73,16 +73,24 @@ static void test_an_hour_without_a_failure_forgets_the_ip() { failTimes(t, IP_A, 7, 0); // Still counted just before the hour: the next failure doubles. TEST_ASSERT_EQUAL_UINT32(240000, t.failed(IP_A, LOGIN_FORGET_MS - 1)); - uint32_t later = 2 * LOGIN_FORGET_MS; + uint64_t later = 2 * LOGIN_FORGET_MS; TEST_ASSERT_EQUAL_UINT32(0, t.failed(IP_A, later)); } -static void test_millis_wrapping_keeps_the_lockout() { +static void test_a_lockout_spans_where_millis_would_wrap() { LoginThrottle t; - uint32_t now = UINT32_MAX - 10000; + uint64_t now = UINT32_MAX - 10000ULL; failTimes(t, IP_A, 5, now); TEST_ASSERT_EQUAL_UINT32(10000, t.retryAfter(IP_A, now + 20000)); TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, now + 30000)); } +static void test_a_slot_left_alone_for_50_days_stays_forgotten() { + LoginThrottle t; + failTimes(t, IP_A, 10, 1000); + // Where a 32-bit millis() would have come round to the same value. + uint64_t wrapped = 1000 + (1ULL << 32); + TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, wrapped)); + TEST_ASSERT_EQUAL_UINT32(0, t.failed(IP_A, wrapped)); +} /********************************************************************* * Several IPs @@ -113,7 +121,7 @@ static void test_a_forgotten_slot_is_taken_again() { LoginThrottle t; for(uint32_t i = 0; i < LOGIN_THROTTLE_SLOTS; i++) failTimes(t, 100 + i, 5, 0); // An hour later every slot is vacant: a new IP has its own free failures. - uint32_t later = LOGIN_FORGET_MS; + uint64_t later = LOGIN_FORGET_MS; TEST_ASSERT_EQUAL_UINT32(0, t.retryAfter(IP_A, later)); failTimes(t, IP_A, 4, later); TEST_ASSERT_EQUAL_UINT32(30000, t.failed(IP_A, later)); @@ -124,7 +132,8 @@ static void test_a_success_doesnt_clear_the_shared_slot() { LoginThrottle t; for(uint32_t i = 0; i < LOGIN_THROTTLE_SLOTS; i++) failTimes(t, 100 + i, 1, 0); failTimes(t, 200, 5, 0); - t.succeeded(201); + // 200 is counted in the shared slot, not one of its own. + t.succeeded(200); TEST_ASSERT_EQUAL_UINT32(30000, t.retryAfter(202, 0)); } @@ -139,7 +148,8 @@ int main(int argc, char **argv) { RUN_TEST(test_after_a_lockout_the_next_failure_locks_out_twice_as_long); RUN_TEST(test_a_success_resets_the_ip); RUN_TEST(test_an_hour_without_a_failure_forgets_the_ip); - RUN_TEST(test_millis_wrapping_keeps_the_lockout); + RUN_TEST(test_a_lockout_spans_where_millis_would_wrap); + RUN_TEST(test_a_slot_left_alone_for_50_days_stays_forgotten); RUN_TEST(test_one_ip_locked_out_leaves_the_others_alone); RUN_TEST(test_a_success_resets_only_its_own_ip); RUN_TEST(test_with_the_table_full_other_ips_share_one_slot); From 3082269e4b5cde202f848b8d018076193b9960c6 Mon Sep 17 00:00:00 2001 From: cjkas Date: Tue, 27 Oct 2026 20:39:04 +0100 Subject: [PATCH 3/3] docs: #81 is fixed by the login throttle Co-Authored-By: Claude Opus 5.5 --- docs/KNOWN_ISSUES.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index 1472703..a5b7694 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -6,7 +6,6 @@ - [#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