Phase 10 — critical & high security fixes (SEC-01, SEC-02, SEC-03, SEC-04, SEC-05, SEC-07, SEC-08, F17)

CHANGELOG.md

@@ -10,6 +10,48 @@ into a new dated `## [<version>] - YYYY-MM-DD` section.
 
 ## [Unreleased]
 
+> ### Upgrading logs everyone out — once
+>
+> **Every existing session is invalidated by this upgrade. Each operator must log in again
+> exactly once.** Nothing is lost and no credentials change; the login page simply appears the
+> first time you open the dashboard after updating.
+>
+> Why: session tokens are now bound to the account credentials, and a token that does not carry
+> that binding is **refused**. Tokens issued by earlier versions do not carry it — and neither
+> does a token forged from a stolen signing secret. The two are indistinguishable on that point,
+> so accepting the ones without it would have left the forgery path open and made the fix
+> cosmetic. A password change now also ends every existing session, which it previously did not.
+>
+> If you are reading this **while responding to an incident**, the companion action is the new
+> `ipmideck rotate-session-secret` command: it replaces the session signing secret, so cookies
+> minted offline from a copied or stolen database stop working. Stop the app, run it, restart.
+
+### Security
+
+- **Fixed a pre-authentication path traversal in the SPA catch-all (SEC-01).** An unauthenticated
+  request for an escaping path (`../../`, and its `%2e` / `%2f` encoded spellings) could read any
+  file the server process could — including the database and the credential encryption key. Paths
+  are now canonicalised and required to resolve inside the web root.
+- **The session signing secret can now be rotated (SEC-02).** New CLI action
+  `ipmideck rotate-session-secret`. Previously a secret read out of a copied database kept minting
+  valid cookies forever, with no supported way to evict them.
+- **Changing the account password now ends every existing session (SEC-03).** It previously
+  revoked nothing.
+- **Completing first-run setup always leaves authentication enabled (SEC-04).** An instance whose
+  authentication had been switched off before setup used to stay open permanently, with no visible
+  symptom. Note that a freshly started, not-yet-configured instance is still claimable by anyone
+  who can reach it, until first-run setup is completed.
+- **Rewriting the account now requires the current password (SEC-05).** A valid-looking session
+  cookie alone could previously replace the sole account through the Security settings — including
+  on an instance with authentication disabled. The Security form has one new
+  `Current password` field for this.
+- **The app-config endpoint no longer returns the session secret (SEC-07).** The read path now
+  enforces the same allow-list the write path already had.
+- **`reset-password` no longer reports success for a username that does not exist (F17).**
+- **Backup archives are credential-grade.** An archive bundles the encryption key, the database
+  and the configuration together, so it must be stored as carefully as the credentials themselves.
+
+
 ## [2.0.1] - 2026-07-25
 
 ### Fixed

backend/api/auth_routes.py

@@ -47,6 +47,10 @@ class SetupRequest(BaseModel):
 class ConfigureRequest(BaseModel):
     username: str
     password: str
+    # SEC-05 (F6/F10): required whenever an account already exists — including on an
+    # auth-DISABLED instance, which is the F10 window. Optional only at genuine first
+    # run (no account), where there is no password to prove knowledge of.
+    current_password: str | None = None
 
 
 class ToggleRequest(BaseModel):
