Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: move pam configuration to sudo_local #1020

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
91 changes: 54 additions & 37 deletions modules/security/pam.nix
Original file line number Diff line number Diff line change
Expand Up @@ -7,63 +7,80 @@ let

# Implementation Notes
#
# We don't use `environment.etc` because this would require that the user manually delete
# `/etc/pam.d/sudo` which seems unwise given that applying the nix-darwin configuration requires
# sudo. We also can't use `system.patchs` since it only runs once, and so won't patch in the
# changes again after OS updates (which remove modifications to this file).
#
# As such, we resort to line addition/deletion in place using `sed`. We add a comment to the
# added line that includes the name of the option, to make it easier to identify the line that
# should be deleted when the option is disabled.
mkSudoTouchIdAuthScript = isEnabled:
# Uses `environment.etc` to create the `/etc/pam.d/sudo_local` file that will be used
# to manage all things pam related for nix-darwin. An activation script will run to check
# for the existance of the line `auth include sudo_local`. This is included
# in macOS Sonoma and later. If the line is not there already then `sed` will add it.
# In those cases, the line will include the name of the option root (`security.pam`),
# to make it easier to identify the line that should be deleted when the option is disabled.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add a comment about what happens to the marker line in /etc/pam.d/sudo when upgrading to Sonoma

mkIncludeSudoLocalScript = isEnabled:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mkIncludeSudoLocalScript = isEnabled:

This being a function seems unnecessary

let
file = "/etc/pam.d/sudo";
option = "security.pam.enableSudoTouchIdAuth";
file = "/etc/pam.d/sudo";
option = "security.pam";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
option = "security.pam";
marker = "security.pam.sudo_local";

deprecatedOption = "security.pam.enableSudoTouchIdAuth";
sed = "${pkgs.gnused}/bin/sed";
in ''
${if isEnabled then ''
# Enable sudo Touch ID authentication, if not already enabled
if ! grep 'pam_tid.so' ${file} > /dev/null; then
${sed} -i '2i\
auth sufficient pam_tid.so # nix-darwin: ${option}
' ${file}
# NOTE: this can be removed at some point when support for older versions are dropped
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# NOTE: this can be removed at some point when support for older versions are dropped
# REMOVEME when macOS 13 no longer supported

# Always clear out older implementation if it exists
if grep '${deprecatedOption}' ${file} > /dev/null; then
${sed} -i '/${option}/d' ${file}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
${sed} -i '/${option}/d' ${file}
${sed} -i '/${deprecatedOption}/d' ${file}

fi
# Check if include line is needed (macOS < 14)
if ! grep 'sudo_local' ${file} > /dev/null; then
${sed} -i '2iauth include sudo_local # nix-darwin: ${option}' ${file}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
${sed} -i '2iauth include sudo_local # nix-darwin: ${option}' ${file}
${sed} -i '2iauth include sudo_local # nix-darwin: ${marker}' ${file}

fi
'' else ''
# Disable sudo Touch ID authentication, if added by nix-darwin
# Remove include line if we added it
if grep '${option}' ${file} > /dev/null; then
${sed} -i '/${option}/d' ${file}
fi
''}
'';
in

{
options = {
security.pam.enableSudoTouchIdAuth = mkEnableOption "" // {
description = ''
Enable sudo authentication with Touch ID.

When enabled, this option adds the following line to
{file}`/etc/pam.d/sudo`:

```
auth sufficient pam_tid.so
```

::: {.note}
macOS resets this file when doing a system update. As such, sudo
authentication with Touch ID won't work after a system update
until the nix-darwin configuration is reapplied.
:::
'';
security.pam = {
enableSudoTouchIdAuth = mkEnableOption "" // {
description = ''
Enable sudo authentication with Touch ID.
When enabled, this option adds the following line to {file}:
```
auth sufficient pam_tid.so
```
'';
};
enablePamReattach = mkEnableOption "" // {
description = ''
Enable re-attaching a program to the user's bootstrap session.
This allows programs like tmux and screen that run in the background to
survive across user sessions to work with PAM services that are tied to the
bootstrap session.
When enabled, this option adds the following line to /etc/pam.d/sudo_local:
```
auth optional /path/in/nix/store/lib/pam/pam_reattach.so"
```
'';
};
};
};

config = {
config =
let
isPamEnabled = (cfg.enableSudoTouchIdAuth || cfg.enablePamReattach);
in
{
environment.etc."pam.d/sudo_local" = {
enable = isPamEnabled;
text = lib.strings.concatStringsSep "\n" [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
text = lib.strings.concatStringsSep "\n" [
text = lib.concatLines [

(lib.optionalString cfg.enablePamReattach "auth optional ${pkgs.pam-reattach}/lib/pam/pam_reattach.so")
(lib.optionalString cfg.enableSudoTouchIdAuth "auth sufficient pam_tid.so")
Comment on lines +76 to +77
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(lib.optionalString cfg.enablePamReattach "auth optional ${pkgs.pam-reattach}/lib/pam/pam_reattach.so")
(lib.optionalString cfg.enableSudoTouchIdAuth "auth sufficient pam_tid.so")
(lib.mkIf cfg.enablePamReattach "auth optional ${pkgs.pam-reattach}/lib/pam/pam_reattach.so")
(lib.mkIf cfg.enableSudoTouchIdAuth "auth sufficient pam_tid.so")

I think this will mean the output file won't have blank lines in it

];
};
system.activationScripts.pam.text = ''
# PAM settings
echo >&2 "setting up pam..."
${mkSudoTouchIdAuthScript cfg.enableSudoTouchIdAuth}
${mkIncludeSudoLocalScript isPamEnabled}
'';
};
}
Loading