[shepherd] 01/02: service: Really set ownership and permissions on Unix sockets.

[Ludovic Courtès]

civodul pushed a commit to branch main
in repository shepherd.

commit f5b74117336d21c76050dd01afc754e2700e500d
Author: Ulrich Baum <ulrich.baum@ubaum.de>
AuthorDate: Sun Dec 17 14:38:12 2023 +0100

    service: Really set ownership and permissions on Unix sockets.
    
    Fixes <https://issues.guix.gnu.org/67867>.
    
    Previously, ownership and permissions of AF_UNIX sockets created by
    make-inetd-constructor and make-systemd-constructor were not set,
    leaving the socket with root:root and 755 permissions.
    
    * modules/shepherd/service.scm (endpoint->listening-socket): For AF_UNIX
    sockets, pass the socket file name in ‘chown’ and ‘chmod’ calls.
    * tests/systemd.sh: Pass #:socket-directory-permissions to ‘endpoint’.
    Check permissions on the socket and on its parent directory.
    * NEWS: Update.
    
    Co-authored-by: Ludovic Courtès <ludo@gnu.org>
---
 NEWS                         |  9 +++++++++
 modules/shepherd/service.scm |  4 ++--
 tests/systemd.sh             | 16 +++++++++++++---
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 9cc8edf..46803d5 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,15 @@ shepherd could eventually hang.  Specifically, the replaced 
service would no
 longer respond to messages, so it would be impossible to start it, to stop it,
 or to unload it.  This is now fixed.
 
+** Fix ownership and permissions on Unix-domain sockets
+   (<https://issues.guix.gnu.org/67867>)
+
+When using an AF_UNIX endpoint with systemd- and inetd-style services, the
+socket file itself would remain owned by root (when shepherd is running as
+root) with permissions 755.  This is now fixed, with ownership set according
+to #:socket-owner and #:socket-group of the endpoint, and permissions on the
+socket set to 666.
+
 ** New #:respawn-delay parameter to ‘service’
    (<https://issues.guix.gnu.org/64665>)
 
diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index f1dd563..5fc3332 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -1871,8 +1871,8 @@ retrying to bind it in one second.")
        (listen sock backlog)
 
        (when (= AF_UNIX (sockaddr:fam address))
-         (chown sock owner group)
-         (chmod sock #o666))
+         (chown (sockaddr:path address) owner group)
+         (chmod (sockaddr:path address) #o666))
 
        sock))))
 
diff --git a/tests/systemd.sh b/tests/systemd.sh
index 90e81f5..8a3b706 100644
--- a/tests/systemd.sh
+++ b/tests/systemd.sh
@@ -23,11 +23,12 @@ socket="t-socket-$$"
 conf="t-conf-$$"
 log="t-log-$$"
 pid="t-pid-$$"
-service_socket="t-service-socket-$$"
+service_socket_dir="t-service-socket-$$"
+service_socket="$service_socket_dir/socket"
 
 herd="herd -s $socket"
 
-trap "cat $log || true; rm -f $service_socket $socket $conf $log;
+trap "cat $log || true; rm -r $service_socket_dir $socket $conf $log;
       test -f $pid && kill \`cat $pid\` || true; rm -f $pid" EXIT
 
 cat > "$conf" <<EOF
@@ -56,7 +57,8 @@ cat > "$conf" <<EOF
               (close-port sock)))))))))
 
 (define %endpoints
-  (list (endpoint (make-socket-address AF_UNIX "$service_socket"))))
+  (list (endpoint (make-socket-address AF_UNIX "$service_socket")
+                  #:socket-directory-permissions #o700)))
 
 (register-services
  (list (service
@@ -80,6 +82,11 @@ while ! test -f "$pid" ; do sleep 0.3 ; done
 
 shepherd_pid="`cat $pid`"
 
+file_permissions ()
+{
+    guile -c "(display (number->string (stat:perms (stat \"$1\")) 8))"
+}
+
 converse_with_echo_server ()
 {
     guile -c "(use-modules (ice-9 match) (ice-9 rdelim))
@@ -97,6 +104,9 @@ $herd start test-systemd-unix
 $herd status test-systemd-unix | grep running
 test $($herd status | grep '\+' | wc -l) -eq 2
 
+test "$(file_permissions "$service_socket_dir")" = "700"
+test "$(file_permissions "$service_socket")" = "666"
+
 for i in $(seq 1 3)
 do
     converse_with_echo_server \