@@ -68,7 +72,7 @@ async def _require_session_if_active(request: Request, auth) -> None:
     """
     if await auth.is_auth_enabled() and await auth.has_user():
         token = request.cookies.get("session")
-        if not token or not auth.verify_session_token(token):
+        if not token or not await auth.verify_session_token_async(token):
             raise HTTPException(status_code=401, detail={"error": "unauthorized"})
 
 
@@ -79,7 +83,7 @@ async def get_me(request: Request):
     if not await auth.is_auth_enabled():
         return {"authenticated": True, "username": "local", "auth_enabled": False, "has_user": has_user}
     token = request.cookies.get("session")
-    username = auth.verify_session_token(token) if token else None
+    username = await auth.verify_session_token_async(token) if token else None
     # REVIEWS #7: mirror require_auth — a token whose subject is no longer the current
     # stored user (e.g. after a credential replace) is NOT authenticated. Keeps /me
     # consistent with protected routes so the frontend boot routing sees the same state.
@@ -125,7 +129,7 @@ async def login(body: LoginRequest, request: Request, response: Response, lang:
 
     # 3. Success: clear any prior failure counter, issue session.
     await auth.reset_failures(body.username)
-    token = auth.create_session_token(body.username)
+    token = await auth.create_session_token_async(body.username)
     _set_session_cookie(response, request, token, auth.session_expiry_seconds)
     return {"success": True, "username": body.username}
 
@@ -138,11 +142,24 @@ async def logout(response: Response):
 
 @router.post("/setup")
 async def setup(body: SetupRequest, request: Request, response: Response, lang: str = Depends(get_lang)):
+    """First-run account creation. Always leaves authentication ENABLED.
+
+    SEC-04 clause 2 (F5): an anonymous caller can disable auth on a
+    not-yet-configured instance (that clause is deferred onto SEC-06), and
+    `setup` used to create the account without
+    touching the flag. The instance therefore stayed open forever, with no UI
+    symptom, even after the real operator finished first run.
+
+    Re-enabling unconditionally removes the DURABILITY and the SILENCE of that
+    attack: whatever the flag was beforehand, completing first run closes the
+    instance and the login page is enforced from then on.
+    """
     from backend.main import auth
     if await auth.has_user():
         return {"success": False, "error": t("user_already_exists", lang)}
     await auth.create_user(body.username, body.password)
-    token = auth.create_session_token(body.username)
+    await auth.set_auth_enabled(True)
+    token = await auth.create_session_token_async(body.username)
     _set_session_cookie(response, request, token, auth.session_expiry_seconds)
     return {"success": True, "username": body.username}
 
@@ -156,15 +173,44 @@ async def configure_auth(body: ConfigureRequest, request: Request, response: Res
     endpoint is NOT an unauthenticated credential-takeover path. Issues a fresh session
     cookie for the new username so the operator stays logged in (and the new cookie
     passes the require_auth current-user check while any old-username cookie is rejected).
+
+    SEC-05 (F6, and F10 as a side effect): a valid-looking session was the ONLY thing
+    standing between a caller and a rewrite of the sole account — the exact action an
+    incident responder takes to evict an attacker. Proving knowledge of the CURRENT
+    password is now required whenever an account exists.
+
+    The gate keys on `has_user()`, NOT on `auth_enabled`. That keying is load-bearing:
+    keying on `auth_enabled` would leave the F10 window wide open, because an
+    auth-disabled instance with an existing account could be seized with no cookie at
+    all — and the frontend only shows this form when auth is OFF, so an `auth_enabled`
+    condition would never fire from the UI.
+
+    With no account present nothing is required: that is genuine first run.
     """
     from backend.main import auth
     await _require_session_if_active(request, auth)
+
+    if await auth.has_user():
+        if not body.current_password:
+            return {"success": False, "error": "Current password is required"}
+        # On an auth-disabled instance there is no session to name the current user,
+        # so fall back to the single stored account row (the users table is single-user).
+        token = request.cookies.get("session")
+        current_username = await auth.verify_session_token_async(token) if token else None
+        if not current_username:
+            row = await auth.db.fetchone("SELECT username FROM users LIMIT 1")
+            current_username = row["username"] if row else None
+        if not current_username or not await auth.verify_password(
+            current_username, body.current_password
+        ):
+            return {"success": False, "error": "Incorrect password"}
+
     try:
         await auth.replace_user(body.username, body.password)
     except ValueError as e:
         return {"success": False, "error": str(e)}
     await auth.set_auth_enabled(True)
-    token = auth.create_session_token(body.username)
+    token = await auth.create_session_token_async(body.username)
     _set_session_cookie(response, request, token, auth.session_expiry_seconds)
     return {"success": True, "username": body.username}
 
@@ -209,7 +255,7 @@ async def toggle_auth(body: ToggleRequest, request: Request, lang: str = Depends
                 "error": "Current password is required to disable authentication",
             }
         token = request.cookies.get("session")
-        username = auth.verify_session_token(token) if token else None
+        username = await auth.verify_session_token_async(token) if token else None
         if not username or not await auth.verify_password(username, body.current_password):
             return {"success": False, "error": "Incorrect password"}
 

backend/api/system_routes.py

