~andir/nixpkgs-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 3

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

Klemens Nanni <klemens@posteo.de>
Details
Message ID
<20201122181332.80207-1-klemens@posteo.de>
DKIM signature
missing
Download raw message
Patch: +4 -5
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 if and only if the user mismatched;  this is
wrong as ownership is always ensured iff `createHome' is set:

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

Furthermode, 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.
---
 nixos/modules/config/update-users-groups.pl | 3 ++-
 nixos/modules/config/users-groups.nix       | 6 ++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/nixos/modules/config/update-users-groups.pl b/nixos/modules/config/update-users-groups.pl
index e220aa61090..0e1d66dfc64 100644
--- a/nixos/modules/config/update-users-groups.pl
+++ b/nixos/modules/config/update-users-groups.pl
@@ -211,10 +211,11 @@ foreach my $u (@{$spec->{users}}) {
        }
    }

    # Create a home directory.
    # Ensure home directory incl. ownership and permissions.
    if ($u->{createHome}) {
        make_path($u->{home}, { mode => 0700 }) if ! -e $u->{home};
        chown $u->{uid}, $u->{gid}, $u->{home};
        chmod 0700, $u->{home};
    }

    if (defined $u->{passwordFile}) {
diff --git a/nixos/modules/config/users-groups.nix b/nixos/modules/config/users-groups.nix
index 72285fe631d..a9576338098 100644
--- a/nixos/modules/config/users-groups.nix
+++ b/nixos/modules/config/users-groups.nix
@@ -198,10 +198,8 @@ let
        type = types.bool;
        default = false;
        description = ''
          If true, the home directory will be created automatically. If this
          option is true and the home directory already exists but is not
          owned by the user, directory owner and group will be changed to
          match the user.
          Whether to create the home directory and ensure ownership as well as
          permissions to match the user.
        '';
      };

-- 
2.29.2
Details
Message ID
<20201122183711.2qotag2pmipscued@ws.flokli.de>
In-Reply-To
<20201122181332.80207-1-klemens@posteo.de> (view parent)
DKIM signature
missing
Download raw message
Thanks for the patch! This should probably still have a entry in
nixos/doc/manual/release-notes/rl-2009.xml. Can you add one?

On 20-11-22 19:13:32, Klemens Nanni wrote:
>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 if and only if the user mismatched;  this is
>wrong as ownership is always ensured iff `createHome' is set:
>
>    if ($u->{createHome}) {
>        make_path($u->{home}, { mode => 0700 }) if ! -e $u->{home};
>        chown $u->{uid}, $u->{gid}, $u->{home};
>    }
>
>Furthermode, 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.
>---
> nixos/modules/config/update-users-groups.pl | 3 ++-
> nixos/modules/config/users-groups.nix       | 6 ++----
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/nixos/modules/config/update-users-groups.pl b/nixos/modules/config/update-users-groups.pl
>index e220aa61090..0e1d66dfc64 100644
>--- a/nixos/modules/config/update-users-groups.pl
>+++ b/nixos/modules/config/update-users-groups.pl
>@@ -211,10 +211,11 @@ foreach my $u (@{$spec->{users}}) {
>         }
>     }
>
>-    # Create a home directory.
>+    # Ensure home directory incl. ownership and permissions.
>     if ($u->{createHome}) {
>         make_path($u->{home}, { mode => 0700 }) if ! -e $u->{home};
>         chown $u->{uid}, $u->{gid}, $u->{home};
>+        chmod 0700, $u->{home};
>     }
>
>     if (defined $u->{passwordFile}) {
>diff --git a/nixos/modules/config/users-groups.nix b/nixos/modules/config/users-groups.nix
>index 72285fe631d..a9576338098 100644
>--- a/nixos/modules/config/users-groups.nix
>+++ b/nixos/modules/config/users-groups.nix
>@@ -198,10 +198,8 @@ let
>         type = types.bool;
>         default = false;
>         description = ''
>-          If true, the home directory will be created automatically. If this
>-          option is true and the home directory already exists but is not
>-          owned by the user, directory owner and group will be changed to
>-          match the user.
>+          Whether to create the home directory and ensure ownership as well as
>+          permissions to match the user.
>         '';
>       };
>
>-- 
>2.29.2

