bug#24275: Misnamed directory in GuixSD

bug#24275: Misnamed directory in GuixSD

[John Darrington]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

Installing GuixSD 0.11.0 creates a directory called /nonexistent
Despite its name, it does actually exist.

I suggest that we rename it or delete it.

J'

-- 
Avoid eavesdropping.  Send strong encryted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

bug#24275: Misnamed directory in GuixSD

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]

On Sat, Aug 20, 2016 at 10:11:00PM +0200, John Darrington wrote:
> Installing GuixSD 0.11.0 creates a directory called /nonexistent
> Despite its name, it does actually exist.

I checked, and it's the 'nobody' user's home directory:
http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/system/shadow.scm#n125

> I suggest that we rename it or delete it.

I think it's very appropriate for nobody to live in a nonexistent
directory ;)

But, I noticed that the directory doesn't exist on my GuixSD system,
which has been reconfigured since the commit that introduced this
directory (2d94702ff). My nobody is still using '/var/empty'.

I wonder if that's a problem?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#24275: Misnamed directory in GuixSD

[Alex Kost]

Leo Famulari (2016-08-22 02:14 +0300) wrote:

> On Sat, Aug 20, 2016 at 10:11:00PM +0200, John Darrington wrote:
>> Installing GuixSD 0.11.0 creates a directory called /nonexistent
>> Despite its name, it does actually exist.
>
> I checked, and it's the 'nobody' user's home directory:
> http://git.savannah.gnu.org/cgit/guix.git/tree/gnu/system/shadow.scm#n125
>
>> I suggest that we rename it or delete it.
>
> I think it's very appropriate for nobody to live in a nonexistent
> directory ;)
>
> But, I noticed that the directory doesn't exist on my GuixSD system,
> which has been reconfigured since the commit that introduced this
> directory (2d94702ff). My nobody is still using '/var/empty'.

The same for me.

> I wonder if that's a problem?

IIUC it happens because the home directory is created only when a user
is added, and is not changed when the user is modified.  See (gnu build
activation) module:

- 'add-user' runs "useradd" with "-d" option to create home dir

- 'modify-user' runs "usermod" without "-d" (and without "--move-home")

So the home of nobody was not changed for us to '/nonexistent' when the
nobody user was changed.

As for me, I wouldn't like to have this directory, and I think it
shouldn't be created (if it is not really needed for nobody user).

-- 
Alex

bug#24275: Misnamed directory in GuixSD

[Vincent Legoll]

Hello,

> IIUC it happens because the home directory is created only when a user
> is added, and is not changed when the user is modified.  See (gnu build
> activation) module:
>
> - 'add-user' runs "useradd" with "-d" option to create home dir

Maybe the nobody user should be special cased, not to run useradd with
-d, the non existent directory, should really not exist for nobody. This is a
(very small ?) security enhancement, I think...

If this is the way to go, I can have a shot at it...

> - 'modify-user' runs "usermod" without "-d" (and without "--move-home")
>
> So the home of nobody was not changed for us to '/nonexistent' when the
> nobody user was changed.
>
> As for me, I wouldn't like to have this directory, and I think it
> shouldn't be created (if it is not really needed for nobody user).

Ditto.

-- 
Vincent Legoll

bug#24275: Misnamed directory in GuixSD

[Leo Famulari]

On Mon, Aug 22, 2016 at 10:47:51AM +0200, Vincent Legoll wrote:
> Hello,
> 
> > IIUC it happens because the home directory is created only when a user
> > is added, and is not changed when the user is modified.  See (gnu build
> > activation) module:
> >
> > - 'add-user' runs "useradd" with "-d" option to create home dir
> 
> Maybe the nobody user should be special cased, not to run useradd with
> -d, the non existent directory, should really not exist for nobody. This is a
> (very small ?) security enhancement, I think...

My Debian system uses '/nonexistent' for the nobody user's passwd entry,
but the directory does not actually exist.

> If this is the way to go, I can have a shot at it...
> 
> > - 'modify-user' runs "usermod" without "-d" (and without "--move-home")
> >
> > So the home of nobody was not changed for us to '/nonexistent' when the
> > nobody user was changed.
> >
> > As for me, I wouldn't like to have this directory, and I think it
> > shouldn't be created (if it is not really needed for nobody user).
> 
> Ditto.

I don't fully understand the implications of the change, but it seems
like a worthwhile thing to try doing. At least you might learn something
while implementing it :)

I'll let more experienced people decide if it's the right thing to do.

bug#24275: Misnamed directory in GuixSD

[Vincent Legoll]

[-- Attachment #1: Type: text/plain, Size: 1741 bytes --]

Hello,

On Mon, Aug 22, 2016 at 8:09 PM, Leo Famulari <leo@famulari.name> wrote:
> On Mon, Aug 22, 2016 at 10:47:51AM +0200, Vincent Legoll wrote:
>>
>> > IIUC it happens because the home directory is created only when a user
>> > is added, and is not changed when the user is modified.  See (gnu build
>> > activation) module:
>> >
>> > - 'add-user' runs "useradd" with "-d" option to create home dir
>>
>> Maybe the nobody user should be special cased, not to run useradd with
>> -d, the non existent directory, should really not exist for nobody. This is a
>> (very small ?) security enhancement, I think...
>
> My Debian system uses '/nonexistent' for the nobody user's passwd entry,
> but the directory does not actually exist.
>
>> If this is the way to go, I can have a shot at it...
>>
>> > - 'modify-user' runs "usermod" without "-d" (and without "--move-home")
>> >
>> > So the home of nobody was not changed for us to '/nonexistent' when the
>> > nobody user was changed.
>> >
>> > As for me, I wouldn't like to have this directory, and I think it
>> > shouldn't be created (if it is not really needed for nobody user).
>>
>> Ditto.
>
> I don't fully understand the implications of the change, but it seems
> like a worthwhile thing to try doing. At least you might learn something
> while implementing it :)
>
> I'll let more experienced people decide if it's the right thing to do.

I came with the attached patch, totally untested, probably wrong for some
cases...

The following is what I think I have implemented:

At account creation time, do not create directories for system? accounts.

At account modification, do not create directories, nor move existing ones,
but change them in /etc/passwd

WDYT ?

-- 
Vincent Legoll

[-- Attachment #2: 0001-Avoid-creating-system-user-s-home-directories.patch --]
[-- Type: text/x-patch, Size: 2346 bytes --]

From 8c83d8cebc3b440a523e714e652b266f7c37b380 Mon Sep 17 00:00:00 2001
From: Vincent Legoll <vincent.legoll@idgrilles.fr>
Date: Tue, 23 Aug 2016 12:37:57 +0200
Subject: [PATCH] Avoid creating system-user's home directories

* gnu/build/activation.scm (modify-user): pass -d to usermod command
                           (add-user): add system? condition to home
                                       dir creation.

Signed-off-by: Vincent Legoll <vincent.legoll@idgrilles.fr>
---
 gnu/build/activation.scm | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 6666cb4..c0f54ae 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -140,9 +140,13 @@ properties.  Return #t on success."
                           '())
                     ,@(if comment `("-c" ,comment) '())
                     ,@(if home
-                          (if (file-exists? home)
-                              `("-d" ,home)     ; avoid warning from 'useradd'
-                              `("-d" ,home "--create-home"))
+                          ;; system? accounts may have non existent home
+                          ;; directories (for example, user nobody)
+                          (if system?
+                              `("-d" ,home)
+                              (if (file-exists? home)
+                                  `("-d" ,home)     ; avoid warning from 'useradd'
+                                  `("-d" ,home "--create-home")))
                           '())
                     ,@(if shell `("-s" ,shell) '())
                     ,@(if password `("-p" ,password) '())
@@ -169,7 +173,10 @@ properties.  Return #t on success."
                       `("-G" ,(string-join supplementary-groups ","))
                       '())
                 ,@(if comment `("-c" ,comment) '())
-                ;; Don't use '--move-home', so ignore HOME.
+                ;; The home directory could have changed, but may be a
+                ;; nonexistent one, so don't use '--move-home'. Manually
+                ;; cleaning things up may be needed in such a case
+                ,@(if home `("-d" ,home) '())
                 ,@(if shell `("-s" ,shell) '())
                 ,name)))
     (zero? (apply system* "usermod" args))))
-- 
1.9.1

bug#24275: Misnamed directory in GuixSD

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

Hello!

What an embarrassing bug.  :-)

Vincent Legoll <vincent.legoll@gmail.com> skribis:

> I came with the attached patch, totally untested, probably wrong for some
> cases...
>
> The following is what I think I have implemented:
>
> At account creation time, do not create directories for system? accounts.
>
> At account modification, do not create directories, nor move existing ones,
> but change them in /etc/passwd
>
> WDYT ?

We currently lack a way to specify whether the home directory should be
created, which would be useful for ‘nobody’.

So what about a patch along these lines instead?  It adds a
‘create-home-directory?’ field to <user-account> and sets it to #f for
‘nobody’.

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3981 bytes --]

diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 6666cb4..10aa58d 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -110,7 +110,8 @@ owner-writable in HOME."
               files)))
 
 (define* (add-user name group
-                   #:key uid comment home shell password system?
+                   #:key uid comment home create-home?
+                   shell password system?
                    (supplementary-groups '())
                    (log-port (current-error-port)))
   "Create an account for user NAME part of GROUP, with the specified
@@ -139,7 +140,7 @@ properties.  Return #t on success."
                           `("-G" ,(string-join supplementary-groups ","))
                           '())
                     ,@(if comment `("-c" ,comment) '())
-                    ,@(if home
+                    ,@(if (and home create-home?)
                           (if (file-exists? home)
                               `("-d" ,home)     ; avoid warning from 'useradd'
                               `("-d" ,home "--create-home"))
@@ -158,7 +159,8 @@ properties.  Return #t on success."
                #t)))))
 
 (define* (modify-user name group
-                      #:key uid comment home shell password system?
+                      #:key uid comment home create-home?
+                      shell password system?
                       (supplementary-groups '())
                       (log-port (current-error-port)))
   "Modify user account NAME to have all the given settings."
@@ -186,7 +188,8 @@ logged in."
   (zero? (system* "groupdel" name)))
 
 (define* (ensure-user name group
-                      #:key uid comment home shell password system?
+                      #:key uid comment home create-home?
+                      shell password system?
                       (supplementary-groups '())
                       (log-port (current-error-port))
                       #:rest rest)
@@ -207,7 +210,8 @@ numeric gid or #f."
 
   (define activate-user
     (match-lambda
-     ((name uid group supplementary-groups comment home shell password system?)
+     ((name uid group supplementary-groups comment home create-home?
+       shell password system?)
       (let ((profile-dir (string-append "/var/guix/profiles/per-user/"
                                         name)))
         (ensure-user name group
@@ -216,6 +220,7 @@ numeric gid or #f."
                      #:supplementary-groups supplementary-groups
                      #:comment comment
                      #:home home
+                     #:create-home? create-home?
                      #:shell shell
                      #:password password)
 
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index c394890..be08646 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -78,6 +78,8 @@
                         (default '()))            ; list of strings
   (comment        user-account-comment (default ""))
   (home-directory user-account-home-directory)
+  (create-home-directory? user-account-create-home-directory? ;Boolean
+                          (default #f))
   (shell          user-account-shell              ; gexp
                   (default #~(string-append #$bash "/bin/bash")))
   (system?        user-account-system?            ; Boolean
@@ -128,6 +130,7 @@
          (group "nogroup")
          (shell #~(string-append #$shadow "/sbin/nologin"))
          (home-directory "/nonexistent")
+         (create-home-directory? #f)
          (system? #t))))
 
 (define (default-skeletons)
@@ -255,6 +258,7 @@ of user '~a' is undeclared")
       #$(user-account-supplementary-groups account)
       #$(user-account-comment account)
       #$(user-account-home-directory account)
+      #$(user-account-create-home-directory? account)
       ,#$(user-account-shell account)             ; this one is a gexp
       #$(user-account-password account)
       #$(user-account-system? account)))

bug#24275: Misnamed directory in GuixSD

[Vincent Legoll]

On Sun, Aug 28, 2016 at 1:32 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> Vincent Legoll <vincent.legoll@gmail.com> skribis:
>
>> I came with the attached patch, totally untested, probably wrong for some
>> cases...

My patch was heuristic, so bound to be wrong sometimes...

> We currently lack a way to specify whether the home directory should be
> created, which would be useful for ‘nobody’.

This is the alternative solution, specifying it, always right.

> So what about a patch along these lines instead?  It adds a
> ‘create-home-directory?’ field to <user-account> and sets it to #f for
> ‘nobody’.

LGTM, but do I understand correctly: the default value being false, we
will have to always specify the added param as true in system definitions ?

Why not the other way around, that would not need any modifications of
current configs.

-- 
Vincent Legoll

bug#24275: Misnamed directory in GuixSD

[Ludovic Courtès]

Hello!

Vincent Legoll <vincent.legoll@gmail.com> skribis:

> On Sun, Aug 28, 2016 at 1:32 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Vincent Legoll <vincent.legoll@gmail.com> skribis:
>>
>>> I came with the attached patch, totally untested, probably wrong for some
>>> cases...
>
> My patch was heuristic, so bound to be wrong sometimes...
>
>> We currently lack a way to specify whether the home directory should be
>> created, which would be useful for ‘nobody’.
>
> This is the alternative solution, specifying it, always right.

Yeah, that’s what I thought.  It may be useful in other situations too.

>> So what about a patch along these lines instead?  It adds a
>> ‘create-home-directory?’ field to <user-account> and sets it to #f for
>> ‘nobody’.
>
> LGTM, but do I understand correctly: the default value being false, we
> will have to always specify the added param as true in system definitions ?
>
> Why not the other way around, that would not need any modifications of
> current configs.

Oops, I meant it to be #t by default, of course.  Good catch!

Pushed as commits eb56ee027b4c6b5682f69fa885d16e55c4495bd8 and
d03db7434b5b2f1e588c65805d38faea973c3cf0.

Thanks!

Ludo’.

bug#24275: Misnamed directory in GuixSD

[Vincent Legoll]

BTW,

I think this may be related to this subject, so...

I've found some user account (other than nobody)
that have /var/empty as their home directory, f.e.:

gnu/services/networking.scm:301

(define %ntp-accounts
  (list (user-account
         (name "ntpd")
         (group "nogroup")
         (system? #t)
         (comment "NTP daemon user")
         (home-directory "/var/empty")
         (shell #~(string-append #$shadow "/sbin/nologin")))))

Is that a glitch, and they can be moved to /nonexistent or do
they really require an existing directory to work ?

-- 
Vincent Legoll

bug#24275: Misnamed directory in GuixSD

[Ludovic Courtès]

Hi,

Vincent Legoll <vincent.legoll@gmail.com> skribis:

> I've found some user account (other than nobody)
> that have /var/empty as their home directory, f.e.:
>
> gnu/services/networking.scm:301
>
> (define %ntp-accounts
>   (list (user-account
>          (name "ntpd")
>          (group "nogroup")
>          (system? #t)
>          (comment "NTP daemon user")
>          (home-directory "/var/empty")
>          (shell #~(string-append #$shadow "/sbin/nologin")))))
>
> Is that a glitch, and they can be moved to /nonexistent or do
> they really require an existing directory to work ?

I don’t know.  /var/empty is a widespread convention.  But it probably
doesn’t matter much in fact.  :-)

Ludo’.