Merge pull request #277620 from nbraud/nixos/pam/ssh-agent-auth-31611

nixos/pam: Add option for ssh-agent auth's trusted authorized_keys files

nixos/doc/manual/release-notes/rl-2405.section.md

@@ -146,6 +146,10 @@ The pre-existing [services.ankisyncd](#opt-services.ankisyncd.enable) has been m
 
 - The source of the `mockgen` package has changed to the [go.uber.org/mock](https://github.com/uber-go/mock) fork because [the original repository is no longer maintained](https://github.com/golang/mock#gomock).
 
+- `security.pam.enableSSHAgentAuth` was renamed to `security.pam.sshAgentAuth.enable` and an `authorizedKeysFiles`
+  option was added, to control which `authorized_keys` files are trusted.  It defaults to the previous behaviour,
+  **which is insecure**: see [#31611](https://github.com/NixOS/nixpkgs/issues/31611).
+
 - [](#opt-boot.kernel.sysctl._net.core.wmem_max_) changed from a string to an integer because of the addition of a custom merge option (taking the highest value defined to avoid conflicts between 2 services trying to set that value), just as [](#opt-boot.kernel.sysctl._net.core.rmem_max_) since 22.11.
 
 - `services.zfs.zed.enableMail` now uses the global `sendmail` wrapper defined by an email module

nixos/modules/security/pam.nix

@@ -654,8 +654,8 @@ let
           { name = "mysql"; enable = cfg.mysqlAuth; control = "sufficient"; modulePath = "${pkgs.pam_mysql}/lib/security/pam_mysql.so"; settings = {
             config_file = "/etc/security/pam_mysql.conf";
           }; }
-          { name = "ssh_agent_auth"; enable = config.security.pam.enableSSHAgentAuth && cfg.sshAgentAuth; control = "sufficient"; modulePath = "${pkgs.pam_ssh_agent_auth}/libexec/pam_ssh_agent_auth.so"; settings = {
+          { name = "ssh_agent_auth"; enable = config.security.pam.sshAgentAuth.enable && cfg.sshAgentAuth; control = "sufficient"; modulePath = "${pkgs.pam_ssh_agent_auth}/libexec/pam_ssh_agent_auth.so"; settings = {
-            file = lib.concatStringsSep ":" config.services.openssh.authorizedKeysFiles;
+            file = lib.concatStringsSep ":" config.security.pam.sshAgentAuth.authorizedKeysFiles;
           }; }
           (let p11 = config.security.pam.p11; in { name = "p11"; enable = cfg.p11Auth; control = p11.control; modulePath = "${pkgs.pam_p11}/lib/security/pam_p11.so"; args = [
             "${pkgs.opensc}/lib/opensc-pkcs11.so"
@@ -943,7 +943,7 @@ let
       value.source = pkgs.writeText "${name}.pam" service.text;
     };
 
-  optionalSudoConfigForSSHAgentAuth = optionalString config.security.pam.enableSSHAgentAuth ''
+  optionalSudoConfigForSSHAgentAuth = optionalString config.security.pam.sshAgentAuth.enable ''
     # Keep SSH_AUTH_SOCK so that pam_ssh_agent_auth.so can do its magic.
     Defaults env_keep+=SSH_AUTH_SOCK
   '';
@@ -956,6 +956,7 @@ in
 
   imports = [
     (mkRenamedOptionModule [ "security" "pam" "enableU2F" ] [ "security" "pam" "u2f" "enable" ])
+    (mkRenamedOptionModule [ "security" "pam" "enableSSHAgentAuth" ] [ "security" "pam" "sshAgentAuth" "enable" ])
   ];
 
   ###### interface
@@ -1025,16 +1026,34 @@ in
       '';
     };
 
-    security.pam.enableSSHAgentAuth = mkOption {
+    security.pam.sshAgentAuth = {
-      type = types.bool;
+      enable = mkEnableOption ''
-      default = false;
+        authenticating using a signature performed by the ssh-agent.
-      description =
+        This allows using SSH keys exclusively, instead of passwords, for instance on remote machines
-        lib.mdDoc ''
+      '';
-          Enable sudo logins if the user's SSH agent provides a key
+
-          present in {file}`~/.ssh/authorized_keys`.
+      authorizedKeysFiles = mkOption {
-          This allows machines to exclusively use SSH keys instead of
+        type = with types; listOf str;
-          passwords.
+        description = ''
+          A list of paths to files in OpenSSH's `authorized_keys` format, containing
+          the keys that will be trusted by the `pam_ssh_agent_auth` module.
+
+          The following patterns are expanded when interpreting the path:
+          - `%f` and `%H` respectively expand to the fully-qualified and short hostname ;
+          - `%u` expands to the username ;
+          - `~` or `%h` expands to the user's home directory.
+
+          ::: {.note}
+          Specifying user-writeable files here result in an insecure configuration:  a malicious process
+          can then edit such an authorized_keys file and bypass the ssh-agent-based authentication.
+
+          See [issue #31611](https://github.com/NixOS/nixpkgs/issues/31611)
+          :::
         '';
+        example = [ "/etc/ssh/authorized_keys.d/%u" ];
+        default = config.services.openssh.authorizedKeysFiles;
+        defaultText = literalExpression "config.services.openssh.authorizedKeysFiles";
+      };
     };
 
     security.pam.enableOTPW = mkEnableOption (lib.mdDoc "the OTPW (one-time password) PAM module");
@@ -1465,6 +1484,16 @@ in
       }
     ];
 
+    warnings = optional
+      (with lib; with config.security.pam.sshAgentAuth;
+        enable && any (s: hasPrefix "%h" s || hasPrefix "~" s) authorizedKeysFiles)
+      ''config.security.pam.sshAgentAuth.authorizedKeysFiles contains files in the user's home directory.
+
+        Specifying user-writeable files there result in an insecure configuration:
+        a malicious process can then edit such an authorized_keys file and bypass the ssh-agent-based authentication.
+        See https://github.com/NixOS/nixpkgs/issues/31611
+      '';
+
     environment.systemPackages =
       # Include the PAM modules in the system path mostly for the manpages.
       [ pkgs.pam ]

nixos/modules/security/sudo.nix

@@ -6,8 +6,6 @@ let
 
   cfg = config.security.sudo;
 
-  inherit (config.security.pam) enableSSHAgentAuth;
-
   toUserString = user: if (isInt user) then "#${toString user}" else "${user}";
   toGroupString = group: if (isInt group) then "%#${toString group}" else "%${group}";
 

nixos/tests/ssh-agent-auth.nix

@@ -15,7 +15,11 @@ import ./make-test-python.nix ({ lib, pkgs, ... }:
         foo.isNormalUser = true;
       };
 
-      security.pam.enableSSHAgentAuth = true;
+      security.pam.sshAgentAuth = {
+        # Must be specified, as nixpkgs CI expects everything to eval without warning
+        authorizedKeysFiles = [ "/etc/ssh/authorized_keys.d/%u" ];
+        enable = true;
+      };
       security.${lib.replaceStrings [ "_" ] [ "-" ] n} = {
         enable = true;
         wheelNeedsPassword = true;  # We are checking `pam_ssh_agent_auth(8)` works for a sudoer