[bug#44700] services: setuid: More configurable setuid support.

["Ludovic Courtès" <ludo@gnu.org>]

Hello!

Christopher Lemmer Webber <cwebber@dustycloud.org> skribis:

>>From eadac673fb22132c555a4e1cee57a6308ecfdad4 Mon Sep 17 00:00:00 2001
> From: Christopher Lemmer Webber <cwebber@dustycloud.org>
> 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.

This looks like the right approach to me!

> +  (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)))

Nitpick: I’d write “program” or “source” instead of “src-path” and avoid
square brackets for consistency with the rest of the code base (you
spent time in Racket-land, didn’t you? ;-)).

> +(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))))

Maybe what we could do is rename ‘operating-system-setuid-programs’ to
’%operating-system-setuid-programs’, keep that internal, and add a new
‘operating-system-setuid-programs’ that calls the other one and
“canonicalizes” list entries so that they’re all <setuid-program>
records.

It would call:

  (warning log (G_ "representing setuid programs with strings is \
deprecated; use 'setuid-program' instead~%"))

WDYT?

Could you also update the “Setuid Programs” section of the manual?

In a subsequent commit, we need to adjust all the services that extend
‘setuid-program-service-type’ so they pass a <setuid-program> and not a
string.

Thanks!

Ludo’.