GNU bug report logs - #75810
[PATCH 0/6] Rootless guix-daemon

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Reepca Russelstein <reepca <at> russelstein.xyz>
Cc: 75810 <at> debbugs.gnu.org
Subject: [bug#75810] [PATCH 0/6] Rootless guix-daemon
Date: Sat, 15 Feb 2025 22:33:58 +0100

Hi,

(Just a quick reply; there’s a lot in here. :-))

Reepca Russelstein <reepca <at> russelstein.xyz> skribis:

> I expect that it should work to:
> 1. go through the entire normal chroot setup
> 2. bind-mount /gnu/store and /tmp to themselves within the chroot using
>    MS_REC so that they are treated as distinct filesystems but also
>    still have their existing bind-mounts underneath them
> 3. bind-mount / to itself using MS_REC
> 4. remount / read-only using MS_RDONLY | MS_REMOUNT | MS_BIND

Yes.  I pushed what I have now at
<https://codeberg.org/civodul/guix/src/branch/wip-rootless-daemon>.  It
does work as expected: / is read-only, /tmp and /gnu/store are
read-write, individual inputs in /gnu/store are read-only.

> Also, _chown does the actual chown on descent, not on return, so it
> first chowns a directory and then goes through its contents.  This means
> that, again, if there weren't the top directory there to block access,
> it would be possible to access a setuid program before it was chown'ed.

Right.

> We seem to be relying entirely on the Linux behavior of chown to reset
> setuid / setgid bits.

Yes, and I think chown(2) is quite clear:

  When the owner or group of an executable file is changed by an
  unprivileged user, the S_ISUID and S_ISGID mode bits are cleared.
  […] since Linux 2.2.13, root is treated like other users.

> Now, for the non-pedantic, significant issue that I came across while
> writing all that: previously, it was not possible for the
> tmpDir-exposing code to be reached without doing the _chown that also
> reset setuid and setgid bits.  But with this patch, in the non-root,
> non-CAP_CHOWN case (which is what is currently proposed for Guix
> System), it can be reached through the catch clause.  In that case it
> will expose tmpDir without changing any permission bits of files beneath
> it, allowing anybody who can access a setuid program in tmpDir (which,
> due to that 0755 chmod, is "everybody") to take control of the build
> user (which in this case would be guix-daemon).

I’m not sure I understand what you mean by “the tmpDir-exposing code”;
are you talking about ‘DerivationGoal::deleteTmpDir’?

> Going by the "Running unprivileged but with CAP_CHOWN" comment, it would
> seem that code is meant to only be reached by reaching the end of the
> "try" block, not by reaching the end of the "catch" block.  I think it
> would be a good idea to call secureFilePerms(tmpDir) before any attempt
> at chown'ing.

Yeah, we can do that to be on the safe side.

> I still think it would be a good idea to call unshare to create an extra
> user and mount namespace just before executing the builder in the
> unprivileged case, just to be sure that the mount-locking behavior is
> triggered in a way that is documented.

The problem with “mount locking” (and “peer group” and in fact most
“concepts” mentioned in mount(2)) is that it’s not clearly defined.
Here I’m relying on unit tests to ensure that the various bits can
indeed not be remounted read-write, for instance.  (“make check” tests
the same setup as unprivileged daemon, which is an advantage over the
current situation where the separate-build-user setup is not covered by
the test suite.)

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.