diff --git a/app/sovran_systemsos_web/security_helpers.py b/app/sovran_systemsos_web/security_helpers.py index 31f0196..ff46e52 100644 --- a/app/sovran_systemsos_web/security_helpers.py +++ b/app/sovran_systemsos_web/security_helpers.py @@ -15,6 +15,7 @@ import json import os import re import tempfile +import threading import time import urllib.parse @@ -245,6 +246,141 @@ def _validate_ssh_pubkey(key: str) -> str: return key +# ── Login throttling ────────────────────────────────────────────────────────── +# +# Delays applied after each failed login, and the lockout that follows once an +# address has tripped LOGIN_FAIL_MAX inside the window. +# +# LOGIN_FAIL_WINDOW has to be longer than the time it takes to reach +# LOGIN_FAIL_MAX failures under the ramping delay: with a 2s ramp capped at +# LOGIN_FAIL_MAX_DELAY, 10 attempts take about 80 seconds, so a 60 second +# window would silently expire the earliest failures and the counter could +# never reach the limit. 900s (15 minutes) keeps the whole ramp inside it. +LOGIN_FAIL_DELAY = 2.0 # base delay; the nth failure waits n x this +LOGIN_FAIL_MAX_DELAY = 10.0 # ceiling for a single delay +LOGIN_FAIL_WINDOW = 900.0 # rolling window failures are counted in +LOGIN_FAIL_MAX = 10 # failures in the window that trigger a lockout +LOGIN_LOCKOUT_SECONDS = 300.0 # how long the lockout lasts + +# Cap on how many addresses are tracked, so a distributed sweep cannot grow +# the table without bound. +_LOGIN_THROTTLE_MAX_IPS = 4096 + + +class LoginThrottle: + """Per-address failed-login tracking with a ramping delay and a lockout. + + The delay ramps so a script hammering the login form slows down as it goes, + and once LOGIN_FAIL_MAX failures land inside the window the address is + refused outright for LOGIN_LOCKOUT_SECONDS. A successful login clears the + address so a legitimate user who fumbles a password is not penalised later. + + ``sleep`` and ``clock`` are injectable so tests run without waiting. + """ + + def __init__( + self, + fail_delay=LOGIN_FAIL_DELAY, + max_delay=LOGIN_FAIL_MAX_DELAY, + window=LOGIN_FAIL_WINDOW, + max_failures=LOGIN_FAIL_MAX, + lockout=LOGIN_LOCKOUT_SECONDS, + max_tracked_ips=_LOGIN_THROTTLE_MAX_IPS, + sleep=None, + clock=None, + ): + self._fail_delay = float(fail_delay) + self._max_delay = float(max_delay) + self._window = float(window) + self._max_failures = int(max_failures) + self._lockout = float(lockout) + self._max_tracked_ips = int(max_tracked_ips) + self._sleep = sleep if sleep is not None else time.sleep + self._clock = clock if clock is not None else time.monotonic + self._lock = threading.Lock() + self._failures: dict[str, list[float]] = {} + + # ── internals ──────────────────────────────────────────────────────────── + + def _prune(self, ip, now): + """Drop timestamps outside the window; return what is left.""" + keep = [t for t in self._failures.get(ip, ()) if now - t < self._window] + if keep: + self._failures[ip] = keep + else: + self._failures.pop(ip, None) + return keep + + def _evict(self, now): + """Forget addresses that can no longer affect anything.""" + horizon = max(self._window, self._lockout) + for ip in [i for i, ts in self._failures.items() + if ts and now - max(ts) > horizon]: + self._failures.pop(ip, None) + while len(self._failures) > self._max_tracked_ips: + oldest = min(self._failures, key=lambda i: max(self._failures[i])) + self._failures.pop(oldest, None) + + # ── public API ─────────────────────────────────────────────────────────── + + def delay_for(self, count): + """Return the delay owed after *count* failures in the current window.""" + if count <= 0: + return 0.0 + return min(self._fail_delay * count, self._max_delay) + + def failure_count(self, ip): + """Return the failures currently counted against *ip*.""" + with self._lock: + return len(self._prune(ip, self._clock())) + + def is_locked_out(self, ip): + """Return True while *ip* is inside a lockout.""" + now = self._clock() + with self._lock: + failures = self._prune(ip, now) + if len(failures) < self._max_failures: + return False + return (now - failures[-1]) < self._lockout + + def remaining_lockout(self, ip): + """Return the seconds left in *ip*'s lockout, or 0.0 if not locked out.""" + now = self._clock() + with self._lock: + failures = self._prune(ip, now) + if len(failures) < self._max_failures: + return 0.0 + return max(0.0, self._lockout - (now - failures[-1])) + + def record_failure(self, ip): + """Record a failure for *ip* and serve out the delay it has earned. + + Returns the delay that was applied. The lock is never held across the + sleep, so one slow client cannot stall every other login. + """ + now = self._clock() + with self._lock: + failures = list(self._prune(ip, now)) + failures.append(now) + self._failures[ip] = failures + count = len(failures) + self._evict(now) + delay = self.delay_for(count) + if delay > 0: + self._sleep(delay) + return delay + + def clear(self, ip): + """Forget *ip*, e.g. after a successful login.""" + with self._lock: + self._failures.pop(ip, None) + + def tracked_addresses(self): + """Return how many addresses are currently being tracked.""" + with self._lock: + return len(self._failures) + + # ── Persistent Hub session store ───────────────────────────────────────────── def load_session_store(path: str) -> dict[str, float]: diff --git a/app/sovran_systemsos_web/server.py b/app/sovran_systemsos_web/server.py index b7423be..5f3a3fd 100644 --- a/app/sovran_systemsos_web/server.py +++ b/app/sovran_systemsos_web/server.py @@ -55,6 +55,12 @@ from .security_helpers import ( _bech32_convertbits_decode, load_session_store, save_session_store, + LoginThrottle, + LOGIN_FAIL_DELAY, + LOGIN_FAIL_MAX_DELAY, + LOGIN_FAIL_WINDOW, + LOGIN_FAIL_MAX, + LOGIN_LOCKOUT_SECONDS, ) from .update_state import effective_update_status @@ -175,11 +181,19 @@ _sessions_lock = Lock() _SESSION_PERSIST_MIN_INTERVAL = 30.0 # seconds _sessions_last_persist = 0.0 -# Failed login tracking: ip → list of failure timestamps -_login_failures: dict[str, list[float]] = {} -LOGIN_FAIL_DELAY = 2.0 # seconds to sleep after a failed attempt -LOGIN_FAIL_WINDOW = 60.0 # rolling window (seconds) for counting failures -LOGIN_FAIL_MAX = 10 # max failures in window before extra delay +# Failed login tracking. +# +# LOGIN_FAIL_MAX used to be declared here and never read anywhere: the only +# thing a failed attempt cost an attacker was a flat 2 second delay, and there +# was no lockout, no escalation and no ban. The throttling now lives in +# security_helpers.LoginThrottle, which ramps the delay and refuses an address +# outright once it has tripped LOGIN_FAIL_MAX inside the window. +# +# The window moved from 60s to 900s. With the ramping delay, reaching +# LOGIN_FAIL_MAX takes about 80 seconds, so a 60 second window expired the +# earliest failures before the limit could ever be reached — the old constant +# could not have worked even if it had been wired up. +_login_throttle = LoginThrottle() # Public paths that are accessible without a valid session _AUTH_EXEMPT_PATHS = {"/login", "/api/login", "/auto-login", "/api/ping"} @@ -770,19 +784,20 @@ def _ensure_onboarding_reopened_for_migration() -> None: logger.warning("Could not clear onboarding flag for migration flow: %s", exc) -def _record_failure(client_ip: str) -> None: - """Record a failed login attempt and apply a rate-limit delay. +def _record_failure(client_ip: str) -> float: + """Record a failed login attempt and apply the throttling delay. Must always be called via loop.run_in_executor() so that the blocking time.sleep() does not stall the asyncio event loop. + + Returns the delay that was applied. """ - now = time.time() - failures = _login_failures.setdefault(client_ip, []) - # Prune old entries outside the window - _login_failures[client_ip] = [t for t in failures if now - t < LOGIN_FAIL_WINDOW] - _login_failures[client_ip].append(now) - # Sleep in the thread-pool thread to slow brute-force without blocking the loop - time.sleep(LOGIN_FAIL_DELAY) + return _login_throttle.record_failure(client_ip) + + +def _is_locked_out(client_ip: str) -> bool: + """Return True while *client_ip* is inside a lockout.""" + return _login_throttle.is_locked_out(client_ip) # ── Authentication middleware ───────────────────────────────────── @@ -2638,10 +2653,24 @@ async def api_login(req: LoginRequest, request: Request): """Validate the Hub password and issue a session cookie.""" client_ip = request.client.host if request.client else "unknown" loop = asyncio.get_event_loop() + + # Refuse outright while the address is locked out. This runs before the + # scrypt hash, so a locked-out client costs almost nothing to reject. + if _is_locked_out(client_ip): + remaining = int(_login_throttle.remaining_lockout(client_ip) // 60) + 1 + raise HTTPException( + status_code=429, + detail=f"Too many failed attempts. Try again in about {remaining} minute(s).", + ) + ok = await loop.run_in_executor(None, _check_password, req.password) if not ok: await loop.run_in_executor(None, _record_failure, client_ip) raise HTTPException(status_code=401, detail="Incorrect password") + + # A real login clears the address, so fumbling a password once in a while + # does not accumulate towards a lockout. + _login_throttle.clear(client_ip) token = _create_session() response = JSONResponse({"ok": True}) response.set_cookie( diff --git a/tests/test_login_throttle.py b/tests/test_login_throttle.py new file mode 100644 index 0000000..4a7b5f1 --- /dev/null +++ b/tests/test_login_throttle.py @@ -0,0 +1,196 @@ +"""Tests for the Hub's login throttling. + +These exercise the exact production implementation in +sovran_systemsos_web.security_helpers.LoginThrottle. The clock and the sleep are +injected, so the tests cover hours of lockout behaviour instantly. + +No network access, no filesystem writes, no real delays. +""" + +import os +import sys +import unittest + +_REPO_ROOT = os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +_APP_PARENT = os.path.join(_REPO_ROOT, "app") +if _APP_PARENT not in sys.path: + sys.path.insert(0, _APP_PARENT) + +from sovran_systemsos_web.security_helpers import ( # noqa: E402 + LoginThrottle, + LOGIN_FAIL_DELAY, + LOGIN_FAIL_MAX_DELAY, + LOGIN_FAIL_WINDOW, + LOGIN_FAIL_MAX, + LOGIN_LOCKOUT_SECONDS, +) + + +class FakeClock: + """A clock that only moves when the test says so.""" + + def __init__(self): + self.now = 1000.0 + + def __call__(self): + return self.now + + def advance(self, seconds): + self.now += seconds + + +class FakeSleeper: + """Records the delays it was asked to apply instead of sleeping.""" + + def __init__(self, clock): + self.clock = clock + self.calls = [] + + def __call__(self, seconds): + self.calls.append(seconds) + self.clock.advance(seconds) + + +def _make(**kwargs): + clock = kwargs.pop("clock", None) or FakeClock() + sleep = kwargs.pop("sleep", None) or FakeSleeper(clock) + return LoginThrottle(clock=clock, sleep=sleep, **kwargs), clock, sleep + + +def _trip(throttle, ip="203.0.113.9"): + """Fail LOGIN_FAIL_MAX times. The fake sleeper advances the clock for us.""" + for _ in range(LOGIN_FAIL_MAX): + throttle.record_failure(ip) + + +class DelayRamp(unittest.TestCase): + + def test_delay_ramps_with_the_failure_count(self): + throttle, _, _ = _make() + self.assertEqual(throttle.delay_for(0), 0.0) + self.assertEqual(throttle.delay_for(1), LOGIN_FAIL_DELAY) + self.assertEqual(throttle.delay_for(3), LOGIN_FAIL_DELAY * 3) + + def test_delay_is_capped(self): + # Unbounded ramping would let a single client park a thread-pool worker + # for minutes at a time. + throttle, _, _ = _make() + self.assertLessEqual(throttle.delay_for(999), LOGIN_FAIL_MAX_DELAY) + self.assertEqual(throttle.delay_for(999), LOGIN_FAIL_MAX_DELAY) + + def test_first_failure_is_not_delayed_much(self): + throttle, _, sleep = _make() + delay = throttle.record_failure("203.0.113.9") + self.assertEqual(delay, LOGIN_FAIL_DELAY) + self.assertEqual(sleep.calls, [LOGIN_FAIL_DELAY]) + + +class Lockout(unittest.TestCase): + + def test_not_locked_out_initially(self): + throttle, _, _ = _make() + self.assertFalse(throttle.is_locked_out("203.0.113.9")) + self.assertEqual(throttle.remaining_lockout("203.0.113.9"), 0.0) + + def test_reaching_the_limit_locks_the_address_out(self): + throttle, _, _ = _make() + _trip(throttle) + self.assertTrue(throttle.is_locked_out("203.0.113.9")) + + def test_the_limit_is_reachable_inside_the_window(self): + # Regression guard for the old 60s window: with a ramping delay it + # takes ~80s to reach LOGIN_FAIL_MAX, so a 60s window expired the + # earliest failures first and the lockout could never fire. + throttle, clock, _ = _make() + start = clock.now + _trip(throttle) + self.assertLess(clock.now - start, LOGIN_FAIL_WINDOW) + self.assertEqual(throttle.failure_count("203.0.113.9"), LOGIN_FAIL_MAX) + self.assertTrue(throttle.is_locked_out("203.0.113.9")) + + def test_one_failure_short_of_the_limit_is_not_a_lockout(self): + throttle, _, _ = _make() + for _ in range(LOGIN_FAIL_MAX - 1): + throttle.record_failure("203.0.113.9") + self.assertFalse(throttle.is_locked_out("203.0.113.9")) + + def test_lockout_expires(self): + throttle, clock, _ = _make() + _trip(throttle) + self.assertTrue(throttle.is_locked_out("203.0.113.9")) + clock.advance(LOGIN_LOCKOUT_SECONDS + 1) + self.assertFalse(throttle.is_locked_out("203.0.113.9")) + + def test_remaining_lockout_counts_down(self): + throttle, clock, _ = _make() + _trip(throttle) + full = throttle.remaining_lockout("203.0.113.9") + # the final record_failure applied a delay, which the fake clock has + # already advanced, so what is left is the lockout minus that delay + self.assertAlmostEqual(full, LOGIN_LOCKOUT_SECONDS, + delta=LOGIN_FAIL_MAX_DELAY + 1.0) + clock.advance(full / 2) + self.assertLess(throttle.remaining_lockout("203.0.113.9"), full) + self.assertGreater(throttle.remaining_lockout("203.0.113.9"), 0.0) + + def test_further_failures_while_locked_out_extend_it(self): + throttle, clock, _ = _make() + _trip(throttle) + clock.advance(LOGIN_LOCKOUT_SECONDS - 1) + throttle.record_failure("203.0.113.9") + self.assertTrue(throttle.is_locked_out("203.0.113.9")) + + +class Isolation(unittest.TestCase): + + def test_one_address_does_not_lock_out_another(self): + throttle, _, _ = _make() + _trip(throttle, "203.0.113.9") + self.assertTrue(throttle.is_locked_out("203.0.113.9")) + self.assertFalse(throttle.is_locked_out("198.51.100.7")) + + def test_successful_login_clears_the_address(self): + throttle, _, _ = _make() + for _ in range(LOGIN_FAIL_MAX - 1): + throttle.record_failure("203.0.113.9") + throttle.clear("203.0.113.9") + self.assertEqual(throttle.failure_count("203.0.113.9"), 0) + self.assertFalse(throttle.is_locked_out("203.0.113.9")) + + def test_old_failures_age_out_of_the_window(self): + throttle, clock, _ = _make() + throttle.record_failure("203.0.113.9") + clock.advance(LOGIN_FAIL_WINDOW + 1) + self.assertEqual(throttle.failure_count("203.0.113.9"), 0) + + +class BoundedMemory(unittest.TestCase): + + def test_tracked_addresses_are_evicted(self): + throttle, clock, _ = _make(max_tracked_ips=8) + for i in range(64): + throttle.record_failure(f"198.51.100.{i}") + clock.advance(LOGIN_FAIL_WINDOW + LOGIN_LOCKOUT_SECONDS + 1) + throttle.record_failure("203.0.113.9") + self.assertLessEqual(throttle.tracked_addresses(), 8) + + def test_sleep_is_never_called_under_the_lock(self): + # If the lock were held across the sleep, one slow client would stall + # every other login — a self-inflicted DoS. + throttle, clock, _ = _make() + order = [] + + def spy(seconds): + order.append("sleep:start") + clock.advance(seconds) + order.append("sleep:end") + + throttle._sleep = spy + throttle.record_failure("203.0.113.9") + self.assertEqual(order, ["sleep:start", "sleep:end"]) + # A second address can still be recorded while the first is "sleeping". + self.assertEqual(throttle.failure_count("198.51.100.7"), 0) + + +if __name__ == "__main__": + unittest.main()