From patchwork Tue Mar 18 14:00:16 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: 40325 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 7F61B27BBE2; Tue, 18 Mar 2025 14:02:10 +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 AFF3B27BBE9 for ; Tue, 18 Mar 2025 14:02:06 +0000 (GMT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tuXVC-0006vq-DP; Tue, 18 Mar 2025 10:01:06 -0400 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 1tuXVA-0006vV-Ma for guix-patches@gnu.org; Tue, 18 Mar 2025 10:01:04 -0400 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 1tuXVA-0001ny-6J for guix-patches@gnu.org; Tue, 18 Mar 2025 10:01:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=xBjgdla+kQknEBqI+xyUQw+JCaa5THMk8Wk84l/80ac=; b=p56yV8oYsMBDDcYV2A6m6OyLwWBwDgFHWul7o9Uwii5AwzpkHJFxyCIfXIr7CWIv4YMpMlqh95mGmOcCuKp+93yk96Cw4Gn/bgY4OJQzuHK+27+yX3pysThUlw4Zbs+qxK23J4oa1+kcvu2DQUrXVsJgAwzhgvwi7RzvWU1Iu+9EuFOYJ0qXgm84slsh9gK8GsseOrVZ/qrzx/36FvedPtnblPBQpHsoykJEs7vZBRuFM4rgJhoDGnK38rVqDflXJPhq7JVDQnpVCfroO3oPVBZqtj3a4xwUrplJwwyVP+j7voTTprANwmgwp2FRWyYkHVfVegPjK8v62D8ZzaGX6g==; Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tuXV8-0000sx-Vq for guix-patches@gnu.org; Tue, 18 Mar 2025 10:01:03 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#75810] [PATCH v6 00/16] Rootless guix-daemon Resent-From: Ludovic =?utf-8?q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 18 Mar 2025 14:01:02 +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: Reepca Russelstein Cc: 75810@debbugs.gnu.org Received: via spool by 75810-submit@debbugs.gnu.org id=B75810.17423064383257 (code B ref 75810); Tue, 18 Mar 2025 14:01:02 +0000 Received: (at 75810) by debbugs.gnu.org; 18 Mar 2025 14:00:38 +0000 Received: from localhost ([127.0.0.1]:41297 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tuXUj-0000qT-7d for submit@debbugs.gnu.org; Tue, 18 Mar 2025 10:00:38 -0400 Received: from hera.aquilenet.fr ([185.233.100.1]:43422) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1tuXUZ-0000p9-Gy for 75810@debbugs.gnu.org; Tue, 18 Mar 2025 10:00:34 -0400 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id BCC793D7; Tue, 18 Mar 2025 15:00:19 +0100 (CET) Authentication-Results: hera.aquilenet.fr; none X-Virus-Scanned: Debian amavis at hera.aquilenet.fr Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavis, port 10024) with ESMTP id b3XMQXplc12u; Tue, 18 Mar 2025 15:00:18 +0100 (CET) Received: from ribbon (unknown [193.50.110.142]) by hera.aquilenet.fr (Postfix) with ESMTPSA id EFC5930A; Tue, 18 Mar 2025 15:00:17 +0100 (CET) From: Ludovic =?utf-8?q?Court=C3=A8s?= In-Reply-To: <875xk7594u.fsf@russelstein.xyz> (Reepca Russelstein's message of "Mon, 17 Mar 2025 22:07:45 -0500") References: <875xk7594u.fsf@russelstein.xyz> Date: Tue, 18 Mar 2025 15:00:16 +0100 Message-ID: <87v7s6h21b.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 X-Rspamd-Queue-Id: BCC793D7 X-Spamd-Result: default: False [4.90 / 15.00]; SPAM_FLAG(5.00)[]; BAYES_HAM(-3.00)[100.00%]; NEURAL_SPAM(3.00)[1.000]; MIME_GOOD(-0.10)[multipart/mixed,text/plain,text/x-patch]; RCVD_COUNT_TWO(0.00)[2]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:+,3:+]; RCPT_COUNT_TWO(0.00)[2]; ARC_NA(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; RCVD_TLS_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; MID_RHS_MATCH_FROM(0.00)[] X-Rspamd-Action: no action X-Spamd-Bar: ++++ X-Rspamd-Server: hera 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 Hi! Reepca Russelstein skribis: > (in patch 7/16, in nix/libstore/build.cc) > > Strictly speaking we should check whether the fds are >= 0, not > 0, > since 0 is technically a valid file descriptor, and we use -1 to > indicate the absence of a file descriptor. > > Also, readiness.readSide isn't explicitly closed in the child after > we're done with it. Right, fixed. >> + # The unprivileged daemon cannot create the log directory by itself. >> + mkdir /var/log/guix >> + chown guix-daemon:guix-daemon /var/log/guix >> + chmod 755 /var/log/guix > > (in patch 15/16, in etc/guix-install.sh) > > Should this be 'mkdir -p' or some other conditional creation? If I > understand correctly this will fail when overwriting an existing install > using GUIX_ALLOW_OVERWRITE. Correct, fixed as well. (I’ve updated the branch on Codeberg.) > Concerning guix-install.sh, to be clear, is the intent to specifically > not support installing the rootful daemon on systemd systems? > > For my two cents, I do think that it's still a tradeoff - not just > because of the reliance on different kernel mechanisms for security, but > also because the rootless daemon currently causes visible changes to the > build environment (EROFS on /, and nothing owned by root, for example). > Which one should we consider the "canonical" build environment going > forward? The way I see it, we would gradually move to the non-root daemon: • First step here is to enable it by default on systemd distros. • Second step would be to allow Guix System users to migrate to non-root daemon, keeping the default unchanged. • Third step (a year later maybe?) would be to default to non-root daemon on Guix System and on remaining distros (though for these it might be trickier because we probably cannot rely on CAP_SYS_CHOWN, not as easily as with systemd at least). The visible changes in the build environment are unfortunate; I’m hoping they won’t have any practical impact, not any more than the other parameters that may change currently (build UID, binfmt_misc, file system, etc.) We could test this hypothesis by rebuilding at least the entire set of packages up to ‘hello’. (I tried doing it just now in a Debian VM but failed since the main partition cannot easily be extended; it’ll be easier to do with Guix System.) > The demo, modified for guix circumstances, would go something like this: > > 1. A derivation is created whose builder is /proc/self/exe, and whose > LD_PRELOAD environment variable points into a malicious store item > for one of its shared libraries - for example, libc. > 2. The daemon reads this in, and, to my knowledge, does no verification > of the builder string. Note that this aspect isn't actually > necessary, as the builder could also be a symlink to /proc/self/exe > from the store. > 3. The daemon sets up the build environment, and execs /proc/self/exe. Then there are 3 possibilities: 1. If /proc/self/exe points to (say) /usr/bin/guix-daemon, outside the store, execve fails with ENOENT because that file is not mounted in the chroot. 2. If /proc/self/exe points to a guix-daemon file inside the store: 2a. If the store item containing guix-daemon is not an input of the derivation, execv fails with ENOENT. 2b. If the store item containing guix-daemon is an input of the derivation, then it’s been remounted read-only and attempts to write to it fail with EROFS. Here’s a test that fails both “rootfull” and “rootless”: --8<---------------cut here---------------start------------->8--- (let* ((builder (add-file-tree-to-store %store `("builder" symlink "/proc/self/exe"))) (drv (derivation %store "attempt-to-run-guix-daemon" builder '() #:env-vars '(("LD_PRELOAD" . "attacker-controlled.so"))))) (guard (c ((store-protocol-error? c) c)) (build-derivations %store (list drv)) #f)) --8<---------------cut here---------------end--------------->8--- > There are several points at which that particular attack could be > stopped, and I'd like to try to stop it at as many of them as possible. > A good start would be canonicalizing the builder prior to executing it > and then checking to make sure it is in the store. A more general > solution could look like writing out and then executing a tiny binary, > something like /tmp/runbuilder, that does nothing but unlink itself and > then exec the actual program. If the above is correct, we’re already safe against this particular attack. Canonicalizing the builder cannot hurt (it’s useful in the ‘--disable-chroot’ case though mostly to prevent programming errors rather than from a security perspective since there are many other issues in that case), apart from adding more code to an already long function. For reference, the extra check I tried but that I’m inclined to not include since it cannot catch anything new: Let me know what you think and I’ll send v7 accordingly. Ludo’. diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 7484ea8fcf..970107c265 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -2374,6 +2374,15 @@ void DerivationGoal::runChild() writeFull(STDERR_FILENO, "error: " + string(e.what()) + "\n"); _exit(1); } + } else { + /* Ensure that the builder is within the store. This prevents + users from using /proc/self/exe (or a symlink to it) as their + builder, which could allow them to overwrite the guix-daemon + binary (CVE-2019-5736). */ + drv.builder = canonPath(drv.builder, true); + printMsg(lvlError, format("builder is `%1%'") % drv.builder); + if (!isInStore(drv.builder)) + throw Error(format("derivation builder '%1%' is outside the store") % drv.builder); } /* Fill in the arguments. */