Can't setuid programs to anybody but root

  • Open
  • quality assurance status badge
Details
3 participants
  • Josselin Poiret
  • edk
  • Edouard Klein
Owner
unassigned
Submitted by
edk
Severity
normal
E
E
edk wrote on 5 Jun 2023 12:00
(name . bug-guix)(address . bug-guix@gnu.org)
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))
E
E
Edouard Klein wrote on 6 Jun 2023 09:21
(address . 63904@debbugs.gnu.org)
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))
J
J
Josselin Poiret wrote on 8 Jun 2023 09:19
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.


HTH,
--
Josselin Poiret
-----BEGIN PGP SIGNATURE-----

iQHEBAEBCgAuFiEEOSSM2EHGPMM23K8vUF5AuRYXGooFAmSBgOQQHGRldkBqcG9p
cmV0Lnh5egAKCRBQXkC5Fhcaigf1DACI1mMO1MSBDjr3TDPJwO18m+j+cLrdNo3+
Bmi+aSL4AmjGJ8RYEDAGtRninFxt1QGaoDs060pe6LPsaA0BGxVkm7ldVWIfob2B
7czkGF55kRAp0Ikx0CQAvjjXfczO0nSVQZx5KOPcbhl2PEaD9e4uZNISPzPipeJF
SM+M2KSceqS+/pE1DCLrNMe2TdmhIsiOwAJN1BnsruusgKCeBdTzHV121pnrrOj6
pQSCeGo84rx4+YLT7tIya92Tly068KPWmo3ZxmGJ74MQiGFt92j0u87BMT8JNY0U
9GXE1eQmYvDyw2en2v3SSAoP5BiDH4MkKzvU1K8cQk+ncGhJbkvbF4h3q7uUfEhK
FccQh9mKeeXSxXObAJBEeLcdVW4JyZKAmYqrhm7wRLYYzKXfZPOwC4pWzkh26qlG
vmSQ0vfXHp3HWqk81XBF1nfktqQyYhwOK7Gxu5fccGErI+qvQlvKX/QbXF/AoeCF
x+pyQ7k2fjLYRKRobZKa6Bbf3MpZ6sQ=
=t6uz
-----END PGP SIGNATURE-----

E
E
Edouard Klein wrote on 6 Jul 2023 14:04
(name . Josselin Poiret)(address . dev@jpoiret.xyz)(address . 63904@debbugs.gnu.org)
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,
?