From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id eDyGHz3v9F8nYQAA0tVLHw (envelope-from ) for ; Tue, 05 Jan 2021 22:59:09 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id eGNZGz3v9F+tTAAA1q6Kng (envelope-from ) for ; Tue, 05 Jan 2021 22:59:09 +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 328339403A4 for ; Tue, 5 Jan 2021 22:59:09 +0000 (UTC) Received: from localhost ([::1]:38010 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kwvIC-0000m3-6O for larch@yhetil.org; Tue, 05 Jan 2021 17:59:08 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:51116) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kwvI6-0000lx-5q for guix-patches@gnu.org; Tue, 05 Jan 2021 17:59:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:59599) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kwvI5-0002SE-V7 for guix-patches@gnu.org; Tue, 05 Jan 2021 17:59:01 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kwvI5-0001Jz-UU for guix-patches@gnu.org; Tue, 05 Jan 2021 17:59:01 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#45409] [PATCH v3 1/3] substitute: Untangle skipping authentication from valid-narinfo?. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 05 Jan 2021 22:59:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 45409 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Received: via spool by 45409-submit@debbugs.gnu.org id=B45409.16098875055038 (code B ref 45409); Tue, 05 Jan 2021 22:59:01 +0000 Received: (at 45409) by debbugs.gnu.org; 5 Jan 2021 22:58:25 +0000 Received: from localhost ([127.0.0.1]:42912 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kwvHV-0001JC-8Y for submit@debbugs.gnu.org; Tue, 05 Jan 2021 17:58:25 -0500 Received: from mira.cbaines.net ([212.71.252.8]:59558) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kwvHU-0001J4-0f for 45409@debbugs.gnu.org; Tue, 05 Jan 2021 17:58:24 -0500 Received: from localhost (188.28.108.198.threembb.co.uk [188.28.108.198]) by mira.cbaines.net (Postfix) with ESMTPSA id BCECB27BC0C; Tue, 5 Jan 2021 22:58:22 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 052057ac; Tue, 5 Jan 2021 22:58:20 +0000 (UTC) References: <87y2hn9l8j.fsf@cbaines.net> <20210104211927.14959-1-mail@cbaines.net> <871rezt5cd.fsf@gnu.org> User-agent: mu4e 1.4.13; emacs 27.1 From: Christopher Baines In-reply-to: <871rezt5cd.fsf@gnu.org> Date: Tue, 05 Jan 2021 22:58:17 +0000 Message-ID: <878s97j8ja.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" 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: , Cc: 45409@debbugs.gnu.org Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -4.44 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: 328339403A4 X-Spam-Score: -4.44 X-Migadu-Scanner: scn0.migadu.com X-TUID: lB8rUd0v1iN/ --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Hi, > > Christopher Baines skribis: > >> Rather than having valid-narinfo? evaluate to #t if >> %allow-unauthenticated-substitutes? is set to #t, just use (const #t) for >> valid-narinfo? when %allow-unauthenticated-substitutes? is set to #t. T= his >> will allow moving valid-narinfo? in to a (guix substitutes) module. >> >> * guix/scripts/substitute.scm (process-query, process-substitution): Cha= nge >> the authorized? argument to lookup-narinfo and lookup-narinfos/diverse b= ased >> on %allow-unauthenticated-substitutes?. >> (valid-narinfo?): Remove use of %allow-unauthenticated-substitutes?. > > Bummer that there are two call sites. > > What about doing away with =E2=80=98%allow-unauthenticated-substitutes?= =E2=80=99 and > instead changing its only user, =E2=80=98tests/substitute.scm=E2=80=99, l= ike so: > > diff --git a/tests/substitute.scm b/tests/substitute.scm > index 542aaf603f..1827ffe8d4 100644 > --- a/tests/substitute.scm > +++ b/tests/substitute.scm > @@ -178,10 +178,10 @@ a file for NARINFO." > (call-with-output-file > (string-append narinfo-directory "/example.nar") > (cute write-file > - (string-append narinfo-directory "/example.out") <>)) > - > - (%allow-unauthenticated-substitutes? #f)) > - thunk > + (string-append narinfo-directory "/example.out") <>))) > + (lambda () > + (mock ((guix narinfo) valid-narinfo?) (const #t) > + (thunk))) > (lambda () > (when (file-exists? cache-directory) > (delete-file-recursively cache-directory)))))) > > That change would have to be made in the patch that creates (guix > narinfo). > > WDYT? I don't know what's up with these tests in particular, adding peek in places makes tests fail... not using Guile debugging helpers and outputting to (current-error-port) seems to not change the result though. I didn't really understand this code, but looking at it more, I'm thinking now that what it actually does is affects all the tests, and for some tests in the (tests substitute) module, the %allow-unauthenticated-substitutes? behaviour is turned off. Commenting out the relevant code in the script seems to support this, the substitute tests still pass, but tests in the store, derivation and guix-daemon modules fail. The substitute tests are actually fine, and break if you disable substitute authentication. The mock approach is probably feasible, but it would need to be done in those modules/tests. I haven't looked at the details, but I'd be a little concerned that it might require mocking in each of the individual 15 failing tests, maybe that's good for being explicit though? Back to the use of %allow-unauthenticated-substitutes? in the code, there are two call sites, for the two separate code paths, but it would be pretty easy to move to one call site. Both process-query and process-substitution take an acl, but they could instead take some (valid? obj) procedure. That would either call (valid-narinfo? obj acl) or just evaluate to #t in the allow unauthorized case. This effectively moves the logic and call site to the command. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAl/07wlfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9Xf64RAAqQ6xADmqmC1avTCo5hHpSLtLqPInecDx Q4v75eLD2uHXZfCnGbpvdvdIujVdtnBqQjfWSPK432zOAw2o0x+o6KBo3ogtKQ4n CGhx+aKpnUC0MjptbA+bWlXnKnwn/+n5OX4U+WxPjkg5zxiuWVvc9nz6GFdd15EZ jzoRz+9Hq/+YfyxPN+rjiw7l48w6T7PgWwpgoprwZZw5u3prLZqXhPWLMYSianf0 iYIC5pxqKEhKCKGFoVZOL6ZXjXQT7r73R6GN2t5TsjuaVafhYImetoWPiK5HAyRq sMKu2q7L+VMrOEtUndmjqYukBRw+yUzWYilfqqCKWoy33WuHbj+CP6VD2EUHQ9S4 LpIrdJKwdQ7a7tiYYw5O1UWeOvsN8lAMaW+yrUvhK1j79yKrxo0tpq51Qda43kdt +QmTSP3cUVrmyY19UVqNa3QrwDI9h2zzgUV8kD5+g7kAkxZmGe2l16mkhPK+Fg4U uaM6YPY+sod73nc//iaiySQ5orz8Dokza7BnELur2IJnqBh6G6qyfanbRBGoXyIE G4Ne46OZuObFvN2qmSt9r8MQYqOYXFmmnpN0XIvkCsX7ErsCfNGHhEzavdJHqel6 y2y5c/CnkLzMnf3jaV+VGWJVDl1+9njPPMX3n9v7uJhCE4zGMWk2ol356JOCb9cj FHKGWJv2trU= =wM9k -----END PGP SIGNATURE----- --=-=-=--