mirror/guix
mirror/guix
1
Fork 0
mirror of https://https.git.savannah.gnu.org/git/guix.git/ synced 2025-07-10 16:50:43 +02:00
Code Releases Activity

daemon: Do not make chroot root directory read-only.

Browse source 
Fixes <https://issues.guix.gnu.org/77570>.

Commit 40f69b586a made chroot root
directory read-only; as a consequence, build processes attempting to
write to the root directory would now get EROFS instead of EACCES.

It turns out that a number of test suites (Go, Ruby, SCons, Shepherd)
would fail because of this observable difference.

To restore previous behavior in build environments while still
preventing build processes from exposing their root directory to outside
processes, this patch (1) keeps the root writable but #o555 by default,
thereby restoring the EACCES behavior, and (2) ensures that the parent
of the chroot root directory is itself user-accessible only.

* nix/libstore/build.cc (class DerivationGoal)[chrootRootTop]: New
field.
(DerivationGoal::startBuilder): Initialize ‘chrootRootTop’ and make it
‘AutoDelete’.  Replace ‘mount’ call that made the root directory
read-only by a mere ‘chmod_’ call.
* tests/store.scm ("build root cannot be made world-readable"): Remove.
("writing to build root leads to EACCES"): New test.

Reported-by: Ada Stevenson <adanskana@gmail.com>
Reported-by: keinflue <keinflue@posteo.net>
Suggested-by: Reepca Russelstein <reepca@russelstein.xyz>
Change-Id: I5912e8b3b293f8242a010cfc79255fc981314445
This commit is contained in:
Ludovic Courtès 2025-04-08 15:18:04 +02:00
parent 13aeb3abf9
commit ff5181e27e
No known key found for this signature in database
GPG key ID: 090B11993D9AEBB5
2 changed files with 38 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

36
nix/libstore/build.cc
View file

@ -657,7 +657,9 @@ private:
/* Whether we're currently doing a chroot build. */
bool useChroot;
Path chrootRootDir;
/* The directory containing the chroot root directory, and the chroot root
directory itself--the latter is a sub-directory of the former. */
Path chrootRootTop, chrootRootDir;
/* RAII object to delete the chroot directory. */
std::shared_ptr<AutoDelete> autoDelChroot;
@ -1810,19 +1812,21 @@ void DerivationGoal::startBuilder()
if (useChroot) {
#if CHROOT_ENABLED
/* Create a temporary directory in which we set up the chroot
environment using bind-mounts. We put it in the store
to ensure that we can create hard-links to non-directory
inputs in the fake store in the chroot (see below). */
chrootRootDir = drvPath + ".chroot";
if (pathExists(chrootRootDir)) deletePath(chrootRootDir);
environment using bind-mounts. Put it in the store to ensure it
can be atomically moved to the store. */
chrootRootTop = drvPath + ".chroot";
chrootRootDir = chrootRootTop + "/top";
if (pathExists(chrootRootTop)) deletePath(chrootRootTop);
/* Clean up the chroot directory automatically. */
autoDelChroot = std::shared_ptr<AutoDelete>(new AutoDelete(chrootRootDir));
autoDelChroot = std::shared_ptr<AutoDelete>(new AutoDelete(chrootRootTop));
printMsg(lvlChatty, format("setting up chroot environment in `%1%'") % chrootRootDir);
if (mkdir(chrootRootTop.c_str(), 0750) == -1)
throw SysError(format("cannot create build root container '%1%'") % chrootRootTop);
if (mkdir(chrootRootDir.c_str(), 0750) == -1)
throw SysError(format("cannot create ‘%1%’") % chrootRootDir);
throw SysError(format("cannot create build root '%1%'") % chrootRootDir);
if (buildUser.enabled() && chown(chrootRootDir.c_str(), 0, buildUser.getGID()) == -1)
throw SysError(format("cannot change ownership of ‘%1%’") % chrootRootDir);
@ -2245,9 +2249,19 @@ void DerivationGoal::runChild()
if (rmdir("real-root") == -1)
throw SysError("cannot remove real-root directory");
/* Remount root as read-only. */
if (mount("/", "/", 0, MS_BIND | MS_REMOUNT | MS_RDONLY, 0) == -1)
throw SysError(format("read-only remount of build root '%1%' failed") % chrootRootDir);
/* Make the root read-only.
When build users are disabled, the build process could make it
world-accessible, but that's OK: since 'chrootRootTop' is *not*
world-accessible, a world-accessible 'chrootRootDir' cannot be
used to grant access to the build environment to external
processes.
Remounting the root as read-only was rejected because it makes
write access fail with EROFS instead of EACCES, which goes
against what some test suites expect (Go, Ruby, SCons,
Shepherd, to name a few). */
chmod_("/", 0555);
if (getuid() != 0) {
/* Create a new mount namespace to "lock" previous mounts.

35
tests/store.scm
View file

@ -498,32 +498,23 @@
(unless (unprivileged-user-namespace-supported?)
(test-skip 1))
(test-assert "build root cannot be made world-readable"
(test-assert "writing to build root leads to EACCES"
(let ((drv
(run-with-store %store
(gexp->derivation
"attempt-to-make-root-world-readable"
(with-imported-modules (source-module-closure
'((guix build syscalls)))
#~(begin
(use-modules (guix build syscalls))
"write-to-root"
#~(begin
(catch 'system-error
(lambda ()
(mkdir "/whatever"))
(lambda args
(format #t "mkdir failed, which is good: ~a~%"
(strerror (system-error-errno args)))
(when (= EACCES (system-error-errno args))
(exit 1))))
(catch 'system-error
(lambda ()
(chmod "/" #o777))
(lambda args
(format #t "failed to make root writable: ~a~%"
(strerror (system-error-errno args)))
(format #t "attempting read-write remount~%")
(mount "none" "/" "/" (logior MS_BIND MS_REMOUNT))
(chmod "/" #o777)))
;; At this point, the build process could create a
;; world-readable setuid binary under its root (so in the
;; store) that would remain visible until the build
;; completes.
(mkdir #$output)))))))
(guard (c ((store-protocol-error? c) #t))
(mkdir #$output))))))
(guard (c ((store-protocol-error? c) c))
(build-derivations %store (list drv))
#f)))