From d1970b77b75e2068ae5baebd42380851eb88826c Mon Sep 17 00:00:00 2001 From: Slawomir Koszewski Date: Sun, 25 Oct 2026 18:00:25 +0100 Subject: [PATCH] fix: vpn-router-setup owns only the creation of the metrics service's account, which is marked internal, and administrators add service accounts with vpn-router-user add --service and /users. --- ADR.md | 15 ++++++----- README.md | 20 ++++++++------ debian-package/debian/changelog | 14 ++++++++++ .../src/usr/lib/vpn-router/vpnrouter_users.py | 27 ++++++++++++++----- debian-package/src/usr/lib/vpn-router/web | 13 ++++++--- debian-package/src/usr/sbin/vpn-router-user | 10 ++++--- 6 files changed, 73 insertions(+), 26 deletions(-) diff --git a/ADR.md b/ADR.md index c1b664b..878e18a 100644 --- a/ADR.md +++ b/ADR.md @@ -558,7 +558,7 @@ purge. A view shows only what its level keeps, so a shorter retention empties th view. A month view needs the hourly rows and a year view the monthly rows. `/metrics` has only the last sample; history is not scraped. -## ADR-23: /metrics takes HTTP Basic authentication of a service account that setup owns +## ADR-23: /metrics takes HTTP Basic authentication of an internal service account that setup creates Status: Accepted @@ -568,11 +568,14 @@ API or the Manager, and its password should be checked and locked out like any o belongs to the package's metrics, not to the people using the router. Decision: A service account is a Manager-only account (ADR-18) with `vpnRouterScope` `service`. -`vpn-router-setup` owns it: while metrics are enabled it adds the account `metrics.service_account` -names, `metrics` by default, with a random password it writes to `/etc/vpn-router/metrics.secret` -(root only), and sets a new random password, ending a lockout, when the file is missing or no -longer holds the password. `vpn-router-user` and `/users` cannot add one. `/auth` refuses it -with the message of a wrong password. `/metrics` accepts HTTP Basic only for that account, and +Administrators add service accounts with `vpn-router-user add --service` and `/users`, for +services of their own that check passwords against the directory, and manage them like other +accounts. The metrics service's is the internal one, created by `vpn-router-setup`: while metrics +are enabled it adds the account `metrics.service_account` names, `metrics` by default, with a +random password it writes to `/etc/vpn-router/metrics.secret` (root only), and sets a new random +password, ending a lockout, when the file is missing or no longer holds the password. The answers +of `/users` mark it as `internal`, and `/users` refuses to set its password, which setup would +replace. `/auth` refuses every service account with the message of a wrong password. `/metrics` accepts HTTP Basic only for that account, and only when it is a service account; the password is checked with a bind as the account, so the password policy's lockout applies (ADR-14). TLS is Caddy's, on `metrics.https_port` (ADR-21); plain HTTP is served on the interfaces `metrics.listen` names, `internal` by default. diff --git a/README.md b/README.md index 6b18f5f..e9420c8 100644 --- a/README.md +++ b/README.md @@ -427,9 +427,11 @@ it is added: name, the password and an optional e-mail address, for notifications: no names and no certificate, so no VPN access. It can sign in to `/auth` and, as a member of `admins`, manage the router. `add` without the first and the last name adds one; its names cannot be set later; -- a service account, a Manager-only account whose `vpnRouterScope` is `service`, which - `vpn-router-setup` adds and keeps for the metrics service (see Metrics); it cannot sign in to - `/auth`, and `vpn-router-user` and `/users` cannot add one. +- a service account, a Manager-only account whose `vpnRouterScope` is `service`, for a service + that checks passwords against the directory; it cannot sign in to `/auth`. `add --service` and + `POST /users` with `"service": true` add one. The internal one is the account + `metrics.service_account` names while metrics are enabled: `vpn-router-setup` adds it and keeps + its password (see Metrics), so `/users` refuses to set its password. Passwords and TOTP secrets are read from the first line of standard input. The answer is text: a table of the users, the client profile, a TOTP secret or a short message, and a failure is a @@ -441,6 +443,7 @@ after a command its options. ```sh echo 's3cr3t-pass' | vpn-router-user add alice --first-name Alice --last-name Smith --email alice@example.com echo 'm4nager-pass' | vpn-router-user add ops +echo 's3rvice-pass' | vpn-router-user add backup --service vpn-router-user update alice --email a.smith@example.com vpn-router-user list vpn-router-user profile alice > alice.ovpn @@ -460,6 +463,7 @@ vpn-router-user otp-remove alice | `list` | The users with their kind, names and e-mail addresses, whether each is enabled or locked, has a certificate and is enrolled for TOTP | | `add --first-name F --last-name L --email E` | Adds a VPN user and issues the certificate | | `add [--email E]` | Adds a Manager-only account, with the e-mail address when given; one name without the other two options is refused | +| `add --service [--email E]` | Adds a service account, with the e-mail address when given; the names are refused | | `update [--first-name F] [--last-name L] [--email E]` | Changes the names or the e-mail address | | `passwd ` | Sets the password | | `disable `, `enable ` | Locks the user out of OpenVPN and `/auth`, and lets the user in again | @@ -474,8 +478,8 @@ A user name is 1 to 32 lowercase letters, digits and hyphens, starting with a le password has 8 to 128 characters; a first or last name has 1 to 64 printable characters; an e-mail address has one `@`, no spaces and a dot in its domain; a TOTP secret is 32 base32 characters, as `otp-new` makes it, and a code 6 digits. Once enrolled, a user logs in to OpenVPN -and `/auth` with the password followed by a current code. In the answers `manager_only` tells the -kind, and a Manager-only account has its names as `null`; `update` changes only its e-mail +and `/auth` with the password followed by a current code. In the answers `manager_only`, `service` +and `internal` tell the kind, and a Manager-only account has its names as `null`; `update` changes only its e-mail address, and `--email ''` removes it. Changing a user's password, disabling, unlocking or removing the user also revokes its refresh tokens of `/auth`. The exit status is 0 on success, 2 for an invalid request, 3 when the user does not exist, 4 when the user exists and 1 for any other failure. @@ -712,10 +716,10 @@ and answer bodies are JSON; a request body is at most 4096 bytes. | Request | Body | Answer | | --- | --- | --- | -| `GET /users` | | `{"users": [{"name", "manager_only", "enabled", "locked", "certificate", "otp", "first_name", "last_name", "email"}, ...]}` | -| `POST /users` | `{"name", "password", "first_name", "last_name", "email"}` for a VPN user; `{"name", "password", "email"?}` for a Manager-only account | 201 and the user | +| `GET /users` | | `{"users": [{"name", "manager_only", "service", "internal", "enabled", "locked", "certificate", "otp", "first_name", "last_name", "email"}, ...]}` | +| `POST /users` | `{"name", "password", "first_name", "last_name", "email"}` for a VPN user; `{"name", "password", "email"?}` for a Manager-only account; `{"name", "password", "service": true, "email"?}` for a service account | 201 and the user | | `PUT /users/` | any of `{"first_name", "last_name", "email"}`; for a Manager-only account only `{"email"}`, `""` removing it | the user | -| `PUT /users//password` | `{"password": ...}` | the user | +| `PUT /users//password` | `{"password": ...}`; refused with 400 for the internal service account | the user | | `POST /users//disable` | | the user | | `POST /users//enable` | | the user | | `POST /users//unlock` | | the user | diff --git a/debian-package/debian/changelog b/debian-package/debian/changelog index 3c01c01..e0c0940 100644 --- a/debian-package/debian/changelog +++ b/debian-package/debian/changelog @@ -1,3 +1,17 @@ +vpn-router (0.18.1-1) unstable; urgency=medium + + fix: vpn-router-setup owns only the creation of the metrics service's account, which is marked internal, and administrators add service accounts with vpn-router-user add --service and /users. + + * POST /users takes "service": true and adds a service account, which + cannot sign in to /auth; vpn-router-user add takes --service. + * The users of /users and vpn-router-user have internal, true for the + account metrics.service_account names while metrics are enabled; + vpn-router-user list shows its kind as internal. + * PUT /users//password refuses the internal service account, + whose password vpn-router-setup keeps. + + -- Slawomir Koszewski Mon, 26 Oct 2026 17:00:00 +0100 + vpn-router (0.18.0-2) unstable; urgency=medium update: release.py tags a Debian revision other than 1 as x.y.z-n. diff --git a/debian-package/src/usr/lib/vpn-router/vpnrouter_users.py b/debian-package/src/usr/lib/vpn-router/vpnrouter_users.py index 985bdc7..e0f2bcf 100644 --- a/debian-package/src/usr/lib/vpn-router/vpnrouter_users.py +++ b/debian-package/src/usr/lib/vpn-router/vpnrouter_users.py @@ -7,9 +7,11 @@ common name, which OpenVPN and IKEv2 accept. A Manager-only account is an accoun simpleSecurityObject and vpnRouterAccount (the package's schema) with a name, a password and optionally an e-mail address, no names, no certificate and so no VPN access; it is added when neither the first nor the last name is given, and can sign in to the web service. A service -account is a Manager-only account with the scope SERVICE_SCOPE in vpnRouterScope, which -vpn-router-setup adds and keeps for the metrics service, as metrics.service_account names it; it -cannot sign in to the web service. Users exist whether or not a P2S system is enabled. +account is a Manager-only account with the scope SERVICE_SCOPE in vpnRouterScope, which cannot +sign in to the web service; an administrator adds one for a service that checks passwords +against the directory. The internal one is the account metrics.service_account names while +metrics are enabled, which vpn-router-setup adds and whose password it keeps (metrics_account). +Users exist whether or not a P2S system is enabled. A group is a groupOfNames in ou=groups with a name, which follows the rules of user names, an optional description and at least one member; a group cannot be empty, so the last member @@ -192,14 +194,27 @@ def service(attributes): return SERVICE_SCOPE.encode() in (value.lower() for value in attributes.get('vpnRouterScope', [])) +def metrics_account(): + """The internal service account: the one metrics.service_account names while metrics are enabled, else None.""" + try: + metrics = vpnrouter.load_config()['metrics'] + except SystemExit: + return None + return metrics['service_account'] if metrics['enabled'] else None + + def summary(attributes): - """A user as the answers show it; a person attribute the entry does not have is None.""" + """A user as the answers show it; a person attribute the entry does not have is None. + + internal is whether it is the internal service account (metrics_account). + """ name = attributes['uid'][0].decode() lock = attributes.get('pwdAccountLockedTime', [None])[0] user = { 'name': name, 'manager_only': manager_only(attributes), 'service': service(attributes), + 'internal': service(attributes) and name == metrics_account(), 'enabled': lock != LOCKED, 'locked': lock not in (None, LOCKED) and locked_since(lock) > time.time() - vpnrouter_ldap.LOCKOUT_SECONDS, 'certificate': certificate(name)[0].is_file(), @@ -225,8 +240,8 @@ def list_users(connection): def add(connection, name, password, values, service=False): """Add a user: a VPN user with its certificate when values gives all three person attributes, a Manager-only account - with at most the e-mail address when it gives neither name, and, with service, which only vpn-router-setup uses, a - service account, with at most the e-mail address too.""" + with at most the e-mail address when it gives neither name, and, with service, a service account, with at most the + e-mail address too.""" check_name(name) details = person(values, required=False) vpn_user = 'givenName' in details or 'sn' in details diff --git a/debian-package/src/usr/lib/vpn-router/web b/debian-package/src/usr/lib/vpn-router/web index e5bf93e..8c8c067 100644 --- a/debian-package/src/usr/lib/vpn-router/web +++ b/debian-package/src/usr/lib/vpn-router/web @@ -441,9 +441,11 @@ def users(state, method, path, body): GET /users the users POST /users add a user: {"name", "password", "first_name", "last_name", "email"}; - without the names a Manager-only account + without the names a Manager-only account, and with "service": true a + service account PUT /users/ change any of {"first_name", "last_name", "email"} - PUT /users//password set the password: {"password": ...} + PUT /users//password set the password: {"password": ...}; refused for the internal service + account, whose password vpn-router-setup keeps POST /users//disable lock the user out of OpenVPN POST /users//enable let the user in again POST /users//unlock end a lockout after failed logins @@ -461,7 +463,8 @@ def users(state, method, path, body): name = parts[1] if len(parts) > 1 else None actions = { ('GET', 1, None): lambda connection: vpnrouter_users.list_users(connection), - ('POST', 1, None): lambda connection: vpnrouter_users.add(connection, body.get('name'), body.get('password'), body), + ('POST', 1, None): lambda connection: vpnrouter_users.add(connection, body.get('name'), body.get('password'), body, + service=body.get('service') is True), ('PUT', 2, None): lambda connection: vpnrouter_users.update(connection, name, body), ('PUT', 3, 'password'): lambda connection: vpnrouter_users.passwd(connection, name, body.get('password')), ('POST', 3, 'disable'): lambda connection: vpnrouter_users.disable(connection, name), @@ -478,6 +481,10 @@ def users(state, method, path, body): ('DELETE', 2, None): 'remove'} if route not in actions: return 404, {'error': f'no route {method} {path}'} + if route == ('POST', 1, None) and not isinstance(body.get('service', False), bool): + return 400, {'error': 'service must be true or false'} + if route == ('PUT', 3, 'password') and name == vpnrouter_users.metrics_account(): + return 400, {'error': f'{name} is the internal service account of the metrics service; vpn-router-setup keeps its password'} try: with vpnrouter_users.session() as connection: document = actions[route](connection) diff --git a/debian-package/src/usr/sbin/vpn-router-user b/debian-package/src/usr/sbin/vpn-router-user index ac887a4..ea7b674 100755 --- a/debian-package/src/usr/sbin/vpn-router-user +++ b/debian-package/src/usr/sbin/vpn-router-user @@ -11,6 +11,7 @@ user exists, 1 anything else. -h or --help shows the commands and their options. vpn-router-user [--json] list vpn-router-user [--json] add --first-name F --last-name L --email E (password on stdin) vpn-router-user [--json] add [--email E] (a Manager-only account; password on stdin) + vpn-router-user [--json] add --service [--email E] (a service account; password on stdin) vpn-router-user [--json] update [--first-name F] [--last-name L] [--email E] vpn-router-user [--json] passwd (password on stdin) vpn-router-user [--json] disable @@ -40,7 +41,8 @@ from vpnrouter_users import Failure HELP = { 'list': 'list the users', 'add': 'add a VPN user with the three person options and issue the certificate, or without the names a ' - 'Manager-only account, with an optional e-mail address; the password is read from standard input', + 'Manager-only account, or with --service a service account, each with an optional e-mail address; the ' + 'password is read from standard input', 'update': "change the first name, the last name or the e-mail address; a Manager-only account has only the " "e-mail address, which '' removes", 'passwd': 'set the password, read from standard input', @@ -76,6 +78,8 @@ def parse(argv): if command in ('add', 'update'): for key, _ in vpnrouter_users.PERSON: sub.add_argument(f'--{key.replace("_", "-")}', dest=key, metavar='TEXT', help=PERSON_HELP[key]) + if command == 'add': + sub.add_argument('--service', action='store_true', help='add a service account, which cannot sign in to the web service') if command == 'otp-enrol': sub.add_argument('code', help='a code of the secret from the authenticator') return parser.parse_args(argv) @@ -93,7 +97,7 @@ def run(connection, options, secret): return vpnrouter_users.list_users(connection) values = {key: getattr(options, key, None) for key, _ in vpnrouter_users.PERSON} if command == 'add': - return vpnrouter_users.add(connection, options.name, secret, values) + return vpnrouter_users.add(connection, options.name, secret, values, service=options.service) if command == 'update': return vpnrouter_users.update(connection, options.name, values) if command == 'passwd': @@ -115,7 +119,7 @@ def text_of(answer): if not users: return 'no users' head = ('USER', 'KIND', 'STATE', 'CERTIFICATE', 'OTP', 'FIRST NAME', 'LAST NAME', 'E-MAIL') - rows = [(user['name'], 'service' if user['service'] else 'manager' if user['manager_only'] else 'vpn', + rows = [(user['name'], 'internal' if user['internal'] else 'service' if user['service'] else 'manager' if user['manager_only'] else 'vpn', 'disabled' if not user['enabled'] else 'locked' if user['locked'] else 'enabled', 'issued' if user['certificate'] else 'missing', 'enrolled' if user['otp'] else '-', user['first_name'] or '-', user['last_name'] or '-', user['email'] or '-')