GNU bug report logs - #41787
[PATCH] Disallow SHA1 signatures on commits

Previous Next

Package: guix-patches;

Reported by: Ludovic Courtès <ludo <at> gnu.org>

Date: Wed, 10 Jun 2020 13:17:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41787 in the body.
You can then email your comments to 41787 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#41787; Package guix-patches. (Wed, 10 Jun 2020 13:17:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ludovic Courtès <ludo <at> gnu.org>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Wed, 10 Jun 2020 13:17:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Subject: [PATCH] Disallow SHA1 signatures on commits
Date: Wed, 10 Jun 2020 15:16:44 +0200
[Message part 1 (text/plain, inline)]
Hello Guix!

The attached patch disallows SHA1 signatures on commits, as recommended
in:

  https://sha-mbles.github.io/

As explained there, SHA1 is no longer the default for signatures since
the release of GnuPG 2.0 in 2006, but there are still users of GnuPG 1.x.

In our repository, there are 132 SHA1 signatures since ‘v1.0.0’ (last
one in April 2020) by three fellow hackers who have since updated their
config along the lines of item #2 at:

  https://guix.gnu.org/manual/devel/en/html_node/Commit-Access.html

With this patch, any commit with a SHA1 signature made after
c91e27c60864faa229198f6f0caf620275c429a2 (May 1st), which introduces
‘.guix-authorizations’, is rejected (it is not a timestamp-based check
because timestamps can always be forged).  If one of us makes a mistake,
we’ll have to hard-reset prior to the faulty commit.

For the record, this was previously discussed at:

  https://issues.guix.gnu.org/issue/22883#62

If you have any questions, please let me know.  Feedback welcome!

Ludo’.

[0001-git-authenticate-Disallow-SHA1-and-MD5-signatures.patch (text/x-patch, inline)]
From e902fdf083627d548541d6cc53643df4071616c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo <at> gnu.org>
Date: Wed, 10 Jun 2020 14:54:13 +0200
Subject: [PATCH] git-authenticate: Disallow SHA1 (and MD5) signatures.

* guix/git-authenticate.scm (commit-signing-key): Add
 #:disallowed-hash-algorithms and honor it.
(authenticate-commit)[recent-commit?]: New variable.
Pass #:disallowed-hash-algorithms to 'commit-signing-key'.
* tests/git-authenticate.scm ("signed commits, SHA1 signature"): New test.
---
 guix/git-authenticate.scm  | 29 ++++++++++++++++++++++++++---
 tests/git-authenticate.scm | 29 +++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/guix/git-authenticate.scm b/guix/git-authenticate.scm
index c333717136..0d6f696a0b 100644
--- a/guix/git-authenticate.scm
+++ b/guix/git-authenticate.scm
@@ -85,9 +85,11 @@
   (signature missing-key-error-signature))
 
 
