Subject: Re: [bug#77288] [PATCH v2 7/8] services: guix: Allow‘guix-daemon’to run without root privileges.

From: Maxim Cournoyer <[email-masked]>
Date: Sun, 20 Apr 2025 23:24:38 +0900

Hi,

Ludovic Courtès <[email-masked]> writes:

> * gnu/services/base.scm (run-with-writable-store)
> (guix-ownership-change-program): New procedures.
> (<guix-configuration>)[privileged?]: New field.
> (guix-shepherd-service): Rename to…
> (guix-shepherd-services): … this.   Add the ‘guix-ownership’ service.
> Change ‘guix-daemon’ service to depend on it; when unprivileged,
> prefix ‘daemon-command’ by ‘run-with-writable-store’ and
> omit ‘--build-users-group’; adjust socket activation endpoints.
> (guix-accounts): When unprivileged, create the “guix-daemon” user and
> group.
> (guix-service-type)[extensions]: Adjust to name change.
> * gnu/tests/base.scm (run-guix-daemon-test): Add ‘name’ parameter.
> (%test-guix-daemon): Adjust accordingly.
> (%test-guix-daemon-unprivileged): New test.
> * doc/guix.texi (Base Services): Document ‘privileged?’.
> (Migrating to the Unprivileged Daemon): Explain that this is automatic
> on Guix System.

About the migrating part: it's currently automatic, unless someone sets
privileged? to #f.

Other than thati, it sounds good, and answers some of my earlier
questions in this thread.

[...]

> +On Guix System, these steps are carried out automatically when you set
> +the @code{privileged?} field of the @code{guix-configuration} record to
> +@code{#f} and reconfigure (@pxref{guix-configuration-type,
> +@code{guix-configuration}}).

So, it's not automatic?  Or is privileged? #f the default?  Perhaps
worth mentioning what the default value is here, to make this clear
without referring to another place.

> +However, on a foreign distribution, the process is manual.  The
> +following paragraphs describe what you need to do.
> +
>  @quotation Warning
>  Follow the instructions below only after making sure you have a recent
>  version of @command{guix-daemon} with support for unprivileged
> @@ -20105,6 +20113,36 @@ Base Services
>  The Guix package to use.  @xref{Customizing the System-Wide Guix} to
>  learn how to provide a package with a pre-configured set of channels.
>  
> +@cindex unprivileged @command{guix-daemon}
> +@cindex rootless @command{guix-daemon}
> +@item @code{privileged?} (default: @code{#t})
> +Whether to run @command{guix-daemon} as root.
> +
> +When true, @command{guix-daemon} runs with root privileges and build
> +processes run under unprivileged user accounts as specified by
> +@code{build-group} and @code{build-accounts} (see below); when false,
> +@command{guix-daemon} run as the @code{guix-daemon} user, which is
> +unprivileged, and so do build processes.  The unprivileged or
> +``rootless'' mode can reduce the impact of some classes of
> +vulnerabilities that could affect the daemon.
> +
> +The default is currently @code{#t} (@command{guix-daemon} runs with root
> +privileges) but may eventually be changed to @code{#f}.

Ah!  It's covered here.

[...]

> +(define (guix-ownership-change-program)
> +  "Return a program that changes ownership of the store and other data files
> +of Guix to the given UID and GID."
> +  (program-file "validate-guix-ownership"
> +                (with-imported-modules (source-module-closure
> +                                        '((guix build utils)))
> +                  #~(begin
> +                      (use-modules (guix build utils)
> +                                   (ice-9 ftw)
> +                                   (ice-9 match))
> +
> +                      (define (lchown file uid gid)
> +                        (let ((parent (open (dirname file) O_DIRECTORY)))
> +                          (chown-at parent (basename file) uid gid
> +                                    AT_SYMLINK_NOFOLLOW)

Why do we need an atomic variant only for symlinks?  Perhaps worth a
comment.

> +                          (close-port parent)))
> +
> +                      (define (change-ownership directory uid gid)
> +                        ;; chown -R UID:GID DIRECTORY
> +                        (file-system-fold (const #t)                 ;enter?
> +                                          (lambda (file stat result) ;leaf
> +                                            (if (eq? 'symlink (stat:type stat))
> +                                                (lchown file uid gid)
> +                                                (chown file uid gid)))
> +                                          (const #t) ;down
> +                                          (lambda (directory stat result) ;up
> +                                            (chown directory uid gid))
> +                                          (const #t) ;skip
> +                                          (lambda (file stat errno result)
> +                                            (format (current-error-port) "i/o error: ~a: ~a~%"

That's too wide for our 80 columns maximum width convention :-).  Easy
to fix by breaking the line either after program-file or
file-system-fold.

> +                                                    file (strerror errno))
> +                                            #f)
> +                                          #t      ;seed
> +                                          directory
> +                                          lstat))
> +
> +                      (define (claim-data-ownership uid gid)
> +                        (format #t "Changing file ownership for /gnu/store \
> +and data directories to ~a:~a...~%"
> +                                uid gid)
> +                        (change-ownership #$(%store-prefix) uid gid)
> +                        (let ((excluded '("." ".." "profiles" "userpool")))
> +                          (for-each (lambda (directory)
> +                                      (change-ownership (in-vicinity "/var/guix" directory)

Likewise.  Also, I never remember why `in-vicinity' is useful, and it's
not documented anywhere.

> +                                                        uid gid))
> +                                    (scandir "/var/guix"
> +                                             (lambda (file)
> +                                               (not (member file
> +                                                            excluded))))))
> +                        (chown "/var/guix" uid gid)
> +                        (change-ownership "/etc/guix" uid gid)
> +                        (mkdir-p "/var/log/guix")
> +                        (change-ownership "/var/log/guix" uid gid))
> +
> +                      (match (command-line)
> +                        ((_ (= string->number (? integer? uid))
> +                            (= string->number (? integer? gid)))
> +                         (setlocale LC_ALL "C.UTF-8") ;for file name decoding

Isn't C.UTF-8 the default locale used in Guile?  Or is there a reason
why it shouldn't be?  I'm still surprised as to why this is needed.

> +                         (setvbuf (current-output-port) 'line)
> +                         (claim-data-ownership uid gid)))))))
> +
>  (define-record-type* <guix-configuration>
>    guix-configuration make-guix-configuration
>    guix-configuration?
> @@ -1959,6 +2053,8 @@ (define-record-type* <guix-configuration>
>                      (default #f))
>    (tmpdir           guix-tmpdir                   ;string | #f
>                      (default #f))
> +  (privileged?      guix-configuration-privileged?
> +                    (default #t))
>    (build-machines   guix-configuration-build-machines ;list of gexps | '()
>                      (default '()))
>    (environment      guix-configuration-environment  ;list of strings
> @@ -2021,7 +2117,7 @@ (define shepherd-discover-action
>                      (environ environment)
>                      #t)))))
>  
> -(define (guix-shepherd-service config)
> +(define (guix-shepherd-services config)
>    "Return a <shepherd-service> for the Guix daemon service with CONFIG."
>    (define locales
>      (let-system (system target)
> @@ -2030,16 +2126,57 @@ (define (guix-shepherd-service config)
>            glibc-utf8-locales)))
>  
>    (match-record config <guix-configuration>
> -    (guix build-group build-accounts chroot? authorize-key? authorized-keys
> +    (guix privileged?
> +          build-group build-accounts chroot? authorize-key? authorized-keys
>            use-substitutes? substitute-urls max-silent-time timeout
>            log-compression discover? extra-options log-file
>            http-proxy tmpdir chroot-directories environment
>            socket-directory-permissions socket-directory-group
>            socket-directory-user)
>      (list (shepherd-service
> +           (provision '(guix-ownership))
> +           (requirement '(user-processes user-homes))
> +           (one-shot? #t)
> +           (start #~(lambda ()
> +                      (let* ((store #$(%store-prefix))
> +                             (stat (lstat store))
> +                             (privileged? #$(guix-configuration-privileged?
> +                                             config))
> +                             (change-ownership #$(guix-ownership-change-program))
> +                             (with-writable-store #$(run-with-writable-store)))
> +                        ;; Check whether we're switching from privileged to
> +                        ;; unprivileged guix-daemon, or vice versa, and adjust
> +                        ;; file ownership accordingly.  Spawn a child process
> +                        ;; if and only if something needs to be changed.
> +                        ;;
> +                        ;; Note: This service remains in 'starting' state for
> +                        ;; as long as CHANGE-OWNERSHIP is running.  That way,
> +                        ;; 'guix-daemon' starts only once we're done.
> +                        (cond ((and (not privileged?)
> +                                    (or (zero? (stat:uid stat))
> +                                        (zero? (stat:gid stat))))
> +                               (let ((user (getpwnam "guix-daemon")))
> +                                 (format #t "Changing to unprivileged guix-daemon.~%")

Too large; did you change your editor settings? :-).

[...]

> -(define (run-guix-daemon-test os)
> +(define (run-guix-daemon-test os name)
>    (define test-image
>      (image (operating-system os)
>             (format 'compressed-qcow2)
> @@ -1161,6 +1162,12 @@ (define (run-guix-daemon-test os)
>              ;; Wait for 'guix-daemon' to be up.
>              (marionette-eval '(begin
>                                  (use-modules (gnu services herd))
> +                                (start-service 'guix-daemon)
> +
> +                                ;; XXX: Do it a second time to work around
> +                                ;; <https://issues.guix.gnu.org/77274> and its
> +                                ;; effect on the 'guix-ownership' service.
> +                                ;; TODO: Remove when Shepherd 1.0.4
> is out.

Shepherd 1.0.4 is out!

>                                  (start-service 'guix-daemon))

Are you sure this translates to 'wait for X to be up?'  In my recent
experience, this returned very quickly, not synchronously to the service
being up.  (I seem to recall for about Shephed 1.0 there's now a
distinction in the state of a service 'starting' and 'running'.).  In
the recently added ngircd-service-type, I've used this in its system
tests:

--8<---------------cut here---------------start------------->8---
+          (test-assert "ngircd service runs"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd))
+                (wait-for-service 'ngircd))
--8<---------------cut here---------------end--------------->8---

With the above nitpicks taken into account,

Reviewed-by: Maxim Cournoyer <[email-masked]>

--
Thanks,
Maxim