pam_ssh_agent_auth allowing users to define own ssh pubkey

[wmertens]

Security impact: passwordless programmatic elevation of privileges via sudo to sudo-defined level.

Issue description

In the manpage it says that if ~ or %h are used, they enable allow_user_owned_authorized_keys_file.

We are using ~, so that means that if you are using security.pam.enableSSHAgentAuth = true;, any program running as a user can use sudo by changing the ~/.authorized_keys file.

I propose we replace % by /users/%U or somehow disable allow_user_owned_authorized_keys_file.

Steps to reproduce

151 $ ssh-agent bash
140 $ sudo -v
[sudo] password for wmertens:
^C

141 $ ssh-keygen -f test
Generating public/private rsa key pair.
Enter passphrase (empty for no passphrase):
Enter same passphrase again:
Your identification has been saved in test.
Your public key has been saved in test.pub.

142 $ cat test.pub >> .ssh/authorized_keys
143 $ ssh-add test
Identity added: test (test)

144 $ sudo -v
145 $ sudo -l
Matching Defaults entries for wmertens on cap-new:
    env_keep+=TERMINFO_DIRS, env_keep+=TERMINFO, env_keep+=SSH_AUTH_SOCK

User wmertens may run the following commands on cap-new:
    (ALL : ALL) SETENV: ALL

Technical details

• System: 17.03.1840.88604ee33a (Gorilla)

Relevant setting is at

nixpkgs/nixos/modules/security/pam.nix

Line 260 in ea35bc9

"auth sufficient ${pkgs.pam_ssh_agent_auth}/libexec/pam_ssh_agent_auth.so file=~/.ssh/authorized_keys:~/.ssh/authorized_keys2:/etc/ssh/authorized_keys.d/%u"}

/cc @edolstra

[orivej]

Does this work when the user does not belong to the wheel group?

[globin]

cc @grahamc, @fpletz, @domenkozar, @rbvermaa

[wmertens]

It will give you whatever permissions your user can receive via sudo.

[fpletz]

The authorized_keys paths should probably be configurable anyway but the default is pretty bad. I think we should just remove the ~ paths. This breaks backward-compatibility but I'm not sure how to disable allow_user_owned_authorized_keys_file after reading the man page.

This is also probably worth a CVE. Opinions? /cc @grahamc

[wmertens]

Well, it looks like we can keep most backwards-compatibility by changing ~ to /home/%U, but that won't catch non-default homedirs. However, personally I would love to only allow ssh keys from /etc. That's what I was assuming when I enabled it…

…

On Tue, Nov 14, 2017 at 11:28 AM Franz Pletz ***@***.***> wrote: The authorized_keys paths should probably be configurable anyway but the default is pretty bad. I think we should just remove the ~ paths. This breaks backward-compatibility but I'm not sure how to disable allow_user_owned_authorized_keys_file after reading the man page. This is also probably worth a CVE. Opinions? /cc @grahamc <https://github.com/grahamc> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#31611 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWlvA8VOt87TWF5BsLsysqxZZXa4GRks5s2WthgaJpZM4Qb_hN> .

[wmertens]

So uhm - kind of disappointing how this is lingering…

[wmertens]

See #32178 for an example fix. I image we should also put it in the changelog, and it's not something we can just merge into stable…

[wmertens]

@fpletz @globin shall I just merge #32178? How do I mark it as a breaking change? What should we do about our stable branches?

[wmertens]

I just thought of a backwards-compatible way to fix this: make a new setting enablePAMSSHAuth with the secure configuration, and make the enableSSHAgentAuth setting deprecated with a warning on every evaluation. Makes sense? @fpletz @GrahamcOfBorg

[sometimes-i-send-pull-requests]

I maintain a multi-user installation of NixOS where I initially create users with one public key they provide me, and thereafter they are free to add their own keys. In my installation, it is intentional that users are able to add their own keys, and sudo needs to follow along. So, if this behavior is changed, it is important to me that there's a way to change it back to the way it is currently, as in my installation, this is intended behavior. (I even submitted PR #23435 to fix this behavior when it broke once.)

Aside from my own use case, I also have qualms about making this change default in general. The root of my objection stems from the inconsistency it would introduce with how OpenSSH handles keys. As a user, I expect that if I turn on PAM SSH authentication, then it will use the same set of keys that OpenSSH uses for authentication. If we now say that configured keys are inherently more trustworthy than user-specified keys (which is reasonable), but apply that only to sudo and not also login to the system in general, that is inconsistent and looks like a bug (even if it is not really a bug, and was an intentional design decision because of this issue).

I agree that this ought to be configurable because I can certainly see some administrators wanting the ability to turn this off. It should also be configurable independently of how OpenSSH chooses which keys to use. It can even default to allowing only configured keys rather than user-provided keys, as long as OpenSSH uses the same default. But, I do think those two defaults need to match, because anything else looks strange and inconsistent.

[wmertens]

Ok so the difference is that for me, strictly the specified key should be allowed to sudo, and for you, any key that ssh accepts is ok. It should definitely be configurable then, but I still think the default should be strict.

…

