Report forwarded
to leo@famulari.name, bug-guix@gnu.org: bug#47229; Package guix.
(Thu, 18 Mar 2021 11:18:02 GMT) (full text, mbox, link).
Acknowledgement sent
to Ludovic Courtès <ludo@gnu.org>:
New bug report received and forwarded. Copy sent to leo@famulari.name, bug-guix@gnu.org.
(Thu, 18 Mar 2021 11:18:02 GMT) (full text, mbox, link).
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.
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 = "";
}
}
Added tag(s) security.
Request was from Ludovic Courtès <ludo@gnu.org>
to control@debbugs.gnu.org.
(Thu, 18 Mar 2021 11:19:02 GMT) (full text, mbox, link).
Severity set to 'serious' from 'normal'
Request was from Ludovic Courtès <ludo@gnu.org>
to control@debbugs.gnu.org.
(Thu, 18 Mar 2021 11:19:02 GMT) (full text, mbox, link).
Information forwarded
to bug-guix@gnu.org: bug#47229; Package guix.
(Thu, 18 Mar 2021 11:46:02 GMT) (full text, mbox, link).
Added tag(s) fixed.
Request was from Ludovic Courtès <ludo@gnu.org>
to control@debbugs.gnu.org.
(Thu, 18 Mar 2021 13:28:01 GMT) (full text, mbox, link).
bug closed, send any further explanations to
47229@debbugs.gnu.org and Ludovic Courtès <ludo@gnu.org>
Request was from Ludovic Courtès <ludo@gnu.org>
to control@debbugs.gnu.org.
(Thu, 18 Mar 2021 13:28:01 GMT) (full text, mbox, link).
Information forwarded
to bug-guix@gnu.org: bug#47229; Package guix.
(Thu, 18 Mar 2021 21:11:02 GMT) (full text, mbox, link).
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
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
Hi Nathan,
Nathan Nye <nnye@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’.
Information forwarded
to bug-guix@gnu.org: bug#47229; Package guix.
(Sat, 10 Apr 2021 17:57:02 GMT) (full text, mbox, link).
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>
Debbugs is free software and licensed under the terms of the
GNU Public License version 2. The current version can be
obtained from https://bugs.debian.org/debbugs-source/.