Subject: [bug#72337] Add /etc/subuid and /etc/subgid support

From: Ludovic Courtès <[email-masked]>
Date: Wed, 04 Sep 2024 23:20:06 +0200

Giacomo Leidi <[email-masked]> skribis:

> This commit adds a Guix System service to handle allocation of subuid
> and subgid requests.  Users that don't care can just add themselves as a
> subid-range and don't need to specify anything but their user name.
> Users that care about specific ranges, such as possibly LXD, can specify
> a start and a count.
>
> * doc/guix.texi: Document the new service.
> * gnu/build/activation.scm (activate-subuids+subgids): New variable.
> * gnu/local.mk: Add gnu/tests/shadow.scm.
> * gnu/system/accounts.scm (sexp->subid-range): New variable.
> * gnu/system/shadow.scm (%root-subid): New variable;
> (subids-configuration): new record;
> (subid-range->gexp): new variable;
> (assert-valid-subids): new variable;
> (delete-duplicate-ranges): new variable;
> (subids-activation): new variable;
> (subids-extension): new record;
> (append-subid-ranges): new variable;
> (subids-extension-merge): new variable;
> (subids-service-type): new variable.
> * gnu/tests/shadow.scm (subids): New system test.
>
> Change-Id: I3755e1c75771220c74fe8ae5de1a7d90f2376635

Nice.

> +The @code{(gnu system shadow)} module exposes the
> +@code{subids-service-type}, its configuration record
> +@code{subids-configuration} and its extension record
> +@code{subids-extension}.

I think this section should start by defining briefly what a
“subordinate ID” is, with a cross-reference to a primary source for that
(unfortunately glibc’s manual has nothing about it, so that’d be Linux
man pages I guess), and by giving an idea of what it’s used for.

It should use “subuid” and “subgid” only after it has introduced them as
abbreviations of “subordinate UID”.

> +for the root account to both @code{/etc/subuid} and @code{/etc/subgid}, possibly

s/@code/@file/

> +(define %sub-id-min
> +  (@@ (gnu build accounts) %sub-id-min))
> +(define %sub-id-max
> +  (@@ (gnu build accounts) %sub-id-max))
> +(define %sub-id-count
> +  (@@ (gnu build accounts) %sub-id-count))

Use single ‘@’ or, better yet, #:use-module the thing.

> +(define (assert-valid-subids ranges)
> +  (cond ((>= (fold + 0 (map subid-range-count ranges))
> +             (- %sub-id-max %sub-id-min -1))
> +         (raise
> +          (string-append
> +           "The configured ranges are more than the "
> +           (number->string
> +            (- %sub-id-max %sub-id-min -1)) " max allowed.")))

Same comment as before regarding ‘raise’.

In this case, you could do: (raise (formatted-message (G_ …) …)).
This is done elsewhere in the code.

> +                (define slurp
> +                  (lambda args
> +                    (let* ((port (apply open-pipe* OPEN_READ args))
> +                           (output (read-lines port))
> +                           (status (close-pipe port)))
> +                      output)))
> +                (let* ((response1 (slurp
> +                                   ,(string-append #$coreutils "/bin/cat")
> +                                   "/etc/subgid"))
> +                       (response2 (slurp
> +                                   ,(string-append #$coreutils "/bin/cat")
> +                                   "/etc/subuid")))
> +                  (list (string-join response1 "\n") (string-join response2 "\n"))))

Instead of running ‘cat’, I would suggest using:

  (call-with-input-file "/etc/subuid" get-string-all)

or similar; it’s much simpler.

Also, it would be nice if the test could actually exercise subordinate
IDs, with ‘newuidmap’ or some such.  Is that within reach?

Thanks,
Ludo’.