GNU bug report logs - #47229
Local privilege escalation via guix-daemon and ‘--keep-failed’

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47229 in the body.
You can then email your comments to 47229 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: <bug-guix <at> gnu.org>
Subject: Local privilege escalation via guix-daemon and
 ‘--keep-failed’
Date: Thu, 18 Mar 2021 12:17:15 +0100

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

A security vulnerability that can lead to local privilege escalation has
been found in ’guix-daemon’.  It affects multi-user setups in which
’guix-daemon’ runs locally.

It does not affect multi-user setups where ‘guix-daemon’ runs on a
separate machine and is accessed over the network, via
‘GUIX_DAEMON_SOCKET’, as is customary on cluster setups.  Machines where
the Linux “protected hardlink”[*] feature is enabled, which is common,
are also unaffected—this is the case when the contents of
/proc/sys/fs/protected_hardlinks are 1.

[*] https://www.kernel.org/doc/Documentation/sysctl/fs.txt


Vulnerability
~~~~~~~~~~~~~

The attack consists in having an unprivileged user spawn a build
process, for instance with ‘guix build’, that makes its build directory
world-writable.  The user then creates a hardlink within the build
directory to a root-owned file from outside of the build directory, such
as ‘/etc/shadow’.  If the user passed the ‘--keep-failed’ option and the
build eventually fails, the daemon changes ownership of the whole build
tree, including the hardlink, to the user.  At that point, the user has
write access to the target file.


Fix
~~~

The fix (patch attached) consists in adding a root-owned “wrapper”
directory in which the build directory itself is located.  If the user
passed the ‘--keep-failed’ option and the build fails, the ‘guix-daemon’
first changes ownership of the build directory, and then, in two stages,
moves the build directory into the location where users expect to find
failed builds, roughly like this:

  1. chown -R USER /tmp/guix-build-foo.drv-0/top
  2. mv /tmp/guix-build-foo.drv-0{,.pivot}
  3. mv /tmp/guix-build-foo.drv-0.pivot/top /tmp/guix-build-foo.drv-0

In step #1, /tmp/guix-build-foo.drv-0 remains root-owned, with
permissions of #o700.  Thus, only root can change directory into it or
into ‘top’.  Likewise in step #2.

The build tree becomes accessible to the user once step #3 has
succeeded, not before.  These steps are performed after the package
build scripts have stopped running.


Additionally, the patch at <https://issues.guix.gnu.org/47013> enables
protected hardlinks and symlinks by default on Guix System, which will
protect against this class of vulnerability from now on.


Credit
~~~~~~

We are grateful to Nathan Nye of WhiteBeam Security for reporting this
bug and discussing fixes with us!


Timeline
~~~~~~~~

We learned about this bug on the private guix-security <at> gnu.org list on
February 7th, and discussed and prepared fixes in the interim.

Ludo’ & Leo Famulari.

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

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 20d83fea4a..4f486f0822 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -1621,6 +1621,24 @@ void DerivationGoal::startBuilder()
     auto drvName = storePathToName(drvPath);
     tmpDir = createTempDir("", "guix-build-" + drvName, false, false, 0700);
 
+    if (useChroot) {
+	/* Make the build directory seen by the build process a sub-directory.
+	   That way, "/tmp/guix-build-foo.drv-0" is root-owned, and thus its
+	   permissions cannot be changed by the build process, while
+	   "/tmp/guix-build-foo.drv-0/top" is owned by the build user.  This
+	   cannot be done when !useChroot because then $NIX_BUILD_TOP would
+	   be inaccessible to the build user by its full file name.
+
+	   If the build user could make the build directory world-writable,
+	   then an attacker could create in it a hardlink to a root-owned file
+	   such as /etc/shadow.  If 'keepFailed' is true, the daemon would
+	   then chown that hardlink to the user, giving them write access to
+	   that file.  */
+	tmpDir += "/top";
+	if (mkdir(tmpDir.c_str(), 0700) == 1)
+	    throw SysError("creating top-level build directory");
+    }
+
     /* In a sandbox, for determinism, always use the same temporary
        directory. */
     tmpDirInSandbox = useChroot ? canonPath("/tmp", true) + "/guix-build-" + drvName + "-0" : tmpDir;
@@ -2626,20 +2644,41 @@ static void _chown(const Path & path, uid_t uid, gid_t gid)
 void DerivationGoal::deleteTmpDir(bool force)
 {
     if (tmpDir != "") {
+	// When useChroot is true, tmpDir looks like
+	// "/tmp/guix-build-foo.drv-0/top".  Its parent is root-owned.
+	string top;
+	if (useChroot) {
+	    if (baseNameOf(tmpDir) != "top") abort();
+	    top = dirOf(tmpDir);
+	} else top = tmpDir;
+
         if (settings.keepFailed && !force) {
             printMsg(lvlError,
                 format("note: keeping build directory `%2%'")
-                % drvPath % tmpDir);
+                % drvPath % top);
             chmod(tmpDir.c_str(), 0755);
+
             // Change the ownership if clientUid is set. Never change the
             // ownership or the group to "root" for security reasons.
             if (settings.clientUid != (uid_t) -1 && settings.clientUid != 0) {
                 _chown(tmpDir, settings.clientUid,
                        settings.clientGid != 0 ? settings.clientGid : -1);
+
+		if (top != tmpDir) {
+		    // Rename tmpDir to its parent, with an intermediate step.
+		    string pivot = top + ".pivot";
+		    if (rename(top.c_str(), pivot.c_str()) == -1)
+			throw SysError("pivoting failed build tree");
+		    if (rename((pivot + "/top").c_str(), top.c_str()) == -1)
+			throw SysError("renaming failed build tree");
+		    rmdir(pivot.c_str());
+		}
             }
         }
-        else
+        else {
             deletePath(tmpDir);
+	    if (top != tmpDir) rmdir(dirOf(tmpDir).c_str());
+	}
         tmpDir = "";
     }
 }

Message #12 received at 47229 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 47229 <at> debbugs.gnu.org
Cc: Leo Famulari <leo <at> famulari.name>
Subject: Re: bug#47229: Local privilege escalation via guix-daemon and
 ‘--keep-failed’
Date: Thu, 18 Mar 2021 12:45:36 +0100

Ludovic Courtès <ludo <at> gnu.org> skribis:

> The fix (patch attached) consists in adding a root-owned “wrapper”
> directory in which the build directory itself is located.

The fix has now been pushed:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=ec7fb669945bfb47c5e1fdf7de3a5d07f7002ccf

Followed by an update of the ‘guix’ package to make the fix available:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=94f03125463ee0dba2f7916fcd43fd19d4b6c892

We recommend upgrading the daemon (using commit 94f03125 or later).
On Guix System, you achieve that by running something along these lines:

  guix pull
  sudo guix system reconfigure /run/current-system/configuration.scm
  sudo herd restart guix-daemon

On other distros, assuming services are managed by systemd:

  sudo --login guix pull
  sudo systemctl restart guix-daemon.service

(See <https://guix.gnu.org/manual/en/html_node/Upgrading-Guix.html>.)

Ludo’.

Message #15 received at 47229 <at> debbugs.gnu.org (full text, mbox):

From: Léo Le Bouter <lle-bout <at> zaclys.net>
To: Ludovic Courtès <ludo <at> gnu.org>, 47229 <at> debbugs.gnu.org
Subject: Re: bug#47229: Local privilege escalation via guix-daemon and
 ‘--keep-failed’
Date: Thu, 18 Mar 2021 12:53:51 +0100

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

Thanks a lot to the reporter and for working on this!

[signature.asc (application/pgp-signature, inline)]

Message #18 received at 47229 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 47229 <at> debbugs.gnu.org
Cc: Leo Famulari <leo <at> famulari.name>
Subject: Re: bug#47229: Local privilege escalation via guix-daemon and
 ‘--keep-failed’
Date: Thu, 18 Mar 2021 14:14:28 +0100

An additional data point: guix-daemon chowns build trees to the caller
upon failure (a very handy feature) since this 2016 commit:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=2608e40988ba8cf51723fe0d21bdedf6b3997c9c

The Nix build daemon, which guix-daemon is based on, did not have this
feature.

Message #25 received at 47229 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 47229 <at> debbugs.gnu.org
Subject: Re: bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’
Date: Thu, 18 Mar 2021 17:10:49 -0400

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

On Thu, Mar 18, 2021 at 12:17:15PM +0100, Ludovic Courtès wrote:
> It does not affect multi-user setups where ‘guix-daemon’ runs on a
> separate machine and is accessed over the network, via
> ‘GUIX_DAEMON_SOCKET’, as is customary on cluster setups.  Machines where
> the Linux “protected hardlink”[*] feature is enabled, which is common,
> are also unaffected—this is the case when the contents of
> /proc/sys/fs/protected_hardlinks are 1.

After publishing the advisory, we received a clarification about the
impact of "protected hardlinks".

When using a guix-daemon that does not include the fix [0] for the bug
reported here, it is still possible for rogue build scripts to escape
the build environment, even when protected hardlinks are enabled.

Protected hardlinks do make exploitation significantly more difficult,
but not impossible.

For this reason, we continue to recommend that all Guix users upgrade
their guix-daemons, as described in the original advisory.

[0]
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=ec7fb669945bfb47c5e1fdf7de3a5d07f7002ccf

[signature.asc (application/pgp-signature, inline)]

Message #28 received at 47229 <at> debbugs.gnu.org (full text, mbox):

From: Nathan Nye <nnye <at> whitebeamsec.com>
To: 47229 <at> debbugs.gnu.org
Subject: Hardlink mitigation limits
Date: Tue, 23 Mar 2021 14:18:14 -0400

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

Hello,

I'm sharing here for future reference why protected hardlinks alone did 
not mitigate the recent LPE security advisory, pre-patch:

"The reasons why are lines 2633 and 2637 of nix/libstore/build.cc:

 * https://git.savannah.gnu.org/cgit/guix.git/tree/nix/libstore/build.cc#n2633
 * https://git.savannah.gnu.org/cgit/guix.git/tree/nix/libstore/build.cc#n2637

When a package fails to build and the keep failed flag is set 
(-K/--keep-failed), it runs a recursive chown on the build directory 
(which is writable following guixbuilder01 changing the permissions to 
777). It starts at the top level and chowns downwards.

The first important thing to notice here is that at any point (even 
pre-chown) the build user has been compromised. The build user can write 
a SUID /bin/sh to the build path, and because a normal user can traverse 
into the directory before and during the chown, they can run a SUID 
shell (allowing them to become guixbuilder01 even after the build user 
processes are terminated). Becoming the build user allows multiple paths 
to privilege escalation, but in this scenario we have faster ways of 
becoming root.

Moving on to getting root, we're choosing not to use a hardlink to show 
why it isn't necessary. Instead, we create a directory under the build 
directory with thousands of sequentially named files, the final entry 
being "passwd" or "shadow". Then we terminate the build and watch for 
the first entry to be chowned to our user ID (possibly with the inotify 
API). This way, we have opened a lengthy window of time where it is 
enumerating over a list of file paths in our chosen directory and 
chowning each of them. Now we can execute our TOCTOU race condition 
vulnerability.

At the time of check (TOC), the guix-daemon has a list of file paths to 
chown under what it assumes is a regular directory (because it ran 
S_ISDIR on the directory). But we can swap out the directory from under 
it with a symlink to /etc (most efficiently with renameat2() and using 
the RENAME_EXCHANGE flag to atomically exchange the paths). At the time 
of use (TOU) lchown() only checks if the file /itself/ that is being 
chowned is a symlink, not if the path components are, as can be 
demonstrated with Python:

$ mkdir td;touch td/tf;python3 -c 'import os;os.lchown("/home/example/td/tf", 1000, 4)';ls -lahtrd td td/tf
-rw-rw-r-- 1 example adm       0    Mar 19 19:20 td/tf
drwxrwxr-x 2 example example   4.0K Mar 19 19:20 td
$ rm -rf td
$ mkdir td; ln -s td td2;touch td2/tf;python3 -c 'import os;os.lchown("/home/example/td2/tf", 1000, 4)';ls -lahtrd td2 td2/tf
lrwxrwxrwx 1 example example 2 Mar 19 19:21 td2 -> td
-rw-rw-r-- 1 example adm     0 Mar 19 19:21 td2/tf

So lchown can blindly chown /etc/passwd to our user by following the 
directory symlink and subsequently verifying that passwd itself is not a 
symlink. I hope this explains the TOCTOU race condition and why 
protected hardlinks help (forcing an attacker to get root using this 
race condition), but they are not a solution to the problem (alone)."

- Nathan

[Message part 2 (text/html, inline)]

Message #31 received at 47229 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Nathan Nye <nnye <at> whitebeamsec.com>
Cc: 47229 <at> debbugs.gnu.org
Subject: Re: bug#47229: Hardlink mitigation limits
Date: Mon, 29 Mar 2021 17:22:30 +0200

Hi Nathan,

Nathan Nye <nnye <at> whitebeamsec.com> skribis:

> I'm sharing here for future reference why protected hardlinks alone
> did not mitigate the recent LPE security advisory, pre-patch:

Thanks a lot for this clarification!

Ludo’.

Message #34 received at 47229 <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 47229 <at> debbugs.gnu.org
Subject: Re: bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’
Date: Sat, 10 Apr 2021 13:56:27 -0400

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

On Thu, Mar 18, 2021 at 12:17:15PM +0100, Ludovic Courtès wrote:
> Vulnerability
> ~~~~~~~~~~~~~
> 
> The attack consists in having an unprivileged user spawn a build
> process, for instance with ‘guix build’, that makes its build directory
> world-writable.  The user then creates a hardlink within the build
> directory to a root-owned file from outside of the build directory, such
> as ‘/etc/shadow’.  If the user passed the ‘--keep-failed’ option and the
> build eventually fails, the daemon changes ownership of the whole build
> tree, including the hardlink, to the user.  At that point, the user has
> write access to the target file.

This has been assigned CVE-2021-27851.

Soon, it should be available in the CVE database at
<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-27851>

[signature.asc (application/pgp-signature, inline)]

Previous Next

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