launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/gpg-fix-verify-retrieve-key into lp:launchpad.

Commit message:
Temporarily allow 64-bit key IDs in GPGHandler.retrieveKey, since getVerifiedSignature may need them.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/gpg-fix-verify-retrieve-key/+merge/364386

An awkward consequence of https://code.launchpad.net/~cjwatson/launchpad/no-auto-key-retrieve/+merge/364100.  I still think this is probably the best approach for now given the keyserver weirdness, but happy to discuss it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/gpg-fix-verify-retrieve-key into lp:launchpad.

=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py	2019-03-12 12:57:14 +0000
+++ lib/lp/services/gpg/handler.py	2019-03-13 17:42:56 +0000
@@ -457,7 +457,12 @@
         if not key.exists_in_local_keyring:
             pubkey = self._getPubKey(fingerprint)
             key = self.importPublicKey(pubkey)
-            if fingerprint != key.fingerprint:
+            # XXX cjwatson 2019-03-13: Remove affordance for 64-bit key IDs
+            # once we're on GnuPG 2.2.7 and GPGME 1.11.0.  See comment in
+            # getVerifiedSignature.
+            if (fingerprint != key.fingerprint and
+                not (len(fingerprint) == 16 and
+                     key.fingerprint.endswith(fingerprint))):
                 ctx = self._getContext()
                 with gpgme_timeline("delete", key.fingerprint):
                     ctx.delete(key.key)

=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
--- lib/lp/services/gpg/tests/test_gpghandler.py	2018-06-25 11:31:00 +0000
+++ lib/lp/services/gpg/tests/test_gpghandler.py	2019-03-13 17:42:56 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import base64
@@ -193,6 +193,48 @@
             GPGKeyMismatchOnServer, gpghandler.retrieveKey, fingerprint)
         self.assertEqual([], list(gpghandler.localKeys()))
 
+    def test_retrieveKey_allows_64bit_key_id(self):
+        # In order to support retrieving keys during signature verification,
+        # retrieveKey temporarily allows 64-bit key IDs.
+        keyserver = self.useFixture(KeyServerTac())
+        fingerprint = "340CA3BB270E2716C9EE0B768E7EB7086C64A8C5"
+        key_id = fingerprint[-16:]
+        shutil.copy2(
+            test_pubkey_file_from_email("foo.bar@xxxxxxxxxxxxx"),
+            os.path.join(keyserver.root, "0x%s.get" % key_id))
+        gpghandler = getUtility(IGPGHandler)
+        self.assertEqual(
+            fingerprint, gpghandler.retrieveKey(key_id).fingerprint)
+        fingerprints = set(key.fingerprint for key in gpghandler.localKeys())
+        self.assertIn(fingerprint, fingerprints)
+
+    def test_retrieveKey_checks_64bit_key_id(self):
+        # If retrieveKey is given a 64-bit key ID, it checks that it's a
+        # suffix of the fingerprint (which is the best it can do).
+        keyserver = self.useFixture(KeyServerTac())
+        key_id = "0000000000000000"
+        shutil.copy2(
+            test_pubkey_file_from_email("foo.bar@xxxxxxxxxxxxx"),
+            os.path.join(keyserver.root, "0x%s.get" % key_id))
+        gpghandler = getUtility(IGPGHandler)
+        self.assertRaises(
+            GPGKeyMismatchOnServer, gpghandler.retrieveKey, key_id)
+        self.assertEqual([], list(gpghandler.localKeys()))
+
+    def test_retrieveKey_forbids_32bit_key_id(self):
+        # 32-bit key IDs are just too terrible, and retrieveKey doesn't
+        # support those.
+        keyserver = self.useFixture(KeyServerTac())
+        fingerprint = "340CA3BB270E2716C9EE0B768E7EB7086C64A8C5"
+        key_id = fingerprint[-8:]
+        shutil.copy2(
+            test_pubkey_file_from_email("foo.bar@xxxxxxxxxxxxx"),
+            os.path.join(keyserver.root, "0x%s.get" % key_id))
+        gpghandler = getUtility(IGPGHandler)
+        self.assertRaises(
+            GPGKeyMismatchOnServer, gpghandler.retrieveKey, key_id)
+        self.assertEqual([], list(gpghandler.localKeys()))
+
     def test_uploadPublicKey_suppress_in_config(self):
         self.useFixture(KeyServerTac())
         logger = BufferLogger()