From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:403:478a::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms9.migadu.com with LMTPS id AN2PJMpZ22Tj0wAASxT56A (envelope-from ) for ; Tue, 15 Aug 2023 12:56:10 +0200 Received: from aspmx1.migadu.com ([2001:41d0:403:478a::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id wLJ8JMpZ22RKVgAA9RJhRA (envelope-from ) for ; Tue, 15 Aug 2023 12:56:10 +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 36D9C4D030 for ; Tue, 15 Aug 2023 12:56:09 +0200 (CEST) Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=VsSRR30q; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1692096969; 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=P/qIV2Gy8zoK25NIJXESficYaDo5tv7FVkwN31l+Iuo=; b=BU/lSmPmhtTrPt9b4LFHxb1Ld4rmVz6WiMZmJtVI9215UxbA4phF7WmCQUZM4J2rkflRRt YRtZ3XoWXAaIjzJbIix6LGVT77A4gmfyQbKLOYMlpEgX98ETLxQ6hAJWWoIwlwLsuzDSCZ ujo/bpOJs7qO9YlcxMq246qjesvK8qHWIEsNIl/06J1RzV+xKudL8sjRln+1tJ9kx7oRi0 jQMSXUGTfgRR7Soowz0vAePTbF1ZxXsDgsnvnIO9y/rqeIk3fhxbCmSVte7wFA4zwunBC3 3VXpiUEwvFOB4K4U9FCQ7HTIRhFY1ht8q6lEEC0JRipkUXXnRcV1qhCyQP2Cjg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gnu.org header.s=fencepost-gnu-org header.b=VsSRR30q; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" ARC-Seal: i=1; s=key1; d=yhetil.org; t=1692096969; a=rsa-sha256; cv=none; b=M/dph0rqa7WKpXLfM9BLaP/ta4w/guRCQGHTM+7aZ8LXhqppeyLU2hScXyrjSR9p3Gfwmc QviW9T+NgiMSkQvfDt1+XE2FrpG5Pil0fsdLTWYXctB0dY7kt4R3/IFlMep6xrT1vRYNMN BlE8gW9GWYbEtHfuhK9vRRjMER7bQBboae0dErO0yjk3OHPQSx0/rD0/JZAeEpK1stdQ8T S7QrDtxxWQz9PykrLcgdjUT1n+IMCdmUgh9ShvRPtdWbHoGTpiyREn1Ag+s0fjEqHOvyi9 XkSE2Tz/GPth/yc3Z0KXQysdO1WfxQL40F3zfxCgG0JdSjK6pQiwRRSma4DKEw== Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qVriW-0001Dz-Bp; Tue, 15 Aug 2023 06:56:04 -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 1qVriU-0001Ce-Aa for guix-patches@gnu.org; Tue, 15 Aug 2023 06:56:02 -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 1qVriU-000583-2G for guix-patches@gnu.org; Tue, 15 Aug 2023 06:56:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qVriT-000816-Oe for guix-patches@gnu.org; Tue, 15 Aug 2023 06:56:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#65221] [PATCH 0/2] Fix EXTRA-PORTS edge cases Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 15 Aug 2023 10:56:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 65221 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: ulfvonbelow Cc: 65221@debbugs.gnu.org Received: via spool by 65221-submit@debbugs.gnu.org id=B65221.169209694930791 (code B ref 65221); Tue, 15 Aug 2023 10:56:01 +0000 Received: (at 65221) by debbugs.gnu.org; 15 Aug 2023 10:55:49 +0000 Received: from localhost ([127.0.0.1]:35050 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qVriH-00080Z-96 for submit@debbugs.gnu.org; Tue, 15 Aug 2023 06:55:49 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:46728) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qVriE-00080L-TY for 65221@debbugs.gnu.org; Tue, 15 Aug 2023 06:55:48 -0400 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 1qVri6-00052B-Uq; Tue, 15 Aug 2023 06:55:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To: From; bh=P/qIV2Gy8zoK25NIJXESficYaDo5tv7FVkwN31l+Iuo=; b=VsSRR30qivIwOKU4xTfg xd0qkDCh8omsvWgHzzDum4B9YV1iFSHney2JEHHKgoapMaP8p3LWGWhPq9eC7akLl46IEFAboWdGM NYPCZpK04kkjlubR6Pc23qR6ksL3zPxgPbOVtVy73VvzCP9NK5JZfz6GNcfcEKrLRDh/m07ZHKrTd XeQkixBU9Wemft0j+DyR6BJMuS4uSP9LnjuCrrtBWTJEkO6lKaGq5zeFkeBIQtLn7+2Y/znYC6KbN wBE6TxY7CCjSTvdcdGKevgBGRIWiD4vob1AEQJTN+qV+C4BVWeLCpIY7hZG6mBrdofAi+5/SidB3F 3Z4LGichRCnxQA==; From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20230811090352.3572-1-striness@tilde.club> <20230811090615.3707-1-striness@tilde.club> Date: Tue, 15 Aug 2023 12:55:03 +0200 In-Reply-To: <20230811090615.3707-1-striness@tilde.club> (ulfvonbelow's message of "Fri, 11 Aug 2023 04:06:14 -0500") Message-ID: <87y1ic8tiw.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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+larch=yhetil.org@gnu.org Sender: guix-patches-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US X-Spam-Score: -6.89 X-Migadu-Queue-Id: 36D9C4D030 X-Migadu-Scanner: mx0.migadu.com X-Migadu-Spam-Score: -6.89 X-TUID: yL7mGonfWl1J Hi, ulfvonbelow skribis: > EXEC-COMMAND (and, by extension, FORK+EXEC-COMMAND) has several issues: > 1. Despite it being documented that "all other file descriptors are closed > prior to yielding control to COMMAND", this is not currently the case - > only other file descriptors that are already marked as FD_CLOEXEC are > closed. For example, if user code happens to have a file descriptor o= pen, > for example with call-with-input-file, while EXEC-COMMAND is run, the = new > process image will inherit that file descriptor. This may cause some > resource waste, but more importantly may cause security issues in cert= ain > situations. Yes. This has been the case since 0.9.2, as noted in =E2=80=98NEWS=E2=80= =99: Previously, services started indirectly with =E2=80=98exec-command=E2=80= =99 (which is usually the case) would not inherit any file descriptor from shepherd because =E2=80=98exec-command=E2=80=99 would explicitly close all of them. Howev= er, services started with =E2=80=98make-system-constructor=E2=80=99 and processes created by s= ome other means, such as calling =E2=80=98system*=E2=80=99, would inherit some of those descrip= tors, giving them more authority than intended. The change here consists in marking all internally-used file descriptors = as =E2=80=9Cclose-on-exec=E2=80=9D (O_CLOEXEC), a feature that=E2=80=99s bee= n available on GNU/Linux and GNU/Hurd for years but that so far wasn=E2=80=99t used consistently in sh= epherd. This is now fixed. As a side-effect, the file-descriptor-closing loop in =E2=80=98exec-command=E2=80=99 is now gone. The FD-closing loop was removed on purpose, in 2c0354258047133db8b885bcc11afdf0def5d885. Now, as you write, it means that service writers must be careful now and not leave any non-CLOEXEC file descriptor behind them. At the time I audited Guix System to check that this was a reasonable thing to expect and that we could indeed ensure no file descriptors were leaked. There=E2=80=99s also =E2=80=98tests/close-on-exec.sh=E2=80=99. If you found cases where it would be necessary, what we could do is have =E2=80=98shepherd=E2=80=99 replace =E2=80=98call-with-input-file=E2=80=99 &= co. with a variant that opens files as O_CLOEXEC by default. WDYT? > 2. EXTRA-PORTS is only honored when either LOG-PORT or LOG-FILE is passed= . I > have no idea why this is the case, it isn't documented anywhere, and it > isn't intuitive. #:extra-ports wasn=E2=80=99t really made to be exposed I guess; it was adde= d for use by systemd-style services in 965f6b61a473ee57a1fc6ec3ea1ad6e35d596031. > 3. Even when LOG-PORT or LOG-FILE is passed, EXTRA-PORTS may not work as > described, because it copies file descriptor contents in an arbitrary > order. For example, suppose that (map fileno EXTRA-PORTS) is (7 6 5 4= 3). > If the underlying file originally stored in fd N is represented by F(N= ), it > will assign > 3 <-- F(7) > 4 <-- F(6) > 5 <-- F(5) > 6 <-- F(6) > 7 <-- F(7) > > In other words, the copying of earlier FDs in EXTRA-PORTS may overwrite > later FDs in EXTRA-PORTS. Good catch! Could you make a more minimal patch fixing this specific issue, also adding a test reproducing the problem being fixed? Thanks for your work! Ludo=E2=80=99.