malob / nixpkgs

My Nix system configs!
MIT License
403 stars 34 forks source link

Module to Require Password as Well as Touch ID for Sudo #7

Open heywoodlh opened 3 years ago

heywoodlh commented 3 years ago

First off, thanks for sharing your great nix-darwin setup. Your contributions to nix-darwin, especially the Homebrew module, have been awesome for me! So thanks.

So I love the idea of using Nix-darwin to manage my Touch ID PAM setup. One suggestion I would love implemented would be to add a module to require a password as well Touch ID -- not just Touch ID. That way, sudo auth would be multi factor.

I could see how not everyone would want this, so I could understand maybe making a separate module for this instead of making it the default behavior of security.pam.enableSudoTouchIdAuth.

I am pretty much going to bed right now but I will give it a bit more thought on how I would implement it. I may try to submit a pull request except I suck at Nix's syntax so I thought I'd write up an issue before I tried anything (I am definitely passively hoping you get around to implementing this before I even try anything ;).

malob commented 3 years ago

🙂

Sounds like a cool idea. I suspect the best course of action here would be to add more options to the security.pam module that implement features like the one you suggested (including potentially changing the existing enableSudoTouchIdAuth option so that it plays well with any new options).

I actually don't know very much about how PAM works though, so that's the biggest blocker to me spending time working on this. If you can tell me what changes need to be made to /etc/pam.d/sudo to implement using TouchID as a second factor, along with some explanation of how those changes accomplish that, I'd be happy to try extending the module to implement that feature.

heywoodlh commented 3 years ago

This is what /etc/pam.d/sudo would look like for that:

# sudo: auth account password session
auth       sufficient     pam_smartcard.so
auth       required       pam_opendirectory.so
auth   required  pam_tid.so
account    required       pam_permit.so
password   required       pam_deny.so
session    required       pam_permit.so

The important snippet:

...
auth   required  pam_tid.so
...

The way I understand it is that PAM goes through each line making sure each module that is set to required is satisfied unless a sufficient module is met.

BIG DRAWBACK:

One huge drawback I just now discovered while writing this up is that it seems that pam_tid.so is completely unavailable when you close your Macbook lid (i.e. connected to an external monitor and keyboard). This means that with this configuration you can't use sudo unless your Macbook lid is open. With this behavior I could understand not wanting to implement this -- for some reason, I had thought that pam_tid.so could fail over to using the Apple Watch if the Touch ID sensor was unavailable (the Apple Watch is usable with pam_tid.so if the Macbook lid is open).

This isn't an issue in your original configuration because you set pam_tid.so to sufficient so when the Macbook is closed then it just skips that module and goes to the password module.

To address this drawback, a solution I have found is installing a third party PAM module: https://github.com/biscuitehh/pam-watchid

And then configuring /etc/pam.d/sudo like this:

# sudo: auth account password session
auth       sufficient     pam_smartcard.so
auth       required       pam_opendirectory.so
account    required       pam_permit.so
password   required       pam_deny.so
session    required       pam_permit.so
auth       sufficient     pam_tid.so
auth       sufficient     pam_watchid.so

This would require a password and then if pam_tid.so is unavailable it will failover to using pam_watchid.so.

This drawback and workaround make things a bit more complicated than I initially realized so I completely understand if this is something you'd rather not implement altogether. At this point, I may just write this all myself because I doubt many users will care to have something like this.

Hopefully I conveyed all of this in a way that makes sense. :)

malob commented 3 years ago

Thanks for the details. Given the complexities here, is seem to me like the best path forward would be to take inspiration from NixOS's security.pam module, to provide a move general way of specifying configuration for these files. I'll probably give that a go next time I'm working on this.