Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ssh: use symlinks for authorizedKeys options #976

Merged
merged 2 commits into from
Jul 10, 2024
Merged

ssh: use symlinks for authorizedKeys options #976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b833d4a
ssh: use symlinks for `authorizedKeys` options
emilazy Jul 16, 2023
36a15e8
write-text: remove support for `copy`
emilazy Jul 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
2024-06-15
- SECURITY NOTICE: The previous implementation of the
`users.users.<name>.openssh.authorizedKeys.*` options would not delete
authorized keys files when the setting for a given user was removed.

This means that if you previously stopped managing a user's authorized
SSH keys with nix-darwin, or intended to revoke their access by
removing the option, the previous set of keys could still be used to
log in as that user.

You can check the /etc/ssh/authorized_keys.d directory to see which
keys were permitted; afterwards, please remove the directory and
re-run activation. The options continue to be supported and will now
correctly permit only the keys in your current system configuration.

2022-08-24
- Major changes to `homebrew` module
`homebrew.cleanup` was renamed to `homebrew.onActivation.cleanup`.
Expand Down
8 changes: 0 additions & 8 deletions modules/lib/write-text.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,6 @@ in
'';
};

copy = mkOption {
type = types.bool;
default = false;
description = ''
Whether this file should be copied instead of symlinking.
'';
};

knownSha256Hashes = mkOption {
internal = true;
type = types.listOf types.str;
Expand Down
37 changes: 14 additions & 23 deletions modules/programs/ssh/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
with lib;

let
cfg = config.programs.ssh;
cfg = config.programs.ssh;

knownHosts = map (h: getAttr h cfg.knownHosts) (attrNames cfg.knownHosts);

Expand Down Expand Up @@ -81,8 +81,7 @@ let
};

