-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
copy over ssh host keys #229
Conversation
src/nixos-anywhere.sh
Outdated
@@ -29,6 +29,8 @@ Options: | |||
use another kexec tarball to bootstrap NixOS | |||
* --post-kexec-ssh-port <ssh_port> | |||
after kexec is executed, use a custom ssh port to connect. Defaults to 22 | |||
* --no-copy-host-keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to make this not a default for the following reasons:
- Users might want to copy their own keys in with --extra-files and this option would override it which is surprising (i.e. when you use agenix and the host key is used to decrypt secrets)
- I wouldn't trust the original OS to have good ssh keys. Some cloud providers are not great and may have the same ssh keys for every Recovery OS by accident. Same issue would be if someone generates an installer iso with a pregenerated ssh key.
So can we invert the logic here and make it opt-in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some cloud providers are not great and may have the same ssh keys for every Recovery OS by accident.
Good point.
Users might want to copy their own keys in with --extra-files and this option would override it which is surprising (i.e. when you use agenix and the host key is used to decrypt secrets)
To be fair, current behavior is also surprising because we lose the host key that is used to decrypt the secrets after installation. We shouldn’t be overwriting existing files in /mnt
though.
So can we invert the logic here and make it opt-in?
Agreed, I’ll invert the logic and avoid copying files if the destination already exists.
Now that I think about it, we can have a more generic --preserve-files
that can be specified multiple times to preserve files from before kexec, and have --copy-host-keys
be a convenience shortcut to keep SSH host keys. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, though that would potentially be non-trivial to implement while keeping all the permissions and directory structure, and I’d like to limit the scope of this PR to copying SSH host keys.
nixos-kexec-installer preserves SSH host keys, so should we, to avoid changing host identity. See also - https://github.com/nix-community/nixos-images/blob/c4c73bce65306a1e747684dd0d4bcf0ab2779585/nix/kexec-installer/kexec-run.sh#L42C1-L46C1 - https://github.com/nix-community/nixos-images/blob/c4c73bce65306a1e747684dd0d4bcf0ab2779585/nix/installer.nix#L44-L55
Have you tested this? I am currently getting this error (see the test I added):
|
Yes, I did test and deploy the initial PR, but didn’t check changes made after review. There is a minor typo in the shell script, Thank you for adding test case, I’ll update it to verify that |
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at ffcbf8c |
nixos-kexec-installer preserves SSH host keys, so should we, to avoid changing host identity.
See also