nixos/users-groups: createHome: Ensure HOME permissions, fix description #106995

andir · 2020-12-16T02:41:22Z

I'm forwarding this patch that I received via email:

commit 8833983f261c6afa0361465f31c4dbc39c45b386
Author: Klemens Nanni <klemens@posteo.de>
Date:   Sun Nov 22 23:42:02 2020 +0100

    nixos/users-groups: createHome: Ensure HOME permissions, fix description

    configuration.nix(1) states

        users.extraUsers.<name>.createHome
            [...] If [...] the home directory already exists but is not
            owned by the user, directory owner and group will be changed to
            match the user.

    i.e. ownership would change only if the user mismatched;  the code
    however ignores the owner, it is sufficient to enable `createHome`:

        if ($u->{createHome}) {
            make_path($u->{home}, { mode => 0700 }) if ! -e $u->{home};
            chown $u->{uid}, $u->{gid}, $u->{home};
        }

    Furthermore, permissions are ignored on already existing directories and
    therefore may allow others to read private data eventually.

    Given that createHome already acts as switch to not only create but
    effectively own the home directory, manage permissions in the same
    manner to ensure the intended default and cover all primary attributes.

    Avoid yet another configuration option to have administrators make a
    clear and simple choice between securely managing home directories
    and optionally defering management to own code (taking care of custom
    location, ownership, mode, extended attributes, etc.).

    While here, simplify and thereby fix misleading documentation.

configuration.nix(1) states users.extraUsers.<name>.createHome [...] If [...] the home directory already exists but is not owned by the user, directory owner and group will be changed to match the user. i.e. ownership would change only if the user mismatched; the code however ignores the owner, it is sufficient to enable `createHome`: if ($u->{createHome}) { make_path($u->{home}, { mode => 0700 }) if ! -e $u->{home}; chown $u->{uid}, $u->{gid}, $u->{home}; } Furthermore, permissions are ignored on already existing directories and therefore may allow others to read private data eventually. Given that createHome already acts as switch to not only create but effectively own the home directory, manage permissions in the same manner to ensure the intended default and cover all primary attributes. Avoid yet another configuration option to have administrators make a clear and simple choice between securely managing home directories and optionally defering management to own code (taking care of custom location, ownership, mode, extended attributes, etc.). While here, simplify and thereby fix misleading documentation.

aanderse

Thanks for the contribution klemens. How do you intend that I have users directories remain 0750 after this is merged? It is important that I retain the ability to effectively set the mode of home directories.

andir · 2020-12-16T14:57:15Z

