bug#36508: GDM files have incorrect owner after temporarily replacing with SDDM

bug#36508: GDM files have incorrect owner after temporarily replacing with SDDM

[ison]

After replacing GDM with SDDM in my Guix System config (to test Wayland) and
then reverting back to my old config and reconfiguring GDM would crash
(printing out around 500 lines about creating a seat)
I also tried rolling back to the generation I had before using SDDM and it would
still crash.
In both instances I also tried "herd restart xorg-server" but same problem.

I then checked the log file /var/log/gdm/greeter.log which had errors such as:
-------------------
Fatal server error:
(EE) Cannot open log file "/var/lib/gdm/.local/share/xorg/Xorg.pid-720.log"
-------------------

And then I could verify that files inside of /var/lib/gdm had incorrect
ownership of 9##:gdm
where 9## was some 3-digit number I can't remember now.
(note: the directory itself /var/lib/gdm still had correct ownership gdm:gdm)

I then manually fixed the ownership with:
chown -R gdm:gdm /var/lib/gdm
and GDM successfully came up without crashing.

The relevant portion of my config when I replaced GDM with SDDM was:
-------------------------------
(operating-system
  ...
  (services
    (cons*
      ...
      (sddm-service
        (sddm-configuration
          (display-server "wayland")))

      ;; Return %desktop-services with GDM removed
      (remove (lambda (service)
                (eq? (service-kind service) gdm-service-type))
              %desktop-services))))
-------------------------------

bug#36508: GDM files have incorrect owner after temporarily removing service

[Brendan Tildesley via Bug reports for GNU Guix]

