diff --git a/APP_PROFILER/routers/server.py b/APP_PROFILER/routers/server.py index 1be1498..230b278 100644 --- a/APP_PROFILER/routers/server.py +++ b/APP_PROFILER/routers/server.py @@ -4,7 +4,7 @@ from fastapi import APIRouter, Depends, HTTPException from sqlalchemy.orm import Session from database import get_db from utils.auth import verify_token -from services import generator +from services import generator, process logger = logging.getLogger(__name__) @@ -16,6 +16,16 @@ def configure_server(db: Session = Depends(get_db)): # Generate to a temporary location or standard location # As per plan, we behave like srvconf output_path = "/etc/openvpn/server.conf" + + # Unprivileged service: render to the staging dir, the root helper validates and installs it + if not process.is_container() and os.geteuid() != 0: + staged = os.path.join(os.getenv("OVPMON_STAGING_DIR", "/var/lib/ovpmon/staging"), "server.conf") + os.makedirs(os.path.dirname(staged), exist_ok=True) + generator.generate_server_config(db, output_path=staged) + ok, msg = process.install_config() + if not ok: + raise HTTPException(status_code=400, detail=f"Configuration rejected: {msg}") + return {"message": "Server configuration generated", "path": output_path} # Ensure we can write to /etc/openvpn if not os.path.exists(os.path.dirname(output_path)) or not os.access(os.path.dirname(output_path), os.W_OK): @@ -29,6 +39,8 @@ def configure_server(db: Session = Depends(get_db)): content = generator.generate_server_config(db, output_path=output_path) return {"message": "Server configuration generated", "path": output_path} + except HTTPException: + raise except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) except Exception as e: diff --git a/APP_PROFILER/services/process.py b/APP_PROFILER/services/process.py index d2c28c7..4443151 100644 --- a/APP_PROFILER/services/process.py +++ b/APP_PROFILER/services/process.py @@ -20,6 +20,34 @@ def is_container(): pass return False +HELPER_PATH = "/usr/local/sbin/ovpmon-helper" + + +def _run_helper(args): + """Call the root-side helper through doas (used when this process is unprivileged). + Returns (returncode, parsed_json_or_None, raw_output).""" + import json + try: + r = subprocess.run(["doas", "-n", HELPER_PATH] + args, capture_output=True, text=True, timeout=90) + except (OSError, subprocess.TimeoutExpired) as e: + return 1, None, str(e) + out = (r.stdout or "").strip() + try: + return r.returncode, json.loads(out.splitlines()[-1]), out + except Exception: + return r.returncode, None, (out + " " + (r.stderr or "")).strip() + + +def install_config(): + """Ask the helper to validate and install the staged server.conf. Returns (ok, message).""" + rc, data, raw = _run_helper(["install-config"]) + if rc == 0 and data and data.get("status") == "ok": + return True, "Configuration installed" + msg = (data or {}).get("error") or raw or "helper failed" + logger.error(f"[PROCESS] install-config failed: {msg}") + return False, msg + + def control_service(action: str): """ Action: start, stop, restart @@ -94,6 +122,14 @@ def control_service(action: str): stop_vpn_direct() return start_vpn_direct() + # Unprivileged service: delegate to the root helper (validated, fixed set of actions) + if os.geteuid() != 0: + rc, data, raw = _run_helper(["service", action]) + if rc == 0 and data and data.get("status") == "ok": + return {"status": "success", "message": f"Service {action} executed successfully via helper", "stdout": data.get("output", "")} + logger.error(f"[PROCESS] helper service {action} failed: {raw}") + return {"status": "error", "message": f"Failed to {action} service via helper", "stderr": (data or {}).get("error") or (data or {}).get("output") or raw} + # On Host OS: Use system service manager os_type = get_os_type() logger.info(f"[PROCESS] Host OS detected ({os_type}), using service manager for {action}") diff --git a/DOCS/Changes/2026-09-30_Privilege_Separation.md b/DOCS/Changes/2026-09-30_Privilege_Separation.md new file mode 100644 index 0000000..196ba64 --- /dev/null +++ b/DOCS/Changes/2026-09-30_Privilege_Separation.md @@ -0,0 +1,54 @@ +# Privilege separation: API services no longer run as root (2026-09-30) + +Problem: `ovpmon-api`, `ovpmon-gatherer` and `ovpmon-profiler` ran as root. Any bug in an API (or a stolen admin token combined with an input-validation gap) meant root on the host. + +## Design + +``` +ovpmon-api / gatherer / profiler (user ovpmon, no login shell) + | + | doas -n /usr/local/sbin/ovpmon-helper (only 5 exact commands allowed) + v +ovpmon-helper (root) -> install-config : validates the staged server.conf against an allowlist, + installs /etc/openvpn/server.conf atomically + -> service start|stop|restart|status : rc-service openvpn +``` + +| Item | Detail | +|---|---| +| Service user | `ovpmon` (system user, nologin), owns `/var/lib/ovpmon` (DBs, `staging/`), `/var/log/ovpmon`, `APP_PROFILER/{easy-rsa,client-config,profiler.log}`, runtime logs and `__pycache__`. Code and virtualenvs stay root-owned (read-only for the service) | +| OpenRC | `command_user="ovpmon:ovpmon"` in the three `ovpmon-*` init scripts; `/etc/ovpmon/env` stays `root:root 600` (read by the init script before the privilege drop) | +| doas | `/etc/doas.d/ovpmon.conf`: `permit nopass ovpmon as root cmd /usr/local/sbin/ovpmon-helper args `; nothing else is permitted | +| Helper | `/usr/local/sbin/ovpmon-helper` (root, 755). Source: `DOCS/General/privilege-separation/ovpmon-helper` | +| Profiler code | `services/process.py`: when not root, calls the helper (`_run_helper`, `install_config`); `routers/server.py`: renders to `/var/lib/ovpmon/staging/server.conf`, then asks the helper to install it (rejection → HTTP 400). Container and root paths are unchanged | +| Status log | the helper sets `root:ovpmon 640` on `openvpn-status.log` before every start/restart so the gatherer can read it | + +### What the helper allows in `server.conf` +Only the directives the template produces, each with checked arguments: `dev tun`, `proto`, `port`, `ca/cert/key/dh/tls-auth/crl-verify` (files must resolve inside the PKI directory), `tun-mtu`, `mssfix`, `topology subnet`, `server`, fixed `ifconfig-pool-persist`, `log`, `log-append`, `status`, `verb`, `push` (only `redirect-gateway def1 bypass-dhcp`, `route `, `dhcp-option DNS `), `user nobody`, `group nogroup`, ciphers/auth/keepalive, `client-to-client`, `duplicate-cn`, `persist-*`, `script-security 2`, `client-connect/disconnect` (script must be root-owned, not group/other-writable, directly in `/etc/openvpn/scripts/`), `management` (loopback only). Everything else (`up`, `down`, `plugin`, `route-up`, `tls-verify`, `setenv`, `config`, ...) is rejected. `user nobody`, `group nogroup`, `server`, `ca`, `cert`, `key` are mandatory. The file is read once (no TOCTOU between check and install), must be an `ovpmon`-owned regular file (no symlinks), ASCII only, at most 64 KiB. + +## Rollout (what was done) +1. Backup: `/root/backup-p2-*.tar` (`/etc/openvpn`, `/var/lib/ovpmon`, `easy-rsa`, `client-config`, init scripts, doas config, changed code) and `/root/app-bak/p2/`. +2. Create the user/group, install the helper and the doas rules; test the helper as `ovpmon` before touching services. +3. Patch `process.py` / `server.py` (root code path unchanged, so nothing changed while services still ran as root). +4. `chown` runtime data, add `command_user`, restart the gatherer, then the API, then the profiler, checking each. + +## Results + +| Check | Result | +|---|---| +| Processes | gunicorn, uvicorn and the gatherer run as `ovpmon`; only the `supervise-daemon` supervisors are root | +| Helper: current live config | accepted, live `server.conf` byte-identical | +| Helper: 17 injected directives (`up`, `plugin`, `script-security 3`, `client-connect /tmp/x`, `route-up`, `tls-verify`, `setenv`, `config`, `ca /etc/shadow`, `management 0.0.0.0`, `log /etc/passwd`, `user root`, `status /etc/cron.d/x`, `push "setenv-safe"`, `dev tap`, multi-argument `push`) | all rejected, live config unchanged | +| Helper: missing `user nobody`, cert outside PKI, CR injection, symlinked staged file | rejected | +| doas: arbitrary command, helper with other args | denied | +| Service user cannot | read `/etc/shadow`, `/root`, `/etc/hysteria/*.yaml`, `/etc/ovpmon/env`; write `/etc/openvpn`, `/etc/init.d`, `authorized_keys`, application code; run `iptables` | +| API: monitoring, config, process stats, `server/configure` via helper (config identical) | 200 | +| API: create profile (easyrsa), download `.ovpn`, revoke | 200 | +| API: OpenVPN restart through the helper | 200; OpenVPN running, `tun0` up, status log readable by the gatherer, egress rules (`ip rule 102`, MASQUERADE to `hytun`) intact, no gatherer errors | + +## Limitations / notes +- The supervisors stay root by design (they only respawn the service user's process). +- Helper checks resolve PKI paths at install time; a service-user-owned PKI directory could later swap a file for a symlink before OpenVPN (root) starts. Impact is limited to OpenVPN failing to parse or reading a key/cert-shaped file; keep the PKI directory owned by `ovpmon` only. +- With `crl_verify` enabled the unprivileged OpenVPN user (`nobody`) must be able to read `crl.pem`, but `easy-rsa` creates `pki/` as `700`. Either keep CRL checking off or publish a copy of `crl.pem` to a root-owned, world-readable location (not automated yet). +- systemd deployments: same idea with `User=ovpmon`, a polkit/sudoers rule for the helper's fixed commands, and the same helper (replace `rc-service` with `systemctl`). +- Rollback: restore `/etc/init.d/ovpmon-*` from `/root/app-bak/p2/`, `chown -R root:root` the data directories, restart the services (the helper and doas rules can stay). diff --git a/DOCS/General/Deployment_Native.md b/DOCS/General/Deployment_Native.md index 552d2d5..3e92b56 100644 --- a/DOCS/General/Deployment_Native.md +++ b/DOCS/General/Deployment_Native.md @@ -44,6 +44,10 @@ OpenRC (Alpine): `supervisor=supervise-daemon`, `respawn_delay=3`, source `/etc/ The Profiler restarts OpenVPN through `rc-service openvpn` (Alpine) or `systemctl openvpn`; on Alpine link the config: `ln -s server.conf /etc/openvpn/openvpn.conf` and enable the `openvpn` service. +## 4a. Run as an unprivileged user (recommended) + +Create the `ovpmon` user, give it the data directories, add `command_user="ovpmon:ovpmon"` to the init scripts (or `User=ovpmon` in systemd units), install the root helper and the doas rules. The API then renders `server.conf` to `/var/lib/ovpmon/staging/`; the helper validates and installs it and controls the `openvpn` service. Full procedure, helper source and results: [Privilege separation](../Changes/2026-09-30_Privilege_Separation.md); files in [`privilege-separation/`](privilege-separation/). + ## 5. UI and Nginx (HTTPS on 8088) ```bash diff --git a/DOCS/General/Index.md b/DOCS/General/Index.md index d4884ef..651e908 100644 --- a/DOCS/General/Index.md +++ b/DOCS/General/Index.md @@ -14,6 +14,7 @@ Welcome to the documentation for the OpenVPN Monitor suite. - [Security hardening (2026-09-30)](../Changes/2026-09-30_Security_Hardening.md) - [Admin username change (2026-09-30)](../Changes/2026-09-30_Admin_Username_Change.md) - [Settings validation (2026-09-30)](../Changes/2026-09-30_Settings_Validation.md) +- [Privilege separation (2026-09-30)](../Changes/2026-09-30_Privilege_Separation.md) - [Egress via Hysteria2 (2026-09-30)](../Changes/2026-09-30_Egress_via_Hysteria2.md) ## 🔍 Core Monitoring (`APP_CORE`) diff --git a/DOCS/General/privilege-separation/doas-ovpmon.conf b/DOCS/General/privilege-separation/doas-ovpmon.conf new file mode 100644 index 0000000..adef4cd --- /dev/null +++ b/DOCS/General/privilege-separation/doas-ovpmon.conf @@ -0,0 +1,7 @@ +# /etc/doas.d/ovpmon.conf (root:root 640) +# The unprivileged ovpmon service user may run exactly these helper commands as root. +permit nopass ovpmon as root cmd /usr/local/sbin/ovpmon-helper args install-config +permit nopass ovpmon as root cmd /usr/local/sbin/ovpmon-helper args service start +permit nopass ovpmon as root cmd /usr/local/sbin/ovpmon-helper args service stop +permit nopass ovpmon as root cmd /usr/local/sbin/ovpmon-helper args service restart +permit nopass ovpmon as root cmd /usr/local/sbin/ovpmon-helper args service status diff --git a/DOCS/General/privilege-separation/ovpmon-helper b/DOCS/General/privilege-separation/ovpmon-helper new file mode 100755 index 0000000..211787c --- /dev/null +++ b/DOCS/General/privilege-separation/ovpmon-helper @@ -0,0 +1,224 @@ +#!/usr/bin/python3 +"""ovpmon-helper: the only root-side entry point for the unprivileged ovpmon services. + +Usage (via doas): ovpmon-helper install-config + ovpmon-helper service start|stop|restart|status +install-config reads the staged OpenVPN server config, validates it against a strict +allowlist of directives and installs it atomically to /etc/openvpn/server.conf. +""" +import ipaddress +import json +import os +import pwd +import re +import shlex +import stat +import subprocess +import sys + +STAGED = "/var/lib/ovpmon/staging/server.conf" +TARGET = "/etc/openvpn/server.conf" +PKI_DIR = "/opt/OpenVPN-Monitoring-Simple/APP_PROFILER/easy-rsa/pki" +SCRIPTS_DIR = "/etc/openvpn/scripts" +STATUS_LOG = "/var/log/openvpn/openvpn-status.log" +SERVICE_USER = "ovpmon" +MAX_SIZE = 64 * 1024 +CIPHERS_RE = re.compile(r"^[A-Za-z0-9:_-]{1,200}$") +os.environ["PATH"] = "/usr/sbin:/usr/bin:/sbin:/bin" + + +class Reject(Exception): + pass + + +def under(path, base): + real = os.path.realpath(path) + return real == base or real.startswith(base.rstrip("/") + "/") + + +def need(cond, msg): + if not cond: + raise Reject(msg) + + +def is_int(x, lo, hi): + return re.fullmatch(r"\d{1,6}", x) is not None and lo <= int(x) <= hi + + +def valid_route(r): + parts = r.split() + try: + if len(parts) == 1: + ipaddress.IPv4Network(parts[0], strict=False) + elif len(parts) == 2: + ipaddress.IPv4Address(parts[0]) + ipaddress.IPv4Network("0.0.0.0/" + parts[1]) + else: + return False + return True + except ValueError: + return False + + +def check_script(path): + need(re.fullmatch(r"/etc/openvpn/scripts/[A-Za-z0-9_.-]{1,64}", path), "script path not allowed") + real = os.path.realpath(path) + need(os.path.dirname(real) == SCRIPTS_DIR and os.path.isfile(real), "script must be a file in " + SCRIPTS_DIR) + st, dst = os.stat(real), os.stat(SCRIPTS_DIR) + need(st.st_uid == 0 and not st.st_mode & 0o022, "script must be root-owned and not group/other-writable") + need(dst.st_uid == 0 and not dst.st_mode & 0o022, SCRIPTS_DIR + " must be root-owned and not writable") + + +def check_line(tokens): + d, a = tokens[0], tokens[1:] + if d == "dev": + need(a == ["tun"], "dev must be tun") + elif d == "proto": + need(len(a) == 1 and a[0] in ("udp", "tcp", "udp4", "tcp4", "udp6", "tcp6"), "bad proto") + elif d in ("tls-server", "client-to-client", "duplicate-cn", "persist-key", "persist-tun"): + need(not a, d + " takes no arguments") + elif d == "explicit-exit-notify": + need(len(a) == 1 and is_int(a[0], 1, 10), "bad explicit-exit-notify") + elif d in ("port", "management-port"): + need(len(a) == 1 and is_int(a[0], 1, 65535), "bad port") + elif d in ("ca", "cert", "key", "dh", "crl-verify"): + need(len(a) == 1 and under(a[0], PKI_DIR), d + " must be a file inside the PKI directory") + elif d == "tls-auth": + need(len(a) == 2 and under(a[0], PKI_DIR) and a[1] in ("0", "1"), "bad tls-auth") + elif d == "tun-mtu": + need(len(a) == 1 and is_int(a[0], 576, 9000), "bad tun-mtu") + elif d == "mssfix": + need(len(a) == 1 and is_int(a[0], 536, 1500), "bad mssfix") + elif d == "topology": + need(a == ["subnet"], "topology must be subnet") + elif d == "server": + need(len(a) == 2, "bad server") + net = ipaddress.IPv4Network(f"{a[0]}/{a[1]}", strict=True) + need(8 <= net.prefixlen <= 30, "bad server prefix") + elif d == "ifconfig-pool-persist": + need(a == ["/etc/openvpn/ipp.txt"], "ifconfig-pool-persist path not allowed") + elif d in ("log", "log-append"): + need(a == ["/var/log/openvpn/openvpn.log"], d + " path not allowed") + elif d == "verb": + need(len(a) == 1 and is_int(a[0], 0, 9), "bad verb") + elif d == "status": + need(len(a) == 2 and a[0] == STATUS_LOG and is_int(a[1], 1, 3600), "bad status") + elif d == "status-version": + need(a in (["1"], ["2"], ["3"]), "bad status-version") + elif d == "push": + need(len(a) == 1, "push takes one quoted argument") + p = a[0] + if p == "redirect-gateway def1 bypass-dhcp": + return + m = re.fullmatch(r"route (.+)", p) + if m: + need(valid_route(m.group(1)), "bad pushed route") + return + m = re.fullmatch(r"dhcp-option DNS (\S+)", p) + need(m is not None, "pushed option not allowed") + ipaddress.ip_address(m.group(1)) + elif d == "user": + need(a == ["nobody"], "user must be nobody") + elif d == "group": + need(a == ["nogroup"], "group must be nogroup") + elif d in ("data-ciphers", "data-ciphers-fallback"): + need(len(a) == 1 and CIPHERS_RE.match(a[0]), "bad cipher list") + elif d == "auth": + need(len(a) == 1 and a[0] in ("SHA256", "SHA384", "SHA512"), "bad auth") + elif d == "keepalive": + need(len(a) == 2 and is_int(a[0], 1, 3600) and is_int(a[1], 1, 7200), "bad keepalive") + elif d == "script-security": + need(a == ["2"], "script-security must be 2") + elif d in ("client-connect", "client-disconnect"): + need(len(a) == 1, d + " takes one argument") + check_script(a[0]) + elif d == "management": + need(len(a) == 2 and ipaddress.ip_address(a[0]).is_loopback and is_int(a[1], 1, 65535), "management must be loopback") + else: + raise Reject("directive not allowed: " + d) + + +def validate(text): + seen = set() + for n, raw in enumerate(text.splitlines(), 1): + line = raw.strip() + if not line or line.startswith("#") or line.startswith(";"): + continue + need(all(32 <= ord(c) < 127 for c in line), f"line {n}: non-printable or non-ASCII character") + try: + tokens = shlex.split(line, comments=False) + except ValueError as e: + raise Reject(f"line {n}: {e}") + try: + check_line(tokens) + except Reject as e: + raise Reject(f"line {n}: {e}") + except ValueError as e: + raise Reject(f"line {n}: invalid value ({e})") + seen.add(tokens[0]) + for req in ("user", "group", "server", "ca", "cert", "key"): + need(req in seen, f"required directive missing: {req}") + + +def install_config(): + uid = pwd.getpwnam(SERVICE_USER).pw_uid + fd = os.open(STAGED, os.O_RDONLY | os.O_NOFOLLOW) + try: + st = os.fstat(fd) + need(st.st_uid == uid and stat.S_ISREG(st.st_mode), "staged config must be a regular file owned by " + SERVICE_USER) + need(st.st_size <= MAX_SIZE, "staged config too large") + data = os.read(fd, MAX_SIZE + 1) + finally: + os.close(fd) + text = data.decode("ascii") # one read: validate exactly what gets installed + validate(text) + tmp = TARGET + ".tmp" + fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC | os.O_NOFOLLOW, 0o644) + with os.fdopen(fd, "w") as f: + f.write(text) + os.chmod(tmp, 0o644) + os.replace(tmp, TARGET) + if not os.path.lexists("/etc/openvpn/openvpn.conf"): + os.symlink("server.conf", "/etc/openvpn/openvpn.conf") + + +def prepare_status_log(): + """Let the unprivileged monitoring gatherer read the status log.""" + import grp + gid = grp.getgrnam(SERVICE_USER).gr_gid + if not os.path.exists(STATUS_LOG): + open(STATUS_LOG, "a").close() + os.chown(STATUS_LOG, 0, gid) + os.chmod(STATUS_LOG, 0o640) + + +def service(action): + need(action in ("start", "stop", "restart", "status"), "invalid action") + if action in ("start", "restart"): + need(os.path.isfile(TARGET), "server.conf is not installed") + prepare_status_log() + r = subprocess.run(["/sbin/rc-service", "openvpn", action], capture_output=True, text=True, timeout=60) + return r.returncode, (r.stdout + r.stderr).strip()[-500:] + + +def main(argv): + try: + if argv == ["install-config"]: + install_config() + print(json.dumps({"status": "ok"})) + return 0 + if len(argv) == 2 and argv[0] == "service": + rc, out = service(argv[1]) + print(json.dumps({"status": "ok" if rc == 0 else "error", "output": out})) + return 0 if rc == 0 else 2 + raise Reject("usage: install-config | service start|stop|restart|status") + except Reject as e: + print(json.dumps({"status": "rejected", "error": str(e)})) + return 3 + except Exception as e: # never leak a traceback with paths to the caller + print(json.dumps({"status": "error", "error": type(e).__name__ + ": " + str(e)[:200]})) + return 4 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/README.md b/README.md index 63e6a43..fa4c7aa 100644 --- a/README.md +++ b/README.md @@ -50,9 +50,10 @@ No default user is created. Seed the initial admin with `OVPMON_INITIAL_ADMIN_US | 2026-09-30 | Security hardening: path traversal, 2FA token bypass, CORS, log leak, HTTPS, SSH, fail2ban | [Security hardening](DOCS/Changes/2026-09-30_Security_Hardening.md) | | 2026-09-30 | Admin username change (API + UI), no built-in default admin | [Admin username change](DOCS/Changes/2026-09-30_Admin_Username_Change.md) | | 2026-09-30 | Validation of server/PKI settings (config injection into the root-written OpenVPN config) | [Settings validation](DOCS/Changes/2026-09-30_Settings_Validation.md) | +| 2026-09-30 | API services run as an unprivileged user; root helper validates and installs the OpenVPN config | [Privilege separation](DOCS/Changes/2026-09-30_Privilege_Separation.md) | | 2026-09-30 | Route OpenVPN clients through a Hysteria2 tunnel to an exit node | [Egress via Hysteria2](DOCS/Changes/2026-09-30_Egress_via_Hysteria2.md) | ## Notes -- `ovpmon-api` and `ovpmon-profiler` currently run as root (they manage OpenVPN and PKI). +- Native deployments run the APIs as user `ovpmon`; OpenVPN config install and service control go through a root helper (`doas`, fixed commands): see [Privilege separation](DOCS/Changes/2026-09-30_Privilege_Separation.md). - Keep `easy-rsa/`, `client-config/`, databases and `*.env` out of git: they contain private keys and secrets.