nixos: add per-user groups by default#199705
Conversation
a474b35 to
57e7dcf
Compare
4f9c5a9 to
e464127
Compare
sternenseemann
left a comment
There was a problem hiding this comment.
Overall LGTM, not sure if the change is good or bad, but moving with other modern distributions can't be terrible.
75b78af to
862d960
Compare
fd21ae0 to
22bc2de
Compare
|
Rebased and updated to target NixOS 23.05. Please review! |
|
This looks great, I was just looking for an option to do exactly this. I do have a couple of questions:
|
I like it. But in a follow-up PR? Or do you feel it belongs in this one?
I think that's handled by this code: nixpkgs/nixos/modules/config/update-users-groups.pl Lines 48 to 60 in 62cace1 Correlating UID and GID would be nice. |
22bc2de to
5b56504
Compare
|
Rebased to resolve conflicts. Please consider for NixOS 23.05. |
|
I'm not too shocked by getting this change for 23.05. But I would rather it early rather than late to smoke out any issue and so we can revert timely. |
|
Apologies, it completely slipped from my mind, we missed the window for this change, let's get this merged after the branch-off. |
You've found the problem ;) |
3870d40 to
a7c297c
Compare
|
Updated with users.solitaryByDefault option. Please review. |
Add a new option, users.solitaryByDefault, that defaults to true for stateVersion >= 24.11. It can be opt-in before that (and opt-out after). When users.solitaryByDefault is true, each user defaults to primary group `<user_name>`, whose group id equals the user id. Before it was `users` (with group id 100) for normal users and system users required explicit setting the group -- because the default was "" (empty) which triggered an assert saying the group must be set. This change allows finer grained control over file access, and aligns NixOS with other distros. Here, the result of `useradd -m user1 && ls -ld /home/user1`: Arch Linux : drwx------ 2 user1 user1 4096 Oct 28 15:17 /home/user1 Debian 11 : drwxr-xr-x 2 user1 user1 4096 Oct 28 15:17 /home/user1 Fedora 36 : drwx------ 2 user1 user1 4096 Oct 28 15:17 /home/user1 Ubuntu 22.04 : drwxr-x--- 2 user1 user1 4096 Oct 28 15:17 /home/user1 Fixes NixOS#198296.
a7c297c to
bad74b7
Compare
|
Re: CI failure:
...seems to be an unrelated Cachix failure? |
bad74b7 to
88d5929
Compare
|
Last rebase/force push was to trigger CI, but I also took the opportunity to improve/correct the release note entry. |
| } | ||
| # Create per-user primary groups | ||
| (lib.optionalAttrs nixosConfig.users.solitaryByDefault | ||
| (lib.mapAttrs' (_: user: { name = user.group; value = { gid = lib.mkDefault user.uid; }; }) nixosConfig.users.users)) |
There was a problem hiding this comment.
This works for statically assigned UIDs, but UID could also be set to null to mean auto-assign upon activation, and that's the default. I don't see any logic in update-users-groups.pl to ensure identical UID and GID are auto-assigned?
There was a problem hiding this comment.
I don't see any logic in update-users-groups.pl to ensure identical UID and GID are auto-assigned?
Correct, I didn't touch that code. Is that a blocker? You still get per-user groups and everything will work fine, except GID != UID, right?
There was a problem hiding this comment.
Users and groups with the same name should have equal uids and gids. We are careful in our static allocation: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/misc/ids.nix#L671.
Also, currently we dynamic allocate GIDs in 400-999 range which are system IDs, and user UID/GIDs should have 1000-29999.
I think before we land this PR we need to change the allocation script to ensure the constraints are met.
There was a problem hiding this comment.
Although I agree having GID = UID is preferable, I don't understand why that should be a blocker for this PR. Apparently we're OK with the current dynamic allocation that does not have GID = UID, so why would this PR make a difference? (I guess I should update the release notes entry to not mention the GID = UID property, since it's only valid for static UIDs.)
There was a problem hiding this comment.
Current the non-ideal behaviour is only exposed to a services that use both a system user and an identical name groups. This does not affect normal users, nor services that use a fixed UID/GID allocation. But with this change, we're now exposing every single normal user (with state version 24.05+) with this behaviour. It's also more difficult to fix user's file permissions than service states if someone wants to change their GID, since systemd will auto fix ownerships.
Also, I think allocating normal user GIDs to <1000 is a hard blocker.
There was a problem hiding this comment.
I wonder if the per-user group switch could happen together with the perlless activation... (background: systemd sysusers have provision to avoid same ID from being used by user/groups with different names)
EDIT: I think currently NixOS sysusers module doesn't look at isSystemUser at all, so it's also broken...
There was a problem hiding this comment.
In perlless NixOS, what code implements the dynamic UID/GID allocations? Is systemd sysusers used for normal users too?
There was a problem hiding this comment.
See this PR that fixes this behaviour (i.e. let sysusers only create system users): #328926
I've reached the point where the only proper solution to our user management is writing my own application that works like systemd-sysusers but also creates normal users and is able to change some data about users (e.g. passwords).
However, I believe it's worth landing this change regardless as it lays the foundation to build software that does this correctly. Since it's gated behind a flag anyway, I don't see a problem.
Btw: isSystemUser is also fundamentally broken (see #328926 (comment))
|
Are there any updates on this? |
I guess I'm waiting for the systemd sysusers thing to stabilize and be the default in NixOS (open-ended thread). |
Userborn will be aiming to be the default instead of sysuser according to #332719 |
|
Any updates? |
Not since #199705 (comment) (two posts up). |
|
The current plan is to use userborn by default on NixOS? |
|
Just did bump into this topic when trying to use per user groups in my nixos setup. Pity that this is open for so long. I am wondering if an interim step could be to add an explicit assertion that the uid must be manually set? It could be enough to save anyone from accidentally running into a mix of auto-assigned uid/gid pairs, and it would give already a partial solution which is helpful until the final ones are ready. The proposed change inspired me to make a little experiment in one of my local flakes and I came up with this: solitary-users = { config, lib, ... }:
let
nixosConfig = config;
in {
key = "solitary-users";
options.users.solitaryByDefault = lib.mkOption {
type = lib.types.bool;
default = true;
description = "Automatically create per-user private groups with GID matching UID";
};
options.users.users = lib.mkOption {
type = lib.types.attrsOf (lib.types.submodule ({ name, config, ... }: {
config.group = lib.mkIf
(config.isNormalUser && nixosConfig.users.solitaryByDefault)
(lib.mkOverride 999 name);
}));
};
config = lib.mkIf nixosConfig.users.solitaryByDefault {
assertions = lib.mapAttrsToList (name: user: {
assertion = user.uid != null;
message = "solitary-users: User '${name}' must have uid set";
}) (lib.filterAttrs (_: u: u.isNormalUser or false) nixosConfig.users.users);
users.groups = lib.mapAttrs' (_: user: {
name = user.group;
value = { gid = lib.mkDefault user.uid; };
}) (lib.filterAttrs (_: u: u.isNormalUser or false) nixosConfig.users.users);
};
};It seems to do the trick for my needs already, with the price that I have to configure fixed uid values. |
Description of changes
Each user now defaults to primary group
<user_name>. Before it wasusersfor normal users and system users required explicit setting thegroup -- because the default was "" (empty) which triggered an assert
saying the group must be set.
This change allows finer grained control over file access, and aligns
NixOS with other distros. Here, the result of
useradd -m user1 && ls -ld /home/user1:The new functionality is gated behind an option called
users.solitaryByDefault, which defaults to true forsystem.stateVersion >= 24.05.Fixes #198296.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes