[bug#41653] [PATCH 0/4] Add (guix git-authenticate) with tests

[Ludovic Courtès]

Ludovic Courtès <ludo@gnu.org> skribis:

>   Add (guix git-authenticate).
>   git-authenticate: Don't hard-code "origin/" for keyring reference.
>   git-authenticate: Raise proper SRFI-35 conditions.
>   git-authenticate: Add tests.

As a followup, I pushed this patch:

  
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=e78275608065ef073775fabb9f1a757da65851f2

Its effect is to prevent removal of ‘.guix-authorizations’ since doing
that would trivially force the authentication code to fall back to
‘default-authorizations’.

Ludo’.

commit e78275608065ef073775fabb9f1a757da65851f2
Author: Ludovic Courtès <ludo@gnu.org>
Date:   Sun Jun 7 23:06:41 2020 +0200

    git-authenticate: Prevent removal of '.guix-authorizations'.
    
    * guix/git-authenticate.scm (commit-authorized-keys)
    [parents-have-authorizations-file?, assert-parents-lack-authorizations]:
    New procedures.
    Use the latter before returning DEFAULT-AUTHORIZATIONS.
    * guix/git.scm (false-if-git-not-found): Export.
    * guix/tests/git.scm (populate-git-repository): Add 'remove' clause.
    * tests/git-authenticate.scm ("signed commits, .guix-authorizations 
removed"):
    New test.

diff --git a/guix/git-authenticate.scm b/guix/git-authenticate.scm
index b73f957105..00d22ef479 100644
--- a/guix/git-authenticate.scm
+++ b/guix/git-authenticate.scm
@@ -19,6 +19,7 @@
 (define-module (guix git-authenticate)
   #:use-module (git)
   #:use-module (guix base16)
+  #:use-module ((guix git) #:select (false-if-git-not-found))
   #:use-module (guix i18n)
   #:use-module (guix openpgp)
   #:use-module ((guix utils)
@@ -145,6 +146,27 @@ return a list of authorized fingerprints."
   "Return the list of OpenPGP fingerprints authorized to sign COMMIT, based on
 authorizations listed in its parent commits.  If one of the parent commits
 does not specify anything, fall back to DEFAULT-AUTHORIZATIONS."
+  (define (parents-have-authorizations-file? commit)
+    ;; Return true if at least one of the parents of COMMIT has the
+    ;; '.guix-authorizations' file.
+    (find (lambda (commit)
+            (false-if-git-not-found
+             (tree-entry-bypath (commit-tree commit)
+                                ".guix-authorizations")))
+          (commit-parents commit)))
+
+  (define (assert-parents-lack-authorizations commit)
+    ;; If COMMIT removes the '.guix-authorizations' file found in one of its
+    ;; parents, raise an error.
+    (when (parents-have-authorizations-file? commit)
+      (raise (condition
+              (&unauthorized-commit-error (commit (commit-id commit))
+                                          (signing-key #f))
+              (&message
+               (message (format #f (G_ "commit ~a attempts \
+to remove '.guix-authorizations' file")
+                                (oid->string (commit-id commit)))))))))
+
   (define (commit-authorizations commit)
     (catch 'git-error
       (lambda ()
@@ -155,7 +177,11 @@ does not specify anything, fall back to 
DEFAULT-AUTHORIZATIONS."
            (open-bytevector-input-port (blob-content blob)))))
       (lambda (key error)
         (if (= (git-error-code error) GIT_ENOTFOUND)
-            default-authorizations
+            (begin
+              ;; Prevent removal of '.guix-authorizations' since it would make
+              ;; it trivial to force a fallback to DEFAULT-AUTHORIZATIONS.
+              (assert-parents-lack-authorizations commit)
+              default-authorizations)
             (throw key error)))))
 
   (apply lset-intersection bytevector=?
diff --git a/guix/git.scm b/guix/git.scm
index 1c45afa050..1671f57d9f 100644
--- a/guix/git.scm
+++ b/guix/git.scm
@@ -39,6 +39,7 @@
             honor-system-x509-certificates!
 
             with-repository
+            false-if-git-not-found
             update-cached-checkout
             url+commit->name
             latest-repository-commit
diff --git a/guix/tests/git.scm b/guix/tests/git.scm
index 5d7056bb53..b8e5f7e643 100644
--- a/guix/tests/git.scm
+++ b/guix/tests/git.scm
@@ -76,6 +76,9 @@ Return DIRECTORY on success."
                       port)))
          (git "add" file)
          (loop rest)))
+      ((('remove file) rest ...)
+       (git "rm" "-f" file)
+       (loop rest))
       ((('commit text) rest ...)
        (git "commit" "-m" text)
        (loop rest))
diff --git a/tests/git-authenticate.scm b/tests/git-authenticate.scm
index 5937c37ee6..84689d628e 100644
--- a/tests/git-authenticate.scm
+++ b/tests/git-authenticate.scm
@@ -282,5 +282,46 @@
                                       merge master3)
                                 #:keyring-reference "master"))))))
 
+(unless (gpg+git-available?) (test-skip 1))
+(test-assert "signed commits, .guix-authorizations removed"
+  (with-fresh-gnupg-setup (list %ed25519-public-key-file
+                                %ed25519-secret-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")
+          (add "a.txt" "A")
+          (commit "first commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file)))
+          (remove ".guix-authorizations")
+          (commit "second commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file)))
+          (add "b.txt" "B")
+          (commit "third commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file))))
+      (with-repository directory repository
+        (let ((commit1 (find-commit repository "first"))
+              (commit2 (find-commit repository "second"))
+              (commit3 (find-commit repository "third")))
+          ;; COMMIT1 and COMMIT2 are fine.
+          (and (authenticate-commits repository (list commit1 commit2)
+                                     #:keyring-reference "master")
+
+               ;; COMMIT3 is rejected because COMMIT2 removes
+               ;; '.guix-authorizations'.
+               (guard (c ((unauthorized-commit-error? c)
+                          (oid=? (git-authentication-error-commit c)
+                                 (commit-id commit2))))
+                 (authenticate-commits repository
+                                       (list commit1 commit2 commit3)
+                                       #:keyring-reference "master")
+                 'failed)))))))
+
 (test-end "git-authenticate")