From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id WCmiMRiqLmBvIgAA0tVLHw (envelope-from ) for ; Thu, 18 Feb 2021 17:55:36 +0000 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id 81tyLRiqLmClBwAAbx9fmQ (envelope-from ) for ; Thu, 18 Feb 2021 17:55:36 +0000 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 7320D320EA for ; Thu, 18 Feb 2021 18:55:36 +0100 (CET) Received: from localhost ([::1]:43418 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lCnWZ-0004ne-Eh for larch@yhetil.org; Thu, 18 Feb 2021 12:55:35 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:38644) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lCnVS-0003jH-9n for guix-devel@gnu.org; Thu, 18 Feb 2021 12:54:26 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]:47277) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lCnVQ-00048w-7K; Thu, 18 Feb 2021 12:54:24 -0500 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=46884 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lCnVP-0008H8-NM; Thu, 18 Feb 2021 12:54:24 -0500 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Maxime Devos Subject: Re: TOCTTOU race References: <87k0rrls0z.fsf@gnu.org> <08F0CD76-DDCF-4CFA-AE8D-5FB165A62B25@lepiller.eu> <87o8h2ehy7.fsf@gnu.org> <69968b3a01d872cabdf55a94b6c82d5057e010c9.camel@telenet.be> <87v9b66dm1.fsf@gnu.org> <56adb5efa894304c27beba99b07e2f8cfd8ee7cb.camel@telenet.be> <87zh0gzy52.fsf@gnu.org> <53c60ce40d68cfc93a9ea2c4a8f865026e12c889.camel@telenet.be> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 30 =?utf-8?Q?Pluvi=C3=B4se?= an 229 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Thu, 18 Feb 2021 18:54:22 +0100 In-Reply-To: <53c60ce40d68cfc93a9ea2c4a8f865026e12c889.camel@telenet.be> (Maxime Devos's message of "Sun, 14 Feb 2021 13:29:29 +0100") Message-ID: <87h7m9p8hd.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: guix-devel@gnu.org Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -1.37 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Queue-Id: 7320D320EA X-Spam-Score: -1.37 X-Migadu-Scanner: scn1.migadu.com X-TUID: ATTzhtAnFK4J Hi Maxime, Maxime Devos skribis: > From ad10c577eb1f13b9b66ea387648671df33b869d7 Mon Sep 17 00:00:00 2001 > From: Maxime Devos > Date: Sun, 14 Feb 2021 12:57:32 +0100 > Subject: [PATCH] services: prevent following symlinks during activation > > Currently, there's a TOCTTOU race. This can be addressed > once guile has bindings for fstatat, openat and friends. > > XXX I'm horrible at naming exceptions: > (throw 'XXX-TODO-does-someone-have-an-idea? path) > > * guix/build/service-utils.scm: new module > with new procedure 'mkdir-p/perms'. > * Makefile.am (MODULES): compile new module. > * gnu/services/authentication.scm > (%nslcd-activation, nslcd-service-type): use new procedure. > * gnu/services/cups.scm (%cups-activation): likewise. > * gnu/services/dbus.scm (dbus-activation): likewise. > * gnu/services/dns.scm (knot-activation): likewise. Nice! > +(define-module (guix build service-utils) > + #:use-module (ice-9 match) > + #:use-module (guix build utils) > + #:export (mkdir-p/perms)) I think this should go either in (gnu build activation) or in a new (gnu build utils) module. (guix build =E2=80=A6) is for non-Guix-System things. > +;; Based upon mkdir-p from (guix build utils) > +(define (verify-not-symbolic dir) > + "Verify DIR or its ancestors aren't symbolic links." > + (define absolute? > + (string-prefix? "/" dir)) > + > + (define not-slash > + (char-set-complement (char-set #\/))) > + > + (define (verify-component path) > + (when (eq? 'symlink (stat:type (lstat path))) > + (throw 'XXX-TODO-does-someone-have-an-idea? path))) It=E2=80=99s tempting to do something like: (error "file name component is a directory" dir) Note that, if that happens at boot time, the system will fail to boot (I think you=E2=80=99d get a REPL rather than a kernel panic, but it=E2=80=99d= be good to check in a VM.) > + (let loop ((components (string-tokenize dir not-slash)) > + (root (if absolute? > + "" > + "."))) > + (match components > + ((head tail ...) > + (let ((path (string-append root "/" head))) Per GNU and Guix convention, =E2=80=9Cpath=E2=80=9D is for =E2=80=9Csearch = paths=E2=80=9D; here it should be =E2=80=9Cfile=E2=80=9D or something. > + (catch 'system-error > + (lambda () > + (verify-component path) > + (loop tail path)) > + (lambda args > + (if (=3D ENOENT (system-error-errno args)) > + #t > + (apply throw args)))))) > + (() #t)))) That reminded me of the =E2=80=98safe_path=E2=80=99 function in OpenSSH, bu= t that one checks the permissions on file name components, and doesn=E2=80=99t check f= or symlinks (maybe it should; there=E2=80=99s an XXX comment there.) > +(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 TOCTOU race!" > + (verify-not-symbolic directory) > + (mkdir-p directory) > + (chown directory (passwd:uid owner) (passwd:gid owner)) > + (chmod directory bits)) Otherwise LGTM, thanks! Ludo=E2=80=99.