nixos/pam: rework of the entire module #105319

rissson · 2020-11-29T05:05:51Z

Hi all!

First, I know this PR looks scary, because it is. But I couldn't see another way of making this module better without starting from scratch.

Alternative names for this PR:

make the PAM module great again

ohno, I won't review this much

ah finally, but boy, is this big

Here is my solution to the current PAM problem in NixOS. The problem is described extensively in #90640 and all related issues and PRs. It basically boils down to the PAM module not being managed by module system, and thus integrates badly with other modules, resulting in PAM services being hardcoded by other modules (yes, I am looking at you display managers 👀, but you had no other choice :/).

Origin, requirements and blockers

This PR builds on top of #105098, which is a step forward, but still has some flaws regarding ordering and the ability to easily override something as a user. See my comments over there for what I think needs improving, and is basically the idea that started my work on this.

We also need #97023 to be able to use mkRenamedOptionModule inside submodules. This isn't a blocker for review, see below.

Design

I'll start with how the PAM module can be interacted with. It is now divided into top level options:

security.pam = {
  modules = {
    u2f = {
      enable = true;  
      cue = true;
    };
  };
  services = {
    login = {
      modules = {
        u2f = { ... };
      };
      
      auth = {
        u2f = {
          control = "sufficient";
          path = "pam_u2f.so";
          args = [ "cue" ];
        };
      };
    };
  };
};

Under the hood, each module does... nothing! Actually, the toplevel modules configuration option only exists to provide default options to the modules defined in each service. And thus, the modules in the services do the work, meaning they will add entries to the relevant PAM types (account, auth, password, session) depending on the options that are set in it. To see how this is rigged, have a look inside nixos/modules/security/pam/modules. I guess the best explanation of how this all works at this point is with examples.

Interacting with the PAM module

Creating a new service with the defaults

security.pam.services.newService = {};

A service overriding something in the modules

security.pam.services.newService = {
  modules = {
    rootOK = true;
  };
};

Changing a module option for all services

security.pam.modules.u2f.enable = true;

Changing a module option for one service

security.pam.services.sshd.modules.u2f.enable = false;

Creating a custom service, using none of the defaults

This will be especially useful for display managers.

security.pam.services.myService = {
  auth = {
    myEntry = {
      control = "sufficient";
      path = "my_entry.so";
      order = 100;
    };
    mySecondEntry = {
      control = "required";
      path = "my_second_entry.so";
      order = 200;
    };
  };
  # The following is required to explicitly remove any entries that are included by default, i.e. by the PAM modules.
  excludeDefaults = [ "account" "auth" "password" "session" ];
  # can also be written as excludeDefaults = utils.pam.entryTypes;
};

This will result in /etc/pam.d/myService containing:

auth sufficient my_entry.so
auth required my_second_entry.so

Changing the order in which the entries appear in the file

Looking at the orders table below, if we want to make krb5 come before ldap and after unix in the account type for the login service, we can do something like this:

security.pam.services.login.account.krb5.order = mkForce 1500;

Include/substack

If we look at the lightdm PAM service on master, we can see it looks like this:

auth      substack      login
account   include       login
password  substack      login
session   include       login

To do the same with this PAM module, we can do the following:

security.pam.services.lightdm = {
  # First let's get rid of the defaults.
  excludeDefaults = [ "auth" "account" "password" "session" ];

  # And then let's define our service the way we want it
  auth.lightdm = {
    control = "substack";
    path = "login"; # no check is made on this
    # even if we only have one entry, we still have to provide
    # a default or the configuration will not evaluate
    order = 1000;
  };
  account.lightdm = {
    control = "include";
    path = "login";
    order = 1000;
  };
  password.lightdm = {
    control = "substack";
    path = "login";
    order = 1000;
  };
  session.lightdm = {
    control = "include";
    path = "login";
    order = 1000;
  };
};

Which will generate the same PAM service file (modulo the order of the types (i.e. account will come before auth) and whitespace).

Ordering of the PAM modules

This used to be done using types.orderOf (see #97392) and comparing the order value of each PAM entry, and then @roberth suggested to me that it could actually be done just by using builtins.sort. The lower that value, the higher the entry ends up in the file.

To avoid more breaking changes than necessary \s, I decided to keep the old order of PAM modules. More seriously, the goal is to leave users who don't interact with the PAM module alone, the interface might change, but they won't see it as they will get the same result.

Here's that table:

Type	Module	Order
account	unix	1000
account	ldap	2000
account	sssd	3000
account	krb5	4000
account	googleOsLoginDie	10000
account	googleOsLoginIgnore	10500

auth	googleOsLogin	10000
auth	rootOK	11000
auth	requireWheel	12000
auth	logFailures	13000
auth	sshAgent	14000
auth	fprintd	15000
auth	p11	16000
auth	u2f	17000
auth	usb	18000
auth	oath	19000
auth	yubico	20000
auth	unixRequired	21000
auth	ecryptfs	22000
auth	mount	23000
auth	kwallet	24000
auth	gnomeKeyring	25000
auth	gnupg	26000
auth	googleAuthenticator	27000
auth	duoSecurity	28000
auth	unix	30000
auth	otpw	31000
auth	ldap	32000
auth	sssd	33000
auth	krb5	34000
auth	deny	100000

password	unix	1000
password	ecryptfs	2000
password	mount	3000
password	ldap	4000
password	sssd	5000
password	krb5	6000
password	gnomeKeyring	10000

session	setEnvironment	500
session	unix	1000
session	setLoginUid	2000
session	makeHomeDir	3000
session	updateWtmp	4000
session	ecryptfs	5000
session	mount	6000
session	ldap	7000
session	sssd	8000
session	krb5	9000
session	otpw	10000
session	startSession	11000
session	forwardXAuth	12000
session	limits	13000
session	motd	14000
session	appArmor	15000
session	kwallet	16000
session	gnomeKeyring	17000
session	gnupg	18000
session	lxcfs	19000

The chosen values are complitely arbitrary, except that they conserve the current order of modules. They might seem excessive, I might have been over enthused, but at least they give people and us a lot of room to move stuff around.

Some other improvements / stuff that changed

PAM configuration has been regrouped under nixos/modules/security/pam and each PAM module has its own file under modules/, except for the PAM modules that have been moved to their related NixOS module
The fprintd PAM module now uses services.fprintd.package instead of hardcoding pkgs.fprintd
Some PAM modules names were changed to match their services names. If possible, the old names were preserved with mkRenamedOptionModule
Some PAM modules' source code were moved to their related NixOS modules:
- `googleOsLogin
- fprintd
- apparmor
- duosec
- gnome-keyring
- ldap
- krb5
- lxcfs

New and renamed PAM modules options

No new PAM modules or options have been added, as it is not the goal of this PR. However, due to the way this module is implemented, new options have appeared nonetheless, as modules are now configurable globally and per-service.

New options

security.pam.modules.apparmor.enable
security.pam.modules.duosec.enable
security.pam.modules.forwardXAuth
security.pam.modules.fprintd.enable
security.pam.modules.gnome-keyring.enable
security.pam.modules.gnupg.*
security.pam.modules.googleAuthenticator.enable
security.pam.modules.googleOsLogin.*
security.pam.modules.krb5.enable
security.pam.modules.kwallet.enable
security.pam.modules.logFailures
security.pam.modules.lxcfs.enable
security.pam.modules.makeHomeDir.enable
security.pam.modules.motd.*
security.pam.modules.requireWheel
security.pam.modules.rootOK
security.pam.modules.setEnvironment
security.pam.modules.setLoginUid
security.pam.modules.sssd.*
security.pam.modules.startSession
security.pam.modules.unix.*
security.pam.services.<name>.account
security.pam.services.<name>.auth
security.pam.services.<name>.password
security.pam.services.<name>.session
security.pam.services.<name>.excludeDefaults
security.pam.services.<name>.modules.ecryptfs.enable
security.pam.services.<name>.modules.krb5
security.pam.services.<name>.modules.ldap.enable
security.pam.services.<name>.modules.limits
security.pam.services.<name>.modules.lxcfs.enable
security.pam.services.<name>.modules.makeHomeDir.skelDirectory
security.pam.services.<name>.modules.motd.motdFile
security.pam.services.<name>.modules.oath (except enable)
security.pam.services.<name>.modules.p11 (except enable)
security.pam.services.<name>.modules.sssd.enable
security.pam.services.<name>.modules.u2f (except enable)
security.pam.services.<name>.modules.unix.debug
security.pam.services.<name>.modules.yubico (except enable)

Renamed options

security.pam.enableEcryptfs -> security.pam.modules.ecryptfs.enable
users.ldap.loginPam -> security.pam.modules.ldap.enable
security.pam.loginLimits -> security.pam.modules.limits: type changed, no possibility of mkRenamedOptionModule
security.pam.makeHomeDir.skelDirectory -> security.pam.modules.makeHomeDir.skelDirectory
security.pam.oath -> security.pam.modules.oath
security.pam.enableOTPW -> security.pam.modules.otpw.enable
security.pam.p11 -> security.pam.modules.p11
security.pam.mount -> security.pam.modules.pam_mount
security.pam.enableSSHAgentAuth -> security.pam.modules.sshAgent.enable
security.pam.services.<name>.sssdStrictAccess -> security.pam.services.<name>.modules.sssd.strictAccess
security.pam.u2f -> security.pam.modules.u2f
security.pam.usb.enable -> security.pam.modules.usb.enable
security.pam.yubico -> security.pam.modules.yubico
security.pam.services.<name>.enableAppArmor -> security.pam.services.<name>.apparmor.enable
security.pam.services.<name>.duoSecurity.enable -> security.pam.services.<name>.modules.duosec.enable
security.pam.services.<name>.forwardXAuth -> security.pam.services.<name>.modules.forwardXAuth
security.pam.services.<name>.fprintAuth -> security.pam.services.<name>.modules.fprintd.enable
security.pam.services.<name>.enableGnomeKeyring -> security.pam.services.<name>.modules.gnome-keyring.enable
security.pam.services.<name>.gnupg -> security.pam.services.<name>.modules.gnupg
security.pam.services.<name>.googleAuthenticator.enable -> security.pam.services.<name>.modules.googleAuthenticator.enable
security.pam.services.<name>.googleOsLoginAccountVerification -> security.pam.services.<name>.modules.googleOsLogin.enableAccountVerification
security.pam.services.<name>.googleOsLoginAuthentication -> security.pam.services.<name>.modules.googleOsLogin.enableAuthentication
security.pam.services.<name>.enableKwallet -> security.pam.services.<name>.modules.kwallet.enable
security.pam.services.<name>.logFailures -> security.pam.services.<name>.modules.logFailures
security.pam.services.<name>.makeHomeDir -> security.pam.services.<name>.modules.makeHomeDir.enable
security.pam.services.<name>.showMotd -> security.pam.services.<name>.modules.motd.enable
security.pam.services.<name>.oathAuth -> security.pam.services.<name>.modules.oath.enable
security.pam.services.<name>.otpwAuth -> security.pam.services.<name>.modules.otpw.enable
security.pam.services.<name>.p11Auth -> security.pam.services.<name>.modules.p11.enable
security.pam.services.<name>.pamMount -> security.pam.services.<name>.modules.pam_mount.enable
security.pam.services.<name>.requireWheel -> security.pam.services.<name>.modules.requireWheel
security.pam.services.<name>.rootOK -> security.pam.services.<name>.modules.rootOK
security.pam.services.<name>.setEnvironment -> security.pam.services.<name>.modules.setEnvironment
security.pam.services.<name>.setLoginUid -> security.pam.services.<name>.modules.setLoginUid
security.pam.services.<name>.sshAgentAuth -> security.pam.services.<name>.modules.sshAgent.enable
security.pam.services.<name>.startSession -> security.pam.services.<name>.modules.startSession
security.pam.services.<name>.u2fAuth -> security.pam.services.<name>.u2fAuth
security.pam.services.<name>.unixAuth -> security.pam.services.<name>.modules.unix.enableAuth
security.pam.services.<name>.allowNullPassword -> security.pam.services.<name>.modules.unix.allowNullPassword
security.pam.services.<name>.nodelay -> security.pam.services.<name>.modules.unix.nodelay
security.pam.services.<name>.updateWtmp -> security.pam.services.<name>.modules.updateWtmp
security.pam.services.<name>.usbAuth -> security.pam.services.<name>.modules.usb.enable
security.pam.services.<name>.yubicoAuth -> security.pam.services.<name>.modules.yubico.enable

TODO

Add @kwohlfahrt as a co-author as I build on top of his changes
Allow other modules to integrate with PAM. This is done via submodules, see nixos/modules/security/pam/modules/* for examples
Keep the current order of PAM entries in order not to break existing configurations. I think this is done, I tripled checked it, but it wouldn't hurt if someone could check again. The current order of PAM modules is provided above, with the ordering values I chose. Checking that order against the current one could already be a huge help!
Refactor the modules in nixos/modules/security/pam/modules with a function that does the submodule part of the work. See nixos/pam: rework of the entire module #105319 (comment)
Allow for creation of services with the defaults provided by the modules. See nixos/pam: rework of the entire module #105319 (comment)
Decide if we should mkDefault all the entries orders
Add mkRenamedOptionModule imports, where relevant and possible. The list is available above, and we need [Reverted] Module-builtin assertions, disabling assertions and submodule assertions #97023 to be able to mkRenamedOptionModule inside submodules. This isn't so much a blocker for review, as you can base that review on the list provided above.
Write a proper commit message, this can wait the end when I squash all of them

Services migration

To remove the text = mkDefault [...] in the pamServiceSubmodule, we have to migrate a bunch of modules that are still using the old interface. If this PR is accepted, I will create an issue with the following list and ping the respective maintainers of the modules so they can migrate, and I'll be happy to give them a hand.

services/x11/desktop-managers/gnome3.nix
services/x11/display-managers/sddm.nix
services/x11/display-managers/gdm.nix
services/x11/display-managers/lightdm.nix
services/networking/vsftpd.nix

Documentation

I still need to write some documentation for this, mainly release notes, and the PAM section of the manual. Most of this is just formatting and expanding the description of the PR above.
Here's a list of TODOs for the documentation:

Check that the module options display correctly.
Write release notes
Write a PAM section in the NixOS manual, probably under III. Administration
Update nixos/doc/manual/development/writing-modules.xml with the new path to the PAM module

Testing

Please, please, please, don't test this on your machine. Try it our in a VM before. As much as I have tested this, some edge cases might remain and you risk getting locked out of our machine. If that is the case, reboot and choose the configuration before you tried out those changes.

How to test this

If you are using flakes, change your nixpkgs input to github:rissson/nixpkgs/nixos-pam and then nix flake update --update-input nixpkgs && nixos-rebuild --flake .#myHost build-vm.
If you're not, you can do something along the lines of nixos-rebuild -I 'nixpkgs=https://github.com/rissson/nixpkgs/archive/nixos-pam.tar.gz build-vm. I haven't tested this.
Then you can execute the commands it gives you to start the VM and try this out.

To make it a bit easier to report your finding, I made a small snippet you can copy paste when commenting on this PR (you can remove everything you didn't tick for a more concise output):

* [ ] I haven't any special PAM configuration.
* [ ] I have some special PAM config (please describe it).
* [ ] Types of auth tested
    * [ ] username/password from `/etc/passwd` from a TTY. You won't test anything if you try this from a display manager, as it probably doesn't use the new PAM module.
    * [ ] ssh
    * [ ] krb5
    * [ ] ldap
    * [ ] sudo
* [ ] Other modules tested
    * [ ] appArmor
    * [ ] duoSecurity
    * [ ] ecryptfs
    * [ ] forwardXAuth
    * [ ] fprintd
    * [ ] gnomeKeyring
    * [ ] gnupg
    * [ ] googleAuthenticator
    * [ ] googleOsLogin
    * [ ] kwallet
    * [ ] limits
    * [ ] logFailures
    * [ ] lxcfs
    * [ ] makeHomeDir
    * [ ] motd
    * [ ] mount (pam_mount)
    * [ ] oath
    * [ ] otpw
    * [ ] p11
    * [ ] requireWheel
    * [ ] rootOK
    * [ ] setEnvironment
    * [ ] setLoginUid
    * [ ] sshAgent
    * [ ] sssd
    * [ ] startSession
    * [ ] u2f
    * [ ] updateWtmp
    * [ ] usb
    * [ ] yubico
* [ ] I ran NixOS tests. If yes, list them below, tick the box if they succeeded. If they didn't, provide the output.

I'll start:

CC.ing people

People part of the original conversation about reworking this module

@flokli @Rudi9719 @kwohlfahrt

People who previously worked or reviewed some related stuff

@Mic92 @aanderse

People working on the blockers for this PR

@infinisil for mkRenamedOptionModule inside submodules

Rudi9719 · 2020-11-29T05:13:37Z

Will it be possible to change the order of modules? For example: SSSD won't work unless it comes before Unix

rissson · 2020-11-29T05:16:10Z

Will it be possible to change the order of modules? For example: SSSD won't work unless it comes before Unix

Yep! That's something I forgot to add as an example.

For example:

security.pam.services.login.password.sssd.order = 100;

Might have to mkForce it as things stand, and thus I might have to put some mkDefault in there.

nixos/modules/security/pam/default.nix

rissson · 2020-11-29T22:52:00Z

find a better way to create services with no defaults. See "Creating a custom service, using none of the defaults" above. The goal is to get rid of those mkForce. I also require help with this. Maybe have the PAM modules add their entries with mkDefault? Solution is in e356a70

Some explanation regarding this. Due to the way the security.pam.services.<...>.{auth,account,password,session} are merged together, we currently (with the commit mentioned above) end up with something like this:

config.security.pam.services.<...>.account = mkDefault {
  unix = { ... };
  ldap = { ... };
# ... you get the gist of it
};

Each PAM pipeline (auth, account, password, session) is a big mkDefault. This is great for services that do not want to register any default. They just have to say:

config.security.pam.services.serviceWithoutAccountDefault = {
  account = {};
  auth = {
    customEntry = { ... };
  };
};

Which means "hey, I don't want any of those defaults for the account pipeline, here is my custom pipeline for auth, and leave the default for the two other pipelines". Which is great! We want that kind of customization (i.e. service can choose which pipeline they want to override). However, where it falls short is for a user who wants to override the order, for example:

config.security.pam.services.login.account.krb5.order = 1;

This overrides the whole account pipeline and evaluation will fail with The option 'security.pam.services.login.account.krb5.control' is used but not defined.. The resulting login.account actually looks like this:

{
  krb5 = {
    order = 1;
  };
}

and that's it. No wonder it fails.

Now, the solution I see:

reverting the mentioned commit above: hard pass, it requires services to mkForce their customization
don't do anything: hard pass, we don't want the users to have to rewrite the whole pipeline just for changing the order
make a security.pam.services.<...>.includeDefaults (or exclude, more on that later) that is a boolean that services can use to get rid of the defaults: meh, it doesn't allow for the level of customization we currently have, i.e. the ability for services to choose which pipeline they want to override
make a security.pam.services.<...>.includeDefaults (or exclude, more on that later) that is a list that defaults to [ "account", "auth", "password", "session" ] ([] if the option is exclude) that explicitly specifies which defaults the services want to keep: and I think I'll go with this one.

Shortcoming of this: requires even more logic then there already is in the modules code, for each pipeline they would have to do something like pipeline = mkIf (any (p: p == "pipeline") config.includeDefaults) { ... }; which is much too heavy. This means that this step is block by the refactoring described above, i.e. make a mkPamModule function to easly create those.

About the include or exclude choice, I don't know which to choose. Services are more likely to drop all pipelines, which would make using includeDefaults easy (just set it to [] to ignore all defaults), but then exclude would be more explicit in which pipelines the service drops. Any take on this?

EDIT: formatting, typos

rissson · 2020-11-29T23:26:46Z

Refactor the modules in nixos/modules/security/pam/modules with a function that does the submodule part of the work. I require help with this, all I can seem to get are infinite recursion errors...

Here is a patch that gives said error. I have no idea why this is happening. Any help would be welcomed and much appreciated! https://bin.lama-corp.space/raw/1k5M1WoUC6_H8MaqGOggc

EDIT: @infinisil found the problem and I was able to implement ~~a PoC of~~ the refactoring in ~~f9e4acb07b04633804d5589235eb686351fc625d~~ e38a39fdaf2f15276fc7f54610d93662e46275fa

rissson · 2020-11-30T15:42:02Z

I wonder if all the orders should be mkDefaulted...

rissson · 2020-11-30T22:16:05Z

nixos/modules/services/x11/desktop-managers/plasma5.nix

-      security.pam.services.lightdm.enableKwallet = true;
-      security.pam.services.sddm.enableKwallet = true;
+      security.pam.services.kde = { modules.unix.allowNullPassword = true; };
+      security.pam.modules.kwallet.enable = true;


Let's make sure this is really what we want.

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>

…module Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>

nixos-discourse · 2020-12-03T17:59:23Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/reworking-the-nixos-pam-module-a-walk-through/10334/1

austinbutler · 2020-12-04T18:49:04Z

Haven't done more than glance at these changes yet, but it seems likely it will at least partially, if not fully, address #95017. Locally I've been using my own override that does what appears to be similar to security.pam.modules.krb5.enable in this PR so that's great.

In that issue there's also the error pam_ccreds: failed to open cached credentials "/var/cache/.security.db": No such file or directory. I mention this because I don't know whether that error happens because in my case Kerberos wasn't set up for login auth, therefore it never succeeded and therefore nothing to cache, or whether something with the existing PAM/Kerberos setup in NixOS didn't create the file or doesn't allow PAM to create the file.

rissson · 2020-12-04T20:00:30Z

Haven't done more than glance at these changes yet, but it seems likely it will at least partially, if not fully, address #95017. Locally I've been using my own override that does what appears to be similar to security.pam.modules.krb5.enable in this PR so that's great.

In that issue there's also the error pam_ccreds: failed to open cached credentials "/var/cache/.security.db": No such file or directory. I mention this because I don't know whether that error happens because in my case Kerberos wasn't set up for login auth, therefore it never succeeded and therefore nothing to cache, or whether something with the existing PAM/Kerberos setup in NixOS didn't create the file or doesn't allow PAM to create the file.

pam_ccreds is a PAM module that saves a hash a a user's password when they logged in kerberos. In the event that a user locked its workstation, and the KDC goes down, the user will still be able to unlock (only unlock, not log in, as in creating a new session) their computer using their password. From your description of the issue, I don't exactly know what's going wrong, but if you're not using kerberos with PAM, with this PR, a simple security.pam.modules.krb5.enable = false; will do the trick.

Also, I'll add a quick comment about PAM activation by default when krb5 is enabled, tell me what you think is best.

rissson · 2020-12-04T20:01:25Z

nixos/modules/config/krb5/default.nix

+        mkSvcConfigCondition = svcCfg: cfg.enable && svcCfg.modules.${pamModName}.enable;
+
+        mkModuleOptions = global: {
+          enable = mkOption {
+            type = types.bool;
+            default = if global then true else pamModCfg.enable;


Suggested change

mkSvcConfigCondition = svcCfg: cfg.enable && svcCfg.modules.${pamModName}.enable;

mkModuleOptions = global: {

enable = mkOption {

type = types.bool;

default = if global then true else pamModCfg.enable;

mkSvcConfigCondition = svcCfg: svcCfg.modules.${pamModName}.enable;

mkModuleOptions = global: {

enable = mkOption {

type = types.bool;

default = if global then cfg.enable else pamModCfg.enable;

Should we do this instead?

kwohlfahrt · 2020-12-05T20:36:50Z

I'll try to have a more detailed review of this tomorrow and Monday, it's a big PR! I've read through the descriptions and the proposals look sensible. I'll try and review the implementation in a bit more detail tomorrow and Monday.

Is the idea of pam.module.foo.enableAuth to give a single switch to enable/disable all auth-related rules for that module? There seems to be some naming inconsistency, with a few enableAuth and a few enableAuthentication, or is there a difference between these?

Perhaps renaming security.pam.modules to defaults or defaultModules would be a little clearer? Otherwise, it's a little confusing whether pam.modules.foo.enable = false; disables it globally (i.e. overriding services) or not.

I also want to discuss the use of orderOf - I think having a order key until that is merged is fine (and a good idea to keep the two PRs independent). However, I think there is an advantage to expressing rules as a partial ordering, because it keeps the intention clearer. For example, if I have

modules.a.after = x: x.name == "d" || x.name == "c";
modules.b.after = x: x.name == "d";
modules.c.after = x: x.name == "d";
modules.d.after = x: false;

This makes the dependencies between modules clear. Whereas an ordering to achieve the same dependencies might be:

modules.a.order = 2500;
modules.b.order = 2000;
modules.c.order = 1500;
modules.d.order = 1000;

This defines an order that satisfies the dependencies, but doesn't make clear which parts of the relationship are important.

I'd like to see the conversations on the other PR resolved before merging this:

Use of attrsets for PAM module arguments
include/substack - allowing nix-defined entries (rather than just paths)

nixos-discourse · 2021-04-10T21:25:02Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-office-hours/11945/14

deliciouslytyped · 2021-04-18T17:30:38Z

I'm currently stumbling in the dark with PAM, slowly making some sense of some things in #119710 , if we can do anything to improve the debugging experience, that would be cool.

As a small note, pam_unix.so also takes an audit flag which the man page says gives more output than debug but I haven't really confirmed that.

I'm currently waiting for a large rebuild, but the --enable-debug flag may be of interest; adding a flag to the nix package and a flag in the module to toggle it.

infinisil · 2021-04-19T01:36:15Z

nixos/modules/security/pam/default.nix

+          The PAM control for this entry. It can be either on of the
+          historical basic control keywords, of a set of result-actions pairs.
+        '';
+        apply = value: if isString value then value else


Don't use apply, it leaks the module implementation to the outside, since config.the.option, accessible from all modules, will have this function applied already. This also makes it difficult for other modules to reuse this option's value. In this case parsing would be needed.

You can achieve this with a let in binding instead.

Same for all the other uses of apply

Ninlives · 2021-07-20T16:25:28Z

Is this pr still active?

rissson · 2021-07-20T22:44:33Z

Is this pr still active?

This PR is way too huge to be merged as is. However, it exposes the ideas I had for refactoring the PAM module.

We had a discussion with @Pamplemousse a while ago about how to break it down into smaller, more reviewable PRs. Hit us up if you want to discuss it!

asdf8dfafjk · 2021-07-23T15:51:02Z

Well you still deserve a huge thank you for the huge effort behind the bold PR, thank you!

nrdxp · 2021-09-10T00:01:43Z

@rissson, thanks for the massive contribution. Hopefully at some point we can get this reworked and merged in more manageable steps. Since you expressed above not wanting to merge this as is, I hope you don't mind if I go ahead and close it for now.

nixos-discourse · 2023-09-21T08:04:05Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/what-do-you-want-from-pam-security-pam-in-nixos/33265/2

nyabinary · 2023-12-31T22:48:30Z

Did anyone rework pam yet?

infinisil · 2023-12-31T23:19:56Z

@nyabinary Some work was done with #255547!

kwohlfahrt reviewed Nov 29, 2020

View reviewed changes

nixos/modules/security/pam/default.nix Show resolved Hide resolved

rissson force-pushed the nixos-pam branch 3 times, most recently from 324293c to 1556605 Compare November 29, 2020 17:39

rissson added 0.kind: enhancement Add something new 2.status: work-in-progress This PR isn't done labels Nov 30, 2020

rissson force-pushed the nixos-pam branch from f9e4acb to e38a39f Compare November 30, 2020 14:49

rissson mentioned this pull request Nov 30, 2020

Introduce types.orderOf #97392

Draft

2 tasks

rissson requested a review from kwohlfahrt November 30, 2020 21:01

rissson commented Nov 30, 2020

View reviewed changes

rissson force-pushed the nixos-pam branch from a6891ea to a588e6c Compare November 30, 2020 22:24

rissson added 4 commits December 1, 2020 12:45

nixos/pam: modules: move duosec to its NixOS module

56b994e

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>

nixos/pam: modules: move gnome-keyring to its NixOS module

aaf3877

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>

virtualisation/lxcfs: pam: use utils.pam.mkPamModule to register the …

a376ec5

…module Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>

nixos/pam: entries: replace orderOf with builtins.sort

e36c614

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>

rissson force-pushed the nixos-pam branch from 7f72331 to e36c614 Compare December 1, 2020 11:46

rissson requested a review from flokli December 1, 2020 14:08

rissson requested a review from Mic92 December 4, 2020 19:54

rissson commented Dec 4, 2020

View reviewed changes

This was referenced Dec 13, 2020

nixos/pam: ability to calls an external command #90488

Open

nixos/pam: ability to calls an external command #90490

Closed

flokli mentioned this pull request Dec 22, 2020

systemd: enable homed #98299

Merged

10 tasks

doronbehar mentioned this pull request Dec 25, 2020

nixos/fprintd: option to inhibit PAM rule creation #107320

Draft

10 tasks

SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 18, 2021

infinisil reviewed Apr 19, 2021

View reviewed changes

nrdxp closed this Sep 10, 2021

Artturin mentioned this pull request Nov 27, 2021

[Tracking] Pam #147565

Closed

Majiir mentioned this pull request Sep 16, 2023

nixos/pam: assemble rules from modular configuration #255547

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/pam: rework of the entire module #105319

nixos/pam: rework of the entire module #105319

rissson commented Nov 29, 2020 •

edited

Loading

Rudi9719 commented Nov 29, 2020

rissson commented Nov 29, 2020 •

edited

Loading

rissson commented Nov 29, 2020 •

edited

Loading

rissson commented Nov 29, 2020 •

edited

Loading

rissson commented Nov 30, 2020

rissson Nov 30, 2020

nixos-discourse commented Dec 3, 2020

austinbutler commented Dec 4, 2020

rissson commented Dec 4, 2020

rissson Dec 4, 2020

kwohlfahrt commented Dec 5, 2020

nixos-discourse commented Apr 10, 2021

deliciouslytyped commented Apr 18, 2021 •

edited

Loading

infinisil Apr 19, 2021

Ninlives commented Jul 20, 2021

rissson commented Jul 20, 2021

asdf8dfafjk commented Jul 23, 2021

nrdxp commented Sep 10, 2021

nixos-discourse commented Sep 21, 2023

nyabinary commented Dec 31, 2023

infinisil commented Dec 31, 2023

nixos/pam: rework of the entire module #105319

nixos/pam: rework of the entire module #105319

Conversation

rissson commented Nov 29, 2020 • edited Loading

Origin, requirements and blockers

Design

Interacting with the PAM module

Creating a new service with the defaults

A service overriding something in the modules

Changing a module option for all services

Changing a module option for one service

Creating a custom service, using none of the defaults

Changing the order in which the entries appear in the file

Include/substack

Ordering of the PAM modules

Some other improvements / stuff that changed

New and renamed PAM modules options

TODO

Services migration

Documentation

Testing

How to test this

CC.ing people

People part of the original conversation about reworking this module

People who previously worked or reviewed some related stuff

People working on the blockers for this PR

Rudi9719 commented Nov 29, 2020

rissson commented Nov 29, 2020 • edited Loading

rissson commented Nov 29, 2020 • edited Loading

rissson commented Nov 29, 2020 • edited Loading

rissson commented Nov 30, 2020

rissson Nov 30, 2020

Choose a reason for hiding this comment

nixos-discourse commented Dec 3, 2020

austinbutler commented Dec 4, 2020

rissson commented Dec 4, 2020

rissson Dec 4, 2020

Choose a reason for hiding this comment

kwohlfahrt commented Dec 5, 2020

nixos-discourse commented Apr 10, 2021

deliciouslytyped commented Apr 18, 2021 • edited Loading

infinisil Apr 19, 2021

Choose a reason for hiding this comment

Ninlives commented Jul 20, 2021

rissson commented Jul 20, 2021

asdf8dfafjk commented Jul 23, 2021

nrdxp commented Sep 10, 2021

nixos-discourse commented Sep 21, 2023

nyabinary commented Dec 31, 2023

infinisil commented Dec 31, 2023

rissson commented Nov 29, 2020 •

edited

Loading

rissson commented Nov 29, 2020 •

edited

Loading

rissson commented Nov 29, 2020 •

edited

Loading

rissson commented Nov 29, 2020 •

edited

Loading

deliciouslytyped commented Apr 18, 2021 •

edited

Loading