On Wed, Dec 16, 2020 at 03:05:26PM +0100, ***@***.*** wrote: There was a comment on the PR (#106995 (review)): @aanderse commented on this pull request. Thanks for the contribution klemens. How do you intend that I have users directories remain `0750` after this is merged? It is important that I retain the ability to effectively set the mode of home directories.

The last paragraph how such scenarios are best dealt with manually, i.e. by not using `createHome': Avoid yet another configuration option to have administrators make a clear and simple choice between securely managing home directories and optionally defering management to own code (taking care of custom location, ownership, mode, extended attributes, etc.). One might also add an option for the mode, but I'd argue against it for the sake of avoiding feature creap; with mode, there's perhaps a custom group option around the corner as well, others might even want to fiddle timestamps on the home directory or what not. Enforcing `0750` is a deliberate choice to ensure a *secure* default.

aanderse · 2020-12-16T15:03:26Z

I understand that I could manually create accounts, but I'm not interested in that. I'm interested in running NixOS with users.mutableUsers = false; and choosing what permissions I want on my home directories.

To clarify - I like 0700 as a default, but I would like the option to choose (and in some cases I need the option to choose).

andir · 2020-12-16T16:07:39Z

----- Forwarded message from Klemens Nanni <klemens@posteo.de> -----

----- Forwarded message from Aaron Andersen ***@***.***> ----- I understand that I could manually create accounts, but I'm not interested in that. I'm interested in running NixOS with `users.mutableUsers = false;` and **choosing** what permissions I want on my home directories.

I'm running with `mutableUsers = false' *and* `creatHome = true' as well, and yet I am able to change home directory permissions without NixOS noticing it. I consider this an implementation bug in nixpkgs, the fact that it matches your desired use case therefore seems to be a concidence.

To clarify - I like `0700` as a default, but I would like the option to choose (and in some cases I *need* the option to choose).

Feel free to implement such option, it should be straight forward and go as its own patch on top of my changes. Personally, I am not interested in adding a `mode' option. Since you like even stricter default permissions, my patch should go inline with your intentions as is, then. Assuming this is a possible way to go, you'd simply have to set `createHome = false' after my change has been merged until you write yours.

…

----- End forwarded message -----

aanderse

After reviewing and considering this PR a bit more I have decided I'm incorrect and this change is good in my opinion 👍 This change does allow declarative, flexible, user creation when combined with tmpfiles:

users.users.foo = {
  isNormalUser = false;
  createHome = false;
};

systemd.tmpfiles.rules = [
  "d ${config.users.users.foo} 0750 foo users"
];

andir · 2020-12-16T17:35:03Z

----- Forwarded message from Klemens Nanni <klemens@posteo.de> -----

----- Forwarded message from Aaron Andersen ***@***.***> ----- @aanderse approved this pull request. After reviewing and considering this PR a bit more I have decided I'm incorrect and this change is good in my opinion 👍 This change **does** allow declarative, flexible, user creation when combined with `tmpfiles`: ``` users.users.foo = { isNormalUser = false; createHome = false; }; systemd.tmpfiles.rules = [ "d ${config.users.users.foo} 0750 foo users" ]; ``` ----- End forwarded message -----

Cool if that works for you, although it comes off as a hack to me: now two components race for or at least manage both ownership and mode of the home directory. Misconfiguration of the tmpfiles.d(5) line cloud lead to time based cleanup of the home directory!

…

----- End forwarded message -----

aanderse · 2020-12-16T17:38:47Z

Yeah, sure, tmpfiles isn't a great solution... but it is declarative, can be made relatively reproducible (under certain circumstances), and will get the job done in my scenario where I need it to.

As far as racing goes - this is not a race, there is a strict ordering here I believe: user creation, followed by tmpfiles creation.

nixos-discourse · 2021-06-14T17:09:34Z

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

https://discourse.nixos.org/t/apache-2-4-47-permission-denied-access-to-denied-file-system-path-home-bavramor-data/13622/3

ofborg bot added 6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) labels Dec 16, 2020

andir changed the title ~~[PATCH] nixos/users-groups: createHome: Ensure HOME permissions, fix description~~ nixos/users-groups: createHome: Ensure HOME permissions, fix description Dec 16, 2020

andir requested review from Mic92 and adisbladis December 16, 2020 02:50

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Dec 16, 2020

aanderse reviewed Dec 16, 2020

View reviewed changes

aanderse approved these changes Dec 16, 2020

View reviewed changes

andir merged commit fa0d499 into NixOS:master Dec 17, 2020

andir deleted the ml2pr/PATCH-nixos-users-groups-createHome-Ensure-HOME-permissions-fix-description branch December 17, 2020 16:23

aanderse mentioned this pull request Mar 7, 2021

nixos/users: require one of users.users.name.{isSystemUser,isNormalUser} #115332

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/users-groups: createHome: Ensure HOME permissions, fix description #106995

nixos/users-groups: createHome: Ensure HOME permissions, fix description #106995

andir commented Dec 16, 2020

aanderse left a comment

andir commented Dec 16, 2020 via email

aanderse commented Dec 16, 2020

andir commented Dec 16, 2020 via email

aanderse left a comment

andir commented Dec 16, 2020 via email

aanderse commented Dec 16, 2020

nixos-discourse commented Jun 14, 2021

nixos/users-groups: createHome: Ensure HOME permissions, fix description #106995

nixos/users-groups: createHome: Ensure HOME permissions, fix description #106995

Conversation

andir commented Dec 16, 2020

aanderse left a comment

Choose a reason for hiding this comment

andir commented Dec 16, 2020 via email

aanderse commented Dec 16, 2020

andir commented Dec 16, 2020 via email

aanderse left a comment

Choose a reason for hiding this comment

andir commented Dec 16, 2020 via email

aanderse commented Dec 16, 2020

nixos-discourse commented Jun 14, 2021