@@ -58,10 +58,16 @@ class AppConfigValueBody(BaseModel):
 async def get_app_config_value(key: str):
     """Read a single app_config value. Returns {success, key, value}.
 
+    SEC-07 (F11): the key must be in the SAME allow-list the PUT path enforces.
+    Without it the endpoint served any app_config row by name — `session_secret`
+    included — to any caller holding a session (real, stolen, or forged).
+
     Bool-shaped storage convention: values stored as 'true'/'false' strings are
     coerced back to JSON booleans in the response so the frontend can use
     them directly. Missing rows return value=None (not an error).
     """
+    if key not in _ALLOWED_APP_CONFIG_KEYS:
+        return {"success": False, "error": "key_not_allowed"}
     from backend.main import db
     raw = await db.get_config(key, default=None)
     if raw is None:

backend/core/auth.py

@@ -290,6 +290,30 @@ async def _setup_session_secret(self) -> None:
             await self.db.set_config("session_secret", secret)
         self._secret = secret
 
+    async def rotate_session_secret(self) -> str:
+        """SEC-02 (F2): replace the session-signing secret; evict every cookie.
+
+        Sessions are stateless HMACs, so whoever reads `app_config.session_secret`
+        out of a copied database can mint valid cookies for the real username
+        forever — patching the read path (SEC-01/SEC-07) stops NEW disclosures but
+        cannot un-disclose a secret already taken. This is the eviction move.
+
+        Generates a fresh secret, persists it, and assigns it to `self._secret`.
+        Note the CLI action runs in a SEPARATE process: a server that is already
+        running keeps serving with the old secret held in memory, so the supported
+        sequence is stop -> rotate -> start. Returns the new secret so the caller
+        can confirm it changed (it is not printed to the operator).
+
+        Deliberately does NOT touch `self._file_key`: the at-rest credential key
+        has a different lifecycle (see the four-case migration in `initialize()`)
+        and re-keying stored BMC credentials is not part of this operation.
+        """
+        secret = secrets.token_hex(32)
+        await self.db.set_config("session_secret", secret)
+        self._secret = secret
+        logger.info("Session signing secret rotated — all existing sessions are now invalid")
+        return secret
+
     async def is_auth_enabled(self) -> bool:
         val = await self.db.get_config("auth_enabled", "true")
         return val.lower() in ("true", "1", "yes")
@@ -318,13 +342,23 @@ async def verify_password(self, username: str, password: str) -> bool:
             return False
         return bcrypt.checkpw(password.encode(), row["password_hash"].encode())
 
-    async def update_password(self, username: str, new_password: str) -> None:
+    async def update_password(self, username: str, new_password: str) -> bool:
+        """Set a new password. Returns True iff a row was actually updated.
+
+        F17: the UPDATE silently matches zero rows for a username that does not
+        exist, and the caller used to print "Password updated" regardless — an
+        operator recovering from an incident was told a password had changed
+        when it had not. The rowcount is returned rather than raised so callers
+        stay simple and decide their own reporting.
+        """
         pw_hash = bcrypt.hashpw(new_password.encode(), bcrypt.gensalt()).decode()
