* [bug#73925] [PATCH] add access control to daemon socket in shepherd service
@ 2024-10-20 23:31 Reepca Russelstein via Guix-patches via
2024-10-24 12:43 ` Ludovic Courtès
0 siblings, 1 reply; 3+ messages in thread
From: Reepca Russelstein via Guix-patches via @ 2024-10-20 23:31 UTC (permalink / raw)
To: 73925
[-- Attachment #1.1: Type: text/plain, Size: 1214 bytes --]
Passing "--disable-chroot" to guix-daemon makes it possible for the
build users to be taken over by anybody who can start a build: they need
only cause a builder to put a setuid binary in /tmp. That being said,
there are some situations where it currently can't be avoided, like on
Hurd. It would also probably be good to have the ability to harden a
guix daemon in general by restricting access to it. For example,
there's no reason that the ntpd user needs access to the guix daemon
(note that this is distinct from access to the *store*, which is of
course always world-readable).
The attached patch implements that restriction for users of
guix-service-type by limiting access to /var/guix/daemon-socket in
accordance with the user-supplied permissions, user, and group.
Example usage:
------------------------------------
;; Limit access to the guix-daemon socket to members of the "users"
;; group
(modify-services %desktop-services
(guix-service-type config =>
(guix-configuration
(inherit config)
(socket-directory-perms #o750)
(socket-directory-group "users"))))
------------------------------------
- reepca
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-services-guix-configuration-add-access-control-to-da.patch --]
[-- Type: text/x-patch, Size: 5814 bytes --]
From b5163889efb544cfe83cd2bcb3ebd3a957c95a18 Mon Sep 17 00:00:00 2001
Message-ID: <b5163889efb544cfe83cd2bcb3ebd3a957c95a18.1729465936.git.reepca@russelstein.xyz>
From: Reepca Russelstein <reepca@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
---
doc/guix.texi | 15 +++++++++++++++
gnu/services/base.scm | 43 ++++++++++++++++++++++++++++++++++++-------
2 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index cb758f9005..0e387f0a17 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19775,6 +19775,21 @@ Base Services
Environment variables to be set before starting the daemon, as a list of
@code{key=value} strings.
+@item @code{socket-directory-perms} (default: @code{#o755})
+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.
+
+@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.
+
@end table
@end deftp
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index fd2cc9d17a..daedc77468 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1880,7 +1880,13 @@ (define-record-type* <guix-configuration>
(build-machines guix-configuration-build-machines ;list of gexps | '()
(default '()))
(environment guix-configuration-environment ;list of strings
- (default '())))
+ (default '()))
+ (socket-directory-perms guix-configuration-socket-directory-perms
+ (default #o755))
+ (socket-directory-group guix-configuration-socket-directory-group
+ (default #f))
+ (socket-directory-user guix-configuration-socket-directory-user
+ (default #f)))
(define %default-guix-configuration
(guix-configuration))
@@ -1941,10 +1947,12 @@ (define (guix-shepherd-service config)
glibc-utf8-locales)))
(match-record config <guix-configuration>
- (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
+ use-substitutes? substitute-urls max-silent-time timeout
+ log-compression discover? extra-options log-file
+ http-proxy tmpdir chroot-directories environment
+ socket-directory-perms socket-directory-group
+ socket-directory-user)
(list (shepherd-service
(documentation "Run the Guix daemon.")
(provision '(guix-daemon))
@@ -1954,11 +1962,13 @@ (define (guix-shepherd-service config)
shepherd-discover-action))
(modules '((srfi srfi-1)
(ice-9 match)
- (gnu build shepherd)))
+ (gnu build shepherd)
+ (guix build utils)))
(start
(with-imported-modules `(((guix config) => ,(make-config.scm))
,@(source-module-closure
- '((gnu build shepherd))
+ '((gnu build shepherd)
+ (guix build utils))
#:select? not-config?))
#~(lambda args
(define proxy
@@ -1969,7 +1979,26 @@ (define (guix-shepherd-service config)
(define discover?
(or (getenv "discover") #$discover?))
+ (mkdir-p "/var/guix")
+ ;; 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))
+
+ (let* ((user #$socket-directory-user)
+ (uid (if user (passwd:uid (getpwnam user)) -1))
+ (group #$socket-directory-group)
+ (gid (if group (group:gid (getgrnam group)) -1)))
+ (chown "/var/guix/daemon-socket" uid gid))
+
;; Start the guix-daemon from a container, when supported,
;; to solve an installation issue. See the comment below for
;; more details.
--
2.45.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 519 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [bug#73925] [PATCH] add access control to daemon socket in shepherd service
2024-10-20 23:31 [bug#73925] [PATCH] add access control to daemon socket in shepherd service Reepca Russelstein via Guix-patches via
@ 2024-10-24 12:43 ` Ludovic Courtès
2024-10-26 0:10 ` Reepca Russelstein via Guix-patches via
0 siblings, 1 reply; 3+ messages in thread
From: Ludovic Courtès @ 2024-10-24 12:43 UTC (permalink / raw)
To: Reepca Russelstein; +Cc: 73925
Hi,
Reepca Russelstein <reepca@russelstein.xyz> skribis:
> From b5163889efb544cfe83cd2bcb3ebd3a957c95a18 Mon Sep 17 00:00:00 2001
> Message-ID: <b5163889efb544cfe83cd2bcb3ebd3a957c95a18.1729465936.git.reepca@russelstein.xyz>
> From: Reepca Russelstein <reepca@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’.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [bug#73925] [PATCH] add access control to daemon socket in shepherd service
2024-10-24 12:43 ` Ludovic Courtès
@ 2024-10-26 0:10 ` Reepca Russelstein via Guix-patches via
0 siblings, 0 replies; 3+ messages in thread
From: Reepca Russelstein via Guix-patches via @ 2024-10-26 0:10 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 73925
[-- Attachment #1.1: Type: text/plain, Size: 2182 bytes --]
Ludovic Courtès <ludo@gnu.org> writes:
>> + ;; 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?
PERMS is passed directly to mkdir; the umask may cause the permissions
the directory is created with to be less permissive than those, but
never more. The only reason I call chmod here is because the umask may
happen to be more strict than PERMS. mkdir-p/perms creates the
directory with the permissions initially restricted only by the umask,
then later chmods it in a separate step, leaving a window during which
the directory is likely world-executable and world-readable. So while
mkdir-p/perms would be an improvement on the "make sure no components
are symlinks" front, it would be a downgrade in restricting access to
the directory.
This behavior can be remedied by ensuring that the final call to
'mkdirat' passes in the specified permission bits. I've submitted a
patch to do this in issue #74002.
There's also a minor annoyance in that the 'owner' argument that
mkdir-p/perms wants MUST be a passwd object. This means that the uid
and gid to use can't be specified independently, nor can they be
specified as -1 or 0, you *have* to do (getpwnam "root") or something
similar.
For now I'm going to keep this part as-is, since currently using
mkdir-p/perms would neither make it more secure nor more concise.
The attached patch incorporates all the other changes you've mentioned.
- reepca
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-services-guix-configuration-add-access-control-to-da.patch --]
[-- Type: text/x-patch, Size: 5373 bytes --]
From b8ea0288a35c27912580bd7fe861dd6e497f4c33 Mon Sep 17 00:00:00 2001
Message-ID: <b8ea0288a35c27912580bd7fe861dd6e497f4c33.1729840060.git.reepca@russelstein.xyz>
From: Reepca Russelstein <reepca@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-{permissions,group,user}): new fields.
(guix-shepherd-service): use them.
* doc/guix.texi: document them.
Change-Id: I8f4c2e20392ced47c09812e62903c87cc0f4a97a
---
doc/guix.texi | 12 ++++++++++++
gnu/services/base.scm | 38 ++++++++++++++++++++++++++++++++++----
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index cb758f9005..fb750bd449 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19775,6 +19775,18 @@ Base Services
Environment variables to be set before starting the daemon, as a list of
@code{key=value} strings.
+@item @code{socket-directory-permissions} (default: @code{#o755})
+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 build
+daemon via its Unix socket. TCP socket operation is unaffected by
+these.
+
+@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 or
+@code{#f} to keep the user or group as root.
+
@end table
@end deftp
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index fd2cc9d17a..0bd60c5eb5 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1880,7 +1880,14 @@ (define-record-type* <guix-configuration>
(build-machines guix-configuration-build-machines ;list of gexps | '()
(default '()))
(environment guix-configuration-environment ;list of strings
- (default '())))
+ (default '()))
+ (socket-directory-permissions
+ guix-configuration-socket-directory-permissions
+ (default #o755))
+ (socket-directory-group guix-configuration-socket-directory-group
+ (default #f))
+ (socket-directory-user guix-configuration-socket-directory-user
+ (default #f)))
(define %default-guix-configuration
(guix-configuration))
@@ -1944,7 +1951,9 @@ (define (guix-shepherd-service config)
(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)
+ http-proxy tmpdir chroot-directories environment
+ socket-directory-permissions socket-directory-group
+ socket-directory-user)
(list (shepherd-service
(documentation "Run the Guix daemon.")
(provision '(guix-daemon))
@@ -1954,11 +1963,13 @@ (define (guix-shepherd-service config)
shepherd-discover-action))
(modules '((srfi srfi-1)
(ice-9 match)
- (gnu build shepherd)))
+ (gnu build shepherd)
+ (guix build utils)))
(start
(with-imported-modules `(((guix config) => ,(make-config.scm))
,@(source-module-closure
- '((gnu build shepherd))
+ '((gnu build shepherd)
+ (guix build utils))
#:select? not-config?))
#~(lambda args
(define proxy
@@ -1969,7 +1980,26 @@ (define (guix-shepherd-service config)
(define discover?
(or (getenv "discover") #$discover?))
+ (mkdir-p "/var/guix")
+ ;; 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-permissions
+ (lognot #o022))))
+ (mkdir "/var/guix/daemon-socket" perms)
+ ;; Override umask
+ (chmod "/var/guix/daemon-socket" perms))
+
+ (let* ((user #$socket-directory-user)
+ (uid (if user (passwd:uid (getpwnam user)) -1))
+ (group #$socket-directory-group)
+ (gid (if group (group:gid (getgrnam group)) -1)))
+ (chown "/var/guix/daemon-socket" uid gid))
+
;; Start the guix-daemon from a container, when supported,
;; to solve an installation issue. See the comment below for
;; more details.
--
2.45.2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 519 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-10-26 0:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-20 23:31 [bug#73925] [PATCH] add access control to daemon socket in shepherd service Reepca Russelstein via Guix-patches via
2024-10-24 12:43 ` Ludovic Courtès
2024-10-26 0:10 ` Reepca Russelstein via Guix-patches via
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.