umask from `/etc/login.defs` is not applied

0n-s commented 2 weeks ago

Issue description

The default umask from /etc/login.defs is not applied for user logins because there's nothing that does so. The usual mechanism for this is pam_umask. This is not configured on NixOS as of writing; the specific proposal here is to add it to the relevant files in /etc/pam.d.

NB: this will inadvertently be a breaking change that should be mentioned in release notes. Reason being that the intended default NixOS umask (again, see /etc/login.defs) is more restrictive than the effective default 0022 that everyone gets right now. The more restrictive umask is better for single-user systems, so it should probably be kept as the default.

Steps to reproduce

Set a non-0022 UMASK in /etc/login.defs (security.loginDefs.settings.UMASK; the current default 077 works fine)
login to a user account
umask in a shell gives 0022

Technical details

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

 - system: `"x86_64-linux"`
 - host os: `Linux 6.11.0, NixOS, 24.11 (Vicuna), 24.11pre688563.bc947f541ae5`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.8`
 - channels(root): `"nixos"`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

0n-s commented 1 week ago

@majiir (as maintainer of nixos/modules/security/pam.nix)

Majiir commented 1 week ago

Makes sense. Switching to 077 could be disruptive to users that are inadvertently relying on 022 in scripts/etc, so I think we should consider putting this change behind a stateVersion check, even though it is a bugfix. It would be difficult for users to check their configs for any possible breakage just by reading a release note. We can add an option so that users on an old stateVersion can upgrade safely.

0n-s commented 1 week ago

Switching to 077 could be disruptive to users that are inadvertently relying on 022 in scripts/etc, so I think we should consider putting this change behind a stateVersion check, even though it is a bugfix.

I don't think it's necessary since the kinds of scripts that generally accidentally rely on umasks, like cronjobs, do not login (i.e. umask remains at kernel default 022, meaning that those scripts won't need to be checked). I think it's highly unlikely to cause problems, but it can't hurt, even if I do think the possibility for regressions there is mostly theoretical.

But if you do go through with the stateVersion check, then…

We can add an option so that users on an old stateVersion can upgrade safely.

…yes, please provide this.

NixOS / nixpkgs