Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nixos: add per-user groups by default #199705

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

nixos: add per-user groups by default #199705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dffdcfd
nixos: add per-user groups option
bjornfor Oct 28, 2022
88d5929
nixos/doc: add per-user groups info to release notes
bjornfor Dec 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions nixos/doc/manual/release-notes/rl-2405.section.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ In addition to numerous new and upgraded packages, this release has the followin
Take the time to review [the release notes](https://gitlab.com/cryptsetup/cryptsetup/-/raw/v2.7.0/docs/v2.7.0-ReleaseNotes).
One of the highlight is that it is now possible to use hardware OPAL-based encryption of your disk with `cryptsetup`, it has a lot of caveats, see the above notes for the full details.

- A new option, `users.solitaryByDefault`, controls whether normal users default to a primary group named the same as the user, and with group id same as user id, or use the shared group "users" with group id 100.
System users always default to a group named after them, with GID = UID.
Per-user groups gives better control over permissions/file sharing and aligns NixOS with other distros like Arch Linux/Debian/Fedora/Ubuntu.
The option defaults to `system.stateVersion >= 24.11`, which means it's opt-in (to per-user groups) up to and including NixOS 24.05, but after that the option must be set to `false` to keep the current/old behaviour.

- `screen`'s module has been cleaned, and will now require you to set `programs.screen.enable` in order to populate `screenrc` and add the program to the environment.

- `linuxPackages_testing_bcachefs` is now fully deprecated by `linuxPackages_latest`, and is therefore no longer available.
Expand Down
76 changes: 49 additions & 27 deletions nixos/modules/config/users-groups.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
with lib;

let
nixosConfig = config;
ids = config.ids;
cfg = config.users;

Expand Down Expand Up @@ -114,7 +115,11 @@ let
group = mkOption {
type = types.str;
apply = x: assert (builtins.stringLength x < 32 || abort "Group name '${x}' is longer than 31 characters which is not allowed!"); x;
default = "";
default = name;
bjornfor marked this conversation as resolved.
Show resolved Hide resolved
defaultText = lib.literalMD ''
For normal users: if users.solitaryByDefault then <user_name> else "users"
For system users: <user_name>
'';
description = lib.mdDoc "The user's primary group.";
};

Expand Down Expand Up @@ -362,13 +367,15 @@ let
shell = mkIf config.useDefaultShell (mkDefault cfg.defaultUserShell);
}
(mkIf config.isNormalUser {
group = mkDefault "users";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
group = mkDefault "users";
group = lib.mkIf (lib.versionOlder nixosConfig.system.stateVersion "23.11") (mkDefault "users");

or like this

createHome = mkDefault true;
home = mkDefault "/home/${config.name}";
homeMode = mkDefault "700";
useDefaultShell = mkDefault true;
isSystemUser = mkDefault false;
})
(mkIf (config.isNormalUser && !nixosConfig.users.solitaryByDefault) {
group = mkDefault "users";
})
# If !mutableUsers, setting ‘initialPassword’ is equivalent to
# setting ‘password’ (and similarly for hashed passwords).
(mkIf (!cfg.mutableUsers && config.initialPassword != null) {
Expand Down Expand Up @@ -584,6 +591,16 @@ in {
'';
};

users.solitaryByDefault = mkOption {
type = types.bool;
default = lib.versionAtLeast nixosConfig.system.stateVersion "24.11";
description = lib.mdDoc ''
Whether normal users should default to a primary group named after
them. When disabled, normal users default to the "users" group. System
users always default to a group named after them.
'';
};

# systemd initrd
boot.initrd.systemd.users = mkOption {
description = ''
Expand Down Expand Up @@ -659,31 +676,36 @@ in {
};
};

users.groups = {
root.gid = ids.gids.root;
wheel.gid = ids.gids.wheel;
disk.gid = ids.gids.disk;
kmem.gid = ids.gids.kmem;
tty.gid = ids.gids.tty;
floppy.gid = ids.gids.floppy;
uucp.gid = ids.gids.uucp;
lp.gid = ids.gids.lp;
cdrom.gid = ids.gids.cdrom;
tape.gid = ids.gids.tape;
audio.gid = ids.gids.audio;
video.gid = ids.gids.video;
dialout.gid = ids.gids.dialout;
nogroup.gid = ids.gids.nogroup;
users.gid = ids.gids.users;
nixbld.gid = ids.gids.nixbld;
utmp.gid = ids.gids.utmp;
adm.gid = ids.gids.adm;
input.gid = ids.gids.input;
kvm.gid = ids.gids.kvm;
render.gid = ids.gids.render;
sgx.gid = ids.gids.sgx;
shadow.gid = ids.gids.shadow;
};
users.groups = mkMerge [
{
root.gid = ids.gids.root;
wheel.gid = ids.gids.wheel;
disk.gid = ids.gids.disk;
kmem.gid = ids.gids.kmem;
tty.gid = ids.gids.tty;
floppy.gid = ids.gids.floppy;
uucp.gid = ids.gids.uucp;
lp.gid = ids.gids.lp;
cdrom.gid = ids.gids.cdrom;
tape.gid = ids.gids.tape;
audio.gid = ids.gids.audio;
video.gid = ids.gids.video;
dialout.gid = ids.gids.dialout;
nogroup.gid = ids.gids.nogroup;
users.gid = ids.gids.users;
bjornfor marked this conversation as resolved.
Show resolved Hide resolved
nixbld.gid = ids.gids.nixbld;
utmp.gid = ids.gids.utmp;
adm.gid = ids.gids.adm;
input.gid = ids.gids.input;
kvm.gid = ids.gids.kvm;
render.gid = ids.gids.render;
sgx.gid = ids.gids.sgx;
shadow.gid = ids.gids.shadow;
}
# 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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@nbdd0121 nbdd0121 Apr 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In perlless NixOS, what code implements the dynamic UID/GID allocations? Is systemd sysusers used for normal users too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))

];

system.activationScripts.users = if !config.systemd.sysusers.enable then {
supportsDryActivation = true;
Expand Down
9 changes: 8 additions & 1 deletion nixos/modules/profiles/hardened.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,21 @@

with lib;

let
normalUsers = lib.filterAttrs (n: v: v.isNormalUser) config.users.users;
in
{
meta = {
maintainers = [ maintainers.joachifm maintainers.emily ];
};

boot.kernelPackages = mkDefault pkgs.linuxPackages_hardened;

nix.settings.allowed-users = mkDefault [ "@users" ];
nix.settings.allowed-users = mkDefault (
if config.users.solitaryByDefault then
lib.mapAttrsToList (n: v: v.name) normalUsers
else [ "@users" ]
);

environment.memoryAllocator.provider = mkDefault "scudo";
environment.variables.SCUDO_OPTIONS = mkDefault "ZeroContents=1";
Expand Down
1 change: 1 addition & 0 deletions nixos/modules/profiles/macos-builder.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ in
];
# swraid's default depends on stateVersion
config.boot.swraid.enable = false;
config.users.solitaryByDefault = true;
options.boot.isContainer = lib.mkOption { default = false; internal = true; };
}
];
Expand Down
39 changes: 39 additions & 0 deletions nixos/tests/user-group-membership.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import ./make-test-python.nix ({ lib, ... }: {
name = "user-group-membership";
meta = with lib.maintainers; { maintainers = [ bjornfor ]; };

nodes =
let
commonConfig = {
users.users.alice = {
isNormalUser = true;
};
users.users.bob = {
isNormalUser = true;
group = "users";
};
};
in
{
shared = {
imports = [ commonConfig ];
users.solitaryByDefault = false;
};
solitary = {
imports = [ commonConfig ];
users.solitaryByDefault = true;
};
};

testScript = ''
start_all()

with subtest("solitaryByDefault = false"):
shared.succeed('[ "$(stat -c %G /home/alice)" == "users" ]')
shared.succeed('[ "$(stat -c %G /home/bob)" == "users" ]')

with subtest("solitaryByDefault = true"):
solitary.succeed('[ "$(stat -c %G /home/alice)" == "alice" ]')
solitary.succeed('[ "$(stat -c %G /home/bob)" == "users" ]')
'';
})