Skip to content

Allow removal of old SSH keys on provision #1576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 20, 2025

Conversation

dalepgrant
Copy link
Contributor

@dalepgrant dalepgrant commented May 14, 2025

Rel: #1087

This PR adds the ability to enable removing old SSH keys from the server.

The added task replaces all SSH keys on the server for the users defined in Trellis with the first found SSH key set in group_vars/all/users.yml. This allows for cases where the Trellis defaults ~/.ssh/id_rsa.pub and ~/.ssh/id_ed25519.pub don't exist. The existing task "Add user SSH keys" then adds the remaining keys as usual.

Given this has the power to lock a user out of their server, a flag has been added to allow making this an optional task to run. To enable, set reset_user_ssh_keys: true. I've placed this in group_vars/all/security.yml but arguments can be made for this in users.yml or main.yml.

Scenarios

Given the default Trellis keys:

  keys:
      - "{{ lookup('file', '~/.ssh/id_rsa.pub', errors='ignore') }}"
      - "{{ lookup('file', '~/.ssh/id_ed25519.pub', errors='ignore') }}"
      # - https://github.com/username.keys

If...

  • ~/.ssh/id_rsa.pub does not exist, ~/.ssh/id_ed25519.pub does exist
    • keys becomes ["", "ssh-ed25519 AAAAC3NzaC1.."] (example)
    • select('truthy') filters out empty strings, keys becomes ["ssh-ed25519 AAAAC3NzaC1.."]
    • The value of ~/.ssh/id_ed25519.pub is used (e.g. ssh-ed25519 AAAAC3NzaC1..)
    • The task is run. Note: Until the subsequent (existing) task is run, access is temporarily only available to ~/.ssh/id_ed25519.pub.
  • Neither ~/.ssh/id_rsa.pub nor ~/.ssh/id_ed25519.pub exist
    • keys becomes ["", ""]
    • select('truthy') filters out empty strings, keys becomes []
    • The when clause (item['keys'] | select('truthy') | list | length) > 0 evaluates to false
    • The task is skipped - no SSH keys removed from the server.

@dalepgrant dalepgrant changed the title Allow removal of old SSH keys by temporarily setting SSH access to th… Allow removal of old SSH keys on provision May 14, 2025
@dalepgrant
Copy link
Contributor Author

dalepgrant commented May 14, 2025

Happy to add documentation if this gets merged.

roots/docs#542

@swalkinshaw
Copy link
Member

🤔 seems useful because it's opt-in. I wonder if we should actually not even define this by default in group_vars/all/security.yml and instead just have docs on it that say to use it as an extra var from the CLI? Then it's only ever set for a single provision. Of course anyone could define it but this will reduce the chances and make it clear how it should be used.

@dalepgrant
Copy link
Contributor Author

So you'd end up with TRELLIS_RESET_USER_SSH_KEYS=true trellis provision --tags users production, if I'm reading you right (correct me if I'm on the wrong track).

That would presumably mean that someone could set that as a Trellis CLI setting in their config file. Could be a pro or a con.

I'm not married to either way. If you'd prefer this not to be added to security.yml by default then I'll push a change up. Doing it through Trellis CLI seems like a progression from this so could be handled separately - if merged I'll open an issue to follow up.

@swalkinshaw
Copy link
Member

Sort of, I mean the Ansible extra vars feature. So it would be used like this: trellis provision --extra-vars reset_user_ssh_keys=true production

They are just normal variables but take precedence. There's no way to set these in the CLI config; they can only be specified in the command like that.

@dalepgrant
Copy link
Contributor Author

dalepgrant commented May 19, 2025

Gotcha, I'm glad I clarified as that's not what I thought you meant!

Understood, I'll remove the entry in security.yml for this PR and add trellis provision --extra-vars reset_user_ssh_keys=true production to the docs PR (which I haven't yet done) roots/docs#542.

@dalepgrant
Copy link
Contributor Author

Removed from security.yml and tested with extra var, confirmed task runs:
ansible-playbook server.yml -e env=staging --extra-vars="reset_user_ssh_keys=true" --tags=users

Screenshot 2025-05-20 at 10 38 02 AM

Re-tested without extra var, confirmed task does not run:

ansible-playbook server.yml -e env=staging --tags=users

Screenshot 2025-05-20 at 10 38 23 AM

For any one else who sees this in future, I'm using the playbook command but the equivalent trellis cli commands for the above would be
trellis provision --tags users --extra-vars reset_user_ssh_keys=true staging
trellis provision --tags users staging

dalepgrant added a commit to dalepgrant/docs that referenced this pull request May 20, 2025
dalepgrant added a commit to dalepgrant/docs that referenced this pull request May 20, 2025
@swalkinshaw swalkinshaw merged commit 1ccf72c into roots:master May 20, 2025
2 checks passed
@swalkinshaw
Copy link
Member

Thank you 🚀

@dalepgrant dalepgrant deleted the feature/reset-user-ssh-keys branch May 20, 2025 23:57
swalkinshaw added a commit to roots/docs that referenced this pull request May 21, 2025
* Update SSH key removal docs

Rel: roots/trellis#1576

* Update ssh-keys authors

* Update SSH key removal docs

Rel: roots/trellis#1576

* Update trellis/ssh-keys.md

Co-authored-by: Scott Walkinshaw <[email protected]>

---------

Co-authored-by: Scott Walkinshaw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants