Merge lp:~cjwatson/launchpad/refine-wrong-key-check into lp:launchpad

Proposed by Colin Watson on 2019-03-15
Status: Merged
Merged at revision: 18908
Proposed branch: lp:~cjwatson/launchpad/refine-wrong-key-check
Merge into: lp:launchpad
Diff against target: 92 lines (+34/-8)
3 files modified
lib/lp/services/gpg/handler.py (+14/-7)
lib/lp/services/gpg/interfaces.py (+4/-1)
lib/lp/services/gpg/tests/test_gpghandler.py (+16/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/refine-wrong-key-check
Reviewer Review Type Date Requested Status
William Grant code 2019-03-15 Approve on 2019-03-18
Review via email: mp+364495@code.launchpad.net

Commit message

Allow retrieving keys by subkey fingerprint. Fixes subkey signature verification regression in r18902.

To post a comment you must log in.
William Grant (wgrant) wrote :

Do please amend the commit message to note that this is a regression fix and why.

review: Approve (code)
18906. By Colin Watson on 2019-03-18

Allow retrieving keys by subkey fingerprint.

This fixes a regression caused by r18902: subkey signatures are now verified
by calling retrieveKey with the subkey ID rather than relying on
auto-key-retrieve, so we need to account for that when checking the
keyserver response.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py 2019-03-13 17:19:50 +0000
+++ lib/lp/services/gpg/handler.py 2019-03-18 10:16:56 +0000
@@ -457,12 +457,7 @@
457 if not key.exists_in_local_keyring:457 if not key.exists_in_local_keyring:
458 pubkey = self._getPubKey(fingerprint)458 pubkey = self._getPubKey(fingerprint)
459 key = self.importPublicKey(pubkey)459 key = self.importPublicKey(pubkey)
460 # XXX cjwatson 2019-03-13: Remove affordance for 64-bit key IDs460 if not key.matches(fingerprint):
461 # once we're on GnuPG 2.2.7 and GPGME 1.11.0. See comment in
462 # getVerifiedSignature.
463 if (fingerprint != key.fingerprint and
464 not (len(fingerprint) == 16 and
465 key.fingerprint.endswith(fingerprint))):
466 ctx = self._getContext()461 ctx = self._getContext()
467 with gpgme_timeline("delete", key.fingerprint):462 with gpgme_timeline("delete", key.fingerprint):
468 ctx.delete(key.key)463 ctx.delete(key.key)
@@ -658,7 +653,7 @@
658 self.keysize, self.algorithm.title, self.fingerprint)653 self.keysize, self.algorithm.title, self.fingerprint)
659654
660 def export(self):655 def export(self):
661 """See `PymeKey`."""656 """See `IPymeKey`."""
662 if self.secret:657 if self.secret:
663 # XXX cprov 20081014: gpgme_op_export() only supports public keys.658 # XXX cprov 20081014: gpgme_op_export() only supports public keys.
664 # See http://www.fifi.org/cgi-bin/info2www?(gpgme)Exporting+Keys659 # See http://www.fifi.org/cgi-bin/info2www?(gpgme)Exporting+Keys
@@ -674,6 +669,18 @@
674669
675 return keydata.getvalue()670 return keydata.getvalue()
676671
672 def matches(self, fingerprint):
673 """See `IPymeKey`."""
674 for subkey in self.key.subkeys:
675 if fingerprint == subkey.fpr:
676 return True
677 # XXX cjwatson 2019-03-13: Remove affordance for 64-bit key IDs
678 # once we're on GnuPG 2.2.7 and GPGME 1.11.0. See comment in
679 # getVerifiedSignature.
680 if len(fingerprint) == 16 and subkey.fpr.endswith(fingerprint):
681 return True
682 return False
683
677684
678@implementer(IPymeUserId)685@implementer(IPymeUserId)
679class PymeUserId:686class PymeUserId:
680687
=== modified file 'lib/lp/services/gpg/interfaces.py'
--- lib/lp/services/gpg/interfaces.py 2018-07-22 17:56:11 +0000
+++ lib/lp/services/gpg/interfaces.py 2019-03-18 10:16:56 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the1# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__all__ = [4__all__ = [
@@ -407,6 +407,9 @@
407 :return: a string containing the exported key.407 :return: a string containing the exported key.
408 """408 """
409409
410 def matches(fingerprint):
411 """Return True if and only if this fingerprint matches this key."""
412
410413
411class IPymeUserId(Interface):414class IPymeUserId(Interface):
412 """pyME user ID"""415 """pyME user ID"""
413416
=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
--- lib/lp/services/gpg/tests/test_gpghandler.py 2019-03-13 17:19:50 +0000
+++ lib/lp/services/gpg/tests/test_gpghandler.py 2019-03-18 10:16:56 +0000
@@ -193,6 +193,22 @@
193 GPGKeyMismatchOnServer, gpghandler.retrieveKey, fingerprint)193 GPGKeyMismatchOnServer, gpghandler.retrieveKey, fingerprint)
194 self.assertEqual([], list(gpghandler.localKeys()))194 self.assertEqual([], list(gpghandler.localKeys()))
195195
196 def test_retrieveKey_allows_subkey(self):
197 # retrieveKey allows retrieving keys by subkey fingerprint.
198 keyserver = self.useFixture(KeyServerTac())
199 primary_fingerprint = "340CA3BB270E2716C9EE0B768E7EB7086C64A8C5"
200 subkey_fingerprint = "A2E916260726EE2BF86501A14244E5A6067595FF"
201 shutil.copy2(
202 test_pubkey_file_from_email("foo.bar@canonical.com"),
203 os.path.join(keyserver.root, "0x%s.get" % subkey_fingerprint))
204 gpghandler = getUtility(IGPGHandler)
205 key = gpghandler.retrieveKey(subkey_fingerprint)
206 self.assertEqual(primary_fingerprint, key.fingerprint)
207 self.assertTrue(key.matches(primary_fingerprint))
208 self.assertTrue(key.matches(subkey_fingerprint))
209 self.assertTrue(key.matches(subkey_fingerprint[-16:]))
210 self.assertFalse(key.matches(subkey_fingerprint[:-1] + "0"))
211
196 def test_retrieveKey_allows_64bit_key_id(self):212 def test_retrieveKey_allows_64bit_key_id(self):
197 # In order to support retrieving keys during signature verification,213 # In order to support retrieving keys during signature verification,
198 # retrieveKey temporarily allows 64-bit key IDs.214 # retrieveKey temporarily allows 64-bit key IDs.