From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id VqnfJ4X44V7edQAA0tVLHw (envelope-from ) for ; Thu, 11 Jun 2020 09:25:25 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id iCskI4X44V6XUgAAB5/wlQ (envelope-from ) for ; Thu, 11 Jun 2020 09:25:25 +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 267D49403EC for ; Thu, 11 Jun 2020 09:25:25 +0000 (UTC) Received: from localhost ([::1]:48146 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jjJSe-0003HP-4O for larch@yhetil.org; Thu, 11 Jun 2020 05:25:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56428) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jjJSJ-000313-0K for guix-patches@gnu.org; Thu, 11 Jun 2020 05:25:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:52725) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jjJSI-0003A8-L2 for guix-patches@gnu.org; Thu, 11 Jun 2020 05:25:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jjJSI-0008Al-Fe for guix-patches@gnu.org; Thu, 11 Jun 2020 05:25:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#41767] [PATCH 4/9] channels: 'latest-channel-instance' authenticates Git checkouts. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 11 Jun 2020 09:25:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 41767 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Maxim Cournoyer Cc: 41767@debbugs.gnu.org Received: via spool by 41767-submit@debbugs.gnu.org id=B41767.159186747331372 (code B ref 41767); Thu, 11 Jun 2020 09:25:02 +0000 Received: (at 41767) by debbugs.gnu.org; 11 Jun 2020 09:24:33 +0000 Received: from localhost ([127.0.0.1]:36038 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jjJRp-00089w-Dg for submit@debbugs.gnu.org; Thu, 11 Jun 2020 05:24:33 -0400 Received: from eggs.gnu.org ([209.51.188.92]:47952) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jjJRn-00089k-37 for 41767@debbugs.gnu.org; Thu, 11 Jun 2020 05:24:31 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:58713) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jjJRh-0002pJ-RZ; Thu, 11 Jun 2020 05:24:25 -0400 Received: from [2a01:e0a:1d:7270:6a6c:dc17:fc02:cfda] (port=57034 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1jjJRh-0004J2-Ey; Thu, 11 Jun 2020 05:24:25 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20200608220256.3267-1-ludo@gnu.org> <20200608220256.3267-4-ludo@gnu.org> <87v9k0i0yj.fsf@gmail.com> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 24 Prairial an 228 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, 11 Jun 2020 11:24:23 +0200 In-Reply-To: <87v9k0i0yj.fsf@gmail.com> (Maxim Cournoyer's message of "Tue, 09 Jun 2020 13:49:24 -0400") Message-ID: <87sgf2araw.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -3.3 (---) 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" X-Scanner: scn0 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-Spam-Score: -1.01 X-TUID: TBKXDHpDcy1f --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Maxim, Maxim Cournoyer skribis: >> +(define %guix-channel-introduction >> + ;; Introduction of the official 'guix channel. The chosen commit is = the >> + ;; first one that introduces '.guix-authorizations' on the 'core-upda= tes' >> + ;; branch that was eventually merged in 'master'. Any branch starting >> + ;; before that commit cannot be merged or it will be rejected by 'gui= x pull' >> + ;; & co. >> + (make-channel-introduction >> + "87a40d7203a813921b3ef0805c2b46c0026d6c31" >> + (base16-string->bytevector >> + (string-downcase >> + (string-filter char-set:hex-digit ;mbakke >> + "BBB0 2DDF 2CEA F6A8 0D1D E643 A2A0 6DF2 A33A 54FA= "))) >> + #f)) ;TODO: Add an intro signature so it can be ex= ported. > > The GnuPG key fingerprint is SHA1 derived, which isn't cryptographically > secure. This doesn't mean fingerprints are unsafe *now* (given that > forging a key to match it isn't currently practical), Fingerprints are used as an index in the keyring here. If somebody introduced a second OpenPGP key with the same fingerprint in the keyring and we picked the wrong one when verifying a signature, signature verification would just fail. So I think it=E2=80=99s perfectly OK here. > but I don't think we should create something *today* that relies on > SHA1 for trust. My point is made moot by the fact that Git uses SHA1 > too... but that's another issue. Just saying, but not blocking or > requesting change, as I don't have a good solution for that, short of > patching GnuPG and Git. Right, we=E2=80=99re following the OpenPGP standard (which I think is fine = in this respect, at least for this use case) and Git (which is less fine). Git commit signatures are over computed over the raw commit, something like: --8<---------------cut here---------------start------------->8--- tree 6e3ed6ad90013f8cb685556b0669c8a12f24bd09 parent 394566f7f4a99748f59b86da5bc551903ece5052 author Ludovic Court=C3=A8s 1591626893 +0200 committer Ludovic Court=C3=A8s 1591648112 +0200 tests: Move OpenPGP helpers to (guix tests gnupg). * tests/git-authenticate.scm (key-id): Remove. (%ed25519-public-key-file, %ed25519-secret-key-file) (%ed25519bis-public-key-file, %ed25519bis-secret-key-file) (read-openpgp-packet, key-fingerprint): Move to... * guix/tests/gnupg.scm: ... here. --8<---------------cut here---------------end--------------->8--- One could forge a tree object with the same SHA1: signature verification would still pass, but the checkout could be very different. The =E2=80=9CSHA-1 is a Shambles=E2=80=9D paper reads: The GIT developers have been working on replacing SHA-1 for a while[16], and they use a collision detection library [SS17] to mitigate the risks of collision attacks. [16] https://git-scm.com/docs/hash-function-transition/ On the Fediverse, someone pointed out that Bitcoin Core developers compute a SHA512 hash of Git trees to mitigate it: https://github.com/bitcoin/bitcoin/tree/master/contrib/verify-commits What they do is add a =E2=80=9CTree-SHA512:=E2=80=9D line to commit logs an= d check those in =E2=80=98verify-commits.py=E2=80=99: --8<---------------cut here---------------start------------->8--- # Check the Tree-SHA512 if (verify_tree or prev_commit =3D=3D "") and current_commit not in incorre= ct_sha512_allowed: tree_hash =3D tree_sha512sum(current_commit) if ("Tree-SHA512: {}".format(tree_hash)) not in subprocess.check_output= ([GIT, 'show', '-s', '--format=3Dformat:%B', current_commit]).decode('utf8'= ).splitlines(): print("Tree-SHA512 did not match for commit " + current_commit, fil= e=3Dsys.stderr) sys.exit(1) --8<---------------cut here---------------end--------------->8--- We could do something similar, maybe optionally, but verification would be expensive (you need to traverse all the blobs of the whole tree for each commit being authenticated). At this point, I=E2=80=99d wait for Git=E2=80=99s SHA256 migration to happe= n, though doesn=E2=80=99t specif= y an ETA. >> + ;; If it's our first time, verify CHANNEL's introductory comm= it. >> + (when (null? authenticated-commits) >> + (verify-introductory-commit repository >> + (channel-introduction channel) >> + keyring)) >> + >> + (call-with-progress-reporter reporter >> + (lambda (report) >> + (authenticate-commits repository commits >> + #:keyring keyring >> + #:report-progress report))) >> + >> + (unless (null? commits) > > That condition is already checked above, but OK to be defensive. Oh that=E2=80=99s a leftover, I=E2=80=99ve removed it for clarity. >> + (when (guix-channel? channel) >> + (warning (G_ "the code of channel '~a' cannot be authenticate= d~%") >> + (channel-name channel)))) >> + > > Perhaps the warning message could say why. Good point. Below are changes I made locally to account for your feedback. Thank you! Ludo=E2=80=99. --=-=-= Content-Type: text/x-patch Content-Disposition: inline diff --git a/guix/channels.scm b/guix/channels.scm index c2ea0e26ff..036c8d9b5d 100644 --- a/guix/channels.scm +++ b/guix/channels.scm @@ -369,10 +369,9 @@ commits ~a to ~a (~h new commits)...~%") #:keyring keyring #:report-progress report))) - (unless (null? commits) - (cache-authenticated-commit cache-key - (oid->string - (commit-id end-commit)))))))) + (cache-authenticated-commit cache-key + (oid->string + (commit-id end-commit))))))) (define* (latest-channel-instance store channel #:key (patches %patches) @@ -392,7 +391,8 @@ relation to STARTING-COMMIT when provided." ;; TODO: Warn for all the channels once the authentication interface ;; is public. (when (guix-channel? channel) - (warning (G_ "the code of channel '~a' cannot be authenticated~%") + (warning (G_ "channel '~a' lacks an introduction and \ +cannot be authenticated~%") (channel-name channel)))) (when (guix-channel? channel) --=-=-=--