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
=== modified file 'lib/lp/archivepublisher/archivesigningkey.py'
--- lib/lp/archivepublisher/archivesigningkey.py 2016-03-16 23:13:58 +0000
+++ lib/lp/archivepublisher/archivesigningkey.py 2016-03-23 18:12:33 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2016 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"""ArchiveSigningKey implementation."""4"""ArchiveSigningKey implementation."""
@@ -23,10 +23,7 @@
23 )23 )
24from lp.registry.interfaces.gpg import IGPGKeySet24from lp.registry.interfaces.gpg import IGPGKeySet
25from lp.services.config import config25from lp.services.config import config
26from lp.services.gpg.interfaces import (26from lp.services.gpg.interfaces import IGPGHandler
27 GPGKeyAlgorithm,
28 IGPGHandler,
29 )
30from lp.services.propertycache import get_property_cache27from lp.services.propertycache import get_property_cache
3128
3229
@@ -133,8 +130,7 @@
133130
134 release_file_content = open(release_file_path).read()131 release_file_content = open(release_file_path).read()
135 signature = gpghandler.signContent(132 signature = gpghandler.signContent(
136 release_file_content, secret_key.fingerprint,133 release_file_content, secret_key, mode=gpgme.SIG_MODE_DETACH)
137 mode=gpgme.SIG_MODE_DETACH)
138134
139 release_signature_file = open(135 release_signature_file = open(
140 os.path.join(suite_path, 'Release.gpg'), 'w')136 os.path.join(suite_path, 'Release.gpg'), 'w')
@@ -142,8 +138,7 @@
142 release_signature_file.close()138 release_signature_file.close()
143139
144 inline_release = gpghandler.signContent(140 inline_release = gpghandler.signContent(
145 release_file_content, secret_key.fingerprint,141 release_file_content, secret_key, mode=gpgme.SIG_MODE_CLEAR)
146 mode=gpgme.SIG_MODE_CLEAR)
147142
148 inline_release_file = open(143 inline_release_file = open(
149 os.path.join(suite_path, 'InRelease'), 'w')144 os.path.join(suite_path, 'InRelease'), 'w')
150145
=== modified file 'lib/lp/bugs/mail/tests/test_handler.py'
--- lib/lp/bugs/mail/tests/test_handler.py 2015-09-08 09:09:28 +0000
+++ lib/lp/bugs/mail/tests/test_handler.py 2016-03-23 18:12:33 +0000
@@ -1,4 +1,4 @@
1# Copyright 2010-2015 Canonical Ltd. This software is licensed under the1# Copyright 2010-2016 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"""Test MaloneHandler."""4"""Test MaloneHandler."""
@@ -682,7 +682,7 @@
682 # isn't too far in the future or past. This test shows that a682 # isn't too far in the future or past. This test shows that a
683 # signature with a timestamp of appxoimately now will be accepted.683 # signature with a timestamp of appxoimately now will be accepted.
684 signing_context = GPGSigningContext(684 signing_context = GPGSigningContext(
685 import_secret_test_key().fingerprint, password='test')685 import_secret_test_key(), password='test')
686 msg = self.factory.makeSignedMessage(686 msg = self.factory.makeSignedMessage(
687 body=' security no', signing_context=signing_context)687 body=' security no', signing_context=signing_context)
688 handler = MaloneHandler()688 handler = MaloneHandler()
689689
=== modified file 'lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt'
--- lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt 2016-03-17 23:32:28 +0000
+++ lib/lp/registry/stories/gpg-coc/gpg-with-gpgservice-ff.txt 2016-03-23 18:12:33 +0000
@@ -261,8 +261,7 @@
261 >>> login(ANONYMOUS)261 >>> login(ANONYMOUS)
262 >>> key = import_secret_test_key('sign.only@canonical.com.sec')262 >>> key = import_secret_test_key('sign.only@canonical.com.sec')
263 >>> bad = gpghandler.signContent(263 >>> bad = gpghandler.signContent(
264 ... 'This is not the verification message!',264 ... 'This is not the verification message!', key, 'test')
265 ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
266 >>> logout()265 >>> logout()
267266
268 >>> browser.getControl('Signed text').value = bad267 >>> browser.getControl('Signed text').value = bad
@@ -298,9 +297,7 @@
298If they sign the text correctly, they are redirected to their home page.297If they sign the text correctly, they are redirected to their home page.
299298
300 >>> login(ANONYMOUS)299 >>> login(ANONYMOUS)
301 >>> good = gpghandler.signContent(300 >>> good = gpghandler.signContent(str(verification_content), key, 'test')
302 ... str(verification_content),
303 ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
304 >>> logout()301 >>> logout()
305302
306 >>> browser.getControl('Signed text').value = good303 >>> browser.getControl('Signed text').value = good
307304
=== modified file 'lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt'
--- lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt 2016-02-05 16:51:12 +0000
+++ lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt 2016-03-23 18:12:33 +0000
@@ -246,8 +246,7 @@
246 >>> login(ANONYMOUS)246 >>> login(ANONYMOUS)
247 >>> key = import_secret_test_key('sign.only@canonical.com.sec')247 >>> key = import_secret_test_key('sign.only@canonical.com.sec')
248 >>> bad = gpghandler.signContent(248 >>> bad = gpghandler.signContent(
249 ... 'This is not the verification message!',249 ... 'This is not the verification message!', key, 'test')
250 ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
251 >>> logout()250 >>> logout()
252251
253 >>> browser.getControl('Signed text').value = bad252 >>> browser.getControl('Signed text').value = bad
@@ -283,9 +282,7 @@
283If they sign the text correctly, they are redirected to their home page.282If they sign the text correctly, they are redirected to their home page.
284283
285 >>> login(ANONYMOUS)284 >>> login(ANONYMOUS)
286 >>> good = gpghandler.signContent(285 >>> good = gpghandler.signContent(str(verification_content), key, 'test')
287 ... str(verification_content),
288 ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
289 >>> logout()286 >>> logout()
290287
291 >>> browser.getControl('Signed text').value = good288 >>> browser.getControl('Signed text').value = good
292289
=== modified file 'lib/lp/services/gpg/doc/gpg-encryption.txt'
--- lib/lp/services/gpg/doc/gpg-encryption.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/services/gpg/doc/gpg-encryption.txt 2016-03-23 18:12:33 +0000
@@ -48,8 +48,8 @@
48 >>> fingerprint48 >>> fingerprint
49 u'A419AE861E88BC9E04B9C26FBA2B9389DFD20543'49 u'A419AE861E88BC9E04B9C26FBA2B9389DFD20543'
5050
51 >>> cipher = gpghandler.encryptContent(content.encode('utf-8'),51 >>> key = gpghandler.retrieveKey(fingerprint)
52 ... fingerprint)52 >>> cipher = gpghandler.encryptContent(content.encode('utf-8'), key)
53 53
54cipher constains the encrypted content.54cipher constains the encrypted content.
5555
@@ -67,15 +67,14 @@
67Verify if the encrytion process support passing another charset string67Verify if the encrytion process support passing another charset string
6868
69 >>> content = u'a\xe7ucar'69 >>> content = u'a\xe7ucar'
70 >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'),70 >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'), key)
71 ... fingerprint)
72 >>> plain = decrypt_content(cipher, 'test') 71 >>> plain = decrypt_content(cipher, 'test')
73 >>> plain.decode('iso-8859-1')72 >>> plain.decode('iso-8859-1')
74 u'a\xe7ucar'73 u'a\xe7ucar'
7574
76Let's try to pass unicode and see if it fails75Let's try to pass unicode and see if it fails
7776
78 >>> cipher = gpghandler.encryptContent(content,fingerprint)77 >>> cipher = gpghandler.encryptContent(content, key)
79 Traceback (most recent call last):78 Traceback (most recent call last):
80 ...79 ...
81 TypeError: Content cannot be Unicode.80 TypeError: Content cannot be Unicode.
@@ -83,8 +82,7 @@
83Decrypt a unicode content:82Decrypt a unicode content:
8483
85 >>> content = u'a\xe7ucar'84 >>> content = u'a\xe7ucar'
86 >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'),85 >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'), key)
87 ... fingerprint)
88 >>> cipher = unicode(cipher)86 >>> cipher = unicode(cipher)
89 >>> plain = decrypt_content(cipher, 'test') 87 >>> plain = decrypt_content(cipher, 'test')
90 Traceback (most recent call last):88 Traceback (most recent call last):
9189
=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py 2016-03-21 00:20:23 +0000
+++ lib/lp/services/gpg/handler.py 2016-03-23 18:12:33 +0000
@@ -26,6 +26,7 @@
26from gpgservice_client import GPGClient26from gpgservice_client import GPGClient
27from lazr.restful.utils import get_current_browser_request27from lazr.restful.utils import get_current_browser_request
28from zope.interface import implementer28from zope.interface import implementer
29from zope.security.proxy import removeSecurityProxy
2930
30from lp.app.validators.email import valid_email31from lp.app.validators.email import valid_email
31from lp.services.config import config32from lp.services.config import config
@@ -320,7 +321,7 @@
320321
321 return key322 return key
322323
323 def encryptContent(self, content, fingerprint):324 def encryptContent(self, content, key):
324 """See IGPGHandler."""325 """See IGPGHandler."""
325 if isinstance(content, unicode):326 if isinstance(content, unicode):
326 raise TypeError('Content cannot be Unicode.')327 raise TypeError('Content cannot be Unicode.')
@@ -333,22 +334,21 @@
333 plain = StringIO(content)334 plain = StringIO(content)
334 cipher = StringIO()335 cipher = StringIO()
335336
336 # retrive gpgme key object337 if key.key is None:
337 try:
338 key = ctx.get_key(fingerprint.encode('ascii'), 0)
339 except gpgme.GpgmeError:
340 return None338 return None
341339
342 if not key.can_encrypt:340 if not key.can_encrypt:
343 raise ValueError('key %s can not be used for encryption'341 raise ValueError('key %s can not be used for encryption'
344 % fingerprint)342 % key.fingerprint)
345343
346 # encrypt content344 # encrypt content
347 ctx.encrypt([key], gpgme.ENCRYPT_ALWAYS_TRUST, plain, cipher)345 ctx.encrypt(
346 [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
347 plain, cipher)
348348
349 return cipher.getvalue()349 return cipher.getvalue()
350350
351 def signContent(self, content, key_fingerprint, password='', mode=None):351 def signContent(self, content, key, password='', mode=None):
352 """See IGPGHandler."""352 """See IGPGHandler."""
353 if not isinstance(content, str):353 if not isinstance(content, str):
354 raise TypeError('Content should be a string.')354 raise TypeError('Content should be a string.')
@@ -361,8 +361,7 @@
361 context = gpgme.Context()361 context = gpgme.Context()
362 context.armor = True362 context.armor = True
363363
364 key = context.get_key(key_fingerprint.encode('ascii'), True)364 context.signers = [removeSecurityProxy(key.key)]
365 context.signers = [key]
366365
367 # Set up containers.366 # Set up containers.
368 plaintext = StringIO(content)367 plaintext = StringIO(content)
@@ -561,6 +560,7 @@
561560
562 def _buildFromGpgmeKey(self, key):561 def _buildFromGpgmeKey(self, key):
563 self.exists_in_local_keyring = True562 self.exists_in_local_keyring = True
563 self.key = key
564 subkey = key.subkeys[0]564 subkey = key.subkeys[0]
565 self.fingerprint = subkey.fpr565 self.fingerprint = subkey.fpr
566 self.revoked = subkey.revoked566 self.revoked = subkey.revoked
567567
=== modified file 'lib/lp/services/gpg/interfaces.py'
--- lib/lp/services/gpg/interfaces.py 2016-03-16 02:08:40 +0000
+++ lib/lp/services/gpg/interfaces.py 2016-03-23 18:12:33 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the1# Copyright 2009-2016 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__ = [
@@ -290,24 +290,23 @@
290 :return: a `PymeKey` object for the just-generated secret key.290 :return: a `PymeKey` object for the just-generated secret key.
291 """291 """
292292
293 def encryptContent(content, fingerprint):293 def encryptContent(content, key):
294 """Encrypt the given content for the given fingerprint.294 """Encrypt the given content for the given key.
295295
296 content must be a traditional string. It's up to the caller to296 content must be a traditional string. It's up to the caller to
297 encode or decode properly. Fingerprint must be hexadecimal string.297 encode or decode properly.
298298
299 :param content: the Unicode content to be encrypted.299 :param content: the Unicode content to be encrypted.
300 :param fingerprint: the OpenPGP key's fingerprint.300 :param key: the `IPymeKey` to encrypt the content for.
301301
302 :return: the encrypted content or None if failed.302 :return: the encrypted content or None if failed.
303 """303 """
304304
305 def signContent(content, key_fingerprint, password='', mode=None):305 def signContent(content, key, password='', mode=None):
306 """Signs content with a given GPG fingerprint.306 """Signs content with a given GPG key.
307307
308 :param content: the content to sign.308 :param content: the content to sign.
309 :param key_fingerprint: the fingerprint of the key to use when309 :param key: the `IPymeKey` to use when signing the content.
310 signing the content.
311 :param password: optional password to the key identified by310 :param password: optional password to the key identified by
312 key_fingerprint, the default value is '',311 key_fingerprint, the default value is '',
313 :param mode: optional type of GPG signature to produce, the312 :param mode: optional type of GPG signature to produce, the
@@ -386,6 +385,7 @@
386 """pyME key model."""385 """pyME key model."""
387386
388 fingerprint = Attribute("Key Fingerprint")387 fingerprint = Attribute("Key Fingerprint")
388 key = Attribute("Underlying GpgmeKey object")
389 algorithm = Attribute("Key Algorithm")389 algorithm = Attribute("Key Algorithm")
390 revoked = Attribute("Key Revoked")390 revoked = Attribute("Key Revoked")
391 expired = Attribute("Key Expired")391 expired = Attribute("Key Expired")
392392
=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
--- lib/lp/services/gpg/tests/test_gpghandler.py 2016-03-21 00:20:23 +0000
+++ lib/lp/services/gpg/tests/test_gpghandler.py 2016-03-23 18:12:33 +0000
@@ -221,7 +221,7 @@
221 secret_key = import_secret_test_key(key_name)221 secret_key = import_secret_test_key(key_name)
222 content = "abc\n"222 content = "abc\n"
223 signed_content = self.gpg_handler.signContent(223 signed_content = self.gpg_handler.signContent(
224 content, secret_key.fingerprint, password)224 content, secret_key, password)
225 signature = self.gpg_handler.getVerifiedSignature(signed_content)225 signature = self.gpg_handler.getVerifiedSignature(signed_content)
226 self.assertEqual(content, signature.plain_data)226 self.assertEqual(content, signature.plain_data)
227 self.assertEqual(secret_key.fingerprint, signature.fingerprint)227 self.assertEqual(secret_key.fingerprint, signature.fingerprint)
228228
=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
--- lib/lp/services/mail/tests/test_incoming.py 2015-07-08 16:05:11 +0000
+++ lib/lp/services/mail/tests/test_incoming.py 2016-03-23 18:12:33 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2012 Canonical Ltd. This software is licensed under the1# Copyright 2009-2016 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
44
@@ -182,7 +182,7 @@
182 """If the signature is nontrivial future-dated, it's not trusted."""182 """If the signature is nontrivial future-dated, it's not trusted."""
183183
184 signing_context = GPGSigningContext(184 signing_context = GPGSigningContext(
185 import_secret_test_key().fingerprint, password='test')185 import_secret_test_key(), password='test')
186 msg = self.factory.makeSignedMessage(signing_context=signing_context)186 msg = self.factory.makeSignedMessage(signing_context=signing_context)
187 # It's not trivial to make a gpg signature with a bogus timestamp, so187 # It's not trivial to make a gpg signature with a bogus timestamp, so
188 # let's just treat everything as invalid, and trust that the regular188 # let's just treat everything as invalid, and trust that the regular
189189
=== modified file 'lib/lp/services/mail/tests/test_signedmessage.py'
--- lib/lp/services/mail/tests/test_signedmessage.py 2015-03-13 19:05:50 +0000
+++ lib/lp/services/mail/tests/test_signedmessage.py 2016-03-23 18:12:33 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2011 Canonical Ltd. This software is licensed under the1# Copyright 2009-2016 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"""Test the SignedMessage class."""4"""Test the SignedMessage class."""
@@ -62,7 +62,7 @@
62 # Create a signed message for the sender specified with the test62 # Create a signed message for the sender specified with the test
63 # secret key.63 # secret key.
64 key = import_secret_test_key()64 key = import_secret_test_key()
65 signing_context = GPGSigningContext(key.fingerprint, password='test')65 signing_context = GPGSigningContext(key, password='test')
66 if body is None:66 if body is None:
67 body = dedent("""\67 body = dedent("""\
68 This is a multi-line body.68 This is a multi-line body.
@@ -136,7 +136,7 @@
136 gpghandler = getUtility(IGPGHandler)136 gpghandler = getUtility(IGPGHandler)
137 signature = gpghandler.signContent(137 signature = gpghandler.signContent(
138 canonicalise_line_endings(body_text.as_string()),138 canonicalise_line_endings(body_text.as_string()),
139 key.fingerprint, 'test', gpgme.SIG_MODE_DETACH)139 key, 'test', gpgme.SIG_MODE_DETACH)
140140
141 attachment = Message()141 attachment = Message()
142 attachment.set_payload(signature)142 attachment.set_payload(signature)
143143
=== modified file 'lib/lp/services/verification/model/logintoken.py'
--- lib/lp/services/verification/model/logintoken.py 2015-09-28 17:38:45 +0000
+++ lib/lp/services/verification/model/logintoken.py 2016-03-23 18:12:33 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2013 Canonical Ltd. This software is licensed under the1# Copyright 2009-2016 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__metaclass__ = type4__metaclass__ = type
@@ -167,8 +167,8 @@
167 # Encrypt this part's content if requested.167 # Encrypt this part's content if requested.
168 if key.can_encrypt:168 if key.can_encrypt:
169 gpghandler = getUtility(IGPGHandler)169 gpghandler = getUtility(IGPGHandler)
170 token_text = gpghandler.encryptContent(token_text.encode('utf-8'),170 token_text = gpghandler.encryptContent(
171 key.fingerprint)171 token_text.encode('utf-8'), key)
172 # In this case, we need to include some clear text instructions172 # In this case, we need to include some clear text instructions
173 # for people who do not have an MUA that can decrypt the ASCII173 # for people who do not have an MUA that can decrypt the ASCII
174 # armored text.174 # armored text.
175175
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2016-03-21 22:03:39 +0000
+++ lib/lp/testing/factory.py 2016-03-23 18:12:33 +0000
@@ -392,10 +392,10 @@
392392
393393
394class GPGSigningContext:394class GPGSigningContext:
395 """A helper object to hold the fingerprint, password and mode."""395 """A helper object to hold the key, password and mode."""
396396
397 def __init__(self, fingerprint, password='', mode=None):397 def __init__(self, key, password='', mode=None):
398 self.fingerprint = fingerprint398 self.key = key
399 self.password = password399 self.password = password
400 self.mode = mode400 self.mode = mode
401401
@@ -2158,8 +2158,8 @@
2158 if signing_context is not None:2158 if signing_context is not None:
2159 gpghandler = getUtility(IGPGHandler)2159 gpghandler = getUtility(IGPGHandler)
2160 body = gpghandler.signContent(2160 body = gpghandler.signContent(
2161 body, signing_context.fingerprint,2161 body, signing_context.key, signing_context.password,
2162 signing_context.password, signing_context.mode)2162 signing_context.mode)
2163 assert body is not None2163 assert body is not None
2164 if attachment_contents is None:2164 if attachment_contents is None:
2165 mail.set_payload(body)2165 mail.set_payload(body)