Add option to enable sudo authentication with Touch ID

[malob]

There's a one line addition to /etc/pam.d/sudo that allows Macs with Touch ID to use Touch ID to authenticate sudo requests. It's a really nice thing to have. Seem like a nice option to add to nix-darwin.

I wasn't quite sure where to put the option, so definitely feel free to ask me to make changes to put it somewhere that makes more sense.

I choose to use system.patch rather than environment.etc since I didn't want to require the user to have to delete the existing file potentially breaking sudo authentication.

[jrolfs]

Does this work outside of Terminal.app??

[malob]

Yes. I use it with Kitty terminal.

[malob]

@LnL7, just wanted to check in and see if there was anything I could do to help get this merged in.

[ethancedwards8]

I would also love for this to get merged as soon as possible! @LnL7

[emilazy]

Gentle ping. Would love to see this merged so I don’t have to sudo -e /etc/pam.d/sudo every macOS upgrade.

[ivankovnatsky]

this looks like ready and tested, any reason not to merge it?

[ivankovnatsky]

maybe not the best solution, but added pam module to fix touch id under tmux: https://github.com/ivankovnatsky/nixos-config/commit/6b3c7462cb1c386ded995c685c98b66aa65a7716

[rubenfonseca]

Why not merge this?

[dhess]

FYI, you should change the use of sed in your script to /usr/bin/sed. If the user has a sed installed from Nixpkgs, it might not work with the syntax you're using.

[shinzui]

Can this option also handle pam_reattach which is also required to make sudo work in tmux?

[shinzui]

Here is an example config that uses both.

[malob]

@shinzui, I could imagine this module being updated to support this. Though I’d rather focus on getting the initial functionality merged before adding more complexity.

Speaking of which, @LnL7, anything I can do/change to get this merged?

[dsyang]

question from a newbie to nix and nix-darwin: When I add this, it works beautifully. However, if I do a darwin-rebuild --rollback, it appears this doesn't delete the added line from my /etc/pam.d/sudo file. I have to do security.pam.enableSudoTouchIdAuth = false and rebuild before it's removed.

Is this expected behavior?

[ethancedwards8]

Can reproduce. Simply removing the option does not remove the addition, you have to explicitly set it to false. This probably is not expected behavior.

[malob]

@dsyang, @ethancedwards8, huh that is surprising to me, you are right that this is not the expected behavior, though I'm not sure what the cause would be.

I tested this on my system (where I've got a copy of the module setup), and was not able to reproduce. Pretty sure the only difference between the two is that in the copy in my repo I'm setting system.activationScripts.extraActivation.text instead of system.activationScripts.pam.text so that I don't have to maintain an up to date copy of modules/system/activation-scripts.nix.

I don't see why that would change anything though. The security.pam.enableSudoTouchIdAuth option defaults to false so I don't know why not specifying the option as opposed to explicitly setting it to false would make a difference.

Any ideas?

[malob]

(Just rebased on master.)

[malob]

I just tested this module directly by changing the nix-darwin flake in my config to be github:malob/nix-darwin/sudo-touchid and still wasn't able to reproduce.

With the option not specified in my config nix repl shows that the option is indeed present and set to false,

nix-repl> darwinConfigurations.MaloBookProM1.config.security.pam.enableSudoTouchIdAuth
false

the activation script is set with the right content,

nix-repl> darwinConfigurations.MaloBookProM1.config.system.activationScripts.pam.text
"# PAM settings\necho >&2 \"setting up pam...\"\n# Disable sudo Touch ID authentication, if added by nix-darwin\nif grep 'security.pam.enableSudoTouchIdAuth' /etc/pam.d/sudo > /dev/null; then\n  sed -i \"\" '/security.pam.enableSudoTouchIdAuth/d' /etc/pam.d/sudo\nfi\n\n\n"

and after running a rebuild and switch, the relevant line from /etc/pam.d/sudo was removed:

❯ cat /etc/pam.d/sudo
# 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

[dsyang]

Hmm interesting. Could this be related to flakes? My setup did not use nix-flakes.

[CorbanR]

@malob @dsyang I ran into the same issue where setting security.pam.enableSudoTouchIdAuth = false didn't remove the added text. However, it was only an issue for me when sed -i "" '/${option}/d' ${file} vs /usr/bin/sed -i "" '/${option}/d' ${file}. As @dhess mentions above, there is a possible issue if the user has gnused installed via nix. The script will use the nix version of sed, which has different syntax than the one that comes installed at /usr/bin/sed

[malob]

@CorbanR (cc: @dsyang, @dhess), gotcha. I've updated the the module to use /usr/bin/sed explicitly.

[pecigonzalo]

@malob how about instead referencing sed from nixpkgs? That way you dont depend on the version being shipped by the OS.

[malob]

@pecigonzalo, good point. I was being a little lazy since I knew the syntax wasn't exactly the same, but using GNU sed from nixpkgs does seem like the better move.

I've updated the code in the PR to use GNU sed now and tested on my system.

[pecigonzalo]

Thank you for implementing this, really looking forward to it.

On Thu, Feb 17, 2022, 19:26 Malo Bourgon @.***> wrote:

@pecigonzalo https://github.com/pecigonzalo, good point. I was being a little lazy since I knew the syntax wasn't exactly the same, but using GNU sed from nixpkgs does seem like the better move.

I've updated code in the PR to use GNU sed now and tested on my system.

— Reply to this email directly, view it on GitHub https://github.com/LnL7/nix-darwin/pull/228#issuecomment-1043273146, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACLS7CJ6OYHM6VILWINPEQDU3U4UDANCNFSM4RIF47RQ . You are receiving this because you were mentioned.Message ID: @.***>

[dsyang]

@malob I'm setting up a new computer today and idk what has changed but I'm not hitting the same bug anymore!

$> darwin-rebuild --list-generations  
   1   2022-03-09 10:38:35   
   2   2022-03-09 10:41:10   
   3   2022-03-09 10:45:53   
   4   2022-03-09 10:55:34   
   5   2022-03-09 11:00:02   (current)

$> git diff . 
diff --git a/darwin-configuration.nix b/darwin-configuration.nix
index ea78c2d..d1fab3c 100644
--- a/darwin-configuration.nix
+++ b/darwin-configuration.nix
@@ -88,7 +88,7 @@
   };
 
   # add touchid support to sudo
-  security.pam.enableSudoTouchIdAuth = true;
+  ### security.pam.enableSudoTouchIdAuth = true;
 
   # List packages installed in system profile. To search by name, run:
   # $ nix-env -qaP | grep wget


$> darwin-rebuild switch
building the system configuration...
trace: warning: literalExample is deprecated, use literalExpression instead, or use literalDocBook for a non-Nix description.
this derivation will be built:
  /nix/store/2rk0gyshrszriyn7dqn321p1g20sw7zk-darwin-system-22.05pre358993.3e644bd6248+darwin4.0000000.drv
building '/nix/store/2rk0gyshrszriyn7dqn321p1g20sw7zk-darwin-system-22.05pre358993.3e644bd6248+darwin4.0000000.drv'...
user defaults...
setting up user launchd services...
setting up pam...
setting up ~/Applications...
applying patches...
setting up /etc...
...

$> cat /etc/pam.d/sudo
# 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

$> darwin-rebuild --list-generations
   1   2022-03-09 10:38:35   
   2   2022-03-09 10:41:10   
   3   2022-03-09 10:45:53   
   4   2022-03-09 10:55:34   
   5   2022-03-09 11:00:02   
   6   2022-03-09 11:05:37   (current)

$> darwin-rebuild -G 5
switching profile from version 6 to 5
user defaults...
setting up user launchd services...
setting up pam...
setting up ~/Applications...
applying patches...
setting up /etc...
...

$> cat /etc/pam.d/sudo
# sudo: auth account password session
auth       sufficient     pam_tid.so # nix-darwin: security.pam.enableSudoTouchIdAuth
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

My setup can be found at https://github.com/dsyang/nixfiles. The patch there doesn't have the new gnused changes though. Will try updating my patch next.

[dsyang]

confirmed patching in the gnused changes still work. I'm not sure what I ran into but it's gone...

[CorbanR]

Tested and also works great for me!

[malob]

@domenkozar, think there's a chance we could get this merged? Any changes you'd like to see before doing so?

[lockejan]

What about devices without touchId? 🤔 There's no mechanism to prevent the user's from shooting themself in the foot. Maybe that's missing. Invalid modules would make Sudo completely unusable and force the user to boot into recovery mode (looking more at third party modules such as pam-reattach for tmux integration).😬

[ivankovnatsky]

What about devices without touchId? thinking There's no mechanism to prevent the user's from shooting themself in the foot. Maybe that's missing. Invalid modules would make Sudo completely unusable and force the user to boot into recovery mode.grimacing

Yes, I've stumbled on something similar when configuring a new mac (but I have a bit more complex config), so you can always enable super user and edit the sudoers file as desired to fix this.

[lockejan]

It's probably to unsafe in it's current state. See also https://github.com/LnL7/nix-darwin/issues/358#issuecomment-922476324