diff --git a/Docs/Architekture.md b/Docs/Architekture.md index 0ca8112..df9f415 100644 --- a/Docs/Architekture.md +++ b/Docs/Architekture.md @@ -647,7 +647,7 @@ BanGUI maintains its **own SQLite database** (separate from the fail2ban databas | Table | Purpose | |---|---| | `settings` | Key-value store for application configuration (master password hash, fail2ban socket path, database path, timezone, session duration) | -| `sessions` | Active session tokens with expiry timestamps | +| `sessions` | Active session token hashes with expiry timestamps. Tokens are stored as one-way SHA256 hashes to prevent token hijacking if the database is exposed. | | `geo_cache` | Resolved IP geolocation results (ip, country_code, country_name, asn, org, cached_at). Loaded into memory at startup via `load_cache_from_db()`; new entries are flushed back by the `geo_cache_flush` background task. | | `blocklist_sources` | Registered blocklist URLs (id, name, url, enabled, created_at, updated_at) | | `import_logs` | Record of every blocklist import run (id, source_id, timestamp, ips_imported, ips_skipped, errors, status) | @@ -665,7 +665,8 @@ BanGUI maintains its **own SQLite database** (separate from the fail2ban databas - **Single-user model** — one master password, no usernames. - Password is hashed with a strong algorithm (e.g., bcrypt or argon2) and stored in the application database during setup. -- Sessions are token-based, stored server-side in the `sessions` table, and delivered to the browser as HTTP-only secure cookies. +- Sessions are token-based, stored server-side in the `sessions` table as one-way SHA256 hashes, and delivered to the browser as HTTP-only secure cookies. +- **Session token hashing** — Session tokens are hashed before storage to prevent token hijacking if the database file is exposed. Only the hash (`token_hash`) is stored in the database; the raw token is never persisted. When validating a session, the incoming token is hashed before the database lookup. This ensures the database alone is not sufficient to usurp a session — an attacker would also need knowledge of the original token value. - Session expiry is configurable (set during setup, stored in `settings`). - The frontend `AuthProvider` checks session validity on mount and redirects to `/login` if invalid. - The backend `dependencies.py` provides an `authenticated` dependency that validates the session cookie on every protected endpoint. diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index c92510a..7cf9388 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -1001,7 +1001,88 @@ async def get_session_repo() -> SessionRepository: - Before each deployment, run `mypy --strict` to ensure all dependency providers return values compatible with their Protocol types. - The `cast()` calls in `dependencies.py` are a documented signal that structural compatibility is being verified externally, not via explicit class inheritance. -#### 13.7.2 Session Cache Pluggability — Process-Local vs. Shared Backends +#### 13.7.2 Session Token Hashing — One-Way Protection Against Database Exposure + +Session tokens must be protected against database exposure. **Session tokens are stored as one-way SHA256 hashes in the database** to ensure that if the database file is compromised (volume mount misconfiguration, backup leak, etc.), the session tokens themselves cannot be directly used to hijack sessions. + +**Implementation pattern:** + +```python +import hashlib +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + import aiosqlite + +from app.models.auth import Session + +def _hash_token(token: str) -> str: + """Return the SHA256 hash of a session token.""" + return hashlib.sha256(token.encode()).hexdigest() + +async def create_session( + db: "aiosqlite.Connection", + token: str, + created_at: str, + expires_at: str, +) -> Session: + """Insert a new session row with the token hash.""" + token_hash = _hash_token(token) + cursor = await db.execute( + "INSERT INTO sessions (token_hash, created_at, expires_at) VALUES (?, ?, ?)", + (token_hash, created_at, expires_at), + ) + await db.commit() + # Return the Session with the ORIGINAL token (not the hash) + # so the service layer can sign and return it to the client. + return Session( + id=int(cursor.lastrowid) if cursor.lastrowid else 0, + token=token, # ← raw token, not the hash + created_at=created_at, + expires_at=expires_at, + ) + +async def get_session( + db: "aiosqlite.Connection", + token: str +) -> Session | None: + """Look up a session by token hash.""" + token_hash = _hash_token(token) + async with db.execute( + "SELECT id, token_hash, created_at, expires_at FROM sessions WHERE token_hash = ?", + (token_hash,), + ) as cursor: + row = await cursor.fetchone() + + if row is None: + return None + + # Return the Session with the INCOMING token (the one the client sent). + return Session( + id=int(row[0]), + token=token, # ← the raw token passed in + created_at=str(row[2]), + expires_at=str(row[3]), + ) +``` + +**Key points:** + +1. **Hash on write** — When inserting a session, hash the token before storage. +2. **Hash on read** — When validating a session, hash the incoming token before the database lookup. +3. **Never store raw tokens** — The `token_hash` column contains only hashes; raw tokens are never persisted. +4. **Return raw tokens to the service layer** — The `Session` model's `token` field contains the raw token (for signing and response), not the hash. +5. **Database schema** — Use `token_hash TEXT NOT NULL UNIQUE` instead of `token TEXT NOT NULL UNIQUE`, and create an index on `token_hash`. +6. **Migration strategy** — When upgrading from plaintext to hashed tokens, drop the old table and recreate it. This invalidates all existing sessions, which is acceptable because the database was exposed in plaintext. + +**Why one-way hashing is safe:** +- If an attacker obtains a token hash from the database, they cannot reverse the SHA256 hash to recover the original token. +- The attacker cannot use the hash directly in a client request — they would need the original token to pass the hash check. +- This forces the attacker to either compromise the client (where they'd also get the raw token) or perform a brute-force attack against the hash space (infeasible for random 128-bit tokens). + +**Never use symmetric encryption** — symmetric encryption stores a key in the database or environment, which merely shifts the exposure risk. A one-way hash is the correct choice for protecting tokens. + +#### 13.7.3 Session Cache Pluggability — Process-Local vs. Shared Backends Session validation is expensive (SQLite lookup + password verification). To improve performance, **validated session tokens are cached** using the `SessionCache` interface (`app.utils.session_cache`). The default implementation, `InMemorySessionCache`, stores cached sessions in process-local memory. diff --git a/Docs/Tasks.md b/Docs/Tasks.md index ca5d3ad..cbae101 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,31 +1,3 @@ -## TASK-021 — `set_jail_config_enabled` and `write_jail_config_file` not atomic - -**Severity:** Medium - -### Where found -`backend/app/services/raw_config_io_service.py` lines ~268 (`set_jail_config_enabled`) and ~344 (`write_jail_config_file`) — both use `path.write_text(updated)` directly. - -### Why this is needed -Same root cause as TASK-018. A process kill mid-write leaves the jail config file corrupted, disabling that jail on next fail2ban reload. - -### Goal -Atomic writes for `set_jail_config_enabled` and `write_jail_config_file`. - -### What to do -Same as TASK-018: replace `path.write_text(content)` with the `NamedTemporaryFile` + `os.replace()` pattern in both functions. This is most efficiently done as part of TASK-018 by extracting a shared `atomic_write(path, content)` helper in `config_file_helpers.py`. - -### Possible traps and issues -- Same as TASK-018. -- Extracting the helper makes TASK-018 and TASK-021 a single coordinated change. - -### Docs changes needed -- `Backend-Development.md` — atomic write helper documentation. - -### Doc references -- [Backend-Development.md](Backend-Development.md) — file I/O conventions - ---- - ## TASK-022 — Session tokens stored in plaintext in SQLite **Severity:** High diff --git a/backend/app/db.py b/backend/app/db.py index 177648c..97b0ae7 100644 --- a/backend/app/db.py +++ b/backend/app/db.py @@ -30,15 +30,15 @@ CREATE TABLE IF NOT EXISTS settings ( _CREATE_SESSIONS: str = """ CREATE TABLE IF NOT EXISTS sessions ( - id INTEGER PRIMARY KEY AUTOINCREMENT, - token TEXT NOT NULL UNIQUE, - created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - expires_at TEXT NOT NULL + id INTEGER PRIMARY KEY AUTOINCREMENT, + token_hash TEXT NOT NULL UNIQUE, + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + expires_at TEXT NOT NULL ); """ _CREATE_SESSIONS_TOKEN_INDEX: str = """ -CREATE UNIQUE INDEX IF NOT EXISTS idx_sessions_token ON sessions (token); +CREATE UNIQUE INDEX IF NOT EXISTS idx_sessions_token_hash ON sessions (token_hash); """ _CREATE_BLOCKLIST_SOURCES: str = """ @@ -107,10 +107,24 @@ _SCHEMA_STATEMENTS: list[str] = [ _CREATE_HISTORY_ARCHIVE, ] -_CURRENT_SCHEMA_VERSION: int = 1 +_CURRENT_SCHEMA_VERSION: int = 2 _MIGRATIONS: dict[int, str] = { 1: "\n".join(_SCHEMA_STATEMENTS), + 2: """ +-- Migration 2: Hash session tokens for security. +-- Drop the old sessions table and recreate with token_hash column. +-- This invalidates all existing sessions, which is acceptable as the DB +-- contents were exposed in plaintext. +DROP TABLE IF EXISTS sessions; +CREATE TABLE sessions ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + token_hash TEXT NOT NULL UNIQUE, + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), + expires_at TEXT NOT NULL +); +CREATE UNIQUE INDEX idx_sessions_token_hash ON sessions (token_hash); +""", } diff --git a/backend/app/repositories/session_repo.py b/backend/app/repositories/session_repo.py index 94cb7f0..f51dc39 100644 --- a/backend/app/repositories/session_repo.py +++ b/backend/app/repositories/session_repo.py @@ -2,10 +2,15 @@ Provides storage, retrieval, and deletion of session records in the ``sessions`` table of the application SQLite database. + +Session tokens are stored as one-way SHA256 hashes to ensure that if the +database is exposed, the session tokens themselves cannot be directly used. +The hash is computed from the raw token before all database operations. """ from __future__ import annotations +import hashlib from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -14,6 +19,18 @@ if TYPE_CHECKING: from app.models.auth import Session +def _hash_token(token: str) -> str: + """Return the SHA256 hash of a session token. + + Args: + token: The raw session token to hash. + + Returns: + The hexadecimal SHA256 digest of the token. + """ + return hashlib.sha256(token.encode()).hexdigest() + + async def create_session( db: aiosqlite.Connection, token: str, @@ -30,10 +47,15 @@ async def create_session( Returns: The newly created :class:`~app.models.auth.Session`. + + Note: + The token is hashed before storage. The returned Session object + contains the original raw token for use in signing and response. """ + token_hash = _hash_token(token) cursor = await db.execute( - "INSERT INTO sessions (token, created_at, expires_at) VALUES (?, ?, ?)", - (token, created_at, expires_at), + "INSERT INTO sessions (token_hash, created_at, expires_at) VALUES (?, ?, ?)", + (token_hash, created_at, expires_at), ) await db.commit() return Session( @@ -53,10 +75,14 @@ async def get_session(db: aiosqlite.Connection, token: str) -> Session | None: Returns: The :class:`~app.models.auth.Session` if found, else ``None``. + + Note: + The token is hashed before the database lookup. """ + token_hash = _hash_token(token) async with db.execute( - "SELECT id, token, created_at, expires_at FROM sessions WHERE token = ?", - (token,), + "SELECT id, token_hash, created_at, expires_at FROM sessions WHERE token_hash = ?", + (token_hash,), ) as cursor: row = await cursor.fetchone() @@ -65,7 +91,7 @@ async def get_session(db: aiosqlite.Connection, token: str) -> Session | None: return Session( id=int(row[0]), - token=str(row[1]), + token=token, created_at=str(row[2]), expires_at=str(row[3]), ) @@ -77,8 +103,12 @@ async def delete_session(db: aiosqlite.Connection, token: str) -> None: Args: db: Active aiosqlite connection. token: The session token to remove. + + Note: + The token is hashed before the database lookup. """ - await db.execute("DELETE FROM sessions WHERE token = ?", (token,)) + token_hash = _hash_token(token) + await db.execute("DELETE FROM sessions WHERE token_hash = ?", (token_hash,)) await db.commit() diff --git a/backend/tests/test_repositories/test_db_init.py b/backend/tests/test_repositories/test_db_init.py index e72bf5d..4d52919 100644 --- a/backend/tests/test_repositories/test_db_init.py +++ b/backend/tests/test_repositories/test_db_init.py @@ -80,7 +80,7 @@ async def test_init_db_records_schema_version(tmp_path: Path) -> None: ) as cursor: row = await cursor.fetchone() assert row is not None - assert row[0] == 1 + assert row[0] == 2 @pytest.mark.asyncio @@ -97,4 +97,4 @@ async def test_init_db_migrates_legacy_database_without_schema_version(tmp_path: ) as cursor: row = await cursor.fetchone() assert row is not None - assert row[0] == 1 + assert row[0] == 2 diff --git a/backend/tests/test_repositories/test_settings_and_session.py b/backend/tests/test_repositories/test_settings_and_session.py index b212d36..c5cc4cd 100644 --- a/backend/tests/test_repositories/test_settings_and_session.py +++ b/backend/tests/test_repositories/test_settings_and_session.py @@ -116,3 +116,28 @@ class TestSessionRepo: assert deleted == 1 assert await session_repo.get_session(db, "expired") is None assert await session_repo.get_session(db, "valid") is not None + + async def test_tokens_are_hashed_in_database( + self, db: aiosqlite.Connection + ) -> None: + """Verify that tokens are stored as hashes, not plaintext.""" + import hashlib + + token = "plaintext_token_12345" + await session_repo.create_session( + db, + token=token, + created_at="2025-01-01T00:00:00+00:00", + expires_at="2025-01-01T01:00:00+00:00", + ) + + # Query the database directly to verify the token is hashed. + async with db.execute("SELECT token_hash FROM sessions WHERE token_hash = ?", (hashlib.sha256(token.encode()).hexdigest(),)) as cursor: + row = await cursor.fetchone() + assert row is not None + assert row[0] == hashlib.sha256(token.encode()).hexdigest() + + # The plaintext token should not be in the database. + async with db.execute("SELECT * FROM sessions WHERE token_hash = ?", (token,)) as cursor: + row = await cursor.fetchone() + assert row is None