search

LnL7 / nix-darwin

nix modules for darwin
MIT License
2.82k stars 431 forks source link

fix: move pam configuration to sudo_local #1020

Open dlubawy opened 2 months ago

dlubawy commented 2 months ago

Addresses #985 and #787.

Implementation uses environment.etc to create the /etc/pam.d/sudo_local file and adds the pkgs.pam-reattach option provided in #662. Follows the comment by @emilazy to have nix-darwin manage the file entirely. If the file exists already, nix-darwin should handle it through the usual warning telling the user to rename the file to sudo_local.before-nix-darwin. As identified by @lilyball in their comment, the symlink approach here shouldn't impact users since it doesn't touch the main /etc/pam.d/sudo file. As long as /etc/pam.d/sudo remains a regular file than this should work fine without disrupting sudo to apply nix-darwin configurations.

I recognize this may be a duplicate PR given all the other open issues/PRs, but I haven't seen movement on them in a while. Feel free to close this if it's unnecessary.

emilazy commented 2 months ago

Thanks! This is definitely the direction I want to go, but we need to keep support for macOS < 14. That means that ensuring the auth include sudo_local line is added to /etc/pam.d/sudo if it’s not present (and cleaning up after the old # nix-darwin: security.pam.enableSudoTouchIdAuth method). So we still need to do some amount of file wrangling here, but the upside is that users of Sonoma and higher won’t have to re‐apply their configuration after every update.

dlubawy commented 2 months ago

Thanks! This is definitely the direction I want to go, but we need to keep support for macOS < 14. That means that ensuring the auth include sudo_local line is added to /etc/pam.d/sudo if it’s not present (and cleaning up after the old # nix-darwin: security.pam.enableSudoTouchIdAuth method). So we still need to do some amount of file wrangling here, but the upside is that users of Sonoma and higher won’t have to re‐apply their configuration after every update.

Good call on the support for older versions and older nix-darwin support. What do you think of the recent changes? I added back an activation script like the old one which will check for the old implementation and delete it using sed -i '/security.pam.enableSudoTouchIdAuth/d'. It will then grep to check if sudo_local is already in the /etc/pam.d/sudo file and add this line using sed -i '2i.... if any option in security.pam is set:

auth       include        sudo_local # nix-darwin: security.pam

Otherwise, if all security.pam options are disabled, then it performs sed -i '/security.pam/d' /etc/pam.d/sudo which should also act to disable it for older implementations as well. Then if enabled again they will just have the single line for including sudo_local.

For users on macOS >=14, the entire thing will just be managed through environment.etc as the /etc/pam.d/sudo file will already include sudo_local. Hopefully, there will be a point where we can remove the activation script entirely as older version support is eventually dropped.