Revert 7deba93bdc76616011a9f493cbc203d60084416f and fix expired-key-sigs properly #12822

TheBlueMatt commented at 6:57 PM on March 28, 2018: member

7deba93bdc76616011a9f493cbc203d60084416f was took the wrong approach to updating verify-commits for a key expiration. Namely, adding each commit to allow-revsig-commits should have been done instead, allowing them to still be validated, but with the expired key.

Revert "test: Update trust git root".

This reverts commit 7deba93bdc76616011a9f493cbc203d60084416f.

This is neither a "test" change, nor should the trusted-git-root
have been updated - there is a process for expired PGP keys.

99f6d48e23

Add Marco-expired-key-signed-commits to allow-revsig-commits de7e93138a

meshcollider requested review from laanwj on Mar 28, 2018

fanquake added the label Scripts and tools on Mar 29, 2018

laanwj commented at 12:43 PM on March 29, 2018: member

ACK 9471576

[verify-commits] Add some additional useful documentation. 94715767a3

TheBlueMatt commented at 2:33 PM on March 29, 2018: member

Added some documentation as requested, should be more clear in the future, I believe.

in contrib/verify-commits/README.md:43 in 94715767a3

  38 | +
  39 | +When a key (or subkey) which has signed old commits expires or is revoked,
  40 | +verify-commits will start failing to verify all commits which were signed by
  41 | +said key. In order to avoid bumping the root-of-trust `trusted-git-root`
  42 | +file, individual commits which were signed by such a key can be added to the
  43 | +`allow-revsig-commits` file. That way, the PGP signatures are still verified

laanwj commented at 2:43 PM on March 29, 2018:

q: Is there a command to get a list of commits signed by a certain key? If so makes sense to mention that here.

TheBlueMatt commented at 2:45 PM on March 29, 2018:

Not currently, I generated the diff here with this patch:

diff --git a/contrib/verify-commits/verify-commits.sh b/contrib/verify-commits/verify-commits.sh
index 532b97a438..eb41d90941 100755
--- a/contrib/verify-commits/verify-commits.sh
+++ b/contrib/verify-commits/verify-commits.sh
@@ -62,17 +62,22 @@ while true; do
        fi
 
        if ! git -c "gpg.program=${DIR}/gpg.sh" verify-commit "$CURRENT_COMMIT" > /dev/null; then
-               if [ "$PREV_COMMIT" != "" ]; then
-                       echo "No parent of $PREV_COMMIT was signed with a trusted key!" > /dev/stderr
-                       echo "Parents are:" > /dev/stderr
-                       PARENTS=$(git show -s --format=format:%P $PREV_COMMIT)
-                       for PARENT in $PARENTS; do
-                               git show -s $PARENT > /dev/stderr
-                       done
+               export BITCOIN_VERIFY_COMMITS_ALLOW_REVSIG=1
+               if git -c "gpg.program=${DIR}/gpg.sh" verify-commit "$CURRENT_COMMIT" > /dev/null; then
+                       echo "$CURRENT_COMMIT" > /dev/stderr
                else
-                       echo "$CURRENT_COMMIT was not signed with a trusted key!" > /dev/stderr
+                       if [ "$PREV_COMMIT" != "" ]; then
+                               echo "No parent of $PREV_COMMIT was signed with a trusted key!" > /dev/stderr
+                               echo "Parents are:" > /dev/stderr
+                               PARENTS=$(git show -s --format=format:%P $PREV_COMMIT)
+                               for PARENT in $PARENTS; do
+                                       git show -s $PARENT > /dev/stderr
+                               done
+                       else
+                               echo "$CURRENT_COMMIT was not signed with a trusted key!" > /dev/stderr
+                       fi
+                       exit 1
                fi
-               exit 1
        fi
 
        # We always verify the top of the tree

laanwj commented at 3:07 PM on March 29, 2018:

Ah yes, patching the tool to print them out makes sense.

laanwj commented at 2:46 PM on March 29, 2018: member

BTW: as I've started timestamping merge commits using opentimestamps, in theory it would be possible to verify the old commits using the expired key without worrying about backdating attacks. But that's probably quite involved, just an idea for the future.

petertodd commented at 6:42 PM on March 29, 2018: contributor

@laanwj Actually it should be as easy as running git log --show-signature with OTS installed. GPG will verify the signature correctly regardless of whether it's expired or revoked, albeit with a large warning. All you have to do is look at the time that OTS reports and decide for yourself if that was prior to when the key became invalid. If it is, the signature can be trusted.

TheBlueMatt commented at 6:47 PM on March 29, 2018: member

@petertodd care to look into a patch for gpg.sh to check if OTS is installed/at a locatoin specified by an env variable and call out to OTS if it is to aut-check that the timestamp is within a day or whatever of the claimed timestamp?

petertodd commented at 8:31 PM on March 29, 2018: contributor

@TheBlueMatt As in, just check that the timestamp on the git commit is valid? Or check that the signature is also valid? I think most of the code required to check the timestamp on an arbitrary git commit exists. So doing that last part wouldn't be too hard.

TheBlueMatt commented at 10:05 PM on March 29, 2018: member

@petertodd well the gpg.sh script already checks the signature, so just have to add the timestamp-checking part :p.

laanwj commented at 10:28 AM on March 30, 2018: member

Going to merge this for now, let's do the timestamp thing separately.

laanwj merged this on Mar 30, 2018

laanwj closed this on Mar 30, 2018

laanwj referenced this in commit ff48f62671 on Mar 30, 2018

PastaPastaPasta referenced this in commit d5bd287ed0 on Sep 27, 2020

PastaPastaPasta referenced this in commit 568b7e1c0c on Oct 22, 2020

harding referenced this in commit 223c489679 on Apr 23, 2021

MarcoFalke locked this on Sep 8, 2021