From patchwork Fri Feb 21 13:05:53 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ludovic_Court=C3=A8s?= X-Patchwork-Id: 38909 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id BF73F27BBEA; Fri, 21 Feb 2025 13:10:39 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,MAILING_LIST_MULTI,RCVD_IN_DNSWL_BLOCKED, RCVD_IN_VALIDITY_CERTIFIED,RCVD_IN_VALIDITY_RPBL,RCVD_IN_VALIDITY_SAFE, SPF_HELO_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id CA79E27BBE2 for ; Fri, 21 Feb 2025 13:10:38 +0000 (GMT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tlSnY-00068M-2D; Fri, 21 Feb 2025 08:10:32 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tlSnK-00060u-T4 for guix-patches@gnu.org; Fri, 21 Feb 2025 08:10:19 -0500 Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tlSnK-00059b-G9; Fri, 21 Feb 2025 08:10:18 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:References:In-Reply-To:Date:From:To:Subject; bh=idfaYvTmznhqv9+JO9+QiD13YMv5X4U5zH6e+nqsgOA=; b=G1XsC7Nd/Voj4Nke8nyJns3hjgKeqIFu15WFP2yS0Rmul4eomsuHEs0Rs02msaF1CvWiUy2E191MgGzmxdYzseEfIsUP/05r+9geWMi1fpd4SqB4GYPFLKlhEPejDAzNZDITMe10MAbzYOzU/4KZGamuFTSf10IR/XBG1TsKHxlH5OP3rzHPVAlg9Sc4SkdV+Gwhp7wRqAzLJHuHCfKhA6I6CBEPrfuK93ZQOxZIll56U9HXbUDYdptPidMaYQsOjx/4wonaRDqsM3xyQi4C6P1pPoyGZ8p27L5MCfIxoXz30F/Sqkv59i6l3rPwTXhTfnh9dkDEMbjEPtX9P46BJg==; Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tlSnG-0005yo-B4; Fri, 21 Feb 2025 08:10:14 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#75810] [PATCH v3 05/11] daemon: Allow running as non-root with unprivileged user namespaces. Resent-From: Ludovic =?utf-8?q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix@cbaines.net, dev@jpoiret.xyz, ludo@gnu.org, othacehe@gnu.org, zimon.toutoune@gmail.com, me@tobias.gr, guix-patches@gnu.org Resent-Date: Fri, 21 Feb 2025 13:10:13 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 75810 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 75810@debbugs.gnu.org Cc: Ludovic =?utf-8?q?Court=C3=A8s?= , Christopher Baines , Josselin Poiret , Ludovic =?utf-8?q?Court=C3=A8s?= , Mathieu Othacehe , Simon Tournier , Tobias Geerinckx-Rice X-Debbugs-Original-Xcc: Christopher Baines , Josselin Poiret , Ludovic =?utf-8?q?Court=C3=A8s?= , Mathieu Othacehe , Simon Tournier , Tobias Geerinckx-Rice Received: via spool by 75810-submit@debbugs.gnu.org id=B75810.174014338522676 (code B ref 75810); Fri, 21 Feb 2025 13:10:13 +0000 Received: (at 75810) by debbugs.gnu.org; 21 Feb 2025 13:09:45 +0000 Received: from localhost ([127.0.0.1]:56713 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tlSmk-0005tL-HY for submit@debbugs.gnu.org; Fri, 21 Feb 2025 08:09:44 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:60040) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1tlSjt-0005OD-RP for 75810@debbugs.gnu.org; Fri, 21 Feb 2025 08:07:02 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tlSjn-0004Yz-Ia; Fri, 21 Feb 2025 08:06:39 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:References:In-Reply-To:Date:Subject:To: From; bh=idfaYvTmznhqv9+JO9+QiD13YMv5X4U5zH6e+nqsgOA=; b=R8mrQ4mlW7XQGnsEkwbQ 0s5fR0rOxQAZqcb3y9C2swDIloNT4IJ/S+Se16uHsQSTA4Qg2rrM6kcwB3hI6gEOdUwDCdiweD9dN inarbhDvq/IszdaORra1+Rv6U8EWKdJG1PKiktE4VcFFmPwZX9Z4oAHYyTqXiH5OkAdlPt2q8ugU6 3kcM+iG+WrrFnXh/JEzQyqLoIoW1O12krwoNAgYlSxcaCRU8yBWqIfOz4f2yo4UwSFEESoRVj9B7q LDybhvqvH56EPOK3dV3ufSy0aYFKVXUPSBzJUlWpn4U6+MPD18hXJDUpqhkKW5UXLSOE+L9TyAyVf 05ixn7KlXYlrxA==; From: Ludovic =?utf-8?q?Court=C3=A8s?= Date: Fri, 21 Feb 2025 14:05:53 +0100 Message-ID: <1f4adc1c09dde70b193e1571b250e6152f0b4ca2.1740142328.git.ludo@gnu.org> X-Mailer: git-send-email 2.48.1 In-Reply-To: References: MIME-Version: 1.0 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org X-getmail-retrieved-from-mailbox: Patches From: Ludovic Courtès * nix/libstore/build.cc (guestUID, guestGID): New variables. (DerivationGoal)[readiness]: New field. (initializeUserNamespace): New function. (DerivationGoal::runChild): When ‘readiness.readSide’ is positive, read from it. (DerivationGoal::startBuilder): Call ‘chown’ only when ‘buildUser.enabled()’ is true. Pass CLONE_NEWUSER to ‘clone’ when ‘buildUser.enabled()’ is false or not running as root. Retry ‘clone’ without CLONE_NEWUSER upon EPERM. (DerivationGoal::registerOutputs): Make ‘actualPath’ writable before ‘rename’. (DerivationGoal::deleteTmpDir): Catch ‘SysError’ around ‘_chown’ call. * nix/libstore/local-store.cc (LocalStore::createUser): Do nothing if ‘dirs’ already exists. Warn instead of failing when failing to chown ‘dir’. * guix/substitutes.scm (%narinfo-cache-directory): Check for ‘_NIX_OPTIONS’ rather than getuid() == 0 to determine the cache location. Change-Id: I38fbe01f80fb45a99cd8a391e55a39a54d64fcb7 --- guix/substitutes.scm | 4 +- nix/libstore/build.cc | 149 ++++++++++++++++++++++++++++-------- nix/libstore/local-store.cc | 22 ++++-- 3 files changed, 135 insertions(+), 40 deletions(-) diff --git a/guix/substitutes.scm b/guix/substitutes.scm index e31b3940203..2761a3dafb4 100644 --- a/guix/substitutes.scm +++ b/guix/substitutes.scm @@ -1,5 +1,5 @@ ;;; GNU Guix --- Functional package management for GNU -;;; Copyright © 2013-2021, 2023-2024 Ludovic Courtès +;;; Copyright © 2013-2021, 2023-2025 Ludovic Courtès ;;; Copyright © 2014 Nikita Karetnikov ;;; Copyright © 2018 Kyle Meyer ;;; Copyright © 2020 Christopher Baines @@ -76,7 +76,7 @@ (define %narinfo-cache-directory ;; time, 'guix substitute' is called by guix-daemon as root and stores its ;; cached data in /var/guix/…. However, when invoked from 'guix challenge' ;; as a user, it stores its cache in ~/.cache. - (if (zero? (getuid)) + (if (getenv "_NIX_OPTIONS") ;invoked by guix-daemon (or (and=> (getenv "XDG_CACHE_HOME") (cut string-append <> "/guix/substitute")) (string-append %state-directory "/substitute/cache")) diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index c87f4f767c5..107ffcfea06 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -747,6 +747,10 @@ private: friend int childEntry(void *); + /* Pipe to notify readiness to the child process when using unprivileged + user namespaces. */ + Pipe readiness; + /* Check that the derivation outputs all exist and register them as valid. */ void registerOutputs(); @@ -1622,6 +1626,25 @@ int childEntry(void * arg) } +/* UID and GID of the build user inside its own user namespace. */ +static const uid_t guestUID = 30001; +static const gid_t guestGID = 30000; + +/* Initialize the user namespace of CHILD. */ +static void initializeUserNamespace(pid_t child) +{ + auto hostUID = getuid(); + auto hostGID = getgid(); + + writeFile("/proc/" + std::to_string(child) + "/uid_map", + (format("%d %d 1") % guestUID % hostUID).str()); + + writeFile("/proc/" + std::to_string(child) + "/setgroups", "deny"); + + writeFile("/proc/" + std::to_string(child) + "/gid_map", + (format("%d %d 1") % guestGID % hostGID).str()); +} + void DerivationGoal::startBuilder() { auto f = format( @@ -1685,7 +1708,7 @@ void DerivationGoal::startBuilder() 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. */ + that file. See CVE-2021-27851. */ tmpDir += "/top"; if (mkdir(tmpDir.c_str(), 0700) == 1) throw SysError("creating top-level build directory"); @@ -1802,7 +1825,7 @@ void DerivationGoal::startBuilder() if (mkdir(chrootRootDir.c_str(), 0750) == -1) throw SysError(format("cannot create ‘%1%’") % chrootRootDir); - if (chown(chrootRootDir.c_str(), 0, buildUser.getGID()) == -1) + if (buildUser.enabled() && chown(chrootRootDir.c_str(), 0, buildUser.getGID()) == -1) throw SysError(format("cannot change ownership of ‘%1%’") % chrootRootDir); /* Create a writable /tmp in the chroot. Many builders need @@ -1821,8 +1844,8 @@ void DerivationGoal::startBuilder() (format( "nixbld:x:%1%:%2%:Nix build user:/:/noshell\n" "nobody:x:65534:65534:Nobody:/:/noshell\n") - % (buildUser.enabled() ? buildUser.getUID() : getuid()) - % (buildUser.enabled() ? buildUser.getGID() : getgid())).str()); + % (buildUser.enabled() ? buildUser.getUID() : guestUID) + % (buildUser.enabled() ? buildUser.getGID() : guestGID)).str()); /* Declare the build user's group so that programs get a consistent view of the system (e.g., "id -gn"). */ @@ -1857,7 +1880,7 @@ void DerivationGoal::startBuilder() createDirs(chrootStoreDir); chmod_(chrootStoreDir, 01775); - if (chown(chrootStoreDir.c_str(), 0, buildUser.getGID()) == -1) + if (buildUser.enabled() && chown(chrootStoreDir.c_str(), 0, buildUser.getGID()) == -1) throw SysError(format("cannot change ownership of ‘%1%’") % chrootStoreDir); foreach (PathSet::iterator, i, inputPaths) { @@ -1948,14 +1971,34 @@ void DerivationGoal::startBuilder() if (useChroot) { char stack[32 * 1024]; int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | SIGCHLD; - if (!fixedOutput) flags |= CLONE_NEWNET; + if (!fixedOutput) { + flags |= CLONE_NEWNET; + } + if (!buildUser.enabled() || getuid() != 0) { + flags |= CLONE_NEWUSER; + readiness.create(); + } + /* Ensure proper alignment on the stack. On aarch64, it has to be 16 bytes. */ - pid = clone(childEntry, + pid = clone(childEntry, (char *)(((uintptr_t)stack + sizeof(stack) - 8) & ~(uintptr_t)0xf), flags, this); - if (pid == -1) - throw SysError("cloning builder process"); + if (pid == -1) { + if ((flags & CLONE_NEWUSER) != 0 && getuid() != 0) + /* 'clone' fails with EPERM on distros where unprivileged user + namespaces are disabled. Error out instead of giving up on + isolation. */ + throw SysError("cannot create process in unprivileged user namespace"); + else + throw SysError("cloning builder process"); + } + + if ((flags & CLONE_NEWUSER) != 0) { + /* Initialize the UID/GID mapping of the child process. */ + initializeUserNamespace(pid); + writeFull(readiness.writeSide, (unsigned char*)"go\n", 3); + } } else #endif { @@ -2001,23 +2044,34 @@ void DerivationGoal::runChild() _writeToStderr = 0; + if (readiness.readSide > 0) { + /* Wait for the parent process to initialize the UID/GID mapping + of our user namespace. */ + char str[20] = { '\0' }; + readFull(readiness.readSide, (unsigned char*)str, 3); + if (strcmp(str, "go\n") != 0) + throw Error("failed to initialize process in unprivileged user namespace"); + } + restoreAffinity(); commonChildInit(builderOut); #if CHROOT_ENABLED if (useChroot) { - /* Initialise the loopback interface. */ - AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP)); - if (fd == -1) throw SysError("cannot open IP socket"); + if (!fixedOutput) { + /* Initialise the loopback interface. */ + AutoCloseFD fd(socket(PF_INET, SOCK_DGRAM, IPPROTO_IP)); + if (fd == -1) throw SysError("cannot open IP socket"); - struct ifreq ifr; - strcpy(ifr.ifr_name, "lo"); - ifr.ifr_flags = IFF_UP | IFF_LOOPBACK | IFF_RUNNING; - if (ioctl(fd, SIOCSIFFLAGS, &ifr) == -1) - throw SysError("cannot set loopback interface flags"); + struct ifreq ifr; + strcpy(ifr.ifr_name, "lo"); + ifr.ifr_flags = IFF_UP | IFF_LOOPBACK | IFF_RUNNING; + if (ioctl(fd, SIOCSIFFLAGS, &ifr) == -1) + throw SysError("cannot set loopback interface flags"); - fd.close(); + fd.close(); + } /* Set the hostname etc. to fixed values. */ char hostname[] = "localhost"; @@ -2463,8 +2517,16 @@ void DerivationGoal::registerOutputs() if (buildMode == bmRepair) replaceValidPath(path, actualPath); else - if (buildMode != bmCheck && rename(actualPath.c_str(), path.c_str()) == -1) - throw SysError(format("moving build output `%1%' from the chroot to the store") % path); + if (buildMode != bmCheck) { + if (S_ISDIR(st.st_mode)) + /* Change mode on the directory to allow for + rename(2). */ + chmod(actualPath.c_str(), st.st_mode | 0700); + if (rename(actualPath.c_str(), path.c_str()) == -1) + throw SysError(format("moving build output `%1%' from the chroot to the store") % path); + if (S_ISDIR(st.st_mode) && chmod(path.c_str(), st.st_mode) == -1) + throw SysError(format("restoring permissions on directory `%1%'") % actualPath); + } } if (buildMode != bmCheck) actualPath = path; } @@ -2723,17 +2785,42 @@ void DerivationGoal::deleteTmpDir(bool force) // 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()); + uid_t uid = settings.clientUid; + gid_t gid = settings.clientGid != 0 ? settings.clientGid : -1; + try { + _chown(tmpDir, uid, gid); + + if (getuid() != 0) { + /* If, without being root, the '_chown' call above + succeeded, then it means we have CAP_CHOWN. Retake + ownership of tmpDir itself so it can be renamed + below. */ + chown(tmpDir.c_str(), getuid(), getgid()); + } + + if (top != tmpDir) { + /* Rename 'tmpDir' to its parent with an intermediate + step. Skip that if the '_chown' call above fails + since in that case the setuid bits are not + removed. */ + 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"); + + if (getuid() != 0) + /* Running unprivileged but with CAP_CHOWN. */ + chown(top.c_str(), uid, gid); + + rmdir(pivot.c_str()); + } + } catch (SysError & e) { + /* When running as an unprivileged user and without + CAP_CHOWN, we cannot chown the build tree. Print a + message and keep going. */ + printMsg(lvlInfo, format("cannot change ownership of build directory '%1%': %2%") + % tmpDir % strerror(e.errNo)); } } } diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc index 0883a4bbcee..4308264a4f3 100644 --- a/nix/libstore/local-store.cc +++ b/nix/libstore/local-store.cc @@ -306,14 +306,14 @@ void LocalStore::openDB(bool create) void LocalStore::makeStoreWritable() { #if HAVE_UNSHARE && HAVE_STATVFS && HAVE_SYS_MOUNT_H && defined(MS_BIND) && defined(MS_REMOUNT) - if (getuid() != 0) return; /* Check if /nix/store is on a read-only mount. */ struct statvfs stat; if (statvfs(settings.nixStore.c_str(), &stat) != 0) throw SysError("getting info about the store mount point"); if (stat.f_flag & ST_RDONLY) { - if (unshare(CLONE_NEWNS) == -1) + int flags = CLONE_NEWNS | (getpid() == 0 ? 0 : CLONE_NEWUSER); + if (unshare(flags) == -1) throw SysError("setting up a private mount namespace"); if (mount(0, settings.nixStore.c_str(), "none", MS_REMOUNT | MS_BIND, 0) == -1) @@ -1614,11 +1614,19 @@ void LocalStore::createUser(const std::string & userName, uid_t userId) { auto dir = settings.nixStateDir + "/profiles/per-user/" + userName; - createDirs(dir); - if (chmod(dir.c_str(), 0755) == -1) - throw SysError(format("changing permissions of directory '%s'") % dir); - if (chown(dir.c_str(), userId, -1) == -1) - throw SysError(format("changing owner of directory '%s'") % dir); + auto created = createDirs(dir); + if (!created.empty()) { + if (chmod(dir.c_str(), 0755) == -1) + throw SysError(format("changing permissions of directory '%s'") % dir); + + /* The following operation requires CAP_CHOWN or can be handled + manually by a user with CAP_CHOWN. */ + if (chown(dir.c_str(), userId, -1) == -1) { + rmdir(dir.c_str()); + string message = strerror(errno); + printMsg(lvlInfo, format("failed to change owner of directory '%1%' to %2%: %3%") % dir % userId % message); + } + } }