-        await self.db.execute(
+        cursor = await self.db.execute(
             "UPDATE users SET password_hash = ? WHERE username = ?",
             (pw_hash, username),
         )
+        changed = bool(getattr(cursor, "rowcount", 0))
         await self.db.commit()
+        return changed
 
     async def replace_user(self, username: str, password: str) -> None:
         """Single-user create-or-replace: clear the users table and insert one row.
@@ -399,6 +433,25 @@ async def reset_failures(self, username: str) -> None:
         async with self._fail_lock:
             self._fail_state.pop(username, None)
 
+    async def _credential_fingerprint(self, username: str) -> str | None:
+        """SEC-03 (F7): a short, stable digest of the user's stored password hash.
+
+        The bcrypt hash changes on every password change (per-hash salt), so a
+        fingerprint derived from it changes with the credentials and nothing extra
+        needs storing. Returns None when the username has no row — the caller then
+        has nothing to compare against and must refuse.
+
+        The claim is an equality check, not a secret: it travels inside a payload
+        that is already HMAC-signed, and a truncated digest of a bcrypt hash
+        reveals nothing usable without the hash itself.
+        """
+        row = await self.db.fetchone(
+            "SELECT password_hash FROM users WHERE username = ?", (username,)
+        )
+        if not row:
+            return None
+        return hashlib.sha256(row["password_hash"].encode()).hexdigest()[:16]
+
     def create_session_token(self, username: str) -> str:
         payload = {
             "sub": username,
@@ -413,8 +466,34 @@ def create_session_token(self, username: str) -> str:
         b64 = base64.urlsafe_b64encode(data.encode()).decode().rstrip("=")
         return f"{b64}.{sig}"
 
+    async def create_session_token_async(self, username: str) -> str:
+        """Mint a session token carrying the credential-fingerprint claim (SEC-03).
+
+        An async SIBLING of `create_session_token()` rather than a change to it:
+        the fingerprint needs a DB read, and the synchronous version is called
+        directly by existing tests. All three cookie issuers in `auth_routes.py`
+        are already async handlers, so awaiting here costs nothing.
+        """
+        payload = {
+            "sub": username,
+            "iat": int(time.time()),
+            "exp": int(time.time()) + self.session_expiry_seconds,
+        }
+        fingerprint = await self._credential_fingerprint(username)
+        if fingerprint is not None:
+            payload["cfp"] = fingerprint
+        data = json.dumps(payload, separators=(",", ":"))
+        sig = hmac.new(self._secret.encode(), data.encode(), hashlib.sha256).hexdigest()
+        b64 = base64.urlsafe_b64encode(data.encode()).decode().rstrip("=")
+        return f"{b64}.{sig}"
+
     def verify_session_token(self, token: str) -> str | None:
-        """Returns username if valid, None otherwise."""
+        """Returns username if valid, None otherwise.
+
+        Signature + expiry only. The credential-fingerprint claim is checked by
+        `verify_session_token_async()`, which needs a DB read; every request path
+        that authenticates a cookie MUST use that one (see the fail-closed rule there).
+        """
         try:
             b64_part, sig_part = token.rsplit(".", 1)
             # Re-pad and decode the base64url data part back to the raw JSON that was signed.
@@ -433,6 +512,50 @@ def verify_session_token(self, token: str) -> str | None:
         except Exception:
             return None
 
+    async def verify_session_token_async(self, token: str) -> str | None:
+        """Full session verification: signature, expiry, AND credential binding.
+
+        SEC-03 — **FAIL-CLOSED**. A token whose payload carries no `cfp`
+        claim is REJECTED. That is the whole point of the change: a forged token
+        (minted from a stolen signing secret) omits the claim in exactly the same
+        way a pre-upgrade token does, so treating "absent" as acceptable would
+        leave the forgery path open and make the fix cosmetic. The cost — every
+        operator is logged out once on upgrade — was accepted with eyes open and
+        is documented prominently in the CHANGELOG.
+
+        Returns the username only when the claim matches a fingerprint recomputed
+        from the CURRENT stored hash, so a password change evicts every token
+        minted before it without adding a session store.
+        """
+        try:
+            b64_part, sig_part = token.rsplit(".", 1)
+            data_part = base64.urlsafe_b64decode(
+                b64_part + "=" * (-len(b64_part) % 4)
+            ).decode()
+            expected_sig = hmac.new(
+                self._secret.encode(), data_part.encode(), hashlib.sha256
+            ).hexdigest()
+            if not hmac.compare_digest(sig_part, expected_sig):
+                return None
+            payload = json.loads(data_part)
+            if payload.get("exp", 0) < time.time():
+                return None
+            username = payload.get("sub")
+            if not username:
+                return None
+        except Exception:
+            return None
+
+        claim = payload.get("cfp")
+        if not claim:
+            return None  # fail-closed: absent claim == forged or pre-upgrade
+        current = await self._credential_fingerprint(username)
+        if current is None:
+            return None  # token subject is no longer the stored user
+        if not hmac.compare_digest(claim, current):
+            return None
+        return username
+
     def get_encryption_key(self) -> bytes:
         """Return the at-rest BMC-credential encryption key.
 
@@ -467,7 +590,7 @@ async def require_auth(request: Request) -> str:
     if not token:
         raise HTTPException(status_code=401, detail={"error": "unauthorized"})
 
-    username = _auth.verify_session_token(token)
+    username = await _auth.verify_session_token_async(token)
     if not username:
         raise HTTPException(status_code=401, detail={"error": "unauthorized"})