-(define (commit-signing-key repo commit-id keyring)
+(define* (commit-signing-key repo commit-id keyring
+                             #:key (disallowed-hash-algorithms '(sha1)))
   "Return the OpenPGP key that signed COMMIT-ID (an OID).  Raise an exception
-if the commit is unsigned, has an invalid signature, or if its signing key is
+if the commit is unsigned, has an invalid signature, has a signature using one
+of the hash algorithms in DISALLOWED-HASH-ALGORITHMS, or if its signing key is
 not in KEYRING."
   (let-values (((signature signed-data)
                 (catch 'git-error
@@ -103,6 +105,17 @@ not in KEYRING."
                                 (oid->string commit-id)))))))
 
     (let ((signature (string->openpgp-packet signature)))
+      (when (memq (openpgp-signature-hash-algorithm signature)
+                  `(,@disallowed-hash-algorithms md5))
+        (raise (condition
+                (&unsigned-commit-error (commit commit-id))
+                (&message
+                 (message (format #f (G_ "commit ~a has a ~a signature, \
+which is not permitted")
+                                  (oid->string commit-id)
+                                  (openpgp-signature-hash-algorithm
+                                   signature)))))))
+
       (with-fluids ((%default-port-encoding "UTF-8"))
         (let-values (((status data)
                       (verify-openpgp-signature signature keyring
@@ -198,8 +211,18 @@ not specify anything, fall back to DEFAULT-AUTHORIZATIONS."
   (define id
     (commit-id commit))
 
+  (define recent-commit?
+    (false-if-git-not-found
+     (tree-entry-bypath (commit-tree commit) ".guix-authorizations")))
+
   (define signing-key
-    (commit-signing-key repository id keyring))
+    (commit-signing-key repository id keyring
+                        ;; Reject SHA1 signatures unconditionally as suggested
+                        ;; by the authors of "SHA-1 is a Shambles" (2019).
+                        ;; Accept it for "historical" commits (there are such
+                        ;; signatures from April 2020 in the repository).
+                        #:disallowed-hash-algorithms
+                        (if recent-commit? '(sha1) '())))
 
   (unless (member (openpgp-public-key-fingerprint signing-key)
                   (commit-authorized-keys repository commit
diff --git a/tests/git-authenticate.scm b/tests/git-authenticate.scm
index 84689d628e..97990acaea 100644
--- a/tests/git-authenticate.scm
+++ b/tests/git-authenticate.scm
@@ -81,6 +81,35 @@
                                 #:keyring-reference "master")
           'failed)))))
 
+(unless (which (git-command)) (test-skip 1))
+(test-assert "signed commits, SHA1 signature"
+  (with-fresh-gnupg-setup (list %ed25519-public-key-file
+                                %ed25519-secret-key-file)
+    ;; Force use of SHA1 for signatures.
+    (call-with-output-file (string-append (getenv "GNUPGHOME") "/gpg.conf")
+      (lambda (port)
+        (display "digest-algo sha1" port)))
+
+    (with-temporary-git-repository directory
+        `((add "a.txt" "A")
+          (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 "first commit"
+                  (signer ,(key-fingerprint %ed25519-public-key-file))))
+      (with-repository directory repository
+        (let ((commit (find-commit repository "first")))
+          (guard (c ((unsigned-commit-error? c)
+                     (oid=? (git-authentication-error-commit c)
+                            (commit-id commit))))
+            (authenticate-commits repository (list commit)
+                                  #:keyring-reference "master")
+            'failed))))))
+
 (unless (gpg+git-available?) (test-skip 1))
 (test-assert "signed commits, default authorizations"
   (with-fresh-gnupg-setup (list %ed25519-public-key-file
-- 
2.26.2


Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Fri, 12 Jun 2020 16:58:02 GMT) Full text and rfc822 format available.

Notification sent to Ludovic Courtès <ludo <at> gnu.org>:
bug acknowledged by developer. (Fri, 12 Jun 2020 16:58:02 GMT) Full text and rfc822 format available.

Message #10 received at 41787-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 41787-done <at> debbugs.gnu.org
Subject: Re: [bug#41787] [PATCH] Disallow SHA1 signatures on commits
Date: Fri, 12 Jun 2020 18:57:44 +0200
Ludovic Courtès <ludo <at> gnu.org> skribis:

>>From e902fdf083627d548541d6cc53643df4071616c7 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo <at> gnu.org>
> Date: Wed, 10 Jun 2020 14:54:13 +0200
> Subject: [PATCH] git-authenticate: Disallow SHA1 (and MD5) signatures.
>
> * guix/git-authenticate.scm (commit-signing-key): Add
>  #:disallowed-hash-algorithms and honor it.
> (authenticate-commit)[recent-commit?]: New variable.
> Pass #:disallowed-hash-algorithms to 'commit-signing-key'.
> * tests/git-authenticate.scm ("signed commits, SHA1 signature"): New test.

Pushed as 52c529ff20b389eb64ac033586e6b1a5c5d82cb5.

Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 11 Jul 2020 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 261 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.