GDM files have incorrect owner after temporarily replacing with SDDM

  • Open
  • quality assurance status badge
Details
7 participants
  • Brendan Tildesley
  • ison
  • Leo Famulari
  • Ludovic Courtès
  • Brendan Tildesley
  • Maxime Devos
  • Mark H Weaver
Owner
unassigned
Submitted by
ison
Severity
normal
Merged with
I
I
ison wrote on 5 Jul 2019 10:36
(address . bug-guix@gnu.org)
20190705083620.lbzu7a33awbymh3d@cf0
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))))
-------------------------------
B
B
Brendan Tildesley wrote on 13 Apr 2021 15:24
GDM files have incorrect owner after temporarily removing service
(name . 36508@debbugs.gnu.org)(address . 36508@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
1576552162.14721.1618320275616@office.mailbox.org
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: file
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

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

Toggle diff (28 lines)
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
M
M
Mark H Weaver wrote on 13 Apr 2021 22:51
(name . Ludovic Courtès)(address . ludo@gnu.org)
87czuxsya5.fsf@netris.org
Hi Brendan,

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

Toggle quote (4 lines)
> 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:

Toggle snippet (22 lines)
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 ~#

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
B
B
Brendan Tildesley wrote on 14 Apr 2021 06:31
(address . 36508@debbugs.gnu.org)
461701204.22088.1618374714507@office.mailbox.org
Toggle quote (55 lines)
> 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.
Toggle quote (12 lines)
>
> 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:


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.
L
L
Ludovic Courtès wrote on 14 Apr 2021 12:32
(name . Mark H Weaver)(address . mhw@netris.org)
875z0pgnqn.fsf@gnu.org
Hi Mark,

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

Toggle quote (12 lines)
> 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.

Toggle quote (9 lines)
> 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’.
B
B
Brendan Tildesley wrote on 14 Apr 2021 14:21
(address . 36508@debbugs.gnu.org)
262720830.30369.1618402863721@office.mailbox.org
Toggle quote (35 lines)
> 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.
L
L
Ludovic Courtès wrote on 15 Apr 2021 16:24
(name . Brendan Tildesley)(address . btild@mailbox.org)
87o8ef8w24.fsf@gnu.org
Hi,

Brendan Tildesley <btild@mailbox.org> skribis:

Toggle quote (2 lines)
>> On 04/14/2021 12:32 PM Ludovic Courtès <ludo@gnu.org> wrote:

[...]

Toggle quote (17 lines)
>> 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’.
M
M
Mark H Weaver wrote on 15 Apr 2021 20:09
(name . Ludovic Courtès)(address . ludo@gnu.org)
87pmyvifmf.fsf@netris.org
Hi Brendan,

Brendan Tildesley <btild@mailbox.org> writes:

Toggle quote (6 lines)
> 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.

Toggle quote (4 lines)
> 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.

Toggle quote (3 lines)
> 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.

Toggle quote (16 lines)
> 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.

Toggle quote (3 lines)
> 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.

Toggle quote (9 lines)
> 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.

Toggle quote (3 lines)
> 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
M
M
Mark H Weaver wrote on 15 Apr 2021 20:30
(name . Ludovic Courtès)(address . ludo@gnu.org)
87lf9jiems.fsf@netris.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (9 lines)
> 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.

Toggle quote (9 lines)
>> 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.

Toggle quote (6 lines)
> 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
M
M
Mark H Weaver wrote on 15 Apr 2021 20:35
(name . Ludovic Courtès)(address . ludo@gnu.org)
87im4nief4.fsf@netris.org
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (3 lines)
> 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
M
M
Mark H Weaver wrote on 15 Apr 2021 20:58
(name . Ludovic Courtès)(address . ludo@gnu.org)
87eefbidcs.fsf@netris.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (11 lines)
> 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
L
L
Ludovic Courtès wrote on 15 Apr 2021 22:05
(name . Mark H Weaver)(address . mhw@netris.org)
878s5j8ga7.fsf@gnu.org
Hi Mark,

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

Toggle quote (31 lines)
> 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?

Toggle quote (6 lines)
>> 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.

Toggle quote (14 lines)
> 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’.
M
M
Mark H Weaver wrote on 16 Apr 2021 00:22
(name . Ludovic Courtès)(address . ludo@gnu.org)
87zgxzgpbf.fsf@netris.org
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (6 lines)
>>> 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:
Toggle quote (5 lines)
>> 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:
Toggle quote (2 lines)
> 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
M
M
Mark H Weaver wrote on 16 Apr 2021 01:04
(name . Ludovic Courtès)(address . ludo@gnu.org)
87wnt3gndy.fsf@netris.org
Hi Ludovic,

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

Toggle quote (8 lines)
> 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.

Toggle quote (3 lines)
> 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.

Toggle quote (3 lines)
> 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
M
M
Maxime Devos wrote on 16 Apr 2021 12:42
38fd9bfdc71c689df1606a0b00d632e423c5defa.camel@telenet.be
On Thu, 2021-04-15 at 14:58 -0400, Mark H Weaver wrote:
Toggle quote (10 lines)
> 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.

Toggle quote (11 lines)
> >
> > 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?)

Toggle quote (5 lines)
> 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.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYHlqLRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7iLeAQCDp8ywXTcQrPOsnQ427HUGcXmo
W23thxgqzXqSPwy5ZAD9EoGaIgOLrMymibzhCeJ/alf6nGRxQjonoxZ9cyuq5A0=
=Ht7l
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 16 Apr 2021 17:14
(name . Mark H Weaver)(address . mhw@netris.org)
871rba5kjg.fsf@gnu.org
Hi Mark,

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

Toggle quote (10 lines)
> 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’.
L
L
Ludovic Courtès wrote on 16 Apr 2021 17:18
(name . Mark H Weaver)(address . mhw@netris.org)
87tuo645sh.fsf@gnu.org
Hi,

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

Toggle quote (14 lines)
> 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:

Toggle quote (18 lines)
> 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’.
M
M
Mark H Weaver wrote on 17 Apr 2021 18:16
(name . Ludovic Courtès)(address . ludo@gnu.org)
87blac9989.fsf@netris.org
Hi Ludovic,

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

Toggle quote (11 lines)
> 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
M
M
Mark H Weaver wrote on 17 Apr 2021 18:28
878s5g98p6.fsf@netris.org
Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

Toggle quote (24 lines)
> 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.

Toggle quote (8 lines)
>> 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 :-)

Toggle quote (2 lines)
> And just in case check whether any live processes have the UID/GID.

Sure, sounds good.

Thanks!
Mark
L
L
Leo Famulari wrote on 9 May 2021 18:46
(no subject)
(address . control@debbugs.gnu.org)
YJgR4pYFIV9kC1Z3@jasmine.lan
merge 39527 36508
B
B
Brendan Tildesley wrote on 9 May 2021 18:51
(address . control@debbugs.gnu.org)
dbff5319-5c49-0891-5fc9-0c67b4e213bb@brendan.scot
merge 39527 36508
M
M
Maxime Devos wrote on 18 Sep 2022 14:22
[DRAFT PATCH] Stable allocation of uids, by keeping a historical mapping.
(address . 36508@debbugs.gnu.org)
020888b6-6e8d-23d8-ee81-92884c717fbe@telenet.be
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
(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:
> 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
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: 0001-DRAFT-Stable-allocation-of-uids.patch
Attachment: OpenPGP_0x49E3EE22191725EE.asc
Attachment: OpenPGP_signature
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 36508@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 36508
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch