GNU bug report logs

#53608 [PATCH 0/2] Rejecting commits unrelated to the introductory commit

PackageSource(s)Maintainer(s)
guix-patches PTS Buildd Popcon
Full log

Message #11 received at 53608@debbugs.gnu.org (full text, mbox, reply):

Received: (at 53608) by debbugs.gnu.org; 28 Jan 2022 17:43:33 +0000
From debbugs-submit-bounces@debbugs.gnu.org Fri Jan 28 12:43:33 2022
Received: from localhost ([127.0.0.1]:60427 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
	id 1nDVHY-0001Bg-Hb
	for submit@debbugs.gnu.org; Fri, 28 Jan 2022 12:43:33 -0500
Received: from eggs.gnu.org ([209.51.188.92]:33928)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <ludo@gnu.org>) id 1nDVHS-0001BA-Bs
 for 53608@debbugs.gnu.org; Fri, 28 Jan 2022 12:43:27 -0500
Received: from [2001:470:142:3::e] (port=34004 helo=fencepost.gnu.org)
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <ludo@gnu.org>)
 id 1nDVHM-0001Xk-0T; Fri, 28 Jan 2022 12:43:20 -0500
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=MIME-Version:References:In-Reply-To:Date:Subject:To:
 From; bh=RNdMfb0GmJ4tGSFrJ1vahpq30SOG+m3iL/btg9vJfhA=; b=nyvfO2p7G9sQnldRmwUi
 inWqHJIiqXQ+283oShWhBheYVTI89PgPCuIzQeKNk6IKbw9uYK8wRMyOUCaLXZl+4njETf4ZMdW2H
 PrFitQKraDq95QMN9g7CTZxbSgRBPpIZQl5vg/elmhApPNKywqNAadKdhCk+dmmDara/Yr5pTkU3R
 vYnUS35bo6BV6oW5qmuTKIxjV/QevEPif/0dASFs7I8eNtmH7mn0pBD2J5O44ULeVTDh6roXJelxf
 FTSNhVOFKXqfbyPJZ9hQIch+0uJ8jAWvbhDIwZgd/gDfetU/O4oenUiWdBS5S0b4q55sGsL4gH3oD
 m5csb9MBXldkIg==;
Received: from [2001:660:6102:320:e120:2c8f:8909:cdfe] (port=33810
 helo=gnu.org)
 by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)
 (Exim 4.90_1) (envelope-from <ludo@gnu.org>)
 id 1nDVHI-0002VO-7J; Fri, 28 Jan 2022 12:43:17 -0500
From: Ludovic Courtès <ludo@gnu.org>
To: 53608@debbugs.gnu.org
Subject: [PATCH 2/2] git-authenticate: Ensure the target is a descendant of
 the introductory commit.
Date: Fri, 28 Jan 2022 18:43:01 +0100
Message-Id: <20220128174301.7632-2-ludo@gnu.org>
X-Mailer: git-send-email 2.34.0
In-Reply-To: <20220128174301.7632-1-ludo@gnu.org>
References: <20220128174301.7632-1-ludo@gnu.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 53608
Cc: Ludovic Courtès <ludo@gnu.org>
X-BeenThere: debbugs-submit@debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request@debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit@debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request@debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request@debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces@debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
X-Spam-Score: -3.3 (---)
Fixes a bug whereby authentication of a commit *not* descending from the
introductory commit could succeed, provided the commit verifies the
authorization invariant.

In the example below, A is a common ancestor of the introductory commit
I and of commit X.  Authentication of X would succeed, even though it is
not a descendant of I, as long as X is authorized according to the
'.guix-authorizations' in A:

   X   	 I
    \   /
      A

This is because, 'authenticate-repository' would not check whether X
descends from I, and the call (commit-difference X I) would return X.

In practice that only affects forks because it means that ancestors of
the introductory commit already contain a '.guix-authorizations' file.

* guix/git-authenticate.scm (authenticate-repository): Add call to
'commit-descendant?'.
* tests/channels.scm ("authenticate-channel, not a descendant of introductory commit"):
New test.
* tests/git-authenticate.scm ("authenticate-repository, target not a descendant of intro"):
New test.
* tests/guix-git-authenticate.sh: Expect earlier test to fail since
9549f0283a78fe36f2d4ff2a04ef8ad6b0c02604 is not a descendant of
$intro_commit.  Add new test targeting an ancestor of the introductory
commit, and another test targeting the v1.2.0 commit.
* doc/guix.texi (Specifying Channel Authorizations): Add a sentence.
---
 doc/guix.texi                  |  4 ++-
 guix/git-authenticate.scm      | 17 ++++++++--
 tests/channels.scm             | 60 +++++++++++++++++++++++++++++++++-
 tests/git-authenticate.scm     | 44 +++++++++++++++++++++++++
 tests/guix-git-authenticate.sh | 17 ++++++++--
 5 files changed, 136 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 62e994ceb1..61f2d7a771 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -5448,7 +5448,9 @@ commit of a channel that should be authenticated.  The first time a
 channel is fetched with @command{guix pull} or @command{guix
 time-machine}, the command looks up the introductory commit and verifies
 that it is signed by the specified OpenPGP key.  From then on, it
-authenticates commits according to the rule above.
+authenticates commits according to the rule above.  Authentication fails
+if the target commit is neither a descendant nor an ancestor of the
+introductory commit.
 
 Additionally, your channel must provide all the OpenPGP keys that were
 ever mentioned in @file{.guix-authorizations}, stored as @file{.key}
diff --git a/guix/git-authenticate.scm b/guix/git-authenticate.scm
index ab3fcd8b2f..419cb85afc 100644
--- a/guix/git-authenticate.scm
+++ b/guix/git-authenticate.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2019, 2020, 2021, 2022 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,7 +22,9 @@ (define-module (guix git-authenticate)
   #:use-module (guix base16)
   #:autoload   (guix base64) (base64-encode)
   #:use-module ((guix git)
-                #:select (commit-difference false-if-git-not-found))
+                #:select (commit-difference
+                          commit-descendant?
+                          false-if-git-not-found))
   #:use-module (guix i18n)
   #:use-module ((guix diagnostics) #:select (formatted-message))
   #:use-module (guix openpgp)
@@ -426,6 +428,17 @@ (define commits
           (verify-introductory-commit repository keyring
                                       start-commit signer))
 
+        ;; Make sure END-COMMIT is a descendant of START-COMMIT or of one of
+        ;; AUTHENTICATED-COMMITS, which are known to be descendants of
+        ;; START-COMMIT.
+        (unless (commit-descendant? end-commit
+                                    (cons start-commit
+                                          authenticated-commits))
+          (raise (formatted-message
+                  (G_ "commit ~a is not a descendant of introductory commit ~a")
+                  (oid->string (commit-id end-commit))
+                  (oid->string (commit-id start-commit)))))
+
         (let ((stats (call-with-progress-reporter reporter
                        (lambda (report)
                          (authenticate-commits repository commits
diff --git a/tests/channels.scm b/tests/channels.scm
index d45c450241..0fe870dbaf 100644
--- a/tests/channels.scm
+++ b/tests/channels.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
-;;; Copyright © 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2019, 2020, 2022 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -525,6 +525,64 @@ (define (find-commit* message)
                                   #:keyring-reference-prefix "")
             'failed))))))
 
+(unless (gpg+git-available?) (test-skip 1))
+(test-equal "authenticate-channel, not a descendant of introductory commit"
+  #t
+  (with-fresh-gnupg-setup (list %ed25519-public-key-file
+                                %ed25519-secret-key-file
+                                %ed25519-2-public-key-file
+                                %ed25519-2-secret-key-file)
+    (with-temporary-git-repository directory
+        `((add ".guix-channel"
+               ,(object->string
+                 '(channel (version 0)
+                           (keyring-reference "master"))))
+          (add ".guix-authorizations"
+               ,(object->string
+                 `(authorizations (version 0)
+                                  ((,(key-fingerprint
+                                      %ed25519-public-key-file)
+                                    (name "Charlie"))))))
+          (add "signer.key" ,(call-with-input-file %ed25519-public-key-file
+                               get-string-all))
+          (commit "first commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file)))
+          (branch "alternate-branch")
+          (checkout "alternate-branch")
+          (add "something.txt" ,(random-text))
+          (commit "intro commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file)))
+          (checkout "master")
+          (add "random" ,(random-text))
+          (commit "second commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file))))
+      (with-repository directory repository
+        (let* ((commit1 (find-commit repository "first"))
+               (commit2 (find-commit repository "second"))
+               (commit0 (commit-lookup
+                         repository
+                         (reference-target
+                          (branch-lookup repository "alternate-branch"))))
+               (intro   (make-channel-introduction
+                         (commit-id-string commit0)
+                         (openpgp-public-key-fingerprint
+                          (read-openpgp-packet
+                           %ed25519-public-key-file))))
+               (channel (channel (name 'example)
+                                 (url (string-append "file://" directory))
+                                 (introduction intro))))
+          (guard (c ((formatted-message? c)
+                     (and (string-contains (formatted-message-string c)
+                                           "not a descendant")
+                          (equal? (formatted-message-arguments c)
+                                  (list
+                                   (oid->string (commit-id commit2))
+                                   (oid->string (commit-id commit0)))))))
+            (authenticate-channel channel directory
+                                  (commit-id-string commit2)
+                                  #:keyring-reference-prefix "")
+            'failed))))))
+
 (unless (gpg+git-available?) (test-skip 1))
 (test-equal "authenticate-channel, .guix-authorizations"
   #t
diff --git a/tests/git-authenticate.scm b/tests/git-authenticate.scm
index 6ec55fb2e5..c063920c12 100644
--- a/tests/git-authenticate.scm
+++ b/tests/git-authenticate.scm
@@ -431,4 +431,48 @@ (define (correct? c commit)
                                       #:keyring-reference "master"
                                       #:cache-key (random-text)))))))))
 
+(unless (gpg+git-available?) (test-skip 1))
+(test-equal "authenticate-repository, target not a descendant of intro"
+  'target-commit-not-a-descendant-of-intro
+  (with-fresh-gnupg-setup (list %ed25519-public-key-file
+                                %ed25519-secret-key-file)
+    (let ((fingerprint (key-fingerprint %ed25519-public-key-file)))
+      (with-temporary-git-repository directory
+          `((add "signer.key" ,(call-with-input-file %ed25519-public-key-file
+                                 get-string-all))
+            (add ".guix-authorizations"
+                 ,(object->string
+                   `(authorizations (version 0)
+                                    ((,(key-fingerprint
+                                        %ed25519-public-key-file)
+                                      (name "Charlie"))))))
+            (commit "zeroth commit" (signer ,fingerprint))
+            (branch "pre-intro-branch")
+            (checkout "pre-intro-branch")
+            (add "b.txt" "B")
+            (commit "alternate commit" (signer ,fingerprint))
+            (checkout "master")
+            (add "a.txt" "A")
+            (commit "first commit" (signer ,fingerprint))
+            (add "c.txt" "C")
+            (commit "second commit" (signer ,fingerprint)))
+        (with-repository directory repository
+          (let ((commit1 (find-commit repository "first"))
+                (commit-alt
+                 (commit-lookup repository
+                                (reference-target
+                                 (branch-lookup repository
+                                                "pre-intro-branch")))))
+            (guard (c ((formatted-message? c)
+                       (and (equal? (formatted-message-arguments c)
+                                    (list (oid->string (commit-id commit-alt))
+                                          (oid->string (commit-id commit1))))
+                            'target-commit-not-a-descendant-of-intro)))
+              (authenticate-repository repository
+                                       (commit-id commit1)
+                                       (openpgp-fingerprint fingerprint)
+                                       #:end (commit-id commit-alt)
+                                       #:keyring-reference "master"
+                                       #:cache-key (random-text)))))))))
+
 (test-end "git-authenticate")
diff --git a/tests/guix-git-authenticate.sh b/tests/guix-git-authenticate.sh
index 8ebbea398b..2b90d8a4af 100644
--- a/tests/guix-git-authenticate.sh
+++ b/tests/guix-git-authenticate.sh
@@ -1,5 +1,5 @@
 # GNU Guix --- Functional package management for GNU
-# Copyright © 2020 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2020, 2022 Ludovic Courtès <ludo@gnu.org>
 #
 # This file is part of GNU Guix.
 #
@@ -34,10 +34,18 @@ intro_signer="BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA"
 
 cache_key="test-$$"
 
-guix git authenticate "$intro_commit" "$intro_signer"	\
+# This must fail because the end commit is not a descendant of $intro_commit.
+! guix git authenticate "$intro_commit" "$intro_signer"	\
      --cache-key="$cache_key" --stats			\
      --end=9549f0283a78fe36f2d4ff2a04ef8ad6b0c02604
 
+# The v1.2.0 commit is a descendant of $intro_commit and it satisfies the
+# authorization invariant.
+v1_2_0_commit="a099685659b4bfa6b3218f84953cbb7ff9e88063"
+guix git authenticate "$intro_commit" "$intro_signer"	\
+     --cache-key="$cache_key" --stats			\
+     --end="$v1_2_0_commit"
+
 rm "$XDG_CACHE_HOME/guix/authentication/$cache_key"
 
 # Commit and signer of the 'v1.0.0' tag.
@@ -45,6 +53,11 @@ v1_0_0_commit="6298c3ffd9654d3231a6f25390b056483e8f407c"
 v1_0_0_signer="3CE4 6455 8A84 FDC6 9DB4  0CFB 090B 1199 3D9A EBB5" # civodul
 v1_0_1_commit="d68de958b60426798ed62797ff7c96c327a672ac"
 
+# This should succeed because v1.0.0 is an ancestor of $intro_commit.
+guix git authenticate "$intro_commit" "$intro_signer"	\
+     --cache-key="$cache_key" --stats			\
+     --end="$v1_0_0_commit"
+
 # This should fail because these commits lack '.guix-authorizations'.
 ! guix git authenticate "$v1_0_0_commit" "$v1_0_0_signer" \
        --cache-key="$cache_key" --end="$v1_0_1_commit"
-- 
2.34.0





Send a report that this bug log contains spam.


debbugs.gnu.org maintainers <help-debbugs@gnu.org>. Last modified: Sun Dec 22 01:47:05 2024; Machine Name: wallace-server

GNU bug tracking system

Debbugs is free software and licensed under the terms of the GNU Public License version 2. The current version can be obtained from https://bugs.debian.org/debbugs-source/.

Copyright © 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson, 2005-2017 Don Armstrong, and many other contributors.