authKeysFiles = let
mkAuthKeyFile = u: nameValuePair "ssh/authorized_keys.d/${u.name}" {
copy = true;
mkAuthKeyFile = u: nameValuePair "ssh/nix_authorized_keys.d/${u.name}" {
text = ''
${concatStringsSep "\n" u.openssh.authorizedKeys.keys}
${concatMapStrings (f: readFile f + "\n") u.openssh.authorizedKeys.keyFiles}
Expand All @@ -97,28 +96,16 @@ let
in

{
imports = [
(mkRemovedOptionModule [ "services" "openssh" "authorizedKeysFiles" ] "No `nix-darwin` equivalent to this NixOS option.")
];
Comment on lines +99 to +101
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this because SSH keys specified in ~/.ssh/authorized_keys stopped working (see #677), ideally it would be nice if this PR could maintain that, potentially by adding it to the AuthorizedKeysCommand

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that should work out of the box now, since we don’t override AuthorizedKeysFile:

          The  program should produce on standard output zero or more lines
          of authorized_keys output (see AUTHORIZED_KEYS in sshd(8)) .  Au‐
          thorizedKeysCommand is tried after the  usual  AuthorizedKeysFile
          files  and will not be executed if a matching key is found there.
          By default, no AuthorizedKeysCommand is run.


options = {

users.users = mkOption {
type = with types; attrsOf (submodule userOptions);
};

services.openssh.authorizedKeysFiles = mkOption {
type = types.listOf types.str;
default = [];
description = ''
Specify the rules for which files to read on the host.

This is an advanced option. If you're looking to configure user
keys, you can generally use [](#opt-users.users._name_.openssh.authorizedKeys.keys)
or [](#opt-users.users._name_.openssh.authorizedKeys.keyFiles).

These are paths relative to the host root file system or home
directories and they are subject to certain token expansion rules.
See AuthorizedKeysFile in man sshd_config for details.
'';
};

programs.ssh.knownHosts = mkOption {
default = {};
type = types.attrsOf (types.submodule host);
Expand Down Expand Up @@ -148,8 +135,6 @@ in
message = "knownHost ${name} must contain either a publicKey or publicKeyFile";
});

services.openssh.authorizedKeysFiles = [ "%h/.ssh/authorized_keys" "/etc/ssh/authorized_keys.d/%u" ];

environment.etc = authKeysFiles //
{ "ssh/ssh_known_hosts" = mkIf (builtins.length knownHosts > 0) {
text = (flip (concatMapStringsSep "\n") knownHosts
Expand All @@ -159,14 +144,20 @@ in
)) + "\n";
};
"ssh/sshd_config.d/101-authorized-keys.conf" = {
text = "AuthorizedKeysFile ${toString config.services.openssh.authorizedKeysFiles}\n";
text = ''
# sshd doesn't like reading from symbolic links, so we cat
# the file ourselves.
AuthorizedKeysCommand /bin/cat /etc/ssh/nix_authorized_keys.d/%u
# Just a simple cat, fine to use _sshd.
AuthorizedKeysCommandUser _sshd
'';
# Allows us to automatically migrate from using a file to a symlink
knownSha256Hashes = [ oldAuthorizedKeysHash ];
};
};

# Clean up .before-nix-darwin file left over from using knownSha256Hashes
system.activationScripts.etc.text = ''
# Clean up .before-nix-darwin file left over from using knownSha256Hashes
auth_keys_orig=/etc/ssh/sshd_config.d/101-authorized-keys.conf.before-nix-darwin

if [ -e "$auth_keys_orig" ] && [ "$(shasum -a 256 $auth_keys_orig | cut -d ' ' -f 1)" = "${oldAuthorizedKeysHash}" ]; then
Expand Down
23 changes: 23 additions & 0 deletions modules/system/checks.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,28 @@ let
exit 2
fi
'';

# TODO: Remove this a couple years down the line when we can assume
# that anyone who cares about security has upgraded.
oldSshAuthorizedKeysDirectory = ''
if [[ -d /etc/ssh/authorized_keys.d ]]; then
printf >&2 '\e[1;31merror: /etc/ssh/authorized_keys.d exists, aborting activation\e[0m\n'
printf >&2 'SECURITY NOTICE: The previous implementation of the\n'
printf >&2 '`users.users.<name>.openssh.authorizedKeys.*` options would not delete\n'
printf >&2 'authorized keys files when the setting for a given user was removed.\n'
printf >&2 '\n'
printf >&2 "This means that if you previously stopped managing a user's authorized\n"
printf >&2 'SSH keys with nix-darwin, or intended to revoke their access by\n'
printf >&2 'removing the option, the previous set of keys could still be used to\n'
printf >&2 'log in as that user.\n'
printf >&2 '\n'
printf >&2 'You can check the /etc/ssh/authorized_keys.d directory to see which\n'
printf >&2 'keys were permitted; afterwards, please remove the directory and\n'
printf >&2 're-run activation. The options continue to be supported and will now\n'
printf >&2 'correctly permit only the keys in your current system configuration.\n'
exit 2
fi
'';
in

{
Expand Down Expand Up @@ -245,6 +267,7 @@ in
(mkIf cfg.verifyNixChannels nixChannels)
nixInstaller
(mkIf cfg.verifyNixPath nixPath)
oldSshAuthorizedKeysDirectory
];

system.activationScripts.checks.text = ''
Expand Down
19 changes: 5 additions & 14 deletions modules/system/etc.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ let
};

etc = filter (f: f.enable) (attrValues config.environment.etc);
etcCopy = filter (f: f.copy) (attrValues config.environment.etc);

in

Expand All @@ -34,9 +33,10 @@ in
''
mkdir -p $out/etc
cd $out/etc
${concatMapStringsSep "\n" (attr: "mkdir -p $(dirname '${attr.target}')") etc}
${concatMapStringsSep "\n" (attr: "ln -s '${attr.source}' '${attr.target}'") etc}
${concatMapStringsSep "\n" (attr: "touch '${attr.target}'.copy") etcCopy}
${concatMapStringsSep "\n" (attr: ''
mkdir -p "$(dirname ${escapeShellArg attr.target})"
ln -s ${escapeShellArgs [ attr.source attr.target ]}
'') etc}
'';

system.activationScripts.etcChecks.text = ''
Expand All @@ -55,10 +55,6 @@ in
etcStaticFile=/etc/static/$subPath
etcFile=/etc/$subPath
if [[ -e $configFile.copy ]]; then
continue
fi
# We need to check files that exist and aren't already links to
# $etcStaticFile for known hashes.
if [[
Expand Down Expand Up @@ -109,11 +105,6 @@ in
mkdir -p "$etcDir"
fi
if [[ -e $etcStaticFile.copy ]]; then
cp "$etcStaticFile" "$etcFile"
continue
fi
if [[ -e $etcFile ]]; then
if [[ $(readlink -- "$etcFile") == "$etcStaticFile" ]]; then
continue
Expand All @@ -130,7 +121,7 @@ in
# Delete stale links into /etc/static.
if [[
$(readlink "$etcFile") == "$etcStaticFile"
$(readlink -- "$etcFile") == "$etcStaticFile"
&& ! -e $etcStaticFile
]]; then
rm "$etcFile"
Expand Down
8 changes: 4 additions & 4 deletions tests/programs-ssh.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@
echo >&2 "checking for github.com in /etc/ssh/ssh_known_hosts"
grep 'github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==' ${config.out}/etc/ssh/ssh_known_hosts

echo >&2 "checking for authorized keys for foo in /etc/ssh/authorized_keys.d/foo"
grep 'ssh-ed25519 AAAA...' ${config.out}/etc/ssh/authorized_keys.d/foo
echo >&2 "checking for authorized keys' path in /etc/ssh/sshd_config.d/101-authorized-keys.conf"
grep 'AuthorizedKeysFile %h/.ssh/authorized_keys /etc/ssh/authorized_keys.d/%u' ${config.out}/etc/ssh/sshd_config.d/101-authorized-keys.conf
echo >&2 "checking for authorized keys for foo in /etc/ssh/nix_authorized_keys.d/foo"
grep 'ssh-ed25519 AAAA...' ${config.out}/etc/ssh/nix_authorized_keys.d/foo
echo >&2 "checking for authorized keys command in /etc/ssh/sshd_config.d/101-authorized-keys.conf"
grep 'AuthorizedKeysCommand /bin/cat /etc/ssh/nix_authorized_keys.d/%u' ${config.out}/etc/ssh/sshd_config.d/101-authorized-keys.conf
'';
}
Loading