-- 
Florian Klink
Details
Message ID
<20201122183900.cpz5opff2cjqcspq@wrt>
In-Reply-To
<20201122183711.2qotag2pmipscued@ws.flokli.de> (view parent)
DKIM signature
missing
Download raw message
On 19:37 22.11.20, Florian Klink wrote:
> Thanks for the patch! This should probably still have a entry in
> nixos/doc/manual/release-notes/rl-2009.xml. Can you add one?

Correction: rl-2109.xml
Klemens Nanni <klemens@posteo.de>
Details
Message ID
<X7rpOmMDamQzHzMC@eru>
In-Reply-To
<20201122183900.cpz5opff2cjqcspq@wrt> (view parent)
DKIM signature
missing
Download raw message
Patch: +11 -5
On Sun, Nov 22, 2020 at 07:39:00PM +0100, andi@srht.l.notmuch.email wrote:
> On 19:37 22.11.20, Florian Klink wrote:
> > Thanks for the patch! This should probably still have a entry in
> > nixos/doc/manual/release-notes/rl-2009.xml. Can you add one?
> 
> Correction: rl-2109.xml

Subject: [PATCH] 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.
---
 nixos/doc/manual/release-notes/rl-2103.xml  | 7 +++++++
 nixos/modules/config/update-users-groups.pl | 3 ++-
 nixos/modules/config/users-groups.nix       | 6 ++----
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/nixos/doc/manual/release-notes/rl-2103.xml b/nixos/doc/manual/release-notes/rl-2103.xml
index 5c017c65a25..fee0b3716e5 100644
--- a/nixos/doc/manual/release-notes/rl-2103.xml
+++ b/nixos/doc/manual/release-notes/rl-2103.xml
@@ -183,6 +183,13 @@
  <title>Other Notable Changes</title>

  <itemizedlist>
   <listitem>
    <para>
     <xref linkend="opt-users.users._name_.createHome" /> now always ensures home directory permissions to be <literal>0700</literal>.
     Permissions had previously been ignored for already existing home directories, possibly leaving them readable by others.
     The option's description was incorrect regarding ownership management and has been simplified greatly.
    </para>
   </listitem>
   <listitem>
    <para>
     The default-version of <literal>nextcloud</literal> is <package>nextcloud20</package>.
diff --git a/nixos/modules/config/update-users-groups.pl b/nixos/modules/config/update-users-groups.pl
index e220aa61090..0e1d66dfc64 100644
--- a/nixos/modules/config/update-users-groups.pl
+++ b/nixos/modules/config/update-users-groups.pl
@@ -211,10 +211,11 @@ foreach my $u (@{$spec->{users}}) {
        }
    }

    # Create a home directory.
    # Ensure home directory incl. ownership and permissions.
    if ($u->{createHome}) {
        make_path($u->{home}, { mode => 0700 }) if ! -e $u->{home};
        chown $u->{uid}, $u->{gid}, $u->{home};
        chmod 0700, $u->{home};
    }

    if (defined $u->{passwordFile}) {
diff --git a/nixos/modules/config/users-groups.nix b/nixos/modules/config/users-groups.nix
index 72285fe631d..a9576338098 100644
--- a/nixos/modules/config/users-groups.nix
+++ b/nixos/modules/config/users-groups.nix
@@ -198,10 +198,8 @@ let
        type = types.bool;
        default = false;
        description = ''
          If true, the home directory will be created automatically. If this
          option is true and the home directory already exists but is not
          owned by the user, directory owner and group will be changed to
          match the user.
          Whether to create the home directory and ensure ownership as well as
          permissions to match the user.
        '';
      };

-- 
2.29.2
Reply to thread Export thread (mbox)