From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id 8BpkLPj9W2MBgAAAbAwnHQ (envelope-from ) for ; Fri, 28 Oct 2022 18:06:16 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id YJhTLPj9W2P8YQAA9RJhRA (envelope-from ) for ; Fri, 28 Oct 2022 18:06:16 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 4C34730930 for ; Fri, 28 Oct 2022 18:06:16 +0200 (CEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ooRs4-0003ls-GU; Fri, 28 Oct 2022 12:06:13 -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 1ooRqx-0002Dq-JU for bug-guix@gnu.org; Fri, 28 Oct 2022 12:05:14 -0400 Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ooRqx-0003Hf-1u for bug-guix@gnu.org; Fri, 28 Oct 2022 12:05:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ooRqw-0005Ud-T5 for bug-guix@gnu.org; Fri, 28 Oct 2022 12:05:02 -0400 X-Loop: help-debbugs@gnu.org Subject: bug#47584: [PATCH 3/3] activation: Fix TOCTTOU in mkdir-p/perms. Resent-From: Maxime Devos Original-Sender: "Debbugs-submit" Resent-CC: bug-guix@gnu.org Resent-Date: Fri, 28 Oct 2022 16:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47584 X-GNU-PR-Package: guix X-GNU-PR-Keywords: security patch To: 47584@debbugs.gnu.org Cc: Maxime Devos Received: via spool by 47584-submit@debbugs.gnu.org id=B47584.166697306621053 (code B ref 47584); Fri, 28 Oct 2022 16:05:02 +0000 Received: (at 47584) by debbugs.gnu.org; 28 Oct 2022 16:04:26 +0000 Received: from localhost ([127.0.0.1]:34127 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ooRqL-0005TU-13 for submit@debbugs.gnu.org; Fri, 28 Oct 2022 12:04:25 -0400 Received: from michel.telenet-ops.be ([195.130.137.88]:56244) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ooRqA-0005Sd-Oc for 47584@debbugs.gnu.org; Fri, 28 Oct 2022 12:04:17 -0400 Received: from localhost.localdomain ([213.119.228.183]) by michel.telenet-ops.be with bizsmtp id dU4B2800F3y3Lwt06U4CSp; Fri, 28 Oct 2022 18:04:12 +0200 From: Maxime Devos Date: Fri, 28 Oct 2022 18:04:09 +0200 Message-Id: <20221028160409.31887-3-maximedevos@telenet.be> X-Mailer: git-send-email 2.38.0 In-Reply-To: <20221028160409.31887-1-maximedevos@telenet.be> References: <20221028160409.31887-1-maximedevos@telenet.be> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telenet.be; s=r22; t=1666973052; bh=cp9gdROeKyKOVhm7ov7oMK+2nzQun7O7h7vn+Qg8fMw=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=XSVvzIGIXYbFDLKseiqrgJZ7JFqodD5dJmoV1UJFgP9QA9GgtuxasVcAUoUMaWk/k QrdlutabopbGdY84ELHUk99pKVJHmkCu/pdOdSxx3WKYAX5AUBquwwAdomJQ96xRRr 2ZNf+M8DTH7UGNUZySTuVBnSch2nWvhkFOSSpYGQ6Ha99twW+wPZ2ywecr3gVSt9w6 5/13/KLz5f+5CZYv54Wyf5YziZUgffKMsXhzSpBnFwofFYWV0bl1OlWZgfJOjQE4P9 5Gic2R/EOIOPcgCssI3a0ZWVD4n2vUxNvgt1IAiYcsVOmyNxMaTRmLug6BUO7o1yeU 8PVPf5oyOHUoA== X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guix@gnu.org List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "bug-Guix" Errors-To: bug-guix-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1666973176; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=cp9gdROeKyKOVhm7ov7oMK+2nzQun7O7h7vn+Qg8fMw=; b=sdiU/c4fTB+JS3k+JowpEYVDsYXD8yrcPOE1j+KbsM0oEUiC/qnxdtLS8pXqBUHTGnb3wS 6MvNL/YZ5Criw2pyiaUgK3KAoojsFFNh7bQZONJjT9ujJI2UZW88Fyf9Sw6QqCvAeCDUI6 bq4TesMV4mBGrX0ej2IWa/0GyWfm5cU+VvuJ9KE/vKDa3VKlJPqb09wj9bPRonI5oCdGJW CIn3T2D5sYwn27qdp5+pc2wHajJp4GEXd98uxvVBApkyA6qqXVta3VDlBja7MRKTtFZyBk l1CsrfxKblxcdSDbQJSNi1XbkYf60p3BlL0l5S1PJfOjd7NF3tmXIrWEC+MPIQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1666973176; a=rsa-sha256; cv=none; b=hoQQl10Gehcy+VhXb7ASwW/fnDtCcr5ncySvfyQ+ICLuB7IqPAuFzTFv2G+R4mpX9lSTwD APqlTypUSjaNPbGs2jBd5diNEK7QQWpeCKeJF2xoCF9mB+mq+tayxgebCTkDJZozQJxygW h65Mu1SsBWsE1iWJW0LMAa2ihX/U1gUXjHfretyqJ6c+vEarTxlnnXNCEV2K2fdNxRDU7z HkobIKaBoWjMUl06aAqsV3DNUjv2jsrfxODte96WymvvGFysLs1ciWunUQWnf83+a7gdQB mnXDMB32XJxzbSSzjZLEBG/HuLSStO6ZBMGkEs5hSZywwQZ4N1ZWr6anOExhfg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=telenet.be header.s=r22 header.b=XSVvzIGI; dmarc=fail reason="SPF not aligned (relaxed)" header.from=telenet.be (policy=none); spf=pass (aspmx1.migadu.com: domain of "bug-guix-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="bug-guix-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: 7.08 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=telenet.be header.s=r22 header.b=XSVvzIGI; dmarc=fail reason="SPF not aligned (relaxed)" header.from=telenet.be (policy=none); spf=pass (aspmx1.migadu.com: domain of "bug-guix-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="bug-guix-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 4C34730930 X-Spam-Score: 7.08 X-Migadu-Scanner: scn1.migadu.com X-TUID: wh8ReM1YYn56 I removed the 'Based upon mkdir-p from (guix build utils)' comment because it's quite a bit different now. * gnu/build/activation.scm (verify-not-symbolic): Delete. (mkdir-p/perms): Rewrite in terms of 'openat'. --- gnu/build/activation.scm | 90 +++++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 33 deletions(-) diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm index 10c9045740..29c6f2ce4c 100644 --- a/gnu/build/activation.scm +++ b/gnu/build/activation.scm @@ -5,7 +5,7 @@ ;;; Copyright © 2015, 2018 Mark H Weaver ;;; Copyright © 2018 Arun Isaac ;;; Copyright © 2018, 2019 Ricardo Wurmus -;;; Copyright © 2021 Maxime Devos +;;; Copyright © 2021, 2022 Maxime Devos ;;; Copyright © 2020 Christine Lemmer-Webber ;;; Copyright © 2021 Brice Waegeneire ;;; @@ -64,46 +64,70 @@ (define %skeleton-directory (define (dot-or-dot-dot? file) (member file '("." ".."))) -;; Based upon mkdir-p from (guix build utils) -(define (verify-not-symbolic dir) - "Verify DIR or its ancestors aren't symbolic links." +(define (mkdir-p/perms directory owner bits) + "Create directory DIRECTORY and all its ancestors. + +Additionally, verify no component of DIRECTORY is a symbolic link, +without TOCTTOU races. However, if OWNER differs from the the current +(process) uid/gid, there is a small window in which DIRECTORY is set to the +current (process) uid/gid instead of OWNER. This is not expected to be +a problem in practice. + +The permission bits and owner of DIRECTORY are set to BITS and OWNER. +Anything above DIRECTORY that already exists keeps +its old owner and bits. For components that do not exist yet, the owner +and bits are set according to the default behaviour of 'mkdir'." (define absolute? - (string-prefix? "/" dir)) + (string-prefix? "/" directory)) (define not-slash (char-set-complement (char-set #\/))) - (define (verify-component file) - (unless (eq? 'directory (stat:type (lstat file))) - (error "file name component is not a directory" dir))) + ;; By combining O_NOFOLLOW and O_DIRECTORY, this procedure automatically + ;; verifies that no components are symlinks. + (define open-flags (logior O_CLOEXEC ; don't pass the port on to subprocesses + O_NOFOLLOW ; don't follow symlinks + O_DIRECTORY)) ; reject anything not a directory - (let loop ((components (string-tokenize dir not-slash)) - (root (if absolute? - "" - "."))) + (let loop ((components (string-tokenize directory not-slash)) + (root (open (if absolute? "/" ".") open-flags))) (match components ((head tail ...) - (let ((file (string-append root "/" head))) - (catch 'system-error - (lambda () - (verify-component file) - (loop tail file)) - (lambda args - (if (= ENOENT (system-error-errno args)) - #t - (apply throw args)))))) - (() #t)))) - -;; TODO: the TOCTTOU race can be addressed once guile has bindings -;; for fstatat, openat and friends. -(define (mkdir-p/perms directory owner bits) - "Create the directory DIRECTORY and all its ancestors. -Verify no component of DIRECTORY is a symbolic link. -Warning: this is currently suspect to a TOCTTOU race!" - (verify-not-symbolic directory) - (mkdir-p directory) - (chown directory (passwd:uid owner) (passwd:gid owner)) - (chmod directory bits)) + (let retry () + ;; In the usual case, we expect HEAD to already exist. + (match (catch 'system-error + (lambda () + (openat root head open-flags)) + (lambda args + (if (= ENOENT (system-error-errno args)) + #false + (begin + (close-port root) + (apply throw args))))) + ((? port? new-root) + (close root) + (loop tail new-root)) + (#false + ;; If not, create it. + (catch 'system-error + (lambda _ + (mkdirat root head)) + (lambda args + ;; Someone else created the directory. Unexpected but fine. + (unless (= EEXIST (system-error-errno args)) + (close-port root) + (apply throw args)))) + (retry))))) + (() + (catch 'system-error + (lambda () + (chown root (passwd:uid owner) (passwd:gid owner)) + (chmod root bits)) + (lambda args + (close-port root) + (apply throw args))) + (close-port root) + (values))))) (define* (copy-account-skeletons home #:key -- 2.38.0