From a7b9b612654fd4cbf4b9e1566e4d1b3b21166fb7 Mon Sep 17 00:00:00 2001 From: Dan Guido Date: Fri, 28 Nov 2025 01:19:36 -0500 Subject: [PATCH] Fix update-users: DNS IP, reboots, key regeneration, and local permissions (#14883) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix DNS IP changing after running update-users The local_service_ip variable is generated using a deterministic random formula seeded by algo_server_name + ansible_fqdn. While algo_server_name is persisted in .config.yml, ansible_fqdn is a runtime fact gathered from the target host that can change if: - Server's hostname configuration changes - DNS resolution changes - User runs ansible from a different control machine When ansible_fqdn differs between initial deployment and users.yml, a different local_service_ip is generated, causing new client configs to point to a DNS IP that doesn't exist on the server. This fix persists local_service_ip and local_service_ipv6 in .config.yml during initial deployment, ensuring users.yml uses the correct DNS IP. Fixes #14614 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Skip apt upgrade and reboot during update-users Previously, running `./algo update-users` would trigger a full apt upgrade and potentially reboot the server because ubuntu.yml was tagged with `update-users`. This caused unexpected disconnections for all VPN clients (gaming, downloads, etc.) during what should be a simple user management operation. Now only facts.yml runs during update-users, which provides the necessary password facts for IPsec certificate generation without running apt upgrade or triggering reboots. Fixes #14518 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Add keys_clean_all support to WireGuard (parity with IPsec) Previously, WireGuard had no option to force credential regeneration for existing users. The keys_clean_all option only affected IPsec certificates. Now WireGuard respects keys_clean_all the same way IPsec does: - keys_clean_all: false (default) - preserve existing keys, only generate for new users - keys_clean_all: true - delete all keys and regenerate for all users Also improved the config.cfg documentation to clarify this option affects both WireGuard and IPsec credentials. Fixes #14610 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Add permission check for local deployment update-users For local deployments, file ownership must be consistent between initial deployment and subsequent update-users runs. When there's a mismatch (e.g., initial deployment without sudo, update with sudo), files get mixed ownership causing permission errors. This adds a pre-flight check that: - Detects local deployments (localhost or algo_provider: local) - Compares config directory owner with current user - Displays a warning with guidance if there's a mismatch - Provides the exact chown command to fix permissions Addresses #14551 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Address review feedback: explicit directory modes and clearer docs - Add explicit mode to WireGuard directory creation in main.yml: - PKI directories (preshared, private, public): 0700 - Config directories (apple/ios, apple/macos): 0755 - Enhance config.cfg keys_clean_all comment to clarify: - When false: new users added (not just preserved) - When true: ALL CLIENTS MUST RECONFIGURE (explicit impact warning) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Address review: fail on permission mismatch, clean up directory creation 1. Permission check now FAILS instead of warning - Prevents continuing with mismatched permissions - Provides clear fix command: sudo chown -R configs// - Simpler condition: just compare owner with current user 2. Clean separation of directory creation - main.yml: Only config directories (apple/ios, apple/macos) with 0755 - keys.yml: Only PKI directories (preshared, private, public) with 0700 - Eliminates duplication and clarifies responsibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Address review: add empty users validation and improve docs 1. Add empty users list validation - Fails early with clear message if no users defined in config.cfg - Prevents confusing downstream errors 2. Improve config.cfg keys_clean_all documentation - Add example use cases: key compromise, removing untrusted users, security audit 3. Rename block for clarity - "Check local deployment permissions" → "Local deployment permission validation" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Fix jinja2 spacing lint warning Remove extra spaces inside brackets in Jinja2 expression per ansible-lint jinja[spacing] rule. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- config.cfg | 5 ++++- roles/common/tasks/main.yml | 4 +++- roles/wireguard/tasks/keys.yml | 17 ++++++++++++++++ roles/wireguard/tasks/main.yml | 6 ++---- server.yml | 2 ++ users.yml | 37 +++++++++++++++++++++++++++++++++- 6 files changed, 64 insertions(+), 7 deletions(-) diff --git a/config.cfg b/config.cfg index ae579ad2..3e43c1a4 100644 --- a/config.cfg +++ b/config.cfg @@ -88,7 +88,10 @@ dns_servers: # Store PKI in RAM disk when not retaining (MacOS/Linux only) pki_in_tmpfs: true -# Regenerate ALL user certs on update-users (not just new users) +# Regenerate ALL user credentials on update-users (not just new users) +# When false: existing WireGuard keys and IPsec certs are preserved, new users added +# When true: all credentials deleted and regenerated - ALL CLIENTS MUST RECONFIGURE +# Use true after: suspected key compromise, removing untrusted users, or security audit keys_clean_all: false ### VPN Network Configuration ### diff --git a/roles/common/tasks/main.yml b/roles/common/tasks/main.yml index 2aed7abe..02bc73e0 100644 --- a/roles/common/tasks/main.yml +++ b/roles/common/tasks/main.yml @@ -11,10 +11,12 @@ - include_tasks: ubuntu.yml when: '"Ubuntu" in OS.stdout or "Linux" in OS.stdout' + +# Include facts separately for update-users (skips apt upgrade/reboot in ubuntu.yml) +- include_tasks: facts.yml tags: - update-users - - name: Sysctl tuning sysctl: name="{{ item.item }}" value="{{ item.value }}" when: item.item is defined and item.item != none diff --git a/roles/wireguard/tasks/keys.yml b/roles/wireguard/tasks/keys.yml index 74f380e9..78dabd9c 100644 --- a/roles/wireguard/tasks/keys.yml +++ b/roles/wireguard/tasks/keys.yml @@ -1,4 +1,21 @@ --- +- name: Ensure the WireGuard pki directory does not exist + file: + dest: "{{ wireguard_pki_path }}" + state: absent + when: keys_clean_all | bool + +- name: Ensure the WireGuard pki directories exist + file: + dest: "{{ wireguard_pki_path }}/{{ item }}" + state: directory + recurse: true + mode: "0700" + with_items: + - preshared + - private + - public + - name: Generate raw private keys community.crypto.openssl_privatekey: type: X25519 diff --git a/roles/wireguard/tasks/main.yml b/roles/wireguard/tasks/main.yml index 307dc357..e982b377 100644 --- a/roles/wireguard/tasks/main.yml +++ b/roles/wireguard/tasks/main.yml @@ -1,13 +1,11 @@ --- -- name: Ensure the required directories exist +- name: Ensure the required config directories exist file: dest: "{{ item }}" state: directory recurse: true + mode: "0755" with_items: - - "{{ wireguard_pki_path }}/preshared" - - "{{ wireguard_pki_path }}/private" - - "{{ wireguard_pki_path }}/public" - "{{ wireguard_config_path }}/apple/ios" - "{{ wireguard_config_path }}/apple/macos" delegate_to: localhost diff --git a/server.yml b/server.yml index a5f89abc..d6234ad6 100644 --- a/server.yml +++ b/server.yml @@ -205,6 +205,8 @@ IP_subject_alt_name: {{ IP_subject_alt_name }} ipsec_enabled: {{ ipsec_enabled }} wireguard_enabled: {{ wireguard_enabled }} + local_service_ip: {{ local_service_ip }} + local_service_ipv6: {{ local_service_ipv6 }} {% if tests | default(false) | bool %} ca_password: '{{ CA_password }}' p12_password: '{{ p12_export_password }}' diff --git a/users.yml b/users.yml index 83588aeb..cf6ebf92 100644 --- a/users.yml +++ b/users.yml @@ -24,7 +24,7 @@ - name: Build list of installed servers set_fact: - server_list: "{{ server_list | default([]) + [ {'server': config.server, 'IP_subject_alt_name': config.IP_subject_alt_name} ] }}" + server_list: "{{ server_list | default([]) + [{'server': config.server, 'IP_subject_alt_name': config.IP_subject_alt_name}] }}" loop: "{{ _configs_list.files }}" loop_control: label: "{{ item.path }}" @@ -51,6 +51,41 @@ include_vars: file: configs/{{ algo_server }}/.config.yml + - name: Validate users list is not empty + fail: + msg: | + NO USERS DEFINED + + The 'users' list in config.cfg is empty. At least one user is required. + Add users to config.cfg before running update-users. + when: users | default([]) | length == 0 + + - name: Local deployment permission validation + block: + - name: Get config directory owner + stat: + path: configs/{{ algo_server }} + register: config_dir_stat + + - name: Fail on permission mismatch + fail: + msg: | + PERMISSION MISMATCH DETECTED + + Config directory owner: {{ config_dir_stat.stat.pw_name }} + Current user: {{ ansible_user_id }} + + Running update-users with mismatched permissions will create + files with inconsistent ownership, breaking future operations. + + TO FIX: Run this command, then retry update-users: + sudo chown -R {{ ansible_user_id }} configs/{{ algo_server }}/ + + PREVENT: Always run update-users the same way as initial deployment + (both with sudo, or both without sudo). + when: config_dir_stat.stat.pw_name != ansible_user_id + when: algo_server == 'localhost' or algo_provider | default('') == 'local' + - name: Test SSH connectivity to server wait_for: host: "{{ algo_server }}"