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

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

[-- 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