bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’

bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 2676 bytes --]

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@gnu.org list on
February 7th, and discussed and prepared fixes in the interim.

Ludo’ & Leo Famulari.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2973 bytes --]

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 = "";
     }
 }

bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’

[Ludovic Courtès]

Ludovic Courtès <ludo@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’.

bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’

[Léo Le Bouter via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 55 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’

[Ludovic Courtès]

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.

bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

bug#47229: Local privilege escalation via guix-daemon and ‘--keep-failed’

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

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>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]