Conversation
- Add a setting to specify the path to the known hosts file - Allow obtaining private keys via a command instead of the ssh agent - Add partial support for the `NIX_SSHOPTS` environment variable
f3220f3 to
d56d8ea
Compare
| var memFd int | ||
| memFd, err = unix.MemfdCreate("nixos-cli-ssh-key", unix.MFD_CLOEXEC|unix.MFD_ALLOW_SEALING|unix.MFD_NOEXEC_SEAL) | ||
| if err != nil { | ||
| return nil, "", fmt.Errorf("failed to create memfd: %v", err) | ||
| } | ||
|
|
||
| if _, err = unix.Write(memFd, stdout.Bytes()); err != nil { | ||
| return nil, "", fmt.Errorf("failed to write to memfd: %v", err) | ||
| } | ||
|
|
||
| _, err = unix.FcntlInt(uintptr(memFd), unix.F_ADD_SEALS, unix.F_SEAL_SEAL|unix.F_SEAL_SHRINK|unix.F_SEAL_GROW|unix.F_SEAL_WRITE) | ||
| if err != nil { | ||
| return nil, "", fmt.Errorf("failed to add seals to memfd: %v", err) | ||
| } | ||
|
|
||
| if err = unix.Fchmod(memFd, 0o400); err != nil { | ||
| return nil, "", fmt.Errorf("failed to change permissions of memfd: %v", err) | ||
| } | ||
|
|
||
| keyPath := fmt.Sprintf("/proc/%v/fd/%v", os.Getpid(), memFd) | ||
| auth := ssh.PublicKeys(signer) | ||
|
|
||
| return auth, keyPath, nil |
There was a problem hiding this comment.
Is it possible to have a fallback mechanism that doesn't use memfd on non-Linux platforms?
Although rather niche, I would like to preserve the ability to deploy a NixOS system from a non-NixOS one such as macOS.
| knownHostsFile := cfg.Ssh.KnownHostsFile | ||
| if knownHostsFile == "" { | ||
| knownHostsFile = filepath.Join(os.Getenv("HOME"), ".ssh", "known_hosts") | ||
| } | ||
| knownHostsKeyCallback, err := knownhosts.New(knownHostsFile) |
There was a problem hiding this comment.
It is possible to pass multiple files to knownhosts.New(), so this conditional isn't really needed.
We can keep a list of well-known paths, such as /etc/ssh/known_hosts, the current $HOME/.ssh/known-hosts, concat that with the ssh.known_hosts_file setting, and pass that to this function most likely.
Following from that, ssh.known_hosts_file could be made plural, and a user is free to specify multiple sources of this if they would like.
| log.Warnf("failed to obtain private key: %v", keyErr) | ||
| log.Warnf("falling back to password auth") | ||
| } | ||
| } else if sock := os.Getenv("SSH_AUTH_SOCK"); sock != "" { |
There was a problem hiding this comment.
Ideally, multiple auth methods should be supported, since SSH runs them in order with all the keys it manages to find, so this shouldn't be gated behind an else if.
| } else if sock := os.Getenv("SSH_AUTH_SOCK"); sock != "" { | |
| } | |
| if sock := os.Getenv("SSH_AUTH_SOCK"); sock != "" { |
| log.CmdArray(argv) | ||
|
|
||
| cmd := NewCommand(argv[0], argv[1:]...) | ||
| sshoptsEnv := strings.TrimSpace(strings.Join(sshopts, " ")) |
There was a problem hiding this comment.
Use shlex.Join() for proper shell quoting.
| } | ||
|
|
||
| argv := []string{"nix-copy-closure"} | ||
| sshopts := []string{os.Getenv("NIX_SSHOPTS")} |
There was a problem hiding this comment.
Use shlex.Split() on NIX_SSHOPTS first, since these are shell args and can be quoted as such, and end up getting joined later.
| address: address, | ||
| port: port, | ||
| password: password, | ||
| sshopts: sshopts, |
There was a problem hiding this comment.
Nit: might be better to name this nixSSHOpts or something along those lines, since this only applies to the Nix command for now.
| return fmt.Sprintf("%s@%s:%d", s.user, s.address, s.port) | ||
| } | ||
|
|
||
| func (s *SSHSystem) Sshopts() []string { |
There was a problem hiding this comment.
Nit: use the uppercase naming for the acronym SSH
| func (s *SSHSystem) Sshopts() []string { | |
| func (s *SSHSystem) NixSSHOpts() []string { |
| speed up activations at the cost of risking data loss if interrupted. | ||
|
|
||
| *NIX_SSHOPTS* | ||
| Additional ssh options to be passed to _nixos-copy-closure_ on the command line. |
There was a problem hiding this comment.
| Additional ssh options to be passed to _nixos-copy-closure_ on the command line. | |
| Additional ssh options to be passed to _nix-copy-closure_ on the command line. |
Add a setting to specify the path to the known hosts file
I need this since I use
programs.ssh.knownHoststo set up known hosts, which stores them in/etc/ssh/ssh_known_hosts, and extra known hosts files can be set withprograms.ssh.knownHostsFiles. This setting could be improved by making it a list of files that are checked in order, but for now I think it's enough.Allow obtaining private keys via a command instead of the ssh agent
Instead of an ssh agent, I use Bitwarden to store my ssh keys and rbw to access them on the command line. I've used
memfd_createto store the key in a volatile anonymous file for automatic cleanup.Add partial support for the
NIX_SSHOPTSenvironment variableSetting this is necessary to support passing the private keys obtained via
ssh.private_key_cmdto thesshcommand run bynix-copy-closure. So adding support for users to set it directly seems natural. It's not full support since it's only used withnix-copy-closure.