Subject: bug#44700: closed (Re: [PATCH v3 2/2] services: Migrate to <setuid-program>.)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 29 Jul 2021 16:19:02 +0000

Your bug report

#44700: services: setuid: More configurable setuid support.

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
44700: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=44700
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [PATCH v3 2/2] services: Migrate to <setuid-program>.
From: Christine Lemmer-Webber <[email-masked]>
To: Brice Waegeneire <[email-masked]>
Cc: [email-masked]
Date: Thu, 29 Jul 2021 12:18:33 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Oh, forgot to close it.

Christine Lemmer-Webber writes:

> Got the all clear to push to master.  Rebased and pushed! :)
>
> Christine Lemmer-Webber writes:
>
>> I rebased the patches and created the branch origin/wip-setuid.
>> (I also updated my name... again.  Should be the final update.)
>>
>> Looks like the tests all pass.  I don't want to let this bitrot again.
>> Does anyone have an objection to me pushing this to master?
>>
>> If nobody objects I'm gonna do it!
>>
>>
>> Chris Lemmer-Webber writes:
>>
>>> Looks good to me.  I'd say push it... let's not let this bitrot again!
>>>
>>> Brice Waegeneire writes:
>>>
>>>> * gnu/services/dbus.scm (dbus-setuid-programs, polkit-setuid-programs):
>>>>   Return setuid-programs.
>>>> * gnu/services/desktop.scm (enlightenment-setuid-programs): Return
>>>>  setuid-programs.
>>>>  (%desktop-services)[mount-setuid-helpers]: Use setuid-programs.
>>>> * gnu/services/docker.scm (singularity-setuid-programs): Return
>>>>  setuid-programs.
>>>> * gnu/services/xorg.scm(screen-locker-setuid-programs): Return
>>>>  setuid-programs.
>>>> * gnu/system.scm (%setuid-programs): Return setuid-programs.
>>>> * doc/guix.texi (Setuid Programs, operating-system Reference): Replace
>>>>   'list of G-expressions' with 'list of <setuid-program>'.
>>>> ---
>>>>  doc/guix.texi            | 19 +++++++++++--------
>>>>  gnu/services/dbus.scm    | 13 +++++++++----
>>>>  gnu/services/desktop.scm | 26 ++++++++++++++++----------
>>>>  gnu/services/docker.scm  |  9 ++++++---
>>>>  gnu/services/xorg.scm    |  4 +++-
>>>>  gnu/system.scm           | 31 ++++++++++++++++---------------
>>>>  6 files changed, 61 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/doc/guix.texi b/doc/guix.texi
>>>> index f7a72b9885..7919332521 100644
>>>> --- a/doc/guix.texi
>>>> +++ b/doc/guix.texi
>>>> @@ -13860,8 +13860,8 @@ Linux @dfn{pluggable authentication module} (PAM) services.
>>>>  @c FIXME: Add xref to PAM services section.
>>>>  
>>>>  @item @code{setuid-programs} (default: @code{%setuid-programs})
>>>> -List of string-valued G-expressions denoting setuid programs.
>>>> -@xref{Setuid Programs}.
>>>> +List of @code{<setuid-program>}.  @xref{Setuid Programs}, for more
>>>> +information.
>>>>  
>>>>  @item @code{sudoers-file} (default: @code{%sudoers-specification})
>>>>  @cindex sudoers file
>>>> @@ -32421,13 +32421,15 @@ the store, we let the system administrator @emph{declare} which programs
>>>>  should be setuid root.
>>>>  
>>>>  The @code{setuid-programs} field of an @code{operating-system}
>>>> -declaration contains a list of G-expressions denoting the names of
>>>> -programs to be setuid-root (@pxref{Using the Configuration System}).
>>>> -For instance, the @command{passwd} program, which is part of the Shadow
>>>> -package, can be designated by this G-expression (@pxref{G-Expressions}):
>>>> +declaration contains a list of @code{<setuid-program>} denoting the
>>>> +names of programs to have a setuid or setgid bit set (@pxref{Using the
>>>> +Configuration System}).  For instance, the @command{passwd} program,
>>>> +which is part of the Shadow package, with a setuid root can be
>>>> +designated like this:
>>>>  
>>>>  @example
>>>> -#~(string-append #$shadow "/bin/passwd")
>>>> +(setuid-program
>>>> +  (program (file-append #$shadow "/bin/passwd")))
>>>>  @end example
>>>>  
>>>>  @deftp {Data Type} setuid-program
>>>> @@ -32458,7 +32460,8 @@ A default set of setuid programs is defined by the
>>>>  @code{%setuid-programs} variable of the @code{(gnu system)} module.
>>>>  
>>>>  @defvr {Scheme Variable} %setuid-programs
>>>> -A list of G-expressions denoting common programs that are setuid-root.
>>>> +A list of @code{<setuid-program>} denoting common programs that are
>>>> +setuid-root.
>>>>  
>>>>  The list includes commands such as @command{passwd}, @command{ping},
>>>>  @command{su}, and @command{sudo}.
>>>> diff --git a/gnu/services/dbus.scm b/gnu/services/dbus.scm
>>>> index af1a1e4c3a..e7b3dac166 100644
>>>> --- a/gnu/services/dbus.scm
>>>> +++ b/gnu/services/dbus.scm
>>>> @@ -2,6 +2,7 @@
>>>>  ;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2019, 2020 Ludovic Courtès <[email-masked]>
>>>>  ;;; Copyright © 2015 Sou Bunnbu <[email-masked]>
>>>>  ;;; Copyright © 2021 Maxime Devos <[email-masked]>
>>>> +;;; Copyright © 2021 Brice Waegeneire <[email-masked]>
>>>>  ;;;
>>>>  ;;; This file is part of GNU Guix.
>>>>  ;;;
>>>> @@ -21,6 +22,7 @@
>>>>  (define-module (gnu services dbus)
>>>>    #:use-module (gnu services)
>>>>    #:use-module (gnu services shepherd)
>>>> +  #:use-module (gnu system setuid)
>>>>    #:use-module (gnu system shadow)
>>>>    #:use-module (gnu system pam)
>>>>    #:use-module ((gnu packages glib) #:select (dbus))
>>>> @@ -156,10 +158,12 @@ includes the @code{etc/dbus-1/system.d} directories of each package listed in
>>>>           (shell (file-append shadow "/sbin/nologin")))))
>>>>  
>>>>  (define dbus-setuid-programs
>>>> -  ;; Return the file name of the setuid program that we need.
>>>> +  ;; Return a list of <setuid-program> for the program that we need.
>>>>    (match-lambda
>>>>      (($ <dbus-configuration> dbus services)
>>>> -     (list (file-append dbus "/libexec/dbus-daemon-launch-helper")))))
>>>> +     (list (setuid-program
>>>> +            (program (file-append
>>>> +                      dbus "/libexec/dbus-daemon-launch-helper")))))))
>>>>  
>>>>  (define (dbus-activation config)
>>>>    "Return an activation gexp for D-Bus using @var{config}."
>>>> @@ -335,8 +339,9 @@ tuples, are all set as environment variables when the bus daemon launches it."
>>>>  (define polkit-setuid-programs
>>>>    (match-lambda
>>>>      (($ <polkit-configuration> polkit)
>>>> -     (list (file-append polkit "/lib/polkit-1/polkit-agent-helper-1")
>>>> -           (file-append polkit "/bin/pkexec")))))
>>>> +     (map file-like->setuid-program
>>>> +          (list (file-append polkit "/lib/polkit-1/polkit-agent-helper-1")
>>>> +                (file-append polkit "/bin/pkexec"))))))
>>>>  
>>>>  (define polkit-service-type
>>>>    (service-type (name 'polkit)
>>>> diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
>>>> index cd800fcc2b..64d0e85301 100644
>>>> --- a/gnu/services/desktop.scm
>>>> +++ b/gnu/services/desktop.scm
>>>> @@ -12,6 +12,7 @@
>>>>  ;;; Copyright © 2019 David Wilson <[email-masked]>
>>>>  ;;; Copyright © 2020 Tobias Geerinckx-Rice <[email-masked]>
>>>>  ;;; Copyright © 2020 Reza Alizadeh Majd <[email-masked]>
>>>> +;;; Copyright © 2021 Brice Waegeneire <[email-masked]>
>>>>  ;;;
>>>>  ;;; This file is part of GNU Guix.
>>>>  ;;;
>>>> @@ -40,6 +41,7 @@
>>>>    #:use-module ((gnu system file-systems)
>>>>                  #:select (%elogind-file-systems file-system))
>>>>    #:use-module (gnu system)
>>>> +  #:use-module (gnu system setuid)
>>>>    #:use-module (gnu system shadow)
>>>>    #:use-module (gnu system pam)
>>>>    #:use-module (gnu packages glib)
>>>> @@ -1034,14 +1036,15 @@ rules."
>>>>  
>>>>  (define (enlightenment-setuid-programs enlightenment-desktop-configuration)
>>>>    (match-record enlightenment-desktop-configuration
>>>> -                <enlightenment-desktop-configuration>
>>>> -                (enlightenment)
>>>> -    (list (file-append enlightenment
>>>> -                       "/lib/enlightenment/utils/enlightenment_sys")
>>>> -          (file-append enlightenment
>>>> -                       "/lib/enlightenment/utils/enlightenment_system")
>>>> -          (file-append enlightenment
>>>> -                       "/lib/enlightenment/utils/enlightenment_ckpasswd"))))
>>>> +      <enlightenment-desktop-configuration>
>>>> +    (enlightenment)
>>>> +    (map file-like->setuid-program
>>>> +         (list (file-append enlightenment
>>>> +                            "/lib/enlightenment/utils/enlightenment_sys")
>>>> +               (file-append enlightenment
>>>> +                            "/lib/enlightenment/utils/enlightenment_system")
>>>> +               (file-append enlightenment
>>>> +                            "/lib/enlightenment/utils/enlightenment_ckpasswd")))))
>>>>  
>>>>  (define enlightenment-desktop-service-type
>>>>    (service-type
>>>> @@ -1204,8 +1207,11 @@ or setting its password with passwd.")))
>>>>           ;; Allow desktop users to also mount NTFS and NFS file systems
>>>>           ;; without root.
>>>>           (simple-service 'mount-setuid-helpers setuid-program-service-type
>>>> -                         (list (file-append nfs-utils "/sbin/mount.nfs")
>>>> -                               (file-append ntfs-3g "/sbin/mount.ntfs-3g")))
>>>> +                         (map (lambda (program)
>>>> +                                (setuid-program
>>>> +                                 (program program)))
>>>> +                              (list (file-append nfs-utils "/sbin/mount.nfs")
>>>> +                               (file-append ntfs-3g "/sbin/mount.ntfs-3g"))))
>>>>  
>>>>           ;; The global fontconfig cache directory can sometimes contain
>>>>           ;; stale entries, possibly referencing fonts that have been GC'd,
>>>> diff --git a/gnu/services/docker.scm b/gnu/services/docker.scm
>>>> index be85316180..ef551480aa 100644
>>>> --- a/gnu/services/docker.scm
>>>> +++ b/gnu/services/docker.scm
>>>> @@ -4,6 +4,7 @@
>>>>  ;;; Copyright © 2020, 2021 Maxim Cournoyer <[email-masked]>
>>>>  ;;; Copyright © 2020 Efraim Flashner <[email-masked]>
>>>>  ;;; Copyright © 2020 Jesse Dowell <[email-masked]>
>>>> +;;; Copyright © 2021 Brice Waegeneire <[email-masked]>
>>>>  ;;;
>>>>  ;;; This file is part of GNU Guix.
>>>>  ;;;
>>>> @@ -26,6 +27,7 @@
>>>>    #:use-module (gnu services base)
>>>>    #:use-module (gnu services dbus)
>>>>    #:use-module (gnu services shepherd)
>>>> +  #:use-module (gnu system setuid)
>>>>    #:use-module (gnu system shadow)
>>>>    #:use-module (gnu packages docker)
>>>>    #:use-module (gnu packages linux)               ;singularity
>>>> @@ -195,9 +197,10 @@ bundles in Docker containers.")
>>>>                                                             "-helper")))
>>>>                                   '("action" "mount" "start")))))
>>>>  
>>>> -  (list (file-append helpers "/singularity-action-helper")
>>>> -        (file-append helpers "/singularity-mount-helper")
>>>> -        (file-append helpers "/singularity-start-helper")))
>>>> +  (map file-like->setuid-program
>>>> +       (list (file-append helpers "/singularity-action-helper")
>>>> +             (file-append helpers "/singularity-mount-helper")
>>>> +             (file-append helpers "/singularity-start-helper"))))
>>>>  
>>>>  (define singularity-service-type
>>>>    (service-type (name 'singularity)
>>>> diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
>>>> index 8ffea3b9dd..d95f8beb7a 100644
>>>> --- a/gnu/services/xorg.scm
>>>> +++ b/gnu/services/xorg.scm
>>>> @@ -8,6 +8,7 @@
>>>>  ;;; Copyright © 2020 shtwzrd <[email-masked]>
>>>>  ;;; Copyright © 2020 Jakub Kądziołka <[email-masked]>
>>>>  ;;; Copyright © 2020 Alex Griffin <[email-masked]>
>>>> +;;; Copyright © 2021 Brice Waegeneire <[email-masked]>
>>>>  ;;;
>>>>  ;;; This file is part of GNU Guix.
>>>>  ;;;
>>>> @@ -29,6 +30,7 @@
>>>>    #:use-module (gnu services)
>>>>    #:use-module (gnu services shepherd)
>>>>    #:use-module (gnu system pam)
>>>> +  #:use-module (gnu system setuid)
>>>>    #:use-module (gnu system keyboard)
>>>>    #:use-module (gnu services base)
>>>>    #:use-module (gnu services dbus)
>>>> @@ -681,7 +683,7 @@ reboot_cmd " shepherd "/sbin/reboot\n"
>>>>                               #:allow-empty-passwords? empty?)))))
>>>>  
>>>>  (define screen-locker-setuid-programs
>>>> -  (compose list screen-locker-program))
>>>> +  (compose list file-like->setuid-program screen-locker-program))
>>>>  
>>>>  (define screen-locker-service-type
>>>>    (service-type (name 'screen-locker)
>>>> diff --git a/gnu/system.scm b/gnu/system.scm
>>>> index 385c36a484..681dd33630 100644
>>>> --- a/gnu/system.scm
>>>> +++ b/gnu/system.scm
>>>> @@ -1105,22 +1105,23 @@ use 'plain-file' instead~%")
>>>>  (define %setuid-programs
>>>>    ;; Default set of setuid-root programs.
>>>>    (let ((shadow (@ (gnu packages admin) shadow)))
>>>> -    (list (file-append shadow "/bin/passwd")
>>>> -          (file-append shadow "/bin/sg")
>>>> -          (file-append shadow "/bin/su")
>>>> -          (file-append shadow "/bin/newgrp")
>>>> -          (file-append shadow "/bin/newuidmap")
>>>> -          (file-append shadow "/bin/newgidmap")
>>>> -          (file-append inetutils "/bin/ping")
>>>> -          (file-append inetutils "/bin/ping6")
>>>> -          (file-append sudo "/bin/sudo")
>>>> -          (file-append sudo "/bin/sudoedit")
>>>> -          (file-append fuse "/bin/fusermount")
>>>> +    (map file-like->setuid-program
>>>> +         (list (file-append shadow "/bin/passwd")
>>>> +               (file-append shadow "/bin/sg")
>>>> +               (file-append shadow "/bin/su")
>>>> +               (file-append shadow "/bin/newgrp")
>>>> +               (file-append shadow "/bin/newuidmap")
>>>> +               (file-append shadow "/bin/newgidmap")
>>>> +               (file-append inetutils "/bin/ping")
>>>> +               (file-append inetutils "/bin/ping6")
>>>> +               (file-append sudo "/bin/sudo")
>>>> +               (file-append sudo "/bin/sudoedit")
>>>> +               (file-append fuse "/bin/fusermount")
>>>>  
>>>> -          ;; To allow mounts with the "user" option, "mount" and "umount" must
>>>> -          ;; be setuid-root.
>>>> -          (file-append util-linux "/bin/mount")
>>>> -          (file-append util-linux "/bin/umount"))))
>>>> +               ;; To allow mounts with the "user" option, "mount" and "umount" must
>>>> +               ;; be setuid-root.
>>>> +               (file-append util-linux "/bin/mount")
>>>> +               (file-append util-linux "/bin/umount")))))
>>>>  
>>>>  (define %sudoers-specification
>>>>    ;; Default /etc/sudoers contents: 'root' and all members of the 'wheel'

Subject: services: setuid: More configurable setuid support.
From: Christopher Lemmer Webber <[email-masked]>
To: [email-masked]
Date: Mon, 16 Nov 2020 18:29:11 -0500
List-Id: <debbugs-submit.debbugs.gnu.org>

This patch allows for configuring the specific user, group, and whether
to set the setuid and setgid bits.

See also:
  https://lists.gnu.org/archive/html/guix-devel/2020-11/msg00369.html

But I thought I'd open this here so we could track changes since this is
technically independent of the postfix stuff.  Anyway, patch attached.
One change since the last email above is that I added support for
string-based username/groups.

This also needs documentation, I suppose, so that should be done.
But it would be good to know if this patch looks like it's on the "right
path" or not.

>From eadac673fb22132c555a4e1cee57a6308ecfdad4 Mon Sep 17 00:00:00 2001
From: Christopher Lemmer Webber <[email-masked]>
Date: Sun, 15 Nov 2020 16:58:52 -0500
Subject: [PATCH] services: setuid: More configurable setuid support.

New record <setuid-program> with fields for setting the specific user and
group, as well as specifically selecting the setuid and setgid bits, for a
program within the setuid-program-service.

* gnu/services.scm (<setuid-program>): New record type.
  (setuid-program, make-setuid-program, setuid-program?)
  (setuid-program-program, stuid-program-setuid?, setuid-program-setgid?)
  (setuid-program-user, setuid-program-group): New variables, export them.
  (setuid-program-entry): New variable, a procedure used for the
  service-extension of activation-service-type as set up by
  setuid-program-service-type.  Unpacks the <setuid-program> record,
  handing off within the gexp to activate-setuid-programs.
  (setuid-program-service-type): Make use of setuid-program-entry.
* gnu/build/activation.scm (activate-setuid-programs): Update to expect a
  ftagged list for each program entry, pre-unpacked from the <setuid-program>
  record before being handed to this procedure.
---
 gnu/build/activation.scm | 46 +++++++++++++++++++++----------------
 gnu/services.scm         | 49 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 22 deletions(-)

diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 4b67926e88..fd17ce0434 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -229,13 +229,6 @@ they already exist."
 (define (activate-setuid-programs programs)
   "Turn PROGRAMS, a list of file names, into setuid programs stored under
 %SETUID-DIRECTORY."
-  (define (make-setuid-program prog)
-    (let ((target (string-append %setuid-directory
-                                 "/" (basename prog))))
-      (copy-file prog target)
-      (chown target 0 0)
-      (chmod target #o6555)))
-
   (format #t "setting up setuid programs in '~a'...~%"
           %setuid-directory)
   (if (file-exists? %setuid-directory)
@@ -247,18 +240,33 @@ they already exist."
                          string<?))
       (mkdir-p %setuid-directory))
 
-  (for-each (lambda (program)
-              (catch 'system-error
-                (lambda ()
-                  (make-setuid-program program))
-                (lambda args
-                  ;; If we fail to create a setuid program, better keep going
-                  ;; so that we don't leave %SETUID-DIRECTORY empty or
-                  ;; half-populated.  This can happen if PROGRAMS contains
-                  ;; incorrect file names: <https://bugs.gnu.org/38800>.
-                  (format (current-error-port)
-                          "warning: failed to make '~a' setuid-root: ~a~%"
-                          program (strerror (system-error-errno args))))))
+  (for-each (match-lambda
+              [('setuid-program src-path setuid? setgid? user group)
+               (let ((uid (match user
+                            [(? string?) (passwd:uid (getpwnam user))]
+                            [(? integer?) user]))
+                     (gid (match group
+                            [(? string?) (group:gid (getgrnam user))]
+                            [(? integer?) group])))
+                 (catch 'system-error
+                   (lambda ()
+                     (let ((target (string-append %setuid-directory
+                                                  "/" (basename src-path)))
+                           (mode (+ #o0555                   ; base permissions
+                                    (if setuid? #o4000 0)    ; setuid bit
+                                    (if setgid? #o2000 0)))) ; setgid bit
+                       (copy-file src-path target)
+                       (chown target uid gid)
+                       (chmod target mode)))
+                   (lambda args
+                     ;; If we fail to create a setuid program, better keep going
+                     ;; so that we don't leave %SETUID-DIRECTORY empty or
+                     ;; half-populated.  This can happen if PROGRAMS contains
+                     ;; incorrect file names: <https://bugs.gnu.org/38800>.
+                     (format (current-error-port)
+                             "warning: failed to make '~a' setuid-root: ~a~%"
+                             (setuid-program-program program)
+                             (strerror (system-error-errno args))))))])
             programs))
 
 (define (activate-special-files special-files)
diff --git a/gnu/services.scm b/gnu/services.scm
index 4b30399adc..a5b4734152 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -87,6 +87,14 @@
             ambiguous-target-service-error-service
             ambiguous-target-service-error-target-type
 
+            setuid-program
+            setuid-program?
+            setuid-program-program
+            setuid-program-setuid?
+            setuid-program-setgid?
+            setuid-program-user
+            setuid-program-group
+
             system-service-type
             provenance-service-type
             sexp->system-provenance
@@ -773,13 +781,48 @@ directory."
 FILES must be a list of name/file-like object pairs."
   (service etc-service-type files))
 
+(define-record-type* <setuid-program> setuid-program make-setuid-program
+  setuid-program?
+  ;; Path to program to link with setuid permissions
+  (program       setuid-program-program)          ;string
+  ;; Whether to set user setuid bit
+  (setuid?       setuid-program-setuid?           ;boolean
+                 (default #t))
+  ;; Whether to set user setgid bit
+  (setgid?       setuid-program-setgid?           ;boolean
+                 (default #t))
+  ;; The user this should be set to (defaults to root)
+  (user          setuid-program-user              ;integer or string
+                 (default 0))
+  ;; Group we want to set this to (defaults to root)
+  (group         setuid-program-group             ;integer or string
+                 (default 0)))
+
+(define (setuid-program-entry programs)
+  #~(activate-setuid-programs
+     ;; convert into a tagged list structure as expected by
+     ;; activate-setuid-programs
+     (list #$@(map (match-lambda
+                     [(? setuid-program? sp)
+                      #~(list 'setuid-program
+                              #$(setuid-program-program sp)
+                              #$(setuid-program-setuid? sp)
+                              #$(setuid-program-setgid? sp)
+                              #$(setuid-program-user sp)
+                              #$(setuid-program-group sp))]
+                     ;; legacy, non-<setuid-program> structure
+                     [program
+                      ;; TODO: Spit out a warning here?
+                      #~(list 'setuid-program
+                              #$program
+                              #t #t 0 0)])
+                   programs))))
+
 (define setuid-program-service-type
   (service-type (name 'setuid-program)
                 (extensions
                  (list (service-extension activation-service-type
-                                          (lambda (programs)
-                                            #~(activate-setuid-programs
-                                               (list #$@programs))))))
+                                          setuid-program-entry)))
                 (compose concatenate)
                 (extend append)
                 (description
-- 
2.29.1