Package: guix-patches;
Reported by: Ludovic Courtès <ludo <at> gnu.org>
Date: Wed, 26 Mar 2025 16:50:01 UTC
Severity: normal
Tags: patch
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Ludovic Courtès <ludo <at> gnu.org> Cc: 77288 <at> debbugs.gnu.org Subject: [bug#77288] [PATCH v2 7/8] services: guix: Allow ‘guix-daemon’ to run without root privileges. Date: Sun, 20 Apr 2025 23:24:38 +0900
Hi,
Ludovic Courtès <ludo <at> gnu.org> 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 <maxim.cournoyer <at> gmail.com>
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.