diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index 5891597..6dc58b8 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -553,6 +553,35 @@ BANGUI_FAIL2BAN_START_COMMAND='"/opt/my tools/fail2ban" start' # Quoted path **Common Pitfall:** Using `.split()` instead of `shlex.split()` would break commands with spaces in paths. Always use quoted strings for paths that contain whitespace. +### Log Path Validation & Allowlisting + +Authenticated users can instruct fail2ban to monitor additional log files through the API endpoint `POST /api/config/jails/{name}/logpath`. To prevent path-traversal attacks and unauthorized reads of sensitive system files, all requested log paths must resolve to locations within a configurable allowlist of safe directories. + +**Allowed Directories:** +- Configured via the `BANGUI_ALLOWED_LOG_DIRS` environment variable (comma-separated list). +- Defaults to: `["/var/log", "/config/log"]`. + +**Path Validation Rules:** +1. The requested path is resolved to its canonical form using `Path(log_path).resolve()`, which: + - Expands relative paths to absolute paths. + - Resolves symbolic links to their real targets. + - Normalizes `.` and `..` components. +2. The resolved path is checked using `Path.is_relative_to()` against each allowed directory prefix. +3. If the resolved path is not relative to any allowed directory, a `ValueError` is raised with a descriptive error message. + +**Implementation:** +- Validation occurs in the Pydantic model `AddLogPathRequest` using a `@field_validator`. +- The validator runs at request time, before the service layer is invoked. +- Symlinks that escape allowed directories are rejected (see [symlink bypass tests](../../backend/tests/test_models.py)). + +**Important:** Use `is_relative_to()`, not `startswith()` or string prefix matching. The latter is bypassable with paths like `/var/log_evil/file.log`. + +**Environment Variables:** +```bash +BANGUI_ALLOWED_LOG_DIRS="/var/log,/config/log" # Default +BANGUI_ALLOWED_LOG_DIRS="/var/log,/config/log,/home/app/logs" # Custom directory +``` + ### Login Rate Limiting The login endpoint (`POST /api/auth/login`) is protected against brute-force attacks using an in-memory rate limiter. diff --git a/Docs/Features.md b/Docs/Features.md index ee83a1c..f891b00 100644 --- a/Docs/Features.md +++ b/Docs/Features.md @@ -212,11 +212,12 @@ A page to inspect and modify the fail2ban configuration without leaving the web - Option to register additional log files that fail2ban should monitor. - For each new log, specify: - - The path to the log file. + - The path to the log file (must be within allowed directories to prevent unauthorized access to sensitive files). - One or more regex patterns that define what constitutes a failure. - The jail name and basic jail settings (ban time, retries, etc.). - Choose whether the file should be read from the beginning or only new lines (head vs. tail). - Preview matching lines from the log against the provided regex before saving, so the user can verify the pattern works. +- **Log Path Security:** Added log paths must resolve to locations within a configured allowlist of safe directories (default: `/var/log` and `/config/log`). This prevents authenticated users from instructing fail2ban to monitor sensitive system files. Paths containing symlinks are resolved to their canonical targets before validation. ### Regex Tester @@ -264,7 +265,7 @@ A page to inspect and modify the fail2ban configuration without leaving the web - Truncation notice when the total log file line count exceeds the requested tail limit. - Container automatically scrolls to the bottom after each data update. - When fail2ban is configured to log to a non-file target (STDOUT, STDERR, SYSLOG, SYSTEMD-JOURNAL), an informational banner explains that file-based log viewing is unavailable. -- The log file path is validated against a safe prefix allowlist on the backend to prevent path-traversal reads. +- Log file paths are validated against a configurable allowlist of safe directories on the backend to prevent unauthorized reads of sensitive system files. --- diff --git a/backend/app/config.py b/backend/app/config.py index dbdd917..97af6de 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -142,6 +142,14 @@ class Settings(BaseSettings): "Used for listing, viewing, and editing configuration files through the web UI." ), ) + allowed_log_dirs: list[str] = Field( + default_factory=lambda: ["/var/log", "/config/log"], + description=( + "List of allowed directory prefixes for jail log paths. " + "Any log path added must resolve to a path within one of these directories. " + "Use absolute paths. Symlinks are resolved before validation." + ), + ) fail2ban_start_command: str = Field( default="fail2ban-client start", description=( @@ -193,4 +201,4 @@ def get_settings() -> Settings: A validated :class:`Settings` object. Raises :class:`pydantic.ValidationError` if required keys are absent or values fail validation. """ - return Settings() # type: ignore[call-arg] # pydantic-settings populates required fields from env vars + return Settings() # pydantic-settings populates required fields from env vars diff --git a/backend/app/models/config.py b/backend/app/models/config.py index 3e6aef2..5173f51 100644 --- a/backend/app/models/config.py +++ b/backend/app/models/config.py @@ -4,9 +4,12 @@ Request, response, and domain models for the config router and service. """ import datetime +from pathlib import Path from typing import Literal -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from app.config import get_settings DNSMode = Literal["yes", "warn", "no", "raw"] LogEncoding = Literal["auto", "ascii", "utf-8", "UTF-8", "latin-1"] @@ -213,6 +216,43 @@ class AddLogPathRequest(BaseModel): description="If true, monitor from current end of file (tail). If false, read from the beginning.", ) + @field_validator("log_path", mode="after") + @classmethod + def validate_log_path(cls, value: str) -> str: + """Validate that the log path is within allowed directories. + + Resolves the path to its canonical form (resolving symlinks) and checks + that it is relative to one of the allowed log directories from settings. + + Args: + value: The log path to validate. + + Returns: + The validated log path. + + Raises: + ValueError: If the path is outside allowed log directories. + """ + settings = get_settings() + try: + resolved_path = Path(value).resolve() + except (OSError, RuntimeError) as e: + raise ValueError(f"Cannot resolve path {value!r}: {e}") from e + + for allowed_dir in settings.allowed_log_dirs: + allowed_path = Path(allowed_dir).resolve() + try: + resolved_path.relative_to(allowed_path) + return value + except ValueError: + continue + + allowed_dirs_str = ", ".join(settings.allowed_log_dirs) + raise ValueError( + f"Log path {value!r} is outside allowed directories: {allowed_dirs_str}" + ) + + class LogPreviewRequest(BaseModel): """Payload for ``POST /api/config/preview-log``.""" diff --git a/backend/tests/test_models.py b/backend/tests/test_models.py new file mode 100644 index 0000000..bc9e81c --- /dev/null +++ b/backend/tests/test_models.py @@ -0,0 +1,141 @@ +"""Unit tests for Pydantic models and their validators.""" + +import tempfile +from pathlib import Path + +import pytest +from pydantic import ValidationError + +from app.config import Settings +from app.models.config import AddLogPathRequest + + +@pytest.fixture +def _mock_allowed_dirs(monkeypatch: pytest.MonkeyPatch) -> None: + """Mock get_settings to return test settings with default allowed directories.""" + def mock_get_settings() -> Settings: + return Settings( + database_path=":memory:", + fail2ban_socket="/tmp/fake.sock", + fail2ban_config_dir="/tmp/fail2ban", + session_secret="test-secret-key-do-not-use", + ) + + monkeypatch.setattr("app.models.config.get_settings", mock_get_settings) + + +def test_add_log_path_request_valid_in_var_log(_mock_allowed_dirs: None) -> None: + """Valid log paths in /var/log are accepted.""" + req = AddLogPathRequest(log_path="/var/log/auth.log", tail=True) + assert req.log_path == "/var/log/auth.log" + assert req.tail is True + + +def test_add_log_path_request_valid_in_config_log(_mock_allowed_dirs: None) -> None: + """Valid log paths in /config/log are accepted.""" + req = AddLogPathRequest(log_path="/config/log/app.log", tail=False) + assert req.log_path == "/config/log/app.log" + assert req.tail is False + + +def test_add_log_path_request_valid_with_subdirectory(_mock_allowed_dirs: None) -> None: + """Log paths in subdirectories of allowed paths are accepted.""" + req = AddLogPathRequest(log_path="/var/log/syslog/auth.log", tail=True) + assert req.log_path == "/var/log/syslog/auth.log" + + +def test_add_log_path_request_rejects_path_outside_allowed(_mock_allowed_dirs: None) -> None: + """Paths outside allowed directories are rejected.""" + with pytest.raises(ValidationError) as exc_info: + AddLogPathRequest(log_path="/etc/passwd", tail=True) + error_msg = str(exc_info.value) + assert "outside allowed directories" in error_msg + assert "/etc/passwd" in error_msg + + +def test_add_log_path_request_rejects_home_directory(_mock_allowed_dirs: None) -> None: + """Paths in home directories are rejected.""" + with pytest.raises(ValidationError) as exc_info: + AddLogPathRequest(log_path="/home/user/app.log", tail=True) + error_msg = str(exc_info.value) + assert "outside allowed directories" in error_msg + + +def test_add_log_path_request_rejects_shadow_file(_mock_allowed_dirs: None) -> None: + """Paths to sensitive files like /etc/shadow are rejected.""" + with pytest.raises(ValidationError) as exc_info: + AddLogPathRequest(log_path="/etc/shadow", tail=True) + error_msg = str(exc_info.value) + assert "outside allowed directories" in error_msg + + +def test_add_log_path_request_rejects_symlink_escape(monkeypatch: pytest.MonkeyPatch) -> None: + """Symlinks that escape allowed directories are rejected.""" + with tempfile.TemporaryDirectory() as tmpdir: + allowed_dir = Path(tmpdir) / "allowed" + escape_dir = Path(tmpdir) / "escape" + allowed_dir.mkdir() + escape_dir.mkdir() + + symlink = allowed_dir / "escape_link" + symlink.symlink_to(escape_dir) + + def mock_get_settings() -> Settings: + return Settings( + database_path=":memory:", + fail2ban_socket="/tmp/fake.sock", + fail2ban_config_dir="/tmp/fail2ban", + session_secret="test-secret-key-do-not-use", + allowed_log_dirs=[str(allowed_dir)], + ) + + monkeypatch.setattr("app.models.config.get_settings", mock_get_settings) + + with pytest.raises(ValidationError) as exc_info: + AddLogPathRequest(log_path=str(symlink / "evil.log"), tail=True) + error_msg = str(exc_info.value) + assert "outside allowed directories" in error_msg + + +def test_add_log_path_request_validates_startswith_bypass(_mock_allowed_dirs: None) -> None: + """Paths like /var/log_evil that bypass startswith() are rejected.""" + with pytest.raises(ValidationError) as exc_info: + AddLogPathRequest(log_path="/var/log_evil/somefile.log", tail=True) + error_msg = str(exc_info.value) + assert "outside allowed directories" in error_msg + + +def test_add_log_path_request_default_tail_is_true(_mock_allowed_dirs: None) -> None: + """Tail defaults to True.""" + req = AddLogPathRequest(log_path="/var/log/app.log") + assert req.tail is True + + +def test_add_log_path_request_error_message_lists_allowed_dirs(_mock_allowed_dirs: None) -> None: + """Error message includes the list of allowed directories.""" + with pytest.raises(ValidationError) as exc_info: + AddLogPathRequest(log_path="/root/secret.log", tail=True) + error_msg = str(exc_info.value) + assert "/var/log" in error_msg + assert "/config/log" in error_msg + + +def test_add_log_path_request_custom_allowed_dirs(monkeypatch: pytest.MonkeyPatch) -> None: + """Custom allowed directories from settings are respected.""" + def mock_get_settings() -> Settings: + return Settings( + database_path=":memory:", + fail2ban_socket="/tmp/fake.sock", + fail2ban_config_dir="/tmp/fail2ban", + session_secret="test-secret-key-do-not-use", + allowed_log_dirs=["/custom/logs", "/another/path"], + ) + + monkeypatch.setattr("app.models.config.get_settings", mock_get_settings) + + req = AddLogPathRequest(log_path="/custom/logs/app.log", tail=True) + assert req.log_path == "/custom/logs/app.log" + + with pytest.raises(ValidationError): + AddLogPathRequest(log_path="/var/log/app.log", tail=True) +