On Thu, Feb 22, 2018, 8:32 AM sometimes-i-send-pull-requests, < ***@***.***> wrote: I maintain a multi-user installation of NixOS where I initially create users with one public key they provide me, and thereafter they are free to add their own keys. In my installation, it is intentional that users are able to add their own keys, and sudo needs to follow along. So, if this behavior is changed, it is important to me that there's a way to change it back to the way it is currently, as in my installation, this is intended behavior. (I even submitted PR #23435 <#23435> to fix this behavior when it broke once.) Aside from my own use case, I also have qualms about making this change default in general. The root of my objection stems from the inconsistency it would introduce with how OpenSSH handles keys. As a user, I expect that if I turn on PAM SSH authentication, then it will use the same set of keys that OpenSSH uses for authentication. If we now say that configured keys are inherently more trustworthy than user-specified keys (which is reasonable), but apply that only to sudo and not also login to the system in general, that is inconsistent and *looks* like a bug (even if it is not really a bug, and was an intentional design decision because of this issue). I agree that this ought to be configurable because I can certainly see some administrators wanting the ability to turn this off. It should also be configurable independently of how OpenSSH chooses which keys to use. It can even default to allowing only configured keys rather than user-provided keys, as long as OpenSSH uses the same default. But, I do think those two defaults need to match, because anything else looks strange and inconsistent. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#31611 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADWll6vMGt3kjzORGx9aFLRAPrnX104ks5tXRf6gaJpZM4Qb_hN> .

[sometimes-i-send-pull-requests]

Sure, I can get behind that. As long as it's configurable! I still think there is something to be said for consistency (⇒ locking down OpenSSH by default too then) but that is a lesser concern.

[benaryorg]

The code referred to (in the opening comment):

"auth sufficient ${pkgs.pam_ssh_agent_auth}/libexec/pam_ssh_agent_auth.so file=~/.ssh/authorized_keys:~/.ssh/authorized_keys2:/etc/ssh/authorized_keys.d/%u"} 

Should have an option (boolean) that removes the ~/.ssh/authorized_keys:~/.ssh/authorized_keys2 part.
That way you can configure the authentication to only use the system-supplied keys.

[bricewge]

Two other PAM modules seems to suffer the same kind of vulnerability: security.pam.yubico and security.pam.enableOTPW.

[wmertens]

I just noticed in the logs that pam_ssh_agent_auth refuses to use my ~/.ssh keys because they have the wrong owner, so looks like it's fixed?

@bricewge could you confirm

[Majiir]

I can still reproduce this vulnerability on 23.05.

The current implementation uses config.services.openssh.authorizedKeysFiles which defaults to %h/.ssh/authorized_keys:/etc/ssh/authorized_keys.d/%u.

[wrvsrx]

After #255547 has been merged, now we can use following config as workaround.

  security.sudo.extraConfig = ''
    # Keep SSH_AUTH_SOCK so that pam_ssh_agent_auth.so can do its magic.
    Defaults env_keep+=SSH_AUTH_SOCK
  '';
  security.pam.services =
    let
      serviceCfg = service: {
        rules.auth.ssh-agent-custom = {
          control = "sufficient";
          # order = config.security.pam.services.${service}.rules.auth.u2f.order + 10;
          modulePath = "${pkgs.pam_ssh_agent_auth}/libexec/pam_ssh_agent_auth.so";
          settings.file = "/etc/ssh/authorized_keys.d/%u";
        };
      };
    in
    lib.flip lib.genAttrs serviceCfg [ "sudo" "i3lock" ];

[musjj]

@wrvsrx

Thanks, this looks like a nice solution.

I'm doing this, which seems to work just fine:

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

I guess the downside is that you can no longer specify authorized keys in ~/.ssh, but IMO this seems more secure in general. Any other possible downsides I'm not seeing?

[wrvsrx]

@musjj As far as I know, there isn't any other downsides. However, IMO it's a big downside that a normal user can't determine which ssh key should be used to connect to its account.

[Et7f3]

In the man I see allow_user_owned_authorized_keys_file ? shouldn't we set it ? so we don't hardcode config path but if you want to allow ~/.ssh they need to be owned by root

[musjj]

@Et7f3

It also says this:

allow_user_owned_authorized_keys_file
A flag which enables authorized_keys files to be owned by the invoking user, instead of root. This flag is enabled automatically whenever the expansions %h or ~ are used.

I wonder if it can be manually overridden, even when there's a %h or ~?

[nbraud]

IMO it's a big downside that a normal user can't determine which ssh key should be used to connect to its account.

Sort of? pam_ssh_agent_auth(8) presupposes there is some trusted source-of-truth for the user's SSH keys, which cannot be altered by a (malicious) process running as the user, otherwise the security model does not make sense at all.

However, it's possible to have both self-service authorized_keys management and securely use pam_ssh_agent_auth, with something allowing users to edit their authorized_keys if they prove control of a currently-accepted key.

There's a standardized way to do that, RFC 4819 “Secure Shell Public Key Subsystem,” and at least one FOSS implementation... but I've never used or reviewed ssh-publickeyd, so don't take this for an endorsement.
Sorry, apparently OpenSSH doesn't support privileged subsystems, so it would have to be either a specialized SSH server (say, listening on port 2222 and providing only the authorized_keys management interface) or some privileged helper which pokes the ssh-agent to check the user does control a currently-trusted key (possibly by using pam_ssh_agent_auth or such)

[nbraud]

@RaitoBezarius What was the context where you mentioned needing user-writable authorized_keys that's trusted by the PAM module?

[VAWVAW]

I'm doing this, which seems to work just fine:

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

I just use

security.pam.sshAgentAuth.authorizedKeysFiles = lib.mkForce [ "/etc/ssh/authorized_keys.d/%u" ];

This still allows to login with user writable keys but not to use them for privilege elevation.

[musjj]

Yeah, the option didn't exist when I wrote that. But it looks like that the new option pretty much solves this issue.

All that's left is setting a saner default for authorizedKeysFiles: #277626