diff --git a/APP_PROFILER/routers/server.py b/APP_PROFILER/routers/server.py index 7b2c45d..1be1498 100644 --- a/APP_PROFILER/routers/server.py +++ b/APP_PROFILER/routers/server.py @@ -29,5 +29,7 @@ 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 ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) except Exception as e: raise HTTPException(status_code=500, detail=str(e)) diff --git a/APP_PROFILER/schemas.py b/APP_PROFILER/schemas.py index f3faa92..c8feed3 100644 --- a/APP_PROFILER/schemas.py +++ b/APP_PROFILER/schemas.py @@ -1,4 +1,7 @@ -from pydantic import BaseModel, Field +import ipaddress +import re +from pydantic import BaseModel, Field, field_validator, model_validator +from services import validation as v from typing import List, Optional, Literal from datetime import datetime @@ -21,7 +24,54 @@ class PKISettingBase(BaseModel): easyrsa_batch: bool = True class PKISettingUpdate(PKISettingBase): - pass + @field_validator("fqdn_ca", "fqdn_server") + @classmethod + def _check_fqdn(cls, val): + if not re.match(r"^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$", val) or ".." in val: + raise ValueError("Invalid name (letters, digits, . _ -; max 64)") + return val + + @field_validator("easyrsa_dn") + @classmethod + def _check_dn(cls, val): + if val not in ("cn_only", "org"): + raise ValueError("easyrsa_dn must be 'cn_only' or 'org'") + return val + + @field_validator("easyrsa_req_country") + @classmethod + def _check_country(cls, val): + if not re.match(r"^[A-Z]{2}$", val): + raise ValueError("Country must be a 2-letter uppercase code") + return val + + @field_validator("easyrsa_req_province", "easyrsa_req_city", "easyrsa_req_org", "easyrsa_req_ou") + @classmethod + def _check_dn_text(cls, val): + if not re.match(r"^[A-Za-z0-9 .,_-]{0,64}$", val): + raise ValueError("Only letters, digits, space and . , _ - are allowed (max 64)") + return val + + @field_validator("easyrsa_req_email") + @classmethod + def _check_email(cls, val): + if not re.match(r"^[A-Za-z0-9._%+-]{1,64}@[A-Za-z0-9.-]{1,190}$", val): + raise ValueError("Invalid email") + return val + + @field_validator("easyrsa_key_size") + @classmethod + def _check_key_size(cls, val): + if val not in (2048, 3072, 4096): + raise ValueError("Key size must be 2048, 3072 or 4096") + return val + + @field_validator("easyrsa_ca_expire", "easyrsa_cert_expire", "easyrsa_cert_renew", "easyrsa_crl_days") + @classmethod + def _check_days(cls, val): + if not 1 <= val <= 36500: + raise ValueError("Days must be between 1 and 36500") + return val class PKISetting(PKISettingBase): id: int @@ -53,7 +103,100 @@ class SystemSettingsBase(BaseModel): mssfix: Optional[int] = None class SystemSettingsUpdate(SystemSettingsBase): - pass + @field_validator("port", "management_port") + @classmethod + def _check_port(cls, val): + if not 1 <= val <= 65535: + raise ValueError("Port must be between 1 and 65535") + return val + + @field_validator("vpn_network") + @classmethod + def _check_network(cls, val): + try: + ipaddress.IPv4Address(val) + except ValueError: + raise ValueError("Invalid IPv4 network address") + return val + + @field_validator("vpn_netmask") + @classmethod + def _check_netmask(cls, val): + if not v.valid_netmask(val): + raise ValueError("Invalid netmask") + return val + + @model_validator(mode="after") + def _check_subnet(self): + try: + net = ipaddress.IPv4Network(f"{self.vpn_network}/{self.vpn_netmask}", strict=True) + except ValueError: + raise ValueError("vpn_network is not a valid network address for vpn_netmask") + if not 8 <= net.prefixlen <= 30: + raise ValueError("VPN subnet prefix must be between /8 and /30") + return self + + @field_validator("split_routes") + @classmethod + def _check_routes(cls, val): + if len(val) > 256: + raise ValueError("Too many routes (max 256)") + for r in val: + if not v.valid_route(r): + raise ValueError(f"Invalid route: {r[:40]!r} (use a.b.c.d/nn or 'a.b.c.d mask')") + return val + + @field_validator("dns_servers") + @classmethod + def _check_dns(cls, val): + if len(val) > 8: + raise ValueError("Too many DNS servers (max 8)") + for d in val: + try: + ipaddress.ip_address(d) + except ValueError: + raise ValueError(f"Invalid DNS server address: {d[:40]!r}") + return val + + @field_validator("connect_script", "disconnect_script") + @classmethod + def _check_script_path(cls, val): + if val and not v.SCRIPT_RE.match(val): + raise ValueError("Script path must be " + v.SCRIPTS_DIR + "/ (letters, digits, . _ -)") + return val + + @field_validator("management_interface_address") + @classmethod + def _check_mgmt_addr(cls, val): + try: + if not ipaddress.ip_address(val).is_loopback: + raise ValueError + except ValueError: + raise ValueError("Management interface must listen on a loopback address") + return val + + @field_validator("public_ip") + @classmethod + def _check_public_ip(cls, val): + if val in (None, ""): + return val + if not v.valid_host(val): + raise ValueError("public_ip must be an IP address or a hostname") + return val + + @field_validator("tun_mtu") + @classmethod + def _check_mtu(cls, val): + if val is not None and not 576 <= val <= 9000: + raise ValueError("tun_mtu must be between 576 and 9000") + return val + + @field_validator("mssfix") + @classmethod + def _check_mss(cls, val): + if val is not None and not 536 <= val <= 1500: + raise ValueError("mssfix must be between 536 and 1500") + return val class SystemSettings(SystemSettingsBase): id: int diff --git a/APP_PROFILER/services/generator.py b/APP_PROFILER/services/generator.py index 21f4b7f..ac3ac9e 100644 --- a/APP_PROFILER/services/generator.py +++ b/APP_PROFILER/services/generator.py @@ -4,6 +4,7 @@ from jinja2 import Environment, FileSystemLoader from sqlalchemy.orm import Session from .config import get_system_settings, get_pki_settings from .pki import PKI_DIR +from . import validation logger = logging.getLogger(__name__) @@ -26,7 +27,7 @@ def generate_server_config(db: Session, output_path: str = "server.conf"): file_crl_path = os.path.join(PKI_DIR, "crl.pem") # Render template - config_content = template.render( + ctx = dict( protocol=settings.protocol, port=settings.port, ca_path=file_ca_path, @@ -53,7 +54,13 @@ def generate_server_config(db: Session, output_path: str = "server.conf"): tun_mtu=settings.tun_mtu, mssfix=settings.mssfix ) - + for _name, _val in ctx.items(): + validation.assert_safe_scalar(_name, _val) + if settings.user_defined_cdscripts: + validation.check_script(settings.connect_script) + validation.check_script(settings.disconnect_script) + config_content = template.render(**ctx) + # Write to file with open(output_path, "w") as f: f.write(config_content) @@ -97,7 +104,9 @@ def generate_client_config(db: Session, username: str, output_path: str): remote_ip = get_public_ip() template = env.get_template("client.ovpn.j2") - + + validation.assert_safe_scalar("remote_ip", remote_ip) + validation.assert_safe_scalar("protocol", settings.protocol) config_content = template.render( protocol=settings.protocol, remote_ip=remote_ip, diff --git a/APP_PROFILER/services/validation.py b/APP_PROFILER/services/validation.py new file mode 100644 index 0000000..df4e295 --- /dev/null +++ b/APP_PROFILER/services/validation.py @@ -0,0 +1,71 @@ +"""Input validation helpers for settings that end up in generated OpenVPN configs.""" +import ipaddress +import os +import re + +SCRIPTS_DIR = "/etc/openvpn/scripts" +SCRIPT_RE = re.compile(r"^/etc/openvpn/scripts/[A-Za-z0-9_.-]{1,64}$") +HOSTNAME_RE = re.compile( + r"^(?=.{1,253}$)([A-Za-z0-9]([A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)*[A-Za-z0-9]([A-Za-z0-9-]{0,61}[A-Za-z0-9])?$" +) +FORBIDDEN_CHARS = set('\n\r"\\\x00') + + +def assert_safe_scalar(name: str, value) -> None: + """Reject values that could break out of a config line (newlines, quotes, backslashes, NUL).""" + if isinstance(value, (list, tuple)): + for item in value: + assert_safe_scalar(name, item) + return + if isinstance(value, str) and (FORBIDDEN_CHARS & set(value) or any(ord(c) < 32 for c in value)): + raise ValueError(f"Unsafe characters in '{name}'") + + +def valid_netmask(mask: str) -> bool: + try: + ipaddress.IPv4Network(f"0.0.0.0/{mask}") + return True + except ValueError: + return False + + +def valid_route(route: str) -> bool: + """'a.b.c.d/nn' or 'a.b.c.d m.m.m.m'.""" + parts = route.split() + try: + if len(parts) == 1: + ipaddress.IPv4Network(parts[0], strict=False) + return True + if len(parts) == 2: + ipaddress.IPv4Address(parts[0]) + return valid_netmask(parts[1]) + except ValueError: + pass + return False + + +def valid_host(value: str) -> bool: + try: + ipaddress.ip_address(value) + return True + except ValueError: + return bool(HOSTNAME_RE.match(value)) + + +def check_script(path: str) -> None: + """A connect/disconnect script must be a root-owned, non-writable file inside SCRIPTS_DIR.""" + if not path: + return + if not SCRIPT_RE.match(path): + raise ValueError(f"Script path must match {SCRIPTS_DIR}/") + real = os.path.realpath(path) + if os.path.dirname(real) != SCRIPTS_DIR: + raise ValueError("Script must reside directly in " + SCRIPTS_DIR) + if not os.path.isfile(real): + raise ValueError("Script file does not exist") + st = os.stat(real) + if st.st_uid != 0 or st.st_mode & 0o022: + raise ValueError("Script must be owned by root and not writable by group/others") + dst = os.stat(SCRIPTS_DIR) + if dst.st_uid != 0 or dst.st_mode & 0o022: + raise ValueError(SCRIPTS_DIR + " must be owned by root and not writable by group/others") diff --git a/DOCS/Changes/2026-09-30_Settings_Validation.md b/DOCS/Changes/2026-09-30_Settings_Validation.md new file mode 100644 index 0000000..da7a007 --- /dev/null +++ b/DOCS/Changes/2026-09-30_Settings_Validation.md @@ -0,0 +1,46 @@ +# Server settings validation (2026-09-30) + +Problem: values of the server/PKI settings (`PUT /profiles-api/config/server|pki`) were free strings that were rendered into `server.conf` (written by a root process) and passed to `easyrsa`. A user with a valid token could inject extra OpenVPN directives (for example `up`/`plugin`) or shell-relevant DN characters, which is a path to code execution as root. + +## Defence in three layers + +| Layer | Where | What it does | +|---|---|---| +| A. Schema | `APP_PROFILER/schemas.py` (`SystemSettingsUpdate`, `PKISettingUpdate`) | Rejects invalid values with 422. Applies to updates only, so already stored values never break `GET /config` | +| B. Scripts | `services/validation.py: check_script`, used in `services/generator.py` | `connect_script`/`disconnect_script` must be a file directly inside `/etc/openvpn/scripts/`, owned by root, not group/other-writable, in a root-owned non-writable directory (symlinks out of the directory are rejected) | +| C. Renderer | `services/generator.py` | Before writing `server.conf` / client `.ovpn`, every value is checked for newline, CR, NUL, other control characters, `"` and `\`; on violation nothing is written and the API returns 400 | + +## Rules (layer A) + +| Field | Rule | +|---|---| +| `port`, `management_port` | 1-65535 | +| `vpn_network` + `vpn_netmask` | valid IPv4 network address for a contiguous mask, prefix /8-/30 | +| `split_routes[]` | `a.b.c.d/nn` or `a.b.c.d mask`, at most 256 | +| `dns_servers[]` | IPv4/IPv6 addresses, at most 8 | +| `public_ip` | IP address or hostname | +| `management_interface_address` | loopback only | +| `tun_mtu`, `mssfix` | 576-9000, 536-1500 | +| `connect_script`, `disconnect_script` | empty or `/etc/openvpn/scripts/` | +| PKI `fqdn_ca`, `fqdn_server` | `^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$`, no `..` | +| PKI `easyrsa_dn` | `cn_only` or `org` | +| PKI country / province, city, org, ou / email | `^[A-Z]{2}$` / `^[A-Za-z0-9 .,_-]{0,64}$` / simple email pattern | +| PKI `key_size`, days | 2048/3072/4096; 1-36500 | + +## Results + +| Check | Result | +|---|---| +| Round trip of current server and PKI settings via `PUT` | 200 | +| 17 malicious/invalid values (newline in DNS or routes, quote, `../` and `/tmp` scripts, port 0/70000, bad mask, host bits in network, non-loopback management, `a b;c` host, MTU 100, `/` in organisation, lowercase country, `../` in FQDN, key size 512) | all 422 with a clear message | +| 4 valid changes (DNS incl. IPv6, routes in both notations, hostname, allowed script path) | 200 | +| Script checks: root-owned 755 file / group-writable / symlink out of the directory / missing / outside the directory / traversal / empty | accepted / rejected / rejected / rejected / rejected / rejected / accepted | +| Layer C with unsafe values injected past the schema (newline in DNS, quote in route, newline in script, script outside the directory, newline in management address) | all blocked, nothing written | +| Regression: settings render to `server.conf` | identical to the live config | + +Settings were restored after the tests and left unchanged. + +## Notes + +- The scripts directory `/etc/openvpn/scripts/` does not exist by default; create it as `root:root 755` and put root-owned `755` scripts there before enabling `user_defined_cdscripts`. +- Next step (planned): run the API as an unprivileged user with a root helper that re-validates the config before installing it. See the project plan. diff --git a/DOCS/General/Index.md b/DOCS/General/Index.md index d6d43cc..d4884ef 100644 --- a/DOCS/General/Index.md +++ b/DOCS/General/Index.md @@ -13,6 +13,7 @@ Welcome to the documentation for the OpenVPN Monitor suite. ## 🛠 Changes and results - [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) - [Egress via Hysteria2 (2026-09-30)](../Changes/2026-09-30_Egress_via_Hysteria2.md) ## 🔍 Core Monitoring (`APP_CORE`) diff --git a/README.md b/README.md index 18f8aec..63e6a43 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,7 @@ 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 | Route OpenVPN clients through a Hysteria2 tunnel to an exit node | [Egress via Hysteria2](DOCS/Changes/2026-09-30_Egress_via_Hysteria2.md) | ## Notes