Merge lp:~cjwatson/launchpad/faster-gpg-operations into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 17967
Proposed branch: lp:~cjwatson/launchpad/faster-gpg-operations
Merge into: lp:launchpad
Diff against target: 381 lines (+48/-61)
12 files modified
lib/lp/archivepublisher/archivesigningkey.py (+4/-9)
lib/lp/bugs/mail/tests/test_handler.py (+2/-2)
lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt (+2/-5)
lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt (+2/-5)
lib/lp/services/gpg/doc/gpg-encryption.txt (+5/-7)
lib/lp/services/gpg/handler.py (+10/-10)
lib/lp/services/gpg/interfaces.py (+9/-9)
lib/lp/services/gpg/tests/test_gpghandler.py (+1/-1)
lib/lp/services/mail/tests/test_incoming.py (+2/-2)
lib/lp/services/mail/tests/test_signedmessage.py (+3/-3)
lib/lp/services/verification/model/logintoken.py (+3/-3)
lib/lp/testing/factory.py (+5/-5)
To merge this branch: bzr merge lp:~cjwatson/launchpad/faster-gpg-operations
Reviewer Review Type Date Requested Status
Celso Providelo (community) Approve
Launchpad code reviewers Pending
Review via email: mp+289950@code.launchpad.net

Commit message

Sign/encrypt content based on an IPymeKey rather than a fingerprint, for performance.

Description of the change

Sign/encrypt content based on an IPymeKey rather than a fingerprint. Getting from a fingerprint to a key involves two subprocess invocations in gpgme, and in nearly all cases we already have the key in hand so this is unnecessary work. This makes a difference when re-signing thousands of PPAs: it saves in the region of .2s per signature on dogfood.

To post a comment you must log in.
Revision history for this message
Celso Providelo (cprov) wrote :

Nice catch! Thank you.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/archivesigningkey.py'
2--- lib/lp/archivepublisher/archivesigningkey.py 2016-03-16 23:13:58 +0000
3+++ lib/lp/archivepublisher/archivesigningkey.py 2016-03-23 18:12:33 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """ArchiveSigningKey implementation."""
10@@ -23,10 +23,7 @@
11 )
12 from lp.registry.interfaces.gpg import IGPGKeySet
13 from lp.services.config import config
14-from lp.services.gpg.interfaces import (
15- GPGKeyAlgorithm,
16- IGPGHandler,
17- )
18+from lp.services.gpg.interfaces import IGPGHandler
19 from lp.services.propertycache import get_property_cache
20
21
22@@ -133,8 +130,7 @@
23
24 release_file_content = open(release_file_path).read()
25 signature = gpghandler.signContent(
26- release_file_content, secret_key.fingerprint,
27- mode=gpgme.SIG_MODE_DETACH)
28+ release_file_content, secret_key, mode=gpgme.SIG_MODE_DETACH)
29
30 release_signature_file = open(
31 os.path.join(suite_path, 'Release.gpg'), 'w')
32@@ -142,8 +138,7 @@
33 release_signature_file.close()
34
35 inline_release = gpghandler.signContent(
36- release_file_content, secret_key.fingerprint,
37- mode=gpgme.SIG_MODE_CLEAR)
38+ release_file_content, secret_key, mode=gpgme.SIG_MODE_CLEAR)
39
40 inline_release_file = open(
41 os.path.join(suite_path, 'InRelease'), 'w')
42
43=== modified file 'lib/lp/bugs/mail/tests/test_handler.py'
44--- lib/lp/bugs/mail/tests/test_handler.py 2015-09-08 09:09:28 +0000
45+++ lib/lp/bugs/mail/tests/test_handler.py 2016-03-23 18:12:33 +0000
46@@ -1,4 +1,4 @@
47-# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
48+# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
49 # GNU Affero General Public License version 3 (see the file LICENSE).
50
51 """Test MaloneHandler."""
52@@ -682,7 +682,7 @@
53 # isn't too far in the future or past. This test shows that a
54 # signature with a timestamp of appxoimately now will be accepted.
55 signing_context = GPGSigningContext(
56- import_secret_test_key().fingerprint, password='test')
57+ import_secret_test_key(), password='test')
58 msg = self.factory.makeSignedMessage(
59 body=' security no', signing_context=signing_context)
60 handler = MaloneHandler()
61
62=== modified file 'lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt'
63--- lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt 2016-03-17 23:32:28 +0000
64+++ lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt 2016-03-23 18:12:33 +0000
65@@ -261,8 +261,7 @@
66 >>> login(ANONYMOUS)
67 >>> key = import_secret_test_key('sign.only@canonical.com.sec')
68 >>> bad = gpghandler.signContent(
69- ... 'This is not the verification message!',
70- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
71+ ... 'This is not the verification message!', key, 'test')
72 >>> logout()
73
74 >>> browser.getControl('Signed text').value = bad
75@@ -298,9 +297,7 @@
76 If they sign the text correctly, they are redirected to their home page.
77
78 >>> login(ANONYMOUS)
79- >>> good = gpghandler.signContent(
80- ... str(verification_content),
81- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
82+ >>> good = gpghandler.signContent(str(verification_content), key, 'test')
83 >>> logout()
84
85 >>> browser.getControl('Signed text').value = good
86
87=== modified file 'lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt'
88--- lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt 2016-02-05 16:51:12 +0000
89+++ lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt 2016-03-23 18:12:33 +0000
90@@ -246,8 +246,7 @@
91 >>> login(ANONYMOUS)
92 >>> key = import_secret_test_key('sign.only@canonical.com.sec')
93 >>> bad = gpghandler.signContent(
94- ... 'This is not the verification message!',
95- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
96+ ... 'This is not the verification message!', key, 'test')
97 >>> logout()
98
99 >>> browser.getControl('Signed text').value = bad
100@@ -283,9 +282,7 @@
101 If they sign the text correctly, they are redirected to their home page.
102
103 >>> login(ANONYMOUS)
104- >>> good = gpghandler.signContent(
105- ... str(verification_content),
106- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
107+ >>> good = gpghandler.signContent(str(verification_content), key, 'test')
108 >>> logout()
109
110 >>> browser.getControl('Signed text').value = good
111
112=== modified file 'lib/lp/services/gpg/doc/gpg-encryption.txt'
113--- lib/lp/services/gpg/doc/gpg-encryption.txt 2011-12-24 17:49:30 +0000
114+++ lib/lp/services/gpg/doc/gpg-encryption.txt 2016-03-23 18:12:33 +0000
115@@ -48,8 +48,8 @@
116 >>> fingerprint
117 u'A419AE861E88BC9E04B9C26FBA2B9389DFD20543'
118
119- >>> cipher = gpghandler.encryptContent(content.encode('utf-8'),
120- ... fingerprint)
121+ >>> key = gpghandler.retrieveKey(fingerprint)
122+ >>> cipher = gpghandler.encryptContent(content.encode('utf-8'), key)
123
124 cipher constains the encrypted content.
125
126@@ -67,15 +67,14 @@
127 Verify if the encrytion process support passing another charset string
128
129 >>> content = u'a\xe7ucar'
130- >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'),
131- ... fingerprint)
132+ >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'), key)
133 >>> plain = decrypt_content(cipher, 'test')
134 >>> plain.decode('iso-8859-1')
135 u'a\xe7ucar'
136
137 Let's try to pass unicode and see if it fails
138
139- >>> cipher = gpghandler.encryptContent(content,fingerprint)
140+ >>> cipher = gpghandler.encryptContent(content, key)
141 Traceback (most recent call last):
142 ...
143 TypeError: Content cannot be Unicode.
144@@ -83,8 +82,7 @@
145 Decrypt a unicode content:
146
147 >>> content = u'a\xe7ucar'
148- >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'),
149- ... fingerprint)
150+ >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'), key)
151 >>> cipher = unicode(cipher)
152 >>> plain = decrypt_content(cipher, 'test')
153 Traceback (most recent call last):
154
155=== modified file 'lib/lp/services/gpg/handler.py'
156--- lib/lp/services/gpg/handler.py 2016-03-21 00:20:23 +0000
157+++ lib/lp/services/gpg/handler.py 2016-03-23 18:12:33 +0000
158@@ -26,6 +26,7 @@
159 from gpgservice_client import GPGClient
160 from lazr.restful.utils import get_current_browser_request
161 from zope.interface import implementer
162+from zope.security.proxy import removeSecurityProxy
163
164 from lp.app.validators.email import valid_email
165 from lp.services.config import config
166@@ -320,7 +321,7 @@
167
168 return key
169
170- def encryptContent(self, content, fingerprint):
171+ def encryptContent(self, content, key):
172 """See IGPGHandler."""
173 if isinstance(content, unicode):
174 raise TypeError('Content cannot be Unicode.')
175@@ -333,22 +334,21 @@
176 plain = StringIO(content)
177 cipher = StringIO()
178
179- # retrive gpgme key object
180- try:
181- key = ctx.get_key(fingerprint.encode('ascii'), 0)
182- except gpgme.GpgmeError:
183+ if key.key is None:
184 return None
185
186 if not key.can_encrypt:
187 raise ValueError('key %s can not be used for encryption'
188- % fingerprint)
189+ % key.fingerprint)
190
191 # encrypt content
192- ctx.encrypt([key], gpgme.ENCRYPT_ALWAYS_TRUST, plain, cipher)
193+ ctx.encrypt(
194+ [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
195+ plain, cipher)
196
197 return cipher.getvalue()
198
199- def signContent(self, content, key_fingerprint, password='', mode=None):
200+ def signContent(self, content, key, password='', mode=None):
201 """See IGPGHandler."""
202 if not isinstance(content, str):
203 raise TypeError('Content should be a string.')
204@@ -361,8 +361,7 @@
205 context = gpgme.Context()
206 context.armor = True
207
208- key = context.get_key(key_fingerprint.encode('ascii'), True)
209- context.signers = [key]
210+ context.signers = [removeSecurityProxy(key.key)]
211
212 # Set up containers.
213 plaintext = StringIO(content)
214@@ -561,6 +560,7 @@
215
216 def _buildFromGpgmeKey(self, key):
217 self.exists_in_local_keyring = True
218+ self.key = key
219 subkey = key.subkeys[0]
220 self.fingerprint = subkey.fpr
221 self.revoked = subkey.revoked
222
223=== modified file 'lib/lp/services/gpg/interfaces.py'
224--- lib/lp/services/gpg/interfaces.py 2016-03-16 02:08:40 +0000
225+++ lib/lp/services/gpg/interfaces.py 2016-03-23 18:12:33 +0000
226@@ -1,4 +1,4 @@
227-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
228+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
229 # GNU Affero General Public License version 3 (see the file LICENSE).
230
231 __all__ = [
232@@ -290,24 +290,23 @@
233 :return: a `PymeKey` object for the just-generated secret key.
234 """
235
236- def encryptContent(content, fingerprint):
237- """Encrypt the given content for the given fingerprint.
238+ def encryptContent(content, key):
239+ """Encrypt the given content for the given key.
240
241 content must be a traditional string. It's up to the caller to
242- encode or decode properly. Fingerprint must be hexadecimal string.
243+ encode or decode properly.
244
245 :param content: the Unicode content to be encrypted.
246- :param fingerprint: the OpenPGP key's fingerprint.
247+ :param key: the `IPymeKey` to encrypt the content for.
248
249 :return: the encrypted content or None if failed.
250 """
251
252- def signContent(content, key_fingerprint, password='', mode=None):
253- """Signs content with a given GPG fingerprint.
254+ def signContent(content, key, password='', mode=None):
255+ """Signs content with a given GPG key.
256
257 :param content: the content to sign.
258- :param key_fingerprint: the fingerprint of the key to use when
259- signing the content.
260+ :param key: the `IPymeKey` to use when signing the content.
261 :param password: optional password to the key identified by
262 key_fingerprint, the default value is '',
263 :param mode: optional type of GPG signature to produce, the
264@@ -386,6 +385,7 @@
265 """pyME key model."""
266
267 fingerprint = Attribute("Key Fingerprint")
268+ key = Attribute("Underlying GpgmeKey object")
269 algorithm = Attribute("Key Algorithm")
270 revoked = Attribute("Key Revoked")
271 expired = Attribute("Key Expired")
272
273=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
274--- lib/lp/services/gpg/tests/test_gpghandler.py 2016-03-21 00:20:23 +0000
275+++ lib/lp/services/gpg/tests/test_gpghandler.py 2016-03-23 18:12:33 +0000
276@@ -221,7 +221,7 @@
277 secret_key = import_secret_test_key(key_name)
278 content = "abc\n"
279 signed_content = self.gpg_handler.signContent(
280- content, secret_key.fingerprint, password)
281+ content, secret_key, password)
282 signature = self.gpg_handler.getVerifiedSignature(signed_content)
283 self.assertEqual(content, signature.plain_data)
284 self.assertEqual(secret_key.fingerprint, signature.fingerprint)
285
286=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
287--- lib/lp/services/mail/tests/test_incoming.py 2015-07-08 16:05:11 +0000
288+++ lib/lp/services/mail/tests/test_incoming.py 2016-03-23 18:12:33 +0000
289@@ -1,4 +1,4 @@
290-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
291+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
292 # GNU Affero General Public License version 3 (see the file LICENSE).
293
294
295@@ -182,7 +182,7 @@
296 """If the signature is nontrivial future-dated, it's not trusted."""
297
298 signing_context = GPGSigningContext(
299- import_secret_test_key().fingerprint, password='test')
300+ import_secret_test_key(), password='test')
301 msg = self.factory.makeSignedMessage(signing_context=signing_context)
302 # It's not trivial to make a gpg signature with a bogus timestamp, so
303 # let's just treat everything as invalid, and trust that the regular
304
305=== modified file 'lib/lp/services/mail/tests/test_signedmessage.py'
306--- lib/lp/services/mail/tests/test_signedmessage.py 2015-03-13 19:05:50 +0000
307+++ lib/lp/services/mail/tests/test_signedmessage.py 2016-03-23 18:12:33 +0000
308@@ -1,4 +1,4 @@
309-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
310+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
311 # GNU Affero General Public License version 3 (see the file LICENSE).
312
313 """Test the SignedMessage class."""
314@@ -62,7 +62,7 @@
315 # Create a signed message for the sender specified with the test
316 # secret key.
317 key = import_secret_test_key()
318- signing_context = GPGSigningContext(key.fingerprint, password='test')
319+ signing_context = GPGSigningContext(key, password='test')
320 if body is None:
321 body = dedent("""\
322 This is a multi-line body.
323@@ -136,7 +136,7 @@
324 gpghandler = getUtility(IGPGHandler)
325 signature = gpghandler.signContent(
326 canonicalise_line_endings(body_text.as_string()),
327- key.fingerprint, 'test', gpgme.SIG_MODE_DETACH)
328+ key, 'test', gpgme.SIG_MODE_DETACH)
329
330 attachment = Message()
331 attachment.set_payload(signature)
332
333=== modified file 'lib/lp/services/verification/model/logintoken.py'
334--- lib/lp/services/verification/model/logintoken.py 2015-09-28 17:38:45 +0000
335+++ lib/lp/services/verification/model/logintoken.py 2016-03-23 18:12:33 +0000
336@@ -1,4 +1,4 @@
337-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
338+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
339 # GNU Affero General Public License version 3 (see the file LICENSE).
340
341 __metaclass__ = type
342@@ -167,8 +167,8 @@
343 # Encrypt this part's content if requested.
344 if key.can_encrypt:
345 gpghandler = getUtility(IGPGHandler)
346- token_text = gpghandler.encryptContent(token_text.encode('utf-8'),
347- key.fingerprint)
348+ token_text = gpghandler.encryptContent(
349+ token_text.encode('utf-8'), key)
350 # In this case, we need to include some clear text instructions
351 # for people who do not have an MUA that can decrypt the ASCII
352 # armored text.
353
354=== modified file 'lib/lp/testing/factory.py'
355--- lib/lp/testing/factory.py 2016-03-21 22:03:39 +0000
356+++ lib/lp/testing/factory.py 2016-03-23 18:12:33 +0000
357@@ -392,10 +392,10 @@
358
359
360 class GPGSigningContext:
361- """A helper object to hold the fingerprint, password and mode."""
362+ """A helper object to hold the key, password and mode."""
363
364- def __init__(self, fingerprint, password='', mode=None):
365- self.fingerprint = fingerprint
366+ def __init__(self, key, password='', mode=None):
367+ self.key = key
368 self.password = password
369 self.mode = mode
370
371@@ -2158,8 +2158,8 @@
372 if signing_context is not None:
373 gpghandler = getUtility(IGPGHandler)
374 body = gpghandler.signContent(
375- body, signing_context.fingerprint,
376- signing_context.password, signing_context.mode)
377+ body, signing_context.key, signing_context.password,
378+ signing_context.mode)
379 assert body is not None
380 if attachment_contents is None:
381 mail.set_payload(body)