Version in base suite: 1.2.0-4+deb11u1 Base version: guix_1.2.0-4+deb11u1 Target version: guix_1.2.0-4+deb11u2 Base file: /srv/ftp-master.debian.org/ftp/pool/main/g/guix/guix_1.2.0-4+deb11u1.dsc Target file: /srv/ftp-master.debian.org/policy/pool/main/g/guix/guix_1.2.0-4+deb11u2.dsc changelog | 8 patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch | 232 ++++++++++ patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch | 106 ++++ patches/series | 2 4 files changed, 348 insertions(+) diff -Nru guix-1.2.0/debian/changelog guix-1.2.0/debian/changelog --- guix-1.2.0/debian/changelog 2023-04-09 01:35:36.000000000 +0000 +++ guix-1.2.0/debian/changelog 2024-04-17 22:39:38.000000000 +0000 @@ -1,3 +1,11 @@ +guix (1.2.0-4+deb11u2) bullseye-security; urgency=medium + + * debian/patches: guix-daemon: Protect against file descriptor escape + when building fixed-output derivations (CVE-2024-27297). + (Closes: #1066113) + + -- Vagrant Cascadian Wed, 17 Apr 2024 15:39:38 -0700 + guix (1.2.0-4+deb11u1) bullseye; urgency=medium [ Santiago Vila ] diff -Nru guix-1.2.0/debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch guix-1.2.0/debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch --- guix-1.2.0/debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch 1970-01-01 00:00:00.000000000 +0000 +++ guix-1.2.0/debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch 2024-04-17 21:22:05.000000000 +0000 @@ -0,0 +1,232 @@ +From 8f4ffb3fae133bb21d7991e97c2f19a7108b1143 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= +Date: Mon, 11 Mar 2024 10:59:42 +0100 +Subject: [PATCH 01/36] daemon: Protect against FD escape when building + fixed-output derivations (CVE-2024-27297). +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +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”. + +Vulnerability discovered by puck . + +Nix security advisory: +https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37 + +Nix fix: +https://github.com/NixOS/nix/commit/244f3eee0bbc7f11e9b383a15ed7368e2c4becc9 + +* 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 , Théophane Hufschmitt +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(-) + +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); +-- +2.39.2 + diff -Nru guix-1.2.0/debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch guix-1.2.0/debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch --- guix-1.2.0/debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch 1970-01-01 00:00:00.000000000 +0000 +++ guix-1.2.0/debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch 2024-04-17 21:22:05.000000000 +0000 @@ -0,0 +1,106 @@ +From ff1251de0bc327ec478fc66a562430fbf35aef42 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= +Date: Tue, 12 Mar 2024 11:53:35 +0100 +Subject: [PATCH 32/36] daemon: Address shortcoming in previous security fix + for CVE-2024-27297. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +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 +--- + nix/libstore/build.cc | 11 ++++++----- + nix/libutil/util.cc | 4 ++++ + nix/libutil/util.hh | 7 ++++--- + 3 files changed, 14 insertions(+), 8 deletions(-) + +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); + +-- +2.39.2 + diff -Nru guix-1.2.0/debian/patches/series guix-1.2.0/debian/patches/series --- guix-1.2.0/debian/patches/series 2023-04-09 01:34:12.000000000 +0000 +++ guix-1.2.0/debian/patches/series 2024-04-17 22:38:18.000000000 +0000 @@ -39,3 +39,5 @@ 0029-tests-swh.scm-Disable-tests-the-fail-with-guile-2.2.patch security/daemon-Prevent-privilege-escalation-with-keep-failed.patch tests-Ensure-test-OpenPGP-keys-never-expire.patch +security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch +security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch