nix-darwin icon indicating copy to clipboard operation
nix-darwin copied to clipboard

pam: add touchIdAuth to sudo_local

Open hgl opened this issue 10 months ago • 4 comments

Apple sanctioned sudo_local in Sonoma, which makes it possible for sudo related pam settings to survive system upgrade. We replicate this design for earlier macOS versions to ensure backward compatibility.

Changes:

  • Rename option to security.pam.touchIdAuth
  • Add pam_reattach.so support
  • Use sudo_local as the entrypoint
  • Include different files for different features

Related PRs:

#784 directly addressed. #591 no longer necessary. The current design is influenced by @malob and @emilazy's suggestion that /etc/pam.d/* should be regular files instead of symlinks. #662 no longer necessary, and it demonstrates using patches is probably not a good idea, since they are not very composable.

Design choices:

  • reattach is nested in touchIdAuth since it doesn't really make sense to use the former without the latter, if I'm not wrong.
  • Make reattach.enable and reattach.ignoreSSH default to true if touchIdAuth is enabled. Since users should generally want them enabled, and there is no harm in enabling them. Lets me know if that's not the case.

This PR currently serves as an overture to design discussion, thus old option migration is currently omitted.

Care has been taken to ensure script error doesn't result in a broken sudo. Let me know if you find otherwise.

hgl avatar Sep 29 '23 12:09 hgl

The current design is influenced by @malob and @emilazy's suggestion that /etc/pam.d/* should be regular files instead of symlinks.

/etc/pam.d/sudo certainly should be, but /etc/pam.d/sudo_local can be a symlink. PAM clearly works when the target of an include directive doesn't exist at all, so I just tested by having sudo_local be a symlink to a path that doesn't exist and that worked. I also tried having it be a symlink to a path that exists but isn't readable by anyone, that also worked. So it should be safe enough to have /etc/pam.d/sudo_local be a symlink into the store as long as /etc/pam.d/sudo itself is still a regular file.

lilyball avatar Oct 02 '23 04:10 lilyball

It's possible that sudo_local already exists before it's deployed. Current awk approach to remove tid & reattach entries should be good enough. But I think it's a good idea to turn /etc/pam.d/nix-darwin-touchIdAuth into a symlink.

hgl avatar Oct 02 '23 04:10 hgl

@hgl any update? thanks

mvillafuertem avatar May 23 '24 14:05 mvillafuertem

Do we really need to support an already‐existing sudo_local here? If we could just generate a file from scratch it seems like we’d save a lot of logic and get a simpler and more robust result. It seems like if you’re managing sudo PAM configuration with nix-darwin it’s okay to expect you to go all‐in, unless there’s existing software that creates the file that we’d really want to be compatible with.

emilazy avatar Jun 13 '24 10:06 emilazy