GNU bug report logs - #73925
[PATCH] add access control to daemon socket in shepherd service

Previous Next

Full log

Message #8 received at 73925 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Reepca Russelstein <reepca <at> russelstein.xyz>
Cc: 73925 <at> debbugs.gnu.org
Subject: Re: [bug#73925] [PATCH] add access control to daemon socket in
 shepherd service
Date: Thu, 24 Oct 2024 14:43:48 +0200

Hi,

Reepca Russelstein <reepca <at> russelstein.xyz> skribis:

> From b5163889efb544cfe83cd2bcb3ebd3a957c95a18 Mon Sep 17 00:00:00 2001
> Message-ID: <b5163889efb544cfe83cd2bcb3ebd3a957c95a18.1729465936.git.reepca <at> russelstein.xyz>
> From: Reepca Russelstein <reepca <at> russelstein.xyz>
> Date: Sat, 19 Oct 2024 22:43:27 -0500
> Subject: [PATCH] services: guix-configuration: add access control to daemon
>  socket.
>
> * gnu/services/base.scm
>   (guix-configuration-socket-directory-{perms,group,user}): new fields.
>   (guix-shepherd-service): use them.
> * doc/guix.texi: document them.
>
> Change-Id: Ic228377b25a83692b0c637dafbd03c4609e332fc

That’s a welcome addition.

> +@item @code{socket-directory-perms} (default: @code{#o755})

s/perms/permissions/

> +Permissions to set for the directory @file{/var/guix/daemon-socket}.
> +This, together with @code{socket-directory-group} and
> +@code{socket-directory-user}, determines who can connect to the guix
> +daemon via its unix socket.  TCP socket operation is unaffected by
> +these.

s/guix daemon/build daemon/ and s/unix/Unix/

> +@item @code{socket-directory-group} (default: @code{#f})
> +Group to set for the directory @file{/var/guix/daemon-socket}, or
> +@code{#f} to keep its group as root.
> +
> +@item @code{socket-directory-user} (default: @code{#f})
> +User to set for the directory @file{/var/guix/daemon-socket}, or
> +@code{#f} to keep its user as root.

Maybe group them together:

  @item @code{socket-directory-user} (default: @code{#f})
  @itemx @code{socket-directory-group} (default: @code{#f})
  User and group owning the @file{/var/guix/daemon-socket} directory.
  …

> -    (guix build-group build-accounts 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)
> +                (guix build-group build-accounts authorize-key? authorized-keys

Please avoid reindenting.

> +                  ;; Ensure that a fresh directory is used, in case the old
> +                  ;; one was more permissive and processes have a file
> +                  ;; descriptor referencing it hanging around, ready to use
> +                  ;; with openat.
> +                  (false-if-exception
> +                   (delete-file-recursively "/var/guix/daemon-socket"))
> +                  (let ((perms #$(logand socket-directory-perms
> +                                         (lognot #o022))))
> +                    (mkdir "/var/guix/daemon-socket" perms)
> +                    ;; Override umask
> +                    (chmod "/var/guix/daemon-socket" perms))

Speaking of ‘openat’, maybe use ‘mkdir-p/perms’ instead of doing it in
two steps?

Apart from that it LGTM.

Could you send an updated patch?

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.