RFC: Add pam_ssh_agent_auth.so PAM module for ssh-agent authentication.

[tobhe]

There is a widely used PAM authentication module called pam_ssh_agent_auth that is used to provide sudo access via SSH keys managed by an SSH agent.

The module is packaged in most Linux distributions such as Debian, Ubuntu and many other systems. It is based on old OpenSSH code and the way it was initially built makes it hard to keep it up to date with the OpenSSH master branch. This resulted in the code bases diverging for newer features like ed25519 keys, while others like FIDO keys are not supported at all.

I was wondering if it would make sense to provide a similar functionality via the official openssh-portable distribution since there certainly is demand and having it in openssh would make it a lot easier to keep it up to date and always have the latest security updates.

This PR contains a rewrite of the original module that supports the basic functionality. It reuses as much ssh code as possible, which means that it supports most features from ssh out of the box. The build system changes are a little hacky and leave a lot of room for improvement as I tried to not change to much in the existing code.

The authentication mechanism is pretty simple:

1. The connected SSH agent is queried for a list of supported keys
2. Each key is compared to an authorized_keys file in /etc/security/authorized_keys
3. If a key matches, a random 32 byte challenge is generated and passed to the ssh agent to sign.
4. If the signature passes verification, access is granted.

[djmdjm]

Thanks - I hadn't realised that the pam_ssh_agent_auth.so module had bitrotted so badly.

As far as including this in the portable OpenSSH distribution, we could host it under contrib/ but it would need its own Makefile and probably a manpage (or other documentation). The Makefile could be generated from a Makefile.in from configure, but I don't think we want to build this by default or from the top-level Makefile.

To keep it updated, IMO the best model would be to follow what we do for contrib/ssh-copy-id.* - we pull these from a separate git repository. This takes the OpenSSH developers out of the critical path for progress on these files and gives users a standalone repository they can file bugs/PRs against, as well as a way to get an updated version between OpenSSH releases.

Would this work? Practically we could update to the latest tagged stable release when upon request or when about to make an OpenSSH release.

A couple of notes on the code:

• randombytes() is unnecessary indirection and should be replaced by arc4random_buf()
• instead of signing a purely random challenge, it would be better to sign something that is downscoped, e.g. by prefixing the signed data with a fixed string such as "pam_ssh_agent_auth"
• I note that it duplicated portions of auth2-pubkey.c's functionality. Maybe we should refactor this file a bit so the code there could be used directly?
• It looks like it uses an authorized_keys-style list of keys, but doesn't support all of the authorized_keys options. I think it should fail safely when options have been specified.
• It would be nice to support cerificate keys, but that would probably require the refactoring mentioned above.

[daztucker]

Here's an example of how it could work with minimal changes outside of its contrib directory: https://github.com/daztucker/openssh-pam-agent-auth/commit/6f5323749b1770c3dfb3ed78711765ba621bd39b

the Makefile is not ideal (the use of VPATH means the top-level .c files need to use $< so the config.h and libopenbsd-compat dependencies are missing but otherwise it seems to work)

[daztucker]

djm points out that we can simplify the contrib Makefile a lot by using libssh.a, but the caveat is that we would then need to build all of the object files in it with -fPIC:

https://github.com/daztucker/openssh-pam-agent-auth/commit/f6294d1d2744da72a43b3bc7fe3ff4fea346f701

[tobhe]

Thanks for your comments!

To keep it updated, IMO the best model would be to follow what we do for contrib/ssh-copy-id.* - we pull these from a separate git repository. This takes the OpenSSH developers out of the critical path for progress on these files and gives users a standalone repository they can file bugs/PRs against, as well as a way to get an updated version between OpenSSH releases.

Would this work? Practically we could update to the latest tagged stable release when upon request or when about to make an OpenSSH release.

Absolutely, this would be a lot better than the current situation!

The newest version contains a working Makefile to build in contrib/ based on the one proposed by @daztucker. I had to reshuffle the link ordering a little and add two more objects to produce a working .so.

• randombytes() is unnecessary indirection and should be replaced by arc4random_buf()
• instead of signing a purely random challenge, it would be better to sign something that is downscoped, e.g. by prefixing the signed data with a fixed string such as "pam_ssh_agent_auth"

Those should be fixed in the new version.

• I note that it duplicated portions of auth2-pubkey.c's functionality. Maybe we should refactor this file a bit so the code there could be used directly?
• It looks like it uses an authorized_keys-style list of keys, but doesn't support all of the authorized_keys options. I think it should fail safely when options have been specified.
• It would be nice to support cerificate keys, but that would probably require the refactoring mentioned above.

I haven't had time to dig into those yet, but I agree that it would be nice to reduce the module specific code even further. The less maintenance work in the module the better. Even more so if we get certs working at the same time.

[tobhe]

The newest version removes some more duplicate code and adds support for the missing options/certs by using check_authkeys_file() from auth2-pubkey.c directly. I am a little uncertain about the struct ssh * I'm passing since it doesn't really represent a valid state but all callers are checking the members they're accessing against NULL.

[djmdjm]

Let me take a look at the refactoring - I think I can do something that gets rid of struct ssh as well as ServerOptions

[tobhe]

Thank you! Rebased on the latest refactoring changes and dropped the now redundant struct ssh and ServerOptions. I also added one more refactoring commit to move the auth_open*file() functions to auth2-pubkeyfile.c, which allowed me to get rid of pam_openfile().