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 v6 00/16] Rootless guix-daemon
Date: Sat, 22 Mar 2025 16:57:45 +0100

[Message part 1 (text/plain, inline)]

Hi Reepca,

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

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>>> While ensuring that what actually gets execve'd is in the store suffices
>>> to eliminate the vulnerability, it may be "conceptually purer" to
>>> require that the links pointing to it are all in the store as well.  For
>>> example, while a builder that is a symlink pointing to /proc/self/exe
>>> wouldn't be able to modify the daemon binary, it's still a piece of
>>> basically "undefined behavior" as far as the build environment is
>>> concerned, which could be closed up.  But that can come later just as
>>> well.
>>
>> Yes.  But in practice, “normal” symlinks (i.e., not /proc/self/exe) will
>> lead ‘canonPath’ to throw if one component is outside the store, since
>> ‘canonPath’ operates within the chroot.
>
> Unless the component actually exists and is outside of the store.  If we
> just rely on canonPath throwing an exception to be safe, then if there
> ever arose a situation where a non-symlink executable existed outside of
> the store, it would still be possible to convince the daemon to execute
> it.

[...]

> I mention this because I see that patch 07/16 of v7 has left out the
> isInStore check, and I think it should remain.

Hmm right (I was very much assuming that /proc/self/exe was the only
non-store executable, but better be safe than sorry).  Re-adding this:

[Message part 2 (text/x-patch, inline)]

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 1733322316..d0fcc99854 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2390,6 +2390,9 @@ void DerivationGoal::runChild()
 	       within the chroot.  */
 	    builderBasename = baseNameOf(drv.builder);
 	    drv.builder = canonPath(drv.builder, true);
+
+	    if (!isInStore(drv.builder))
+		throw Error(format("derivation builder '%1%' is outside the store") % drv.builder);
 	}
 
         /* Fill in the arguments. */

[Message part 3 (text/plain, inline)]

> While researching container escape vulnerabilities, I recently came
> across CAP_DAC_READ_SEARCH and open_by_handle_at, which is a system call
> so insanely powerful it is outright banned in all but the root user
> namespace.  Or at least, it was.  10 months ago, in commit
> 620c266f394932e5decc4b34683a75dfc59dc2f4 of
> https://github.com/torvalds/linux, the requirements were relaxed so
> that, in certain cases, processes in non-root user namespaces could use
> open_by_handle_at.

The way ‘open_by_handle_at’ is documented (“half” of ‘openat’) does not
make it immediately obvious to me what makes it “powerful”.  I see the
risk of a confused deputy problem though because of the ‘mount_id’
argument in addition to ‘handle’.  Is that what you have in mind?

> The consequences of this for same-user containers are not clear to me
> yet, as I haven't studied the kernel source enough to know what exactly
> that commit message means by "privileges over the filesystem" or
> "privileges over a subtree".  I also haven't been able to test this
> behavior yet, because my kernel is actually too old (I do my rebases and
> upgrades rather less regularly than is recommended).  I'll try to look
> into this more once I update my system (and man-pages!), but figured I
> should mention it, because aside from that, and the aforementioned
> isInStore check, I can't think of any remaining concerns.

Alright.  I’ll send v8 with the change above.

Thanks again!

Ludo’.

Previous Next

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