[-- Attachment #1.1: Type: text/plain, Size: 1054 bytes --]

I recently encountered what is likely the same bug. The directory /var/lib/gdm
had the correct permissions gdm:gdm, but all the files inside had something like
973:gdm

a43e9157ef479e94c19951cc9d228cf153bf78ee is supposed to fix this (duplicate bug
37423) but it only checks the permissions of /var/lib/gdm/ itself. Not all of
the files in it. This explains why in my case it failed to fix the permissions,
because the directory was gdm:gdm. How it got that way I don't know, and infact
it doesn't really matter. The directory is mutable, and thus can theoretically be
changed for any number of reasons. Therefore if we wish for Guix to be robust
with it's Functional design, and have meaningful rollbacks, we perhaps have no
choice but to assert the required invariants like these on mutable files.

A better solution may be to make it fully chown -R on reconfigure, but not each time
on boot?

I've attached an untested patch with a suggested solution of making
%gdm-activation operate every single time, instead of just after checking
/var/lib/gdm.



[-- Attachment #1.2: Type: text/html, Size: 1658 bytes --]

[-- Attachment #2: 0001-services-gdm-Correctly-set-ownership-on-var-lib-gdm.patch --]
[-- Type: text/x-patch, Size: 2015 bytes --]

From 31cb6dbd756af695bd6a1f4d4c89b42367b13307 Mon Sep 17 00:00:00 2001
From: Brendan Tildesley <mail@brendan.scot>
Date: Tue, 13 Apr 2021 23:04:28 +1000
Subject: [PATCH] services: gdm: Correctly set ownership on /var/lib/gdm.

* gnu/services/xorg.scm (%gdm-activation): Always chown /var/lib/gdm,
instead of only when it appears to be correct, because it's still
possible the files inside could be wrong and break GDM. I encountered
this once: https://issues.guix.gnu.org/36508 .

Perhaps it is with good intentions to try not running this code every
single time on boot, but when it fails, the consequence is that GDM can
break not just for the current revision, but all previous rollback
systems in GRUB will fail, and subsequent reconfigure-ings fail
too. That totally destroys a desktop system and our rollback
functionally, which is much much worse!
---
 gnu/services/xorg.scm | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index 17d983ff8d..a206c7c93a 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -861,16 +861,11 @@ the GNOME desktop environment.")
 
         (let* ((gdm (getpwnam "gdm"))
                (uid (passwd:uid gdm))
-               (gid (passwd:gid gdm))
-               (st  (stat "/var/lib/gdm" #f)))
-          ;; Recurse into /var/lib/gdm only if it has wrong ownership.
-          (when (and st
-                     (or (not (= uid (stat:uid st)))
-                         (not (= gid (stat:gid st)))))
-            (for-each (lambda (file)
-                        (chown file uid gid))
-                      (find-files "/var/lib/gdm"
-                                  #:directories? #t)))))))
+               (gid (passwd:gid gdm)))
+          (for-each (lambda (file)
+                      (chown file uid gid))
+                    (find-files "/var/lib/gdm"
+                                #:directories? #t))))))
 
 (define dbus-daemon-wrapper
   (program-file
-- 
2.31.1

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Hi Brendan,

Brendan Tildesley via Bug reports for GNU Guix <bug-guix@gnu.org>
writes:

> I recently encountered what is likely the same bug. The directory /var/lib/gdm
> had the correct permissions gdm:gdm, but all the files inside had something like
> 973:gdm

The underlying problem here, which I've also experienced, is that if you
reconfigure your system with fewer users/groups, and then later add
those users/groups back, there is no guarantee that they will be
assigned the same UIDs and GIDs.

This problem is made much worse by the fact that files may be left
around, e.g. in /var, with the old UIDs and GIDs.

In your case, I guess that the 'gdm' user was previously assigned UID
973, but now it has been given a different UID.

In my case, after reconfiguring to a minimal system and later switching
back to a full GNOME-based desktop system, I found that many files and
directories in /var had the wrong owner or group.  Here's what I saw
before I cleaned things up:

--8<---------------cut here---------------start------------->8---
root@jojen ~# ls -l /var/lib/
total 4
drwxr-xr-x 1 colord colord    40 Mar 28  2017 colord
drwx------ 1 995    978       56 Sep  3 02:10 gdm
drwx------ 1 root   root   30400 Dec 25 01:55 NetworkManager
-rw------- 1 root   root     512 Dec 25 01:35 random-seed
drwxr-xr-x 1 colord colord   164 Dec 28  2017 sddm
drwx------ 1 tor    tor      178 Dec 19 21:28 tor
drwx------ 1 root   root      20 Sep  5 01:32 udisks2
drwxr-xr-x 1 root   root     274 Dec 25 01:55 upower
drwxr-xr-x 1 root   root      86 Mar 28  2017 wicd
root@jojen ~# ls -la /var/lib/gdm/
total 4
drwx------ 1  995    978  56 Sep  3 02:10 .
drwxr-xr-x 1 root root   750 Dec 25 01:59 ..
drwxr-xr-x 1  994 colord  64 Sep  3 02:10 .cache
drwx------ 1  994 colord  54 Sep  3 02:10 .config
-rw------- 1  994 colord  16 Sep  3 02:10 .esd_auth
drwxr-xr-x 1  994 colord  10 Sep  3 02:10 .local
root@jojen ~# 
--8<---------------cut here---------------end--------------->8---

Given the fact that existing files and directories in /var can
*effectively* have their ownership changed, I think that this issue
could be a security risk.

There's some discussion of this issue at <https://bugs.gnu.org/44944>,
although I'm not sure that Danny's suggested solution is practical.

Here's one idea: when activating a system, *never* delete users or
groups if files still exist that are owned by those users/groups.
Checking all filesystems would likely be too expensive, but perhaps it
would be sufficient to check certain directories such as /var, /etc, and
possibly the top directory of /home.

What do you think?

      Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Brendan Tildesley via Bug reports for GNU Guix]

> On 04/13/2021 10:51 PM Mark H Weaver <mhw@netris.org> wrote:
> 
>  
> Hi Brendan,
> 
> Brendan Tildesley via Bug reports for GNU Guix <bug-guix@gnu.org>
> writes:
> 
> > I recently encountered what is likely the same bug. The directory /var/lib/gdm
> > had the correct permissions gdm:gdm, but all the files inside had something like
> > 973:gdm
> 
> The underlying problem here, which I've also experienced, is that if you
> reconfigure your system with fewer users/groups, and then later add
> those users/groups back, there is no guarantee that they will be
> assigned the same UIDs and GIDs.
> 
> This problem is made much worse by the fact that files may be left
> around, e.g. in /var, with the old UIDs and GIDs.
> 
> In your case, I guess that the 'gdm' user was previously assigned UID
> 973, but now it has been given a different UID.
> 
> In my case, after reconfiguring to a minimal system and later switching
> back to a full GNOME-based desktop system, I found that many files and
> directories in /var had the wrong owner or group.  Here's what I saw
> before I cleaned things up:
> 
> --8<---------------cut here---------------start------------->8---
> root@jojen ~# ls -l /var/lib/
> total 4
> drwxr-xr-x 1 colord colord    40 Mar 28  2017 colord
> drwx------ 1 995    978       56 Sep  3 02:10 gdm
> drwx------ 1 root   root   30400 Dec 25 01:55 NetworkManager
> -rw------- 1 root   root     512 Dec 25 01:35 random-seed
> drwxr-xr-x 1 colord colord   164 Dec 28  2017 sddm
> drwx------ 1 tor    tor      178 Dec 19 21:28 tor
> drwx------ 1 root   root      20 Sep  5 01:32 udisks2
> drwxr-xr-x 1 root   root     274 Dec 25 01:55 upower
> drwxr-xr-x 1 root   root      86 Mar 28  2017 wicd
> root@jojen ~# ls -la /var/lib/gdm/
> total 4
> drwx------ 1  995    978  56 Sep  3 02:10 .
> drwxr-xr-x 1 root root   750 Dec 25 01:59 ..
> drwxr-xr-x 1  994 colord  64 Sep  3 02:10 .cache
> drwx------ 1  994 colord  54 Sep  3 02:10 .config
> -rw------- 1  994 colord  16 Sep  3 02:10 .esd_auth
> drwxr-xr-x 1  994 colord  10 Sep  3 02:10 .local
> root@jojen ~# 
> --8<---------------cut here---------------end--------------->8---
> 
> Given the fact that existing files and directories in /var can
> *effectively* have their ownership changed, I think that this issue
> could be a security risk.

Yes and they could change for any reason under the sun, and so we have no
choice but to set them right on service activation.

Guix system rollbacks should be a supported feature of Guix, not just a gimmick
that falls out of its design. It should be that a Guix user could leave their
system for 5 years, and then do a guix pull; guix system reconfigure in the year
2026. Perhaps at that time the new system will break, and then its desirable
that they can rollback to the previous generation. So what fixes we put in to 
Guix services today need to consider not just how files could have changed in
the past, but how they might change in breaking ways in the future, within reason.
I don't know off the top of my head of any way that can be done other than to
have chmod -R gdm:gdm /var/lib/gdm always executed.
> 
> There's some discussion of this issue at <https://bugs.gnu.org/44944>,
> although I'm not sure that Danny's suggested solution is practical.
> 
> Here's one idea: when activating a system, *never* delete users or
> groups if files still exist that are owned by those users/groups.
> Checking all filesystems would likely be too expensive, but perhaps it
> would be sufficient to check certain directories such as /var, /etc, and
> possibly the top directory of /home.
> 
> What do you think

Wouldn't that imply that uids could be randomly different on different systems
with the same configuration, and then remain statically different permanently?
We want as little randomness and moving parts as possible. It's yet another
way the system is not actually Functional, but has state.

Seems this bug spans 3 or so different bug reports. In http://issues.guix.gnu.org/45571
I commented that Nix uses hard coded id's, sorta like how ports are allocated
for a purpose:

https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/misc/ids.nix

Perhaps you are thinking of other kinds of security issues that could be caused that
I'm not thinking of. In that case maybe Nix devs have already made the best choice by
making them static?

... After all, if the permissions can change, then it is possible another user could
actually modify the contents of /var/lib/gdm its self, thereby infecting other users,
if for some reason that other malicious user gets allocated that ID.
That further points towards static ID's like Nix has as a solution.

bug#36508: GDM files have incorrect owner after temporarily removing service

[Ludovic Courtès]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> Brendan Tildesley via Bug reports for GNU Guix <bug-guix@gnu.org>
> writes:
>
>> I recently encountered what is likely the same bug. The directory /var/lib/gdm
>> had the correct permissions gdm:gdm, but all the files inside had something like
>> 973:gdm
>
> The underlying problem here, which I've also experienced, is that if you
> reconfigure your system with fewer users/groups, and then later add
> those users/groups back, there is no guarantee that they will be
> assigned the same UIDs and GIDs.

Yes.

The patch Brendan posted LGTM (though I’m surprised the directory itself
can have the right UID/GID while files inside it don’t; perhaps this was
made possible by 2161820ebbbab62a5ce76c9101ebaec54dc61586, which chowns
the home directory unconditionally.)

Note that there are other places, in addition to GDM, where we
forcefully reset the UID/GID of the home directory (e.g., for the
‘knot-resolver’ service.)

My preferred solution to this would be to unconditionally chown -R home
directories upon activation (for efficiency, it would be best if we
could do that if and only if the home directory itself has wrong
ownership).  Thoughts?

systemd-homed does something like that.  The intuition here is that
UIDs/GIDs are implementation details that should get out of the way.

> There's some discussion of this issue at <https://bugs.gnu.org/44944>,
> although I'm not sure that Danny's suggested solution is practical.
>
> Here's one idea: when activating a system, *never* delete users or
> groups if files still exist that are owned by those users/groups.
> Checking all filesystems would likely be too expensive, but perhaps it
> would be sufficient to check certain directories such as /var, /etc, and
> possibly the top directory of /home.

How would you determine which directories to look at though?  What if we
miss an important one?

Note that the ID allocation strategy in (gnu build accounts) ensures
UIDs/GIDs aren’t reused right away (same strategy as implemented by
Shadow, etc.).  So if you remove “bob”, then add “alice”, “alice” won’t
be able to access the left-behind /home/bob because it has a different
UID.

Ludo’.

bug#36508: GDM files have incorrect owner after temporarily removing service

[Brendan Tildesley via Bug reports for GNU Guix]

> On 04/14/2021 12:32 PM Ludovic Courtès <ludo@gnu.org> wrote:
> 
>  
> Hi Mark,
> 
> Mark H Weaver <mhw@netris.org> skribis:
> 
> > Brendan Tildesley via Bug reports for GNU Guix <bug-guix@gnu.org>
> > writes:
> >
> >> I recently encountered what is likely the same bug. The directory /var/lib/gdm
> >> had the correct permissions gdm:gdm, but all the files inside had something like
> >> 973:gdm
> >
> > The underlying problem here, which I've also experienced, is that if you
> > reconfigure your system with fewer users/groups, and then later add
> > those users/groups back, there is no guarantee that they will be
> > assigned the same UIDs and GIDs.
> 
> Yes.
> 
> The patch Brendan posted LGTM (though I’m surprised the directory itself
> can have the right UID/GID while files inside it don’t; perhaps this was
> made possible by 2161820ebbbab62a5ce76c9101ebaec54dc61586, which chowns
> the home directory unconditionally.)
> 
> Note that there are other places, in addition to GDM, where we
> forcefully reset the UID/GID of the home directory (e.g., for the
> ‘knot-resolver’ service.)
> 
> My preferred solution to this would be to unconditionally chown -R home
> directories upon activation (for efficiency, it would be best if we
> could do that if and only if the home directory itself has wrong
> ownership).  Thoughts?
> 
I'm confused. It sounds like you're suggesting to add the very IF condition that my
patch removes from %gdm-activation in order to fix the problem.

bug#36508: GDM files have incorrect owner after temporarily removing service

[Ludovic Courtès]

Hi,

Brendan Tildesley <btild@mailbox.org> skribis:

>> On 04/14/2021 12:32 PM Ludovic Courtès <ludo@gnu.org> wrote:

[...]

>> The patch Brendan posted LGTM (though I’m surprised the directory itself
>> can have the right UID/GID while files inside it don’t; perhaps this was
>> made possible by 2161820ebbbab62a5ce76c9101ebaec54dc61586, which chowns
>> the home directory unconditionally.)
>> 
>> Note that there are other places, in addition to GDM, where we
>> forcefully reset the UID/GID of the home directory (e.g., for the
>> ‘knot-resolver’ service.)
>> 
>> My preferred solution to this would be to unconditionally chown -R home
>> directories upon activation (for efficiency, it would be best if we
>> could do that if and only if the home directory itself has wrong
>> ownership).  Thoughts?
>> 
> I'm confused. It sounds like you're suggesting to add the very IF condition that my
> patch removes from %gdm-activation in order to fix the problem.

I’d like to understand why the ‘if’ the patch removes was problematic.
I think it relates to the commit above, but that needs more
investigation.

Ludo’.

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Hi Brendan,

Brendan Tildesley <btild@mailbox.org> writes:

> Guix system rollbacks should be a supported feature of Guix, not just a gimmick
> that falls out of its design. It should be that a Guix user could leave their
> system for 5 years, and then do a guix pull; guix system reconfigure in the year
> 2026. Perhaps at that time the new system will break, and then its desirable
> that they can rollback to the previous generation.

This sounds like a good set of goals to strive for.  I'm not sure that
Guix, on its own, will be able to achieve reliable 5-year rollback.  I
think that would require _all_ software in Guix that maintains mutable
state on disk to gracefully support downgrading to a version from 5
years earlier.

Nonetheless, Guix can certainly do its part to try to ensure that
multi-year rollbacks can work, and I agree that it's a good thing to
keep in mind.

> So what fixes we put in to 
> Guix services today need to consider not just how files could have changed in
> the past, but how they might change in breaking ways in the future, within reason.

It's a good thing to keep in mind, yes.

> I don't know off the top of my head of any way that can be done other than to
> have chmod -R gdm:gdm /var/lib/gdm always executed.

I'm not necessarily opposed to doing that, at least as a temporary
workaround for GDM, but I don't think this is a satisfactory solution to
the larger problem.  A few points:

(1) I don't think we can assume that all files owned by a given user
    will be in that user's home directory, especially for "system"
    users.

(2) I also don't think we can assume that all files in a user's home
    directory *should* be owned by that user.  Even if it's true today,
    it might not be true tomorrow.

(3) Groups don't even have home directories.

> On 04/13/2021 10:51 PM Mark H Weaver <mhw@netris.org> wrote:
>> 
>> There's some discussion of this issue at <https://bugs.gnu.org/44944>,
>> although I'm not sure that Danny's suggested solution is practical.
>> 
>> Here's one idea: when activating a system, *never* delete users or
>> groups if files still exist that are owned by those users/groups.
>> Checking all filesystems would likely be too expensive, but perhaps it
>> would be sufficient to check certain directories such as /var, /etc, and
>> possibly the top directory of /home.
>> 
>> What do you think
>
> Wouldn't that imply that uids could be randomly different on different systems
> with the same configuration, and then remain statically different permanently?

Yes, and I agree that it's suboptimal.

> We want as little randomness and moving parts as possible. It's yet another
> way the system is not actually Functional, but has state.

Agreed.  Danny's suggested solution (UID = hash username) is clearly the
optimal approach in many respects.  It has the nice properties above.

The practical problem I see with Danny's approach is that in order to
make hash collisions sufficiently improbable, our UIDs and GIDs would
need to be much larger than the 16 bits that is widely supported by
POSIX software.  With 16-bit UIDs, the probability of a collision would
be 1.85% with 50 users, and 7.28% with 100 users.

To adopt this approach, I think that our UIDs and GIDs would need to be
at least 31 bits.  These are the problems I see:

(1) It's unlikely that all software in Guix robustly handles such large
    UIDs/GIDs.  Desktop systems with UIDs/GIDs larger than 65533 have
    not been widely tested, as far as I know.

(2) Even with 31 bit IDs, the probability of collisions would still be
    uncomfortably high when large numbers of users are present: with 10k
    users, the probability of hash collisions would be about 2.3%.

(3) We'd need a transition plan for users' existing file systems.

(4) It would be aesthetically unpleasant for our UIDs and GIDs to be
    random-looking numbers with 10 decimal digits.

> Seems this bug spans 3 or so different bug reports. In http://issues.guix.gnu.org/45571
> I commented that Nix uses hard coded id's, sorta like how ports are allocated
> for a purpose:
>
> https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/misc/ids.nix
>
> Perhaps you are thinking of other kinds of security issues that could be caused that
> I'm not thinking of.

I'm not sure what you're getting at here.  The only security issue I've
raised so far is that ownership of files can _effectively_ be changed
when removing services and later adding them back.  For example, in my
case, 'colord' ended up being the owner of files in /var/lib/gdm.

> In that case maybe Nix devs have already made the best choice by
> making them static?

That might well be true.  At the present time, this is the option that
seems most appealing to me.

One possible approach would be to add a field to our 'operating-system'
record that explicitly specifies a total mapping from user/group names
to UIDs/GIDs.  It could either be a procedure (to support Danny's
hashing approach with its nice properties) or possibly also an alist for
convenience.  If any entries were missing, it would raise an error.

One risk to this approach is that users could accidentally make a mess
of their system if they made a mistake while changing that field.

What do you think?

    Thanks,
      Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Ludovic Courtès <ludo@gnu.org> writes:

> Note that there are other places, in addition to GDM, where we
> forcefully reset the UID/GID of the home directory (e.g., for the
> ‘knot-resolver’ service.)
>
> My preferred solution to this would be to unconditionally chown -R home
> directories upon activation (for efficiency, it would be best if we
> could do that if and only if the home directory itself has wrong
> ownership).  Thoughts?

It might be okay to do this in specific cases like /var/lib/gdm, but I'm
very uncomfortable doing it for *all* users, because:

(1) We shouldn't assume that all files within a home directory are
    supposed to be owned by that user.

(2) We shouldn't assume that all files owned by a user will be within
    their home directory.

(3) We shouldn't assume that all files within a home directory are
    supposed to have the same 'group'.  I, for one, have sometimes had
    subdirectories of my home directory with a different 'group', to
    either restrict or grant other users access to selected files or
    directories.

(4) Groups do not, in general, have home directories.

(5) I consider it unsatifactory for there to be *any* window of time
    during system activation when the ownership of files is incorrect.

>> Here's one idea: when activating a system, *never* delete users or
>> groups if files still exist that are owned by those users/groups.
>> Checking all filesystems would likely be too expensive, but perhaps it
>> would be sufficient to check certain directories such as /var, /etc, and
>> possibly the top directory of /home.
>
> How would you determine which directories to look at though?  What if we
> miss an important one?

Yes, that's a good point.  I suppose that my idea above is not
satifactory either.

> Note that the ID allocation strategy in (gnu build accounts) ensures
> UIDs/GIDs aren’t reused right away (same strategy as implemented by
> Shadow, etc.).  So if you remove “bob”, then add “alice”, “alice” won’t
> be able to access the left-behind /home/bob because it has a different
> UID.

This mechanism is insufficient, because it only avoids the problem if
you add "alice" at the same time that "bob" is removed.  If you remove
"bob" during one system activation, and then later add "alice", then
"alice" might well be able to access bob's left-behind files.

In the case that I personally witnessed on my Guix system, files within
/var/lib/gdm ended up with 'colord' as their group.  That's not good.

Increasingly, I'm leaning toward the idea that the complete mapping from
names to IDs should somehow be explicitly given as part of the OS
configuration, as I advocated in <https://bugs.gnu.org/36508#26>.

What do you think?

     Thanks,
       Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Ludovic Courtès <ludo@gnu.org> writes:
> My preferred solution to this would be to unconditionally chown -R home
> directories upon activation

I also wonder if this could lead to security flaws similar to
CVE-2021-27851 <https://bugs.gnu.org/47229>, but perhaps 'chown' has
been written carefully to avoid such problems.

       Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Ludovic Courtès <ludo@gnu.org> writes:

> Mark H Weaver <mhw@netris.org> skribis:
>
>> Here's one idea: when activating a system, *never* delete users or
>> groups if files still exist that are owned by those users/groups.
>> Checking all filesystems would likely be too expensive, but perhaps it
>> would be sufficient to check certain directories such as /var, /etc, and
>> possibly the top directory of /home.
>
> How would you determine which directories to look at though?  What if we
> miss an important one?

I have another idea:

Maintain historical mappings from user/group names to UIDs/GIDs, perhaps
in some file in /etc, where entries are added but *never* automatically
removed.  When allocating UIDs/GIDs, we would avoid any UIDs/GIDs in the
range of those mappings.

Then, provide a UID/GID garbage collector, to be explicitly run by users
if desired, which would scan all filesystems to find the set of UID/GIDs
currently referenced, and remove entries from the historical mappings
that are no longer needed.

What do you think?

      Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Ludovic Courtès]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Note that there are other places, in addition to GDM, where we
>> forcefully reset the UID/GID of the home directory (e.g., for the
>> ‘knot-resolver’ service.)
>>
>> My preferred solution to this would be to unconditionally chown -R home
>> directories upon activation (for efficiency, it would be best if we
>> could do that if and only if the home directory itself has wrong
>> ownership).  Thoughts?
>
> It might be okay to do this in specific cases like /var/lib/gdm, but I'm
> very uncomfortable doing it for *all* users, because:
>
> (1) We shouldn't assume that all files within a home directory are
>     supposed to be owned by that user.
>
> (2) We shouldn't assume that all files owned by a user will be within
>     their home directory.
>
> (3) We shouldn't assume that all files within a home directory are
>     supposed to have the same 'group'.  I, for one, have sometimes had
>     subdirectories of my home directory with a different 'group', to
>     either restrict or grant other users access to selected files or
>     directories.
>
> (4) Groups do not, in general, have home directories.
>
> (5) I consider it unsatifactory for there to be *any* window of time
>     during system activation when the ownership of files is incorrect.

I agree this raises questions and we should take time to think through
it.  For system accounts though, I think 1–4 do not apply.

Perhaps a first step would be to do that for system accounts?

>> Note that the ID allocation strategy in (gnu build accounts) ensures
>> UIDs/GIDs aren’t reused right away (same strategy as implemented by
>> Shadow, etc.).  So if you remove “bob”, then add “alice”, “alice” won’t
>> be able to access the left-behind /home/bob because it has a different
>> UID.

To be clear, it’s doing the same as any other GNU/Linux distro.

> This mechanism is insufficient, because it only avoids the problem if
> you add "alice" at the same time that "bob" is removed.  If you remove
> "bob" during one system activation, and then later add "alice", then
> "alice" might well be able to access bob's left-behind files.
>
> In the case that I personally witnessed on my Guix system, files within
> /var/lib/gdm ended up with 'colord' as their group.  That's not good.
>
> Increasingly, I'm leaning toward the idea that the complete mapping from
> names to IDs should somehow be explicitly given as part of the OS
> configuration, as I advocated in <https://bugs.gnu.org/36508#26>.
>
> What do you think?

IDs as hash of the user names are interesting because that’d be
stateless (conversely, the current ID allocation strategy is stateful:
it arranges to not reuse recently-freed IDs.)

But like you write, we’d need 32-bit UIDs.  In libc, ‘uid_t’
(specifically ‘__UID_T_TYPE’ in typesizes.h) is 32-bit, so it might work
rather well in user space.

It still sounds like a change with significant implications though, and
it’s hard to predict exactly how it would go or what would break.  For
example, that does away with the system/non-system ranges, and wouldn’t
play well with “special” IDs like 0 and 65535.

To me, it’s a potential way out, but not a solution for the bug Brendan
reported today, nor a change we could implement in the coming
weeks/months; the time scale is probably longer.

WDYT?

Thanks,
Ludo’.

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> wrote:
>>> Note that the ID allocation strategy in (gnu build accounts) ensures
>>> UIDs/GIDs aren’t reused right away (same strategy as implemented by
>>> Shadow, etc.).  So if you remove “bob”, then add “alice”, “alice” won’t
>>> be able to access the left-behind /home/bob because it has a different
>>> UID.

I replied:
>> This mechanism is insufficient, because it only avoids the problem if
>> you add "alice" at the same time that "bob" is removed.  If you remove
>> "bob" during one system activation, and then later add "alice", then
>> "alice" might well be able to access bob's left-behind files.

Ludovic Courtès <ludo@gnu.org> responded:
> To be clear, it’s doing the same as any other GNU/Linux distro.

I don't think that's quite right.

It's true that if you delete a user or group on another distro and then
re-add it, it might not be assigned the same UID/GID.  That much is the
same as any other distro.

The key difference is this: On Debian, at least in my experience, users
and groups are *never* deleted automatically.  They are only added
automatically, but never removed unless you explicitly ask to remove
them.  So, this problem does not arise in practice.

      Thanks,
        Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> IDs as hash of the user names are interesting because that’d be
> stateless (conversely, the current ID allocation strategy is stateful:
> it arranges to not reuse recently-freed IDs.)
>
> But like you write, we’d need 32-bit UIDs.  In libc, ‘uid_t’
> (specifically ‘__UID_T_TYPE’ in typesizes.h) is 32-bit, so it might work
> rather well in user space.

The kernel and core system components certainly support 32-bit UIDs, and
have for around 20 years.

> It still sounds like a change with significant implications though, and
> it’s hard to predict exactly how it would go or what would break.

Right, my concern is with the vast majority of programs and libraries in
Guix, most of which probably haven't seen much (if any) testing with
large UIDs.

> For example, that does away with the system/non-system ranges, and
> wouldn’t play well with “special” IDs like 0 and 65535.

This particular issue is easily addressed.  It's easy enough to find a
function from 31-hash values to 32-bit IDs that's injective and avoids
any chosen subset of special IDs, as long as there are fewer than 2^31
special IDs.

Simply adding 65536 (or even 2^31) to the hash value would be one easy
option.

What do you think?

       Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Maxime Devos]

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

On Thu, 2021-04-15 at 14:58 -0400, Mark H Weaver wrote:
> Ludovic Courtès <ludo@gnu.org> writes:
> 
> > Mark H Weaver <mhw@netris.org> skribis:
> > 
> > > Here's one idea: when activating a system, *never* delete users or
> > > groups if files still exist that are owned by those users/groups.
> > > Checking all filesystems would likely be too expensive, but perhaps it
> > > would be sufficient to check certain directories such as /var, /etc, and
> > > possibly the top directory of /home.

And /tmp, /media and /run/user.

> > 
> > How would you determine which directories to look at though?  What if we
> > miss an important one?
> 
> I have another idea:
> 
> Maintain historical mappings from user/group names to UIDs/GIDs, perhaps
> in some file in /etc, where entries are added but *never* automatically
> removed.  When allocating UIDs/GIDs, we would avoid any UIDs/GIDs in the
> range of those mappings.

This seems rather convoluted to me.  Why not reuse /etc/passwd and /etc/groups?
My suggestion:

1. *never* automatically delete users/groups from /etc/passwd, /etc/groups
   (I thought that was how Guix already worked ...)
2. as users and groups appearing in /etc/passwd and /etc/groups, but not
   in the operating system configuration can be confusing, change the comment
   string of these users and groups, to something like

   "account removed"

   Add a group 'user-graveyard' for (3), and move these 'pseudo-removed' users
   to the 'user-graveyard' group.
3. Don't forget to remove graveyard users from all groups (except user-graveyard),
   make sure the graveyard users can't log in anymore ... (Perhaps add a rule to
   the SSH and PAM configuration that forbids logging in to graveyard accounts,
   by checking whether the user is in the 'user-graveyard' group?)

> Then, provide a UID/GID garbage collector, to be explicitly run by users
> if desired, which would scan all filesystems to find the set of UID/GIDs
> currently referenced, and remove entries from the historical mappings
> that are no longer needed.

That seems useful for if /etc/passwd and /etc/group is getting full, or just for
cleaning up.  You may want to exclude /gnu/store though, for efficiency (-:.
And just in case check whether any live processes have the UID/GID.

Suggested command name: "guix user-gc".

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#36508: GDM files have incorrect owner after temporarily removing service

[Ludovic Courtès]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> This particular issue is easily addressed.  It's easy enough to find a
> function from 31-hash values to 32-bit IDs that's injective and avoids
> any chosen subset of special IDs, as long as there are fewer than 2^31
> special IDs.
>
> Simply adding 65536 (or even 2^31) to the hash value would be one easy
> option.
>
> What do you think?

Yes, but these special IDs are just examples of things that could go
wrong.  I don’t know, maybe I’m just overly cautious; we have to try to
get a better understanding of how things will go!

Thanks,
Ludo’.

bug#36508: GDM files have incorrect owner after temporarily removing service

[Ludovic Courtès]

Hi,

Mark H Weaver <mhw@netris.org> skribis:

> It's true that if you delete a user or group on another distro and then
> re-add it, it might not be assigned the same UID/GID.  That much is the
> same as any other distro.
>
> The key difference is this: On Debian, at least in my experience, users
> and groups are *never* deleted automatically.  They are only added
> automatically, but never removed unless you explicitly ask to remove
> them.  So, this problem does not arise in practice.

>> Maintain historical mappings from user/group names to UIDs/GIDs, perhaps
>> in some file in /etc, where entries are added but *never* automatically
>> removed.  When allocating UIDs/GIDs, we would avoid any UIDs/GIDs in the
>> range of those mappings.

If we’re just worried about ID allocation, we could keep state in, say,
/etc/previous-uids, and feed that as input to the (gnu build accounts)
allocation code.

Thoughts?

Maxime Devos <maximedevos@telenet.be> skribis:

> This seems rather convoluted to me.  Why not reuse /etc/passwd and /etc/groups?
> My suggestion:
>
> 1. *never* automatically delete users/groups from /etc/passwd, /etc/groups
>    (I thought that was how Guix already worked ...)
> 2. as users and groups appearing in /etc/passwd and /etc/groups, but not
>    in the operating system configuration can be confusing, change the comment
>    string of these users and groups, to something like
>
>    "account removed"
>
>    Add a group 'user-graveyard' for (3), and move these 'pseudo-removed' users
>    to the 'user-graveyard' group.
> 3. Don't forget to remove graveyard users from all groups (except user-graveyard),
>    make sure the graveyard users can't log in anymore ... (Perhaps add a rule to
>    the SSH and PAM configuration that forbids logging in to graveyard accounts,
>    by checking whether the user is in the 'user-graveyard' group?)

Problem is that things like GDM would still propose those old accounts
(unless maybe their password is uninitialized, I’m not sure; but it’s
still hacky.)

Thanks,
Ludo’.

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Mark H Weaver <mhw@netris.org> skribis:
>
>>> Maintain historical mappings from user/group names to UIDs/GIDs, perhaps
>>> in some file in /etc, where entries are added but *never* automatically
>>> removed.  When allocating UIDs/GIDs, we would avoid any UIDs/GIDs in the
>>> range of those mappings.
>
> If we’re just worried about ID allocation, we could keep state in, say,
> /etc/previous-uids, and feed that as input to the (gnu build accounts)
> allocation code.

Sounds good to me, or at least better than the other available options.

     Thanks,
       Mark

bug#36508: GDM files have incorrect owner after temporarily removing service

[Mark H Weaver]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

> On Thu, 2021-04-15 at 14:58 -0400, Mark H Weaver wrote:
>> Maintain historical mappings from user/group names to UIDs/GIDs, perhaps
>> in some file in /etc, where entries are added but *never* automatically
>> removed.  When allocating UIDs/GIDs, we would avoid any UIDs/GIDs in the
>> range of those mappings.
>
> This seems rather convoluted to me.  Why not reuse /etc/passwd and /etc/groups?
> My suggestion:
>
> 1. *never* automatically delete users/groups from /etc/passwd, /etc/groups
>    (I thought that was how Guix already worked ...)
> 2. as users and groups appearing in /etc/passwd and /etc/groups, but not
>    in the operating system configuration can be confusing, change the comment
>    string of these users and groups, to something like
>
>    "account removed"
>
>    Add a group 'user-graveyard' for (3), and move these 'pseudo-removed' users
>    to the 'user-graveyard' group.
> 3. Don't forget to remove graveyard users from all groups (except user-graveyard),
>    make sure the graveyard users can't log in anymore ... (Perhaps add a rule to
>    the SSH and PAM configuration that forbids logging in to graveyard accounts,
>    by checking whether the user is in the 'user-graveyard' group?)

I would be okay with this approach as well, although it's not obvious to
me that it's any cleaner than having a separate /etc/previous-uids file,
given items 2 and 3 above.

>> Then, provide a UID/GID garbage collector, to be explicitly run by users
>> if desired, which would scan all filesystems to find the set of UID/GIDs
>> currently referenced, and remove entries from the historical mappings
>> that are no longer needed.
>
> That seems useful for if /etc/passwd and /etc/group is getting full, or just for
> cleaning up.  You may want to exclude /gnu/store though, for efficiency (-:.

Good point!  That's one directory that would clearly be a waste to scan :-)

> And just in case check whether any live processes have the UID/GID.

Sure, sounds good.

     Thanks!
       Mark

bug#36508: [DRAFT PATCH] Stable allocation of uids, by keeping a historical mapping.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 1564 bytes --]

Hi,

The attached patch maintains a historical mapping from user names to 
UIDs/GIDs (even when those users are removed from the user accounts), to 
solve <https://issues.guix.gnu.org/36508>, as proposed by Mark H Weaver 
in <https://issues.guix.gnu.org/36508#10>.

(The proposed garbage collector is not included, however.)

As I proposed in <https://issues.guix.gnu.org/36508#14>, the information 
of this mapping is kept in a special 'user'.  However, I didn't 
implement the same for groups (raises some questions about atomicity -- 
/etc/passwd and /etc/groups are separate files).

I didn't completely follow that proposal though, since:

 > https://issues.guix.gnu.org/36508#16:
 > Problem is that things like GDM would still propose those old accounts
 > (unless maybe their password is uninitialized, I’m not sure; but it’s
 > still hacky.)

, though that could perhaps be solved by adjusting GDM appropriately.

"make check TESTS=tests/accounts.scm" passes, but otherwise the patch is 
rather untested (hence, 'DRAFT') -- the new functionality will need a 
few additional tests in tests/account.scm, to avoid introducing bugs in 
the handling of /etc/passwd.

On second thought, a separate /etc/previous-uids (as proposed in
<https://issues.guix.gnu.org/36508#16> by Ludovic Courtès)
might be better (there are atomicity issues anyway, due to /etc/passwd 
and /etc/groups being separate files, and losing the historical mapping
is relatively harmless and seems unlikely to happen in practice).

Greetings,
Maxime

[-- Attachment #1.1.2: 0001-DRAFT-Stable-allocation-of-uids.patch --]
[-- Type: text/x-patch, Size: 16012 bytes --]

From 1e0eb78fb7ae1a7b6c40e364d88b0b33945ef7c1 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Sun, 18 Sep 2022 14:18:10 +0200
Subject: [PATCH] DRAFT: Stable allocation of uids

---
 gnu/build/accounts.scm | 186 +++++++++++++++++++++++++++++++++++------
 tests/accounts.scm     |  14 +++-
 2 files changed, 173 insertions(+), 27 deletions(-)

diff --git a/gnu/build/accounts.scm b/gnu/build/accounts.scm
index 1247fc640c..5e04e9f526 100644
--- a/gnu/build/accounts.scm
+++ b/gnu/build/accounts.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -17,9 +18,11 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu build accounts)
+  #:use-module (guix base64)
   #:use-module (guix records)
   #:use-module (guix combinators)
   #:use-module (gnu system accounts)
+  #:use-module (rnrs bytevectors)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-19)
@@ -72,10 +75,16 @@ (define-module (gnu build accounts)
 ;;; <shadow.h>, <pwd.h>, and <grp.h> routines, as well as a subset of the
 ;;; functionality of the Shadow command-line tools.  It can parse and write
 ;;; /etc/passwd, /etc/shadow, and /etc/group.  It can also take care of UID
-;;; and GID allocation in a way similar to what 'useradd' does.
+;;; and GID allocation, in a different way than 'useradd' does.
 ;;;
-;;; The benefit is twofold: less code is involved, and the ID allocation
-;;; strategy and state preservation is made explicit.
+;;; The benefit is threefold: less code is involved, the ID allocation
+;;; strategy and state preservation is made explicit and it avoids
+;;; allocating IDs that were in the past allocated for some user user
+;;; and removed but still present somewhere in the file system.
+;;; Additionally, when such an user is re-added, Guix will use the same
+;;; ID again
+;;;
+;;; TODO: do the same 'keep old IDs' for groups as well.
 ;;;
 ;;; Code:
 
@@ -288,6 +297,85 @@ (define read-shadow
 (define read-group
   (database-reader "/etc/group" string->group-entry))
 
+(define (decode-guix-deleted-users encoded)
+  (alist->vhash (call-with-input-string
+                    (utf8->string (base64-decode encoded))
+                  read)))
+(define (encode-guix-deleted-users deleted-username->id)
+  (define (less this that)
+    (< (cdr this) (cdr that)))
+  (base64-encode
+   (string->utf8
+    (object->string
+     ;; Sort the mappings, to avoid hashing causing non-determinism.
+     (sort
+      (vhash-fold (lambda (username value accumulated)
+                    (unless (string? username)
+                      (error "username should be a string"))
+                    (unless (integer? value)
+                      (error "uid should be a string"))
+                    (cons (cons username value) accumulated))
+                  '()
+                  deleted-username->id)
+      less)))))
+
+(define (passwd-deleted passwd)
+  (define guix-deleted-users
+    ((lookup-procedure passwd password-entry-name)
+     "guix-deleted-users"))
+  (if guix-deleted-users
+      (decode-guix-deleted-users (password-entry-real-name guix-deleted-users))
+      vlist-null))
+
+(define (adjust-deleted-users current-passwd-entries new-passwd-entries
+                              current-deleted)
+  (define lookup-by-name/new
+    (lookup-procedure new-passwd-entries password-entry-name))
+  (define lookup-by-id/new
+    (lookup-procedure new-passwd-entries password-entry-uid))
+  (define lookup-by-id/old
+    (lookup-procedure current-passwd-entries password-entry-uid))
+  ;; Remove all usernames of 'current-deleted' that were allocated
+  ;; (in new-passwd-entries) and similarily, remove all IDs that were
+  ;; allocated.
+  (define current-deleted/filtered
+    (vhash-fold (lambda (username id accumulated)
+                  (if (or (lookup-by-name/new username)
+                          (lookup-by-id/new id))
+                      accumulated
+                      (vhash-cons username id accumulated)))
+                vlist-null
+                current-deleted))
+  (define (id-reused? username id)
+    ;; Was the ID used by a different username than USERNAME in
+    ;; CURRENT-PASSWD-ENTRIES?
+    (let ((old-password-entry (lookup-by-id/old id)))
+      (and old-password-entry (string=? (password-entry-name old-password-entry)
+                                        username))))
+  ;; Add usernames that are present in CURRENT-PASSWD-ENTRIES but
+  ;; not in NEW-PASSWD-ENTRIES, but only if the ID wasn't reused
+  ;; for some other user name.
+  (define new-deleted
+    (fold (lambda (username id accumulated)
+            (if (and (not (lookup-by-name/new username))
+                     (not (id-reused? username id)))
+                (vhash-cons username id accumulated)
+                accumulated))
+          current-deleted/filtered
+          current-passwd-entries))
+
+  (define (adjust password-entry* accumulated)
+    (cons (if (string=? (password-entry-name password-entry*) "guix-deleted-users")
+              (password-entry
+               (inherit password-entry*)
+               (real-name (encode-guix-deleted-users new-deleted)))
+              password-entry*)
+          accumulated))
+  ;; The 'reverse' is to preserve the order -- in theory, this should be
+  ;; unnecessary, but reverting the ordering of the entries in /etc/passwd
+  ;; after each reconfiguration would be surprising.
+  (fold adjust '() (reverse new-passwd-entries)))
+
 \f
 ;;;
 ;;; Building databases.
@@ -321,7 +409,8 @@ (define (user-id? id)
 
 (define* (allocate-id assignment #:key system?)
   "Return two values: a newly allocated ID, and an updated <allocation> record
-based on ASSIGNMENT.  If SYSTEM? is true, return a system ID."
+based on ASSIGNMENT.  If SYSTEM? is true, return a system ID.  This requires
+that no ID is allocated yet."
   (define next
     ;; Return the next available ID, looping if necessary.
     (if system?
@@ -441,20 +530,28 @@ (define previous-entry
           gids
           groups)))
 
-(define* (allocate-passwd users groups #:optional (current-passwd '()))
+(define* (allocate-passwd users groups #:optional (current-passwd '())
+                          (deleted-username->id vlist-null))
   "Return a list of password entries for USERS, a list of <user-account>.
 Take GIDs from GROUPS, a list of group entries.  Reuse UIDs from
-CURRENT-PASSWD, a list of password entries, when possible; otherwise allocate
-new UIDs."
+CURRENT-PASSWD, a list of password entries, or deleted-username->id,
+a vhash of usernames that were deleted in the previous /etc/passwd to IDs,
+when possible; otherwise allocate new UIDs."
   (define uids
-    (reserve-ids (reserve-ids (allocation)
-                              (map password-entry-uid current-passwd))
-                 (filter-map user-account-uid users)
-                 #:skip? #f))
-
-  (define previous-entry
+    (reserve-ids ; <-- TODO: is #:skip? #false needed here as well?
+     (reserve-ids (reserve-ids (allocation)
+                               (map password-entry-uid current-passwd))
+                  (filter-map user-account-uid users)
+                  #:skip? #f)
+     (vhash-fold (lambda (username id accumulated)
+                   (cons id accumulated)) '() deleted-username->id)))
+
+  (define previous-undeleted-entry
     (lookup-procedure current-passwd password-entry-name))
 
+  (define (previous-deleted-entry username)
+    (and=> (vhash-assoc username deleted-username->id) cdr))
+
   (define (group-id name)
     (or (any (lambda (entry)
                (and (string=? (group-entry-name entry) name)
@@ -471,17 +568,28 @@ (define (group-id name)
                   (directory    (user-account-home-directory user))
                   (shell        (user-account-shell user))
                   (system?      (user-account-system? user)))
-              (let*-values (((previous)
-                             (previous-entry name))
+              (let*-values (((previous-undeleted)
+                             (previous-undeleted-entry name))
+                            ((previous-deleted)
+                             (previous-deleted-entry name))
                             ((allocation id)
                              (cond
                               ((number? requested-id)
                                (values (reserve-ids allocation
                                                     (list requested-id))
                                        requested-id))
-                              (previous
+                              (previous-undeleted
                                (values allocation
-                                       (password-entry-uid previous)))
+                                       (password-entry-uid previous-undeleted)))
+                              ((and previous-deleted
+                                    (not (allocated? allocation previous-deleted)))
+                               ;; This deleted user might still have some files
+                               ;; in the file system, reuse the old id such
+                               ;; that it remains correct, unless some other
+                               ;; user has choosen that id.
+                               (values (reserve-ids allocation
+                                                    (list previous-deleted))
+                                       previous-deleted))
                               (else
                                (allocate-id allocation
                                             #:system? system?)))))
@@ -494,9 +602,10 @@ (define (group-id name)
                                ;; Users might change their name to something
                                ;; other than what the sysadmin chose, with
                                ;; 'chfn'.  Thus consider it "stateful".
-                               (real-name (if (and previous (not system?))
-                                              (password-entry-real-name previous)
-                                              real-name))
+                               (real-name
+                                (if (and previous-undeleted (not system?))
+                                    (password-entry-real-name previous-undeleted)
+                                    real-name))
 
                                ;; Do not reuse the shell of PREVIOUS since (1)
                                ;; that could lead to confusion, and (2) the
@@ -556,6 +665,25 @@ (define* (user+group-databases users groups
 entries, and the list of shadow entries corresponding to USERS and GROUPS.
 Preserve stateful bits from CURRENT-PASSWD, CURRENT-GROUPS, and
 CURRENT-SHADOW: UIDs, GIDs, passwords, user shells, etc."
+  (define users*
+    (if ((lookup-procedure users user-account-name)
+         "guix-deleted-users")
+        users
+        (append users
+                (list (user-account
+                       (name "guix-deleted-users")
+                       (group "guix-deleted-users")
+                       (home-directory "/var/empty")
+                       (system? #true))))))
+  (define groups*
+    (if ((lookup-procedure groups user-group-name)
+         "guix-deleted-users")
+        groups
+        (append groups
+                (list (user-group
+                       (name "guix-deleted-users")
+                       (system? #true))))))
+
   (define members
     ;; Map group name to user names.
     (fold (lambda (user members)
@@ -563,16 +691,24 @@ (define members
                   members
                   (user-account-supplementary-groups user)))
           vlist-null
-          users))
+          users*))
+
+  (define current-deleted (passwd-deleted current-passwd))
 
   (define group-entries
-    (allocate-groups groups members current-groups))
+    (allocate-groups groups* members current-groups))
 
   (define passwd-entries
-    (allocate-passwd users group-entries current-passwd))
+    (allocate-passwd users* group-entries current-passwd
+                     (passwd-deleted current-passwd)))
 
   (define shadow-entries
-    (passwd->shadow users passwd-entries current-shadow
+    (passwd->shadow users* passwd-entries current-shadow
                     #:current-time current-time))
 
-  (values group-entries passwd-entries shadow-entries))
+  ;; TODO: adjust new 'deleted'
+  (values group-entries (adjust-deleted-users
+                         current-passwd
+                         passwd-entries
+                         current-deleted)
+          shadow-entries))
diff --git a/tests/accounts.scm b/tests/accounts.scm
index 78136390bb..4e29bfe285 100644
--- a/tests/accounts.scm
+++ b/tests/accounts.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -272,7 +273,9 @@ (define allocate-passwd (@@ (gnu build accounts) allocate-passwd))
                            (members '("bob")))
               (group-entry (name "b") (gid (+ 1 %id-min))
                            (members '("alice")))
-              (group-entry (name "s") (gid %system-id-max)))
+              (group-entry (name "s") (gid %system-id-max))
+              (group-entry (name "guix-deleted-users")
+                           (gid (- %system-id-max 1))))
         (list (password-entry (name "alice") (real-name "Alice")
                               (uid %id-min) (gid %id-min)
                               (directory "/a"))
@@ -281,12 +284,19 @@ (define allocate-passwd (@@ (gnu build accounts) allocate-passwd))
                               (directory "/b"))
               (password-entry (name "nobody")
                               (uid 65534) (gid %system-id-max)
+                              (directory "/var/empty"))
+              (password-entry (name "guix-deleted-users")
+                              ;; empty list, start without deleted users
+                              (real-name "KCk=")
+                              (uid %system-id-max)
+                              (gid (- %system-id-max 1)) ; XXX: why not %system-id-max?  Bug or OK?
                               (directory "/var/empty")))
         (list (shadow-entry (name "alice") (last-change 100)
                             (password (crypt "initial pass" "$6$")))
               (shadow-entry (name "bob") (last-change 50)
                             (password (crypt "foo" "$6$")))
-              (shadow-entry (name "nobody") (last-change 100))))
+              (shadow-entry (name "nobody") (last-change 100))
+              (shadow-entry (name "guix-deleted-users") (last-change 100))))
   (call-with-values
       (lambda ()
         (user+group-databases (list (user-account

base-commit: 17f646aeba39f0d297f6c911d83b3bd9e88a227b
prerequisite-patch-id: 1f7c45cf2480f4e6f1e9563660e1b73a8682425e
prerequisite-patch-id: 0caac311875ee39cb48573657ebb960e90da6dfb
prerequisite-patch-id: 418285493d89ebf102175902d9b09a0174e88190
prerequisite-patch-id: 3c39eb839d9d3ff3fca6cd98621a5d5c411b7af4
prerequisite-patch-id: 8d5662e874c469f5ee496ef5181cf2d0a30ad1d8
prerequisite-patch-id: 26513c3b3b86963df718ee41d14a25d1cc6a8f3f
prerequisite-patch-id: 2b2497e2edec0afc48ebadd6f09f0c661c466127
prerequisite-patch-id: 2712efb97bf33985fd0658e4dd8e936dc08be5fe
prerequisite-patch-id: 9d2409b480a8bff0fef029b4b095922d4957e06f
prerequisite-patch-id: 51a32abca3efec1ba67ead59b8694c5ea3129ad3
prerequisite-patch-id: 7d55e3b39eb8803f058857d4412796b3f5dc0856
prerequisite-patch-id: 9092927761a340c07a99f5f3ed314a6add04cdee
prerequisite-patch-id: eafeeba1e6816dee3f8df671631bbeb5c373237a
-- 
2.37.3


[-- Attachment #1.1.3: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]