Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Login password. Set this to require a login; leave empty to run open (dev/demo).
# Login password (single-password mode). Leave empty to run open (dev/demo).
# Once you add users on the Settings page, Bastion switches to multi-user login
# and this password is ignored.
BASTION_AUTH_PASSWORD=
# Secret used to sign multi-user session cookies. Optional — if unset, a random
# key is generated and stored at <data_dir>/.secret. Set it to share sessions
# across replicas or to control rotation.
# BASTION_SECRET_KEY=
# Only needed for bare-metal (systemd) runs as an unprivileged user. Leave as-is for Docker.
BASTION_SUDO=false
# Jails to query (empty -> auto-discover via `fail2ban-client status`)
Expand Down
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ __pycache__/
*.mmdb
.pytest_cache/

# Runtime state written by the app (user preferences, trend history).
# Runtime state written by the app (preferences, users, history, session key).
data/prefs.json
data/history.jsonl
data/users.json
data/.secret
data/*.tmp
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
independently toggleable in Settings (in addition to ban-spike alerts).
- Home **trend sparklines** for banned IPs and attackers, backed by a capped
time-series recorded by the poller. New `GET /api/history` endpoint.
- **Multi-user login** — named accounts with PBKDF2-hashed passwords, managed in
Settings → Users, with signed session cookies. Falls back to the single shared
password when no users exist; HTTP Basic auth works with user credentials.

### Changed
- Firewall page now renders rules in **readable nft syntax** (e.g. `tcp dport 22
Expand Down
21 changes: 13 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,19 @@ take effect immediately.

## Authentication

Set `BASTION_AUTH_PASSWORD` to require a login; the whole dashboard is then gated
behind a single-password sign-in (session stored in an HMAC-signed cookie).
Repeated failures from one IP are locked out (see *Login security* above). Leave
the password empty to run open (dev/demo) — a warning is logged on startup.
`/healthz`, `/login` and static assets stay public.

This is a deliberately minimal gate; keep the app behind HTTPS. A full
multi-user model can be layered on later without changing call sites.
Three modes, in order of precedence:

1. **Multi-user** — add accounts under **Settings → Users**; login is then
username + password (PBKDF2-hashed, stored in `data/users.json`; sessions are
signed with a key at `data/.secret` or `BASTION_SECRET_KEY`).
2. **Single password** — with no users but `BASTION_AUTH_PASSWORD` set, the whole
dashboard is gated behind one shared password (HMAC-signed cookie).
3. **Open** — neither configured; a warning is logged on startup.

Repeated failures from one IP are locked out (see *Login security* above).
`/healthz`, `/login` and static assets stay public. Header-only clients can use
HTTP Basic auth (username + password in multi-user mode, or any username + the
shared password otherwise). Keep the app behind HTTPS regardless.

## GeoIP (country flags)

Expand Down
44 changes: 35 additions & 9 deletions bastion/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import hmac

COOKIE_NAME = "bastion_auth"
USER_COOKIE = "bastion_user"
COOKIE_MAX_AGE = 60 * 60 * 24 * 7 # 7 days


Expand All @@ -34,18 +35,43 @@ def is_authenticated(cookie_value: str | None, password: str) -> bool:
return bool(cookie_value) and hmac.compare_digest(cookie_value, expected_token(password))


def verify_basic(auth_header: str | None, password: str) -> bool:
"""Accept HTTP Basic auth where the password half matches. The username is
ignored. Lets header-only API clients (dashboards, scripts) reach the JSON
API without the cookie login flow. Returns False when no password is set."""
if not password or not auth_header:
return False
def sign_session(username: str, secret: str) -> str:
"""Return a `username.signature` session token for multi-user mode."""
sig = hmac.new(secret.encode(), username.encode(), hashlib.sha256).hexdigest()
return f"{username}.{sig}"


def verify_session(cookie_value: str | None, secret: str) -> str | None:
"""Return the username if the session token is valid, else None."""
if not cookie_value or "." not in cookie_value:
return None
username, _, sig = cookie_value.rpartition(".")
if not username:
return None
expected = hmac.new(secret.encode(), username.encode(), hashlib.sha256).hexdigest()
return username if hmac.compare_digest(sig, expected) else None


def decode_basic(auth_header: str | None) -> tuple[str, str] | None:
"""Return (username, password) from an HTTP Basic header, or None."""
if not auth_header:
return None
scheme, _, encoded = auth_header.partition(" ")
if scheme.lower() != "basic" or not encoded:
return False
return None
try:
decoded = base64.b64decode(encoded, validate=True).decode("utf-8")
except (binascii.Error, ValueError, UnicodeDecodeError):
return None
username, sep, password = decoded.partition(":")
return (username, password) if sep else None


def verify_basic(auth_header: str | None, password: str) -> bool:
"""Accept HTTP Basic auth where the password half matches. The username is
ignored. Lets header-only API clients (dashboards, scripts) reach the JSON
API without the cookie login flow. Returns False when no password is set."""
if not password:
return False
_, sep, candidate = decoded.partition(":")
return bool(sep) and hmac.compare_digest(candidate, password)
creds = decode_basic(auth_header)
return creds is not None and hmac.compare_digest(creds[1], password)
12 changes: 12 additions & 0 deletions bastion/i18n.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
"wrong_password": "Wrong password",
"too_many_attempts": "Too many attempts. Please wait and try again.",
"logout": "Logout",
"username": "Username",
"sec_users": "Users",
"sec_users_hint": "Named accounts with their own passwords. Adding the first user switches login to username + password (the single password is then ignored).",
"add_user": "Add user",
"change_password": "Change password",
"no_users": "No users yet — single-password mode is active.",
"ban": "Ban",
"unban": "Unban",
"ban_ip": "Ban an IP",
Expand Down Expand Up @@ -140,6 +146,12 @@
"wrong_password": "비밀번호가 틀렸습니다",
"too_many_attempts": "시도가 너무 많습니다. 잠시 후 다시 시도하세요.",
"logout": "로그아웃",
"username": "사용자명",
"sec_users": "사용자",
"sec_users_hint": "각자 비밀번호를 가진 계정입니다. 첫 사용자를 추가하면 로그인이 사용자명+비밀번호로 전환됩니다(단일 비밀번호는 무시됨).",
"add_user": "사용자 추가",
"change_password": "비밀번호 변경",
"no_users": "사용자 없음 — 단일 비밀번호 모드가 활성입니다.",
"ban": "차단",
"unban": "차단 해제",
"ban_ip": "IP 차단",
Expand Down
41 changes: 41 additions & 0 deletions bastion/secretkey.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""Stable server secret used to sign multi-user session cookies.

Resolution order: BASTION_SECRET_KEY env, then a persisted random key under the
data directory (created once, 0600), then an in-memory fallback if the data dir
is not writable (sessions won't survive a restart, but auth still works).
"""

from __future__ import annotations

import logging
import os
import secrets

from bastion.config import settings

log = logging.getLogger("bastion.secretkey")

_ephemeral: str | None = None


def get() -> str:
env = os.environ.get("BASTION_SECRET_KEY")
if env:
return env
path = os.path.join(settings.data_dir, ".secret")
try:
if os.path.exists(path):
with open(path, "r", encoding="utf-8") as f:
return f.read().strip()
os.makedirs(settings.data_dir, exist_ok=True)
key = secrets.token_hex(32)
with open(path, "w", encoding="utf-8") as f:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The .secret file is created with the process's default umask permissions (via open(path, "w")) before being locked down to 0o600 by the subsequent os.chmod call. A local attacker or another process on the system could read the secret during this microsecond window. Use os.open with stat.S_IRUSR | stat.S_IWUSR to create the file with restricted permissions from the instant it hits disk, eliminating the race.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bastion/secretkey.py, line 32:

<comment>The `.secret` file is created with the process's default umask permissions (via `open(path, "w")`) before being locked down to `0o600` by the subsequent `os.chmod` call. A local attacker or another process on the system could read the secret during this microsecond window. Use `os.open` with `stat.S_IRUSR | stat.S_IWUSR` to create the file with restricted permissions from the instant it hits disk, eliminating the race.</comment>

<file context>
@@ -0,0 +1,41 @@
+                return f.read().strip()
+        os.makedirs(settings.data_dir, exist_ok=True)
+        key = secrets.token_hex(32)
+        with open(path, "w", encoding="utf-8") as f:
+            f.write(key)
+        os.chmod(path, 0o600)
</file context>

f.write(key)
os.chmod(path, 0o600)
return key
except OSError as e:
global _ephemeral
if _ephemeral is None:
log.warning("data dir not writable (%s); using an ephemeral session key", e)
_ephemeral = secrets.token_hex(32)
return _ephemeral
123 changes: 123 additions & 0 deletions bastion/users.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
"""User account store with salted password hashing.

Layered on top of the single-password gate: when at least one user exists,
Bastion runs in multi-user mode (username + password login); with no users it
falls back to `BASTION_AUTH_PASSWORD`, and with neither it runs open.

Passwords are hashed with PBKDF2-HMAC-SHA256 (per-user random salt). The hashing
helpers are pure so they can be unit-tested with a fixed salt; the store does
best-effort JSON IO under the data directory.
"""

from __future__ import annotations

import hashlib
import hmac
import json
import logging
import os
import re

from bastion.config import settings

log = logging.getLogger("bastion.users")

USERNAME_RE = re.compile(r"^[A-Za-z0-9_.-]{1,32}$")
_ITERATIONS = 200_000


def hash_password(password: str, *, salt: bytes | str | None = None, iterations: int = _ITERATIONS) -> str:
"""Return a self-describing `pbkdf2_sha256$iters$salt$hash` string.
Pass `salt` (hex or bytes) for a deterministic result in tests."""
if salt is None:
salt = os.urandom(16)
elif isinstance(salt, str):
salt = bytes.fromhex(salt)
dk = hashlib.pbkdf2_hmac("sha256", password.encode("utf-8"), salt, iterations)
return f"pbkdf2_sha256${iterations}${salt.hex()}${dk.hex()}"


def verify_hash(password: str, stored: str) -> bool:
"""Constant-time check of `password` against a stored hash string."""
try:
algo, iters, salt_hex, _ = stored.split("$")
except (ValueError, AttributeError):
return False
if algo != "pbkdf2_sha256":
return False
candidate = hash_password(password, salt=salt_hex, iterations=int(iters))
return hmac.compare_digest(candidate, stored)
Comment on lines +48 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A malformed hash entry in data/users.json can crash login/basic-auth checks, because verify_hash() only catches split errors but not invalid iteration counts or salt hex. Since this file is best-effort runtime JSON, treating those parse failures as False would keep a corrupt user record from causing 500 responses.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bastion/users.py, line 48:

<comment>A malformed hash entry in `data/users.json` can crash login/basic-auth checks, because `verify_hash()` only catches split errors but not invalid iteration counts or salt hex. Since this file is best-effort runtime JSON, treating those parse failures as `False` would keep a corrupt user record from causing 500 responses.</comment>

<file context>
@@ -0,0 +1,123 @@
+        return False
+    if algo != "pbkdf2_sha256":
+        return False
+    candidate = hash_password(password, salt=salt_hex, iterations=int(iters))
+    return hmac.compare_digest(candidate, stored)
+
</file context>
Suggested change
candidate = hash_password(password, salt=salt_hex, iterations=int(iters))
return hmac.compare_digest(candidate, stored)
try:
candidate = hash_password(password, salt=salt_hex, iterations=int(iters))
except ValueError:
return False
return hmac.compare_digest(candidate, stored)



# ---------- store (best-effort JSON IO) ----------
def _path() -> str:
return os.path.join(settings.data_dir, "users.json")


def load() -> dict[str, str]:
path = _path()
if not os.path.exists(path):
return {}
try:
with open(path, "r", encoding="utf-8") as f:
data = json.load(f)
return data if isinstance(data, dict) else {}
except (OSError, ValueError) as e:
log.warning("could not read users (%s)", e)
return {}


def save(users: dict[str, str]) -> None:
os.makedirs(settings.data_dir, exist_ok=True)
path = _path()
tmp = path + ".tmp"
with open(tmp, "w", encoding="utf-8") as f:
json.dump(users, f, indent=2, sort_keys=True)
os.replace(tmp, path)


def count() -> int:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: count(), exists(), and usernames() each re-read and parse the full users.json from disk on every call. The auth middleware calls count() up to 6 times per request, each triggering I/O. Consider caching the loaded dict (e.g., keep it in a module-level variable updated by load() and invalidated by save()) to avoid redundant reads.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bastion/users.py, line 79:

<comment>`count()`, `exists()`, and `usernames()` each re-read and parse the full `users.json` from disk on every call. The auth middleware calls `count()` up to 6 times per request, each triggering I/O. Consider caching the loaded dict (e.g., keep it in a module-level variable updated by `load()` and invalidated by `save()`) to avoid redundant reads.</comment>

<file context>
@@ -0,0 +1,123 @@
+    os.replace(tmp, path)
+
+
+def count() -> int:
+    return len(load())
+
</file context>

return len(load())


def exists(username: str) -> bool:
return username in load()


def usernames() -> list[str]:
return sorted(load().keys())


def add(username: str, password: str) -> None:
if not USERNAME_RE.match(username or ""):
raise ValueError(f"invalid username: {username!r}")
if not password:
raise ValueError("password must not be empty")
users = load()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Concurrent writes in add(), set_password(), and remove() can lose data. Each function reads the full user store (load), modifies the dict, then writes it back (save). If two admin requests arrive at the same time, the second load() may read the first's changes but the second save() could overwrite the first's write after the first's os.replace. Consider a simple file-lock (fcntl.flock or a threading.Lock) around the load+modify+save sequence.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bastion/users.py, line 96:

<comment>Concurrent writes in `add()`, `set_password()`, and `remove()` can lose data. Each function reads the full user store (load), modifies the dict, then writes it back (save). If two admin requests arrive at the same time, the second `load()` may read the first's changes but the second `save()` could overwrite the first's write after the first's `os.replace`. Consider a simple file-lock (fcntl.flock or a threading.Lock) around the load+modify+save sequence.</comment>

<file context>
@@ -0,0 +1,123 @@
+        raise ValueError(f"invalid username: {username!r}")
+    if not password:
+        raise ValueError("password must not be empty")
+    users = load()
+    users[username] = hash_password(password)
+    save(users)
</file context>

users[username] = hash_password(password)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: add() silently overwrites an existing user's password instead of rejecting the duplicate. An admin who re-submits the add-user form with an existing username (or a script hitting the POST endpoint) will change that user's password without any error or warning. add() should either check exists(username) and raise ValueError, or the web route should call exists() first.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bastion/users.py, line 97:

<comment>`add()` silently overwrites an existing user's password instead of rejecting the duplicate. An admin who re-submits the add-user form with an existing username (or a script hitting the POST endpoint) will change that user's password without any error or warning. `add()` should either check `exists(username)` and raise `ValueError`, or the web route should call `exists()` first.</comment>

<file context>
@@ -0,0 +1,123 @@
+    if not password:
+        raise ValueError("password must not be empty")
+    users = load()
+    users[username] = hash_password(password)
+    save(users)
+    log.info("user added: %s", username)
</file context>

save(users)
log.info("user added: %s", username)


def set_password(username: str, password: str) -> None:
if not password:
raise ValueError("password must not be empty")
users = load()
if username not in users:
raise ValueError(f"no such user: {username!r}")
users[username] = hash_password(password)
save(users)
log.info("password changed: %s", username)


def remove(username: str) -> None:
users = load()
if username in users:
del users[username]
save(users)
log.info("user removed: %s", username)


def verify(username: str, password: str) -> bool:
stored = load().get(username)
return bool(stored) and verify_hash(password, stored)
Loading
Loading