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
1=== modified file 'lib/lp/services/gpg/handler.py'
2--- lib/lp/services/gpg/handler.py 2019-03-13 17:19:50 +0000
3+++ lib/lp/services/gpg/handler.py 2019-03-18 10:16:56 +0000
4@@ -457,12 +457,7 @@
5 if not key.exists_in_local_keyring:
6 pubkey = self._getPubKey(fingerprint)
7 key = self.importPublicKey(pubkey)
8- # XXX cjwatson 2019-03-13: Remove affordance for 64-bit key IDs
9- # once we're on GnuPG 2.2.7 and GPGME 1.11.0. See comment in
10- # getVerifiedSignature.
11- if (fingerprint != key.fingerprint and
12- not (len(fingerprint) == 16 and
13- key.fingerprint.endswith(fingerprint))):
14+ if not key.matches(fingerprint):
15 ctx = self._getContext()
16 with gpgme_timeline("delete", key.fingerprint):
17 ctx.delete(key.key)
18@@ -658,7 +653,7 @@
19 self.keysize, self.algorithm.title, self.fingerprint)
20
21 def export(self):
22- """See `PymeKey`."""
23+ """See `IPymeKey`."""
24 if self.secret:
25 # XXX cprov 20081014: gpgme_op_export() only supports public keys.
26 # See http://www.fifi.org/cgi-bin/info2www?(gpgme)Exporting+Keys
27@@ -674,6 +669,18 @@
28
29 return keydata.getvalue()
30
31+ def matches(self, fingerprint):
32+ """See `IPymeKey`."""
33+ for subkey in self.key.subkeys:
34+ if fingerprint == subkey.fpr:
35+ return True
36+ # XXX cjwatson 2019-03-13: Remove affordance for 64-bit key IDs
37+ # once we're on GnuPG 2.2.7 and GPGME 1.11.0. See comment in
38+ # getVerifiedSignature.
39+ if len(fingerprint) == 16 and subkey.fpr.endswith(fingerprint):
40+ return True
41+ return False
42+
43
44 @implementer(IPymeUserId)
45 class PymeUserId:
46
47=== modified file 'lib/lp/services/gpg/interfaces.py'
48--- lib/lp/services/gpg/interfaces.py 2018-07-22 17:56:11 +0000
49+++ lib/lp/services/gpg/interfaces.py 2019-03-18 10:16:56 +0000
50@@ -1,4 +1,4 @@
51-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
52+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
53 # GNU Affero General Public License version 3 (see the file LICENSE).
54
55 __all__ = [
56@@ -407,6 +407,9 @@
57 :return: a string containing the exported key.
58 """
59
60+ def matches(fingerprint):
61+ """Return True if and only if this fingerprint matches this key."""
62+
63
64 class IPymeUserId(Interface):
65 """pyME user ID"""
66
67=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
68--- lib/lp/services/gpg/tests/test_gpghandler.py 2019-03-13 17:19:50 +0000
69+++ lib/lp/services/gpg/tests/test_gpghandler.py 2019-03-18 10:16:56 +0000
70@@ -193,6 +193,22 @@
71 GPGKeyMismatchOnServer, gpghandler.retrieveKey, fingerprint)
72 self.assertEqual([], list(gpghandler.localKeys()))
73
74+ def test_retrieveKey_allows_subkey(self):
75+ # retrieveKey allows retrieving keys by subkey fingerprint.
76+ keyserver = self.useFixture(KeyServerTac())
77+ primary_fingerprint = "340CA3BB270E2716C9EE0B768E7EB7086C64A8C5"
78+ subkey_fingerprint = "A2E916260726EE2BF86501A14244E5A6067595FF"
79+ shutil.copy2(
80+ test_pubkey_file_from_email("foo.bar@canonical.com"),
81+ os.path.join(keyserver.root, "0x%s.get" % subkey_fingerprint))
82+ gpghandler = getUtility(IGPGHandler)
83+ key = gpghandler.retrieveKey(subkey_fingerprint)
84+ self.assertEqual(primary_fingerprint, key.fingerprint)
85+ self.assertTrue(key.matches(primary_fingerprint))
86+ self.assertTrue(key.matches(subkey_fingerprint))
87+ self.assertTrue(key.matches(subkey_fingerprint[-16:]))
88+ self.assertFalse(key.matches(subkey_fingerprint[:-1] + "0"))
89+
90 def test_retrieveKey_allows_64bit_key_id(self):
91 # In order to support retrieving keys during signature verification,
92 # retrieveKey temporarily allows 64-bit key IDs.