Can't setuid programs to anybody but root

edk wrote on 5 Jun 2023 12:00

Recipients:(name . bug-guix)(address . bug-guix@gnu.org)

Message-ID:87h6rmtdzk.fsf@rdklein.fr

Dear Guix developers,

At the end of the email is the code for a minimal container, which tries
to setuid =true=, the simplest binary of all, to user suc.

When line 26 is commented, and the container is built and run with:
sudo $(guix system container  mwe.scm)

One can login to the container and run:
ls -l /run/setuid-programs/true

which yields:
-r-sr-xr-x 1 root root 39488 Jun  5 09:59 /run/setuid-programs/true
as it should.

Also, one can fire up guile and run (getpw "suc") and get in return:
$1 = #("suc" "x" 1000 998 "" "/home/suc" "/gnu/store/m6c5hgqg569mbcjjbp8l8m7q82ascpdl-bash-5.1.16/bin/bash")

However, when line 26 is uncommented, the container can be built, but
when run fails with the error below.
My hunch is that things are done out of order, with setuid binaries
being set up before user creation, but I have no way of checking that.

Please do not hesitate to ping me if I can be of help.

Cheers,

Edouard.

The error:
system container is running as PID 9825
WARNING: (guile-user): imported module (guix build utils) overrides core binding `delete'
Run 'sudo guix container exec 9825 /run/current-system/profile/bin/bash --login'
or run 'sudo nsenter -a -t 9825' to get a shell into it.

WARNING: (guile-user): imported module (guix build utils) overrides core binding `delete'
making '/gnu/store/mnc9lfpn01frmffqa31jy3c381dkgrwl-system' the current system...
WARNING: (guile-user): imported module (guix build utils) overrides core binding `delete'
setting up setuid programs in '/run/setuid-programs'...
Backtrace:
          12 (primitive-load "/gnu/store/bygckv7p4091xqykjnkay4qnazn…")
In gnu/build/linux-container.scm:
    300:8 11 (call-with-temporary-directory #<procedure 7fb026898d70…>)
   397:16 10 (_ "/tmp/guix-directory.B9dmTN")
     62:6  9 (call-with-clean-exit #<procedure 7fb0268a5380 at gnu/b…>)
In unknown file:
           8 (primitive-load "/gnu/store/mnc9lfpn01frmffqa31jy3c381d…")
In ice-9/eval.scm:
    619:8  7 (_ #f)
In unknown file:
           6 (primitive-load "/gnu/store/dib6wfh2r52dfaydz78n33267qx…")
In srfi/srfi-1.scm:
    634:9  5 (for-each #<procedure primitive-load (_)> ("/gnu/sto…" …))
In unknown file:
           4 (primitive-load "/gnu/store/ypwqsx11k2qmxkscmzan6srq87q…")
In srfi/srfi-1.scm:
    634:9  3 (for-each #<procedure 7fb026380538 at gnu/build/activa…> …)
In ice-9/boot-9.scm:
  1747:15  2 (with-exception-handler #<procedure 7fb02683c6f0 at ic…> …)
In gnu/build/activation.scm:
   317:57  1 (_)
In unknown file:
           0 (getpw "suc")

ERROR: In procedure getpw:
In procedure getpw: entry not found



The code

(use-modules
 (guix gexp)
 (gnu system)
 (gnu bootloader)
 (gnu bootloader grub)
 (gnu system file-systems)
 (gnu services)
 (gnu services base)
 (gnu system setuid)
 (gnu packages base))

(operating-system
  (host-name "minimal-container")
  (timezone "UTC")
  (locale "en_US.utf8")
  (bootloader (bootloader-configuration
               (bootloader grub-bootloader)))
  (file-systems %base-file-systems)
  (users (cons
          (user-account
           (name "suc")
           (group "users"))
          %base-user-accounts))
  (setuid-programs
   (cons (setuid-program (program (file-append coreutils "/bin/true"))
                         (user "suc")
                         )
         %setuid-programs))
  (packages %base-packages)
  (services %base-services))

Edouard Klein wrote on 6 Jun 2023 09:21

Recipients:(address . 63904@debbugs.gnu.org)

Message-ID:878rcxt4jt.fsf@rdklein.fr

Dear Guix, CCing the core team,

I tried tracking down the bug.

The fatidic call to getpw was easy enough to find:

The make-setuid-program procedure is given a numeric uid argument. This

numeric uid is found from the user name string by

activate-setuid-program which calls getpwnam

(gnu/build/activation.scm:317).

Now this gave me an idea to sidestep the bug: See below the modified

part of the minimal reproductible example: I just force-assign a uid to

the user I want to setuid to, and give this uid instead of the username

to the setuid record.

This is cumbersome, but it does the job: the call to getpw is averted

and I get a system in which I can setuid to somebody other than root.

However, I'm lost as to how to solve the bug for good. I tried to

understand the call stack, but I can't figure out how in the folding

service machinery the services are ordered. My intuition is that I need

to make it so the folding of non-root setuids happen after the folding

of user and groups (I also have the intuition that root-setuids must

happen before, because folding users and group may require that root

setuid binaries are there, but I have not been able to verify that).

Here is what I was able to find.

getpw is called by activate-setuid-program

activate-setuid-program is called in setuid-program->activation-gexp

setuid-program->activation-gexp is the activation procedure for setuid-program-service-type

setuid-program-service-type is itself an extension of activation-service-type

I'm trying to follow how the service DAG is constructed, and then

walked, from there, but I don't think I have a very clear model of how

it works in my head.

I think the devil may be in:

(define (compute-boot-script _ gexps)

;; Reverse GEXPS so that extensions appear in the boot script in the right

;; order. That is, user extensions would come first, and extensions added

;; by 'essential-services' (e.g., running shepherd) are guaranteed to come

;; last.

(gexp->file "boot"

;; Clean up and activate the system, then spawn shepherd.

#~(begin #$@(reverse gexps))))

Any help there would be greatly appreciated.

Thanks in advance,

Cheers,

Edouard.

(operating-system

(host-name "minimal-container")

(timezone "UTC")

(locale "en_US.utf8")

(bootloader (bootloader-configuration

(bootloader grub-bootloader)))

(file-systems %base-file-systems)

(users (cons

(user-account

(name "suc")

(group "users")

(uid 1042))

%base-user-accounts))

(setuid-programs

(cons (setuid-program (program (file-append coreutils "/bin/true"))

;; (user "suc")

(user 1042)

)

%setuid-programs))

(packages %base-packages)

(services %base-services))

edk@beaver-labs.com writes:

Toggle quote (101 lines)> Dear Guix developers,
>
> At the end of the email is the code for a minimal container, which tries
> to setuid =true=, the simplest binary of all, to user suc.
>
> When line 26 is commented, and the container is built and run with:
> sudo $(guix system container  mwe.scm)
>
> One can login to the container and run:
> ls -l /run/setuid-programs/true
>
> which yields:
> -r-sr-xr-x 1 root root 39488 Jun  5 09:59 /run/setuid-programs/true
> as it should.
>
> Also, one can fire up guile and run (getpw "suc") and get in return:
> $1 = #("suc" "x" 1000 998 "" "/home/suc" "/gnu/store/m6c5hgqg569mbcjjbp8l8m7q82ascpdl-bash-5.1.16/bin/bash")
>
> However, when line 26 is uncommented, the container can be built, but
> when run fails with the error below.
> My hunch is that things are done out of order, with setuid binaries
> being set up before user creation, but I have no way of checking that.
>
> Please do not hesitate to ping me if I can be of help.
>
> Cheers,
>
> Edouard.
>
> The error:
> system container is running as PID 9825
> WARNING: (guile-user): imported module (guix build utils) overrides core binding `delete'
> Run 'sudo guix container exec 9825 /run/current-system/profile/bin/bash --login'
> or run 'sudo nsenter -a -t 9825' to get a shell into it.
>
> WARNING: (guile-user): imported module (guix build utils) overrides core binding `delete'
> making '/gnu/store/mnc9lfpn01frmffqa31jy3c381dkgrwl-system' the current system...
> WARNING: (guile-user): imported module (guix build utils) overrides core binding `delete'
> setting up setuid programs in '/run/setuid-programs'...
> Backtrace:
>           12 (primitive-load "/gnu/store/bygckv7p4091xqykjnkay4qnazn…")
> In gnu/build/linux-container.scm:
>     300:8 11 (call-with-temporary-directory #<procedure 7fb026898d70…>)
>    397:16 10 (_ "/tmp/guix-directory.B9dmTN")
>      62:6  9 (call-with-clean-exit #<procedure 7fb0268a5380 at gnu/b…>)
> In unknown file:
>            8 (primitive-load "/gnu/store/mnc9lfpn01frmffqa31jy3c381d…")
> In ice-9/eval.scm:
>     619:8  7 (_ #f)
> In unknown file:
>            6 (primitive-load "/gnu/store/dib6wfh2r52dfaydz78n33267qx…")
> In srfi/srfi-1.scm:
>     634:9  5 (for-each #<procedure primitive-load (_)> ("/gnu/sto…" …))
> In unknown file:
>            4 (primitive-load "/gnu/store/ypwqsx11k2qmxkscmzan6srq87q…")
> In srfi/srfi-1.scm:
>     634:9  3 (for-each #<procedure 7fb026380538 at gnu/build/activa…> …)
> In ice-9/boot-9.scm:
>   1747:15  2 (with-exception-handler #<procedure 7fb02683c6f0 at ic…> …)
> In gnu/build/activation.scm:
>    317:57  1 (_)
> In unknown file:
>            0 (getpw "suc")
>
> ERROR: In procedure getpw:
> In procedure getpw: entry not found
>
>
>
> The code
>
> (use-modules
>  (guix gexp)
>  (gnu system)
>  (gnu bootloader)
>  (gnu bootloader grub)
>  (gnu system file-systems)
>  (gnu services)
>  (gnu services base)
>  (gnu system setuid)
>  (gnu packages base))
>
> (operating-system
>   (host-name "minimal-container")
>   (timezone "UTC")
>   (locale "en_US.utf8")
>   (bootloader (bootloader-configuration
>                (bootloader grub-bootloader)))
>   (file-systems %base-file-systems)
>   (users (cons
>           (user-account
>            (name "suc")
>            (group "users"))
>           %base-user-accounts))
>   (setuid-programs
>    (cons (setuid-program (program (file-append coreutils "/bin/true"))
>                          (user "suc")
>                          )
>          %setuid-programs))
>   (packages %base-packages)
>   (services %base-services))

Josselin Poiret wrote on 8 Jun 2023 09:19

Recipients:

Message-ID:87edmma0bf.fsf@jpoiret.xyz

Hi everyone,

You might want to have a look at [1], which should resolve this. I've

held off on reviewing it for quite a bit but have talked on IRC recently

with bjc about it. With this approach, while cleaner, we'll need to

identify which services rely on the setuid binaries being present, as

well as ensure they're up before any interaction with the user is

possible.

[1] https://issues.guix.gnu.org/62726

HTH,

Josselin Poiret

Edouard Klein wrote on 6 Jul 2023 14:04

Recipients:(name . Josselin Poiret)(address . dev@jpoiret.xyz)(address . 63904@debbugs.gnu.org)

Message-ID:87edll9ra0.fsf@rdklein.fr

Thank you Josselin :)

I ended up basically reusing this code in my own system layer, while we

wait on 62726 to be merged.

Also, I needed a new keyword argument to change the name of the setuid

binary.

It works flawlessly, thanks !

Cheers,

Edouard.

Josselin Poiret <dev@jpoiret.xyz> writes:

Toggle quote (13 lines)> [[PGP Signed Part:Undecided]]
> Hi everyone,
>
> You might want to have a look at [1], which should resolve this.  I've
> held off on reviewing it for quite a bit but have talked on IRC recently
> with bjc about it.  With this approach, while cleaner, we'll need to
> identify which services rely on the setuid binaries being present, as
> well as ensure they're up before any interaction with the user is
> possible.
>
> [1] https://issues.guix.gnu.org/62726
>
> HTH,

Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 63904@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 63904

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date