From 0e83d6737370e1cfeb11bf3c42a428e7279bbaf8 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 3 Jul 2024 21:22:25 +0200 Subject: [PATCH 1/4] Revert "nixos/gnome-keyring: enable gnome-keyring for fingerprint authentication'" `gdm-fingerprint` PAM module is defined using the `text` option, so the option here is a no-op. This reverts commit 6bb516d45f2cbb56a817adf4c7f0ee680e3cf9e9. --- nixos/modules/services/desktops/gnome/gnome-keyring.nix | 3 --- 1 file changed, 3 deletions(-) diff --git a/nixos/modules/services/desktops/gnome/gnome-keyring.nix b/nixos/modules/services/desktops/gnome/gnome-keyring.nix index 96089d718c17..5f0542769ca6 100644 --- a/nixos/modules/services/desktops/gnome/gnome-keyring.nix +++ b/nixos/modules/services/desktops/gnome/gnome-keyring.nix @@ -43,9 +43,6 @@ in gdm-password.enableGnomeKeyring = true; gdm-autologin.enableGnomeKeyring = true; }) - (lib.mkIf (config.services.xserver.displayManager.gdm.enable && config.services.fprintd.enable) { - gdm-fingerprint.enableGnomeKeyring = true; - }) ]; security.wrappers.gnome-keyring-daemon = { From 68211b354677d26e0e90b0a0cf7b62bd863c3a54 Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 3 Jul 2024 21:22:27 +0200 Subject: [PATCH 2/4] Revert "nixos/gnome-keyring: unlock keyring with gdm-password, gdm-autologin" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `gdm-autologin` and `gdm-password` PAM modules are defined using the `text` option, so the option here is a no-op. Furthermore, `gdm-password` already includes `login` for all module types, and that invokes `pam_gnome_keyring.so` in the same way Arch’s `gdm-password` module would: https://gitlab.gnome.org/GNOME/gdm/-/blob/81ee658c11381912131dd4a29e84190f7f9cd039/data/pam-arch/gdm-password.pam This reverts commit c24c7933ba2e5266d91978f1eaefdd81c760af67. --- .../modules/services/desktops/gnome/gnome-keyring.nix | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/nixos/modules/services/desktops/gnome/gnome-keyring.nix b/nixos/modules/services/desktops/gnome/gnome-keyring.nix index 5f0542769ca6..550c6ba8eff5 100644 --- a/nixos/modules/services/desktops/gnome/gnome-keyring.nix +++ b/nixos/modules/services/desktops/gnome/gnome-keyring.nix @@ -35,15 +35,7 @@ in xdg.portal.extraPortals = [ pkgs.gnome-keyring ]; - security.pam.services = lib.mkMerge [ - { - login.enableGnomeKeyring = true; - } - (lib.mkIf config.services.xserver.displayManager.gdm.enable { - gdm-password.enableGnomeKeyring = true; - gdm-autologin.enableGnomeKeyring = true; - }) - ]; + security.pam.services.login.enableGnomeKeyring = true; security.wrappers.gnome-keyring-daemon = { owner = "root"; From 1cf415549842a076d3ec578c766c6caa5263262b Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 3 Jul 2024 23:26:31 +0200 Subject: [PATCH 3/4] nixos/gdm: Clean up gdm-fingerprint pam module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `optional pam_permit.so` comes from the [upstream Arch config] we used as a template in 9d41fe6fcc4df838a56b1cfb2512b65e5e655958. But I do not think it does anything in this position – see also the discussion at https://bbs.archlinux.org/viewtopic.php?id=245892 – so let’s just remove it. Let’s also add a comment about disabling `fprintAuth` and a blank line for clarity. [upstream Arch config]: https://gitlab.gnome.org/GNOME/gdm/-/blob/81ee658c11381912131dd4a29e84190f7f9cd039/data/pam-arch/gdm-fingerprint.pam --- nixos/modules/services/x11/display-managers/gdm.nix | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nixos/modules/services/x11/display-managers/gdm.nix b/nixos/modules/services/x11/display-managers/gdm.nix index 82cc80417fa1..d4f0de2c3bfa 100644 --- a/nixos/modules/services/x11/display-managers/gdm.nix +++ b/nixos/modules/services/x11/display-managers/gdm.nix @@ -321,13 +321,15 @@ in session include login ''; + # This would block password prompt when included by gdm-password. + # GDM will instead run gdm-fingerprint in parallel. login.fprintAuth = mkIf config.services.fprintd.enable false; + gdm-fingerprint.text = mkIf config.services.fprintd.enable '' auth required pam_shells.so auth requisite pam_nologin.so auth requisite pam_faillock.so preauth auth required ${pkgs.fprintd}/lib/security/pam_fprintd.so - auth optional pam_permit.so auth required pam_env.so auth [success=ok default=1] ${pkgs.gnome.gdm}/lib/security/pam_gdm.so From af0cdb44a0016f8b7572483a39f5a80fb518721d Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Wed, 3 Jul 2024 15:56:12 +0200 Subject: [PATCH 4/4] nixos/gdm: Fix fingerprint auth rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We introduced the gdm-fingerprint.pam in 9d41fe6fcc4df838a56b1cfb2512b65e5e655958. We used the [upstream Arch config] as a template, which contains an extended control field that jumps over **one** immediately-following `auth` rule unless `pam_gdm.so` succeeds. But we decided to not include `pam_gnome_keyring.so` so there was no rule to skip over, resulting in a broken control flow and the PAM module failing with “PAM bad jump in stack”, breaking the fingerprint authentication in GDM. Let’s actually add `pam_gnome_keyring.so`, like the Arch config does. Because we are creating the PAM file using the `text` option, `security.pam.services.gdm-fingerprint.enableGnomeKeyring` does not do anything so we need to do it manually. For the case where gnome-keyring is not enabled, we could add a no-op rule like `optional pam_permit.so` after `pam_gdm.so` so that the branching always has something to jump over but it will be simpler to just make the both conditional. There are no further `auth` rules that could benefit from `pam_gdm.so` doing something so it should be fine. Unlike in Arch, we are not going to invoke `pam_gnome_keyring.so` in a `session` rule since that is already done by the included `login` module. [upstream Arch config]: https://gitlab.gnome.org/GNOME/gdm/-/blob/81ee658c11381912131dd4a29e84190f7f9cd039/data/pam-arch/gdm-fingerprint.pam --- nixos/modules/services/x11/display-managers/gdm.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nixos/modules/services/x11/display-managers/gdm.nix b/nixos/modules/services/x11/display-managers/gdm.nix index d4f0de2c3bfa..1a39b365db5f 100644 --- a/nixos/modules/services/x11/display-managers/gdm.nix +++ b/nixos/modules/services/x11/display-managers/gdm.nix @@ -6,6 +6,7 @@ let cfg = config.services.xserver.displayManager; gdm = pkgs.gnome.gdm; + pamCfg = config.security.pam.services; settingsFormat = pkgs.formats.ini { }; configFile = settingsFormat.generate "custom.conf" cfg.gdm.settings; @@ -331,7 +332,10 @@ in auth requisite pam_faillock.so preauth auth required ${pkgs.fprintd}/lib/security/pam_fprintd.so auth required pam_env.so - auth [success=ok default=1] ${pkgs.gnome.gdm}/lib/security/pam_gdm.so + ${lib.optionalString pamCfg.login.enableGnomeKeyring '' + auth [success=ok default=1] ${pkgs.gnome.gdm}/lib/security/pam_gdm.so + auth optional ${pkgs.gnome-keyring}/lib/security/pam_gnome_keyring.so + ''} account include login