From f01be41191c4913445298376842398006bc2a6bd Mon Sep 17 00:00:00 2001 From: Slawomir Koszewski Date: Sun, 25 Oct 2026 18:18:03 +0100 Subject: [PATCH] fix: service accounts are managed at /service-accounts, not /users, and the internal one of the metrics service is only listed. --- ADR.md | 25 +++--- README.md | 39 +++++++-- debian-package/debian/changelog | 17 ++++ debian-package/src/usr/lib/vpn-router/web | 96 +++++++++++++++++++---- 4 files changed, 143 insertions(+), 34 deletions(-) diff --git a/ADR.md b/ADR.md index 878e18a..c069c3c 100644 --- a/ADR.md +++ b/ADR.md @@ -568,22 +568,27 @@ 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`. -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 +Administrators add service accounts with `vpn-router-user add --service` and the web service's +`/service-accounts`, for services of their own that check passwords against the directory. +`/service-accounts` is a path of its own, apart from `/users`, which does not list or serve them: +a service account has no names, certificate, profile or TOTP, and is not a person an +administrator manages with the users. 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. `/service-accounts` marks it as `internal` and only lists it: every other +request on it, and putting it in or taking it out of a group at `/groups`, gets 403, whatever the +token. Setup +changes it through the directory's user-manager account, not the API, and root may still with +`vpn-router-user`. `/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. Consequences: Every scrape binds to slapd once. The collector's operator copies the password from the router; deleting the file and running setup changes it. A user who already has the name is not -taken over; setup fails until `metrics.service_account` names another. A disabled account stays -disabled. A scraper on the external interface over plain HTTP sends the password in the clear, so +taken over; setup fails until `metrics.service_account` names another. An account root disabled +stays disabled. A scraper on the external interface over plain HTTP sends the password in the clear, so `metrics.listen` stays `internal` unless HTTPS is used. ## ADR-24: The Router Manager upgrades the router's packages in a transient unit outside its own diff --git a/README.md b/README.md index e9420c8..66166a5 100644 --- a/README.md +++ b/README.md @@ -428,10 +428,11 @@ it is added: 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`, 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. + that checks passwords against the directory; it cannot sign in to `/auth`. `add --service` adds + one, and the web service manages them at `/service-accounts`, not at `/users`. 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). The API only lists it: `/service-accounts` refuses every + other request on it, and `/groups` refuses to put it in or take it out of a group, with 403. 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 @@ -610,6 +611,7 @@ curl http://10.0.0.4:17081/ | `/auth` | Signing in with a user's password and TOTP code, refreshing and signing out | | `/config` | The configuration without its secret options, and what the router resolved and runs, as JSON; needs a token | | `/users` | The users of OpenVPN and road-warrior access; needs a token | +| `/service-accounts` | The service accounts; needs a token | | `/groups` | The groups of users; needs a token | | `/site-to-site` | The site-to-site connections: list, add, change and remove; needs a token | | `/account` | The account of the signed-in user: its details, its password and its TOTP enrolment; needs a user's token | @@ -634,7 +636,7 @@ curl http://10.0.0.4:7081/status ### Tokens and signing in -`/config`, `/users`, `/groups`, `/site-to-site` and `/account` need a JWT in an `Authorization: Bearer` header, signed with +`/config`, `/users`, `/service-accounts`, `/groups`, `/site-to-site` and `/account` need a JWT in an `Authorization: Bearer` header, signed with HS256 by the key the web service makes on its first run, `/etc/vpn-router/web.key` (root only). The key never leaves the router; while it does not exist these paths and `/auth` are not served (404). A missing, expired or wrongly signed token, or one of another algorithm, gets 401 with @@ -717,9 +719,9 @@ and answer bodies are JSON; a request body is at most 4096 bytes. | Request | Body | Answer | | --- | --- | --- | | `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 | +| `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 | | `PUT /users/` | any of `{"first_name", "last_name", "email"}`; for a Manager-only account only `{"email"}`, `""` removing it | the user | -| `PUT /users//password` | `{"password": ...}`; refused with 400 for the internal service account | the user | +| `PUT /users//password` | `{"password": ...}` | the user | | `POST /users//disable` | | the user | | `POST /users//enable` | | the user | | `POST /users//unlock` | | the user | @@ -730,7 +732,8 @@ and answer bodies are JSON; a request body is at most 4096 bytes. | `DELETE /users//otp` | | the user | A failure is `{"error": ...}` with 400 for an invalid request, 404 for a user that does not -exist, 409 for a user that exists and 500 otherwise. `/users` works whether or not OpenVPN or +exist or is a service account, 409 for a user that exists and 500 otherwise. `/users` does not +list service accounts; `/service-accounts` serves them. `/users` works whether or not OpenVPN or road-warrior access is enabled; the profile needs OpenVPN. Passwords, private keys and TOTP secrets cross the internal network in plain HTTP, like the token. @@ -740,6 +743,26 @@ curl -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \ http://10.0.0.4:7081/users ``` +### The /service-accounts API + +`/service-accounts` manages the service accounts with the same token and the same module as +`/users`. + +| Request | Body | Answer | +| --- | --- | --- | +| `GET /service-accounts` | | `{"service_accounts": [{"name", "internal", "enabled", "locked", "email", ...}, ...]}`, each as `/users` shows a user | +| `POST /service-accounts` | `{"name", "password", "email"?}` | 201 and the account | +| `PUT /service-accounts/` | `{"email"}`; `""` removes it | the account | +| `PUT /service-accounts//password` | `{"password": ...}` | the account | +| `POST /service-accounts//disable` | | the account | +| `POST /service-accounts//enable` | | the account | +| `POST /service-accounts//unlock` | | the account | +| `DELETE /service-accounts/` | | `{"name", "removed": true}` | + +The internal account, `internal` true, is only listed: every other request on it gets 403, as +`vpn-router-setup` adds and keeps it. A failure is otherwise as for `/users`, with 404 also for a +name that is not a service account. + ### The /account API `/account` serves the user the access token names, whether or not the user is a member of diff --git a/debian-package/debian/changelog b/debian-package/debian/changelog index e0c0940..dc3e2a0 100644 --- a/debian-package/debian/changelog +++ b/debian-package/debian/changelog @@ -1,3 +1,20 @@ +vpn-router (0.18.2-1) unstable; urgency=medium + + fix: service accounts are managed at /service-accounts, not /users, and the internal one of the metrics service is only listed. + + * /service-accounts, for members of admins and admin tokens, lists the + service accounts, adds one with a password and an optional e-mail + address, changes the e-mail address, sets the password, disables, + enables, unlocks and removes them. + * /users no longer lists service accounts or takes "service" on POST, + and answers 404 to a request naming one. + * /service-accounts answers 403 to every request but the list on the + account metrics.service_account names while metrics are enabled, and + /groups to putting it in or taking it out of a group; + vpn-router-setup and root's vpn-router-user still change it. + + -- Slawomir Koszewski Mon, 26 Oct 2026 18:00:00 +0100 + 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. diff --git a/debian-package/src/usr/lib/vpn-router/web b/debian-package/src/usr/lib/vpn-router/web index 8c8c067..a224868 100644 --- a/debian-package/src/usr/lib/vpn-router/web +++ b/debian-package/src/usr/lib/vpn-router/web @@ -12,7 +12,11 @@ Pages: runs, as JSON /users the users in the router's directory, whether or not a P2S system is enabled: list, add, change, set the password, disable, enable, remove, the OpenVPN client - profile, and TOTP enrolment (see users()) + profile, and TOTP enrolment (see users()); service accounts are not among them + /service-accounts + the service accounts: list, add, change the e-mail address, set the password, + disable, enable, unlock and remove; the internal one is only listed (see + service_accounts()) /groups the groups of the router's directory: list, add, rename or describe, remove, and add and take out members (see groups()) /account the account of the signed-in user: its details, its password and its TOTP @@ -28,7 +32,7 @@ Pages: the databases of each PostgreSQL cluster on the router with their sizes, as JSON (see postgresql_databases()); /config has the clusters' settings -/config, /metrics/history, /postgresql, /users, /groups, /site-to-site and /account need an HS256 JWT in an Authorization: Bearer header signed +/config, /metrics/history, /postgresql, /users, /service-accounts, /groups, /site-to-site and /account need an HS256 JWT in an Authorization: Bearer header signed with vpnrouter_auth.WEB_KEY (see vpnrouter_auth.verify). A token of /auth names a user in sub; /account serves that user, and the other paths need a member of vpnrouter_users.ADMIN_GROUP. A token of vpn-router-token has the admin claim and names no user; it may call every path but @@ -441,11 +445,9 @@ 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, and with "service": true a - service account + without the names a Manager-only account PUT /users/ change any of {"first_name", "last_name", "email"} - PUT /users//password set the password: {"password": ...}; refused for the internal service - account, whose password vpn-router-setup keeps + PUT /users//password set the password: {"password": ...} 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 @@ -454,6 +456,8 @@ def users(state, method, path, body): POST /users//otp a new TOTP secret and its otpauth URI; nothing is stored PUT /users//otp store a TOTP secret: {"secret", "code"}, code a code of the secret DELETE /users//otp remove the TOTP token + + Service accounts are served by /service-accounts: they are not listed, and a request naming one is 404. """ if body == 'invalid' or (body is not None and not isinstance(body, dict)): return 400, {'error': 'the body must be a JSON object'} @@ -462,9 +466,9 @@ def users(state, method, path, body): route = (method, len(parts), parts[2] if len(parts) == 3 else None) 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, - service=body.get('service') is True), + ('GET', 1, None): lambda connection: { + 'users': [user for user in vpnrouter_users.list_users(connection)['users'] if not user['service']]}, + ('POST', 1, None): lambda connection: vpnrouter_users.add(connection, body.get('name'), body.get('password'), body), ('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), @@ -481,12 +485,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: + if name is not None and vpnrouter_users.service(vpnrouter_users.entry(connection, name)): + raise vpnrouter_users.Failure(f'{name} is a service account; see /service-accounts', 3) document = actions[route](connection) if operations.get(route) in vpnrouter_auth.REVOKING: vpnrouter_auth.revoke(vpnrouter.load_config(), name) @@ -502,6 +504,56 @@ def users(state, method, path, body): return (201 if route == ('POST', 1, None) else 200), document +def service_accounts(method, path, body): + """Answer a /service-accounts request with vpnrouter_users: the HTTP status and the JSON document. + + GET /service-accounts the service accounts: {"service_accounts": [...]} + POST /service-accounts add one: {"name", "password", "email"?} + PUT /service-accounts/ change the e-mail address: {"email"}; "" removes it + PUT /service-accounts//password set the password: {"password": ...} + POST /service-accounts//disable lock it out + POST /service-accounts//enable let it in again + POST /service-accounts//unlock end a lockout after failed logins + DELETE /service-accounts/ remove it + + A name that is not a service account is 404. The internal one (vpnrouter_users.metrics_account), which + vpn-router-setup adds and keeps, is only listed: every other request on it is refused with 403. + """ + if body == 'invalid' or (body is not None and not isinstance(body, dict)): + return 400, {'error': 'the body must be a JSON object'} + body = body or {} + parts = path.strip('/').split('/') + route = (method, len(parts), parts[2] if len(parts) == 3 else None) + name = parts[1] if len(parts) > 1 else None + email = {'email': body.get('email')} + actions = { + ('GET', 1, None): lambda connection: { + 'service_accounts': [user for user in vpnrouter_users.list_users(connection)['users'] if user['service']]}, + ('POST', 1, None): lambda connection: vpnrouter_users.add(connection, body.get('name'), body.get('password'), email, + service=True), + ('PUT', 2, None): lambda connection: vpnrouter_users.update(connection, name, email), + ('PUT', 3, 'password'): lambda connection: vpnrouter_users.passwd(connection, name, body.get('password')), + ('POST', 3, 'disable'): lambda connection: vpnrouter_users.disable(connection, name), + ('POST', 3, 'enable'): lambda connection: vpnrouter_users.enable(connection, name), + ('POST', 3, 'unlock'): lambda connection: vpnrouter_users.unlock(connection, name), + ('DELETE', 2, None): lambda connection: vpnrouter_users.remove(connection, name), + } + if route not in actions: + return 404, {'error': f'no route {method} {path}'} + if name is not None and name == vpnrouter_users.metrics_account(): + return 403, {'error': f'{name} is the internal service account of the metrics service; only vpn-router-setup changes it'} + try: + with vpnrouter_users.session() as connection: + if name is not None and not vpnrouter_users.service(vpnrouter_users.entry(connection, name)): + raise vpnrouter_users.Failure(f'{name} is not a service account', 3) + document = actions[route](connection) + except vpnrouter_users.Failure as failure: + return USER_STATUS.get(failure.status, 500), {'error': str(failure)} + except (ldap.LDAPError, OSError) as exc: + return 500, {'error': f'the user directory failed: {exc}'} + return (201 if route == ('POST', 1, None) else 200), document + + def groups(state, method, path, body): """Answer a /groups request with vpnrouter_users: the HTTP status and the JSON document. @@ -511,6 +563,9 @@ def groups(state, method, path, body): DELETE /groups/ remove the group; its members are kept POST /groups//members add a member: {"user": ...} DELETE /groups//members/ take a member out; the last member cannot leave + + The internal service account (vpnrouter_users.metrics_account) is not put in or taken out of a group: such a + request is refused with 403. """ if body == 'invalid' or (body is not None and not isinstance(body, dict)): return 400, {'error': 'the body must be a JSON object'} @@ -518,6 +573,11 @@ def groups(state, method, path, body): parts = path.strip('/').split('/') name = parts[1] if len(parts) > 1 else None route = (method, len(parts), parts[2] if len(parts) > 2 else None) + internal = vpnrouter_users.metrics_account() + members = body.get('members') if route == ('POST', 1, None) else [body.get('user')] if route == ('POST', 3, 'members') \ + else [parts[3]] if route == ('DELETE', 4, 'members') else [] + if internal is not None and isinstance(members, list) and internal in members: + return 403, {'error': f'{internal} is the internal service account of the metrics service; only vpn-router-setup changes it'} actions = { ('GET', 1, None): lambda connection: vpnrouter_users.list_groups(connection), ('POST', 1, None): lambda connection: vpnrouter_users.group_add( @@ -680,7 +740,8 @@ def metrics_history(view): def changes_admins(connection, method, parts, body): - """Whether a /users or /groups request creates, renames to, changes or removes ADMIN_GROUP, or removes a member of it.""" + """Whether a /users, /service-accounts or /groups request creates, renames to, changes or removes ADMIN_GROUP, or + removes a member of it.""" admins = vpnrouter_users.ADMIN_GROUP if parts[0] == 'groups': if method == 'POST' and len(parts) == 1: @@ -688,7 +749,7 @@ def changes_admins(connection, method, parts, body): if method != 'GET' and len(parts) > 1 and parts[1] == admins: return True return method == 'PUT' and len(parts) == 2 and body.get('name') == admins - if parts[0] == 'users' and method == 'DELETE' and len(parts) == 2: + if parts[0] in ('users', 'service-accounts') and method == 'DELETE' and len(parts) == 2: return admins in vpnrouter_users.groups_of(connection, parts[1]) return False @@ -802,7 +863,7 @@ class Handler(BaseHTTPRequestHandler): # A body that is not read must not be taken for the next request. if self.command not in ('GET', 'HEAD'): self.close_connection = True - directory = next((name for name in ('users', 'groups', 'account', 'site-to-site') + directory = next((name for name in ('users', 'service-accounts', 'groups', 'account', 'site-to-site') if path == f'/{name}' or path.startswith(f'/{name}/')), None) protected = path in ('/config', '/metrics/history', '/postgresql') or directory is not None signing = path in ('/auth', '/auth/refresh', '/auth/logout') @@ -843,6 +904,9 @@ class Handler(BaseHTTPRequestHandler): elif directory == 'site-to-site': code, document = site_to_site(self.command, path, payload) body, kind = json.dumps(document, indent=2).encode() + b'\n', 'application/json' + elif directory == 'service-accounts': + code, document = service_accounts(self.command, path, payload) + body, kind = json.dumps(document, indent=2).encode() + b'\n', 'application/json' elif directory is not None: code, document = (users if directory == 'users' else groups)(state, self.command, path, payload) body, kind = json.dumps(document, indent=2).encode() + b'\n', 'application/json'