launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20154
[Merge] lp:~cjwatson/launchpad/faster-gpg-operations into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/faster-gpg-operations into lp:launchpad.
Commit message:
Sign/encrypt content based on an IPymeKey rather than a fingerprint, for performance.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/faster-gpg-operations/+merge/289950
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/faster-gpg-operations into lp:launchpad.
=== 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 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""ArchiveSigningKey implementation."""
@@ -23,10 +23,7 @@
)
from lp.registry.interfaces.gpg import IGPGKeySet
from lp.services.config import config
-from lp.services.gpg.interfaces import (
- GPGKeyAlgorithm,
- IGPGHandler,
- )
+from lp.services.gpg.interfaces import IGPGHandler
from lp.services.propertycache import get_property_cache
@@ -133,8 +130,7 @@
release_file_content = open(release_file_path).read()
signature = gpghandler.signContent(
- release_file_content, secret_key.fingerprint,
- mode=gpgme.SIG_MODE_DETACH)
+ release_file_content, secret_key, mode=gpgme.SIG_MODE_DETACH)
release_signature_file = open(
os.path.join(suite_path, 'Release.gpg'), 'w')
@@ -142,8 +138,7 @@
release_signature_file.close()
inline_release = gpghandler.signContent(
- release_file_content, secret_key.fingerprint,
- mode=gpgme.SIG_MODE_CLEAR)
+ release_file_content, secret_key, mode=gpgme.SIG_MODE_CLEAR)
inline_release_file = open(
os.path.join(suite_path, 'InRelease'), 'w')
=== 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 @@
-# Copyright 2010-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test MaloneHandler."""
@@ -682,7 +682,7 @@
# isn't too far in the future or past. This test shows that a
# signature with a timestamp of appxoimately now will be accepted.
signing_context = GPGSigningContext(
- import_secret_test_key().fingerprint, password='test')
+ import_secret_test_key(), password='test')
msg = self.factory.makeSignedMessage(
body=' security no', signing_context=signing_context)
handler = MaloneHandler()
=== 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 @@
>>> login(ANONYMOUS)
>>> key = import_secret_test_key('sign.only@xxxxxxxxxxxxxxxxx')
>>> bad = gpghandler.signContent(
- ... 'This is not the verification message!',
- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
+ ... 'This is not the verification message!', key, 'test')
>>> logout()
>>> browser.getControl('Signed text').value = bad
@@ -298,9 +297,7 @@
If they sign the text correctly, they are redirected to their home page.
>>> login(ANONYMOUS)
- >>> good = gpghandler.signContent(
- ... str(verification_content),
- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
+ >>> good = gpghandler.signContent(str(verification_content), key, 'test')
>>> logout()
>>> browser.getControl('Signed text').value = good
=== 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 @@
>>> login(ANONYMOUS)
>>> key = import_secret_test_key('sign.only@xxxxxxxxxxxxxxxxx')
>>> bad = gpghandler.signContent(
- ... 'This is not the verification message!',
- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
+ ... 'This is not the verification message!', key, 'test')
>>> logout()
>>> browser.getControl('Signed text').value = bad
@@ -283,9 +282,7 @@
If they sign the text correctly, they are redirected to their home page.
>>> login(ANONYMOUS)
- >>> good = gpghandler.signContent(
- ... str(verification_content),
- ... '447DBF38C4F9C4ED752246B77D88913717B05A8F', 'test')
+ >>> good = gpghandler.signContent(str(verification_content), key, 'test')
>>> logout()
>>> browser.getControl('Signed text').value = good
=== 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 @@
>>> fingerprint
u'A419AE861E88BC9E04B9C26FBA2B9389DFD20543'
- >>> cipher = gpghandler.encryptContent(content.encode('utf-8'),
- ... fingerprint)
+ >>> key = gpghandler.retrieveKey(fingerprint)
+ >>> cipher = gpghandler.encryptContent(content.encode('utf-8'), key)
cipher constains the encrypted content.
@@ -67,15 +67,14 @@
Verify if the encrytion process support passing another charset string
>>> content = u'a\xe7ucar'
- >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'),
- ... fingerprint)
+ >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'), key)
>>> plain = decrypt_content(cipher, 'test')
>>> plain.decode('iso-8859-1')
u'a\xe7ucar'
Let's try to pass unicode and see if it fails
- >>> cipher = gpghandler.encryptContent(content,fingerprint)
+ >>> cipher = gpghandler.encryptContent(content, key)
Traceback (most recent call last):
...
TypeError: Content cannot be Unicode.
@@ -83,8 +82,7 @@
Decrypt a unicode content:
>>> content = u'a\xe7ucar'
- >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'),
- ... fingerprint)
+ >>> cipher = gpghandler.encryptContent(content.encode('iso-8859-1'), key)
>>> cipher = unicode(cipher)
>>> plain = decrypt_content(cipher, 'test')
Traceback (most recent call last):
=== 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 @@
from gpgservice_client import GPGClient
from lazr.restful.utils import get_current_browser_request
from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
from lp.app.validators.email import valid_email
from lp.services.config import config
@@ -320,7 +321,7 @@
return key
- def encryptContent(self, content, fingerprint):
+ def encryptContent(self, content, key):
"""See IGPGHandler."""
if isinstance(content, unicode):
raise TypeError('Content cannot be Unicode.')
@@ -333,22 +334,21 @@
plain = StringIO(content)
cipher = StringIO()
- # retrive gpgme key object
- try:
- key = ctx.get_key(fingerprint.encode('ascii'), 0)
- except gpgme.GpgmeError:
+ if key.key is None:
return None
if not key.can_encrypt:
raise ValueError('key %s can not be used for encryption'
- % fingerprint)
+ % key.fingerprint)
# encrypt content
- ctx.encrypt([key], gpgme.ENCRYPT_ALWAYS_TRUST, plain, cipher)
+ ctx.encrypt(
+ [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
+ plain, cipher)
return cipher.getvalue()
- def signContent(self, content, key_fingerprint, password='', mode=None):
+ def signContent(self, content, key, password='', mode=None):
"""See IGPGHandler."""
if not isinstance(content, str):
raise TypeError('Content should be a string.')
@@ -361,8 +361,7 @@
context = gpgme.Context()
context.armor = True
- key = context.get_key(key_fingerprint.encode('ascii'), True)
- context.signers = [key]
+ context.signers = [removeSecurityProxy(key.key)]
# Set up containers.
plaintext = StringIO(content)
@@ -561,6 +560,7 @@
def _buildFromGpgmeKey(self, key):
self.exists_in_local_keyring = True
+ self.key = key
subkey = key.subkeys[0]
self.fingerprint = subkey.fpr
self.revoked = subkey.revoked
=== 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 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__all__ = [
@@ -290,24 +290,23 @@
:return: a `PymeKey` object for the just-generated secret key.
"""
- def encryptContent(content, fingerprint):
- """Encrypt the given content for the given fingerprint.
+ def encryptContent(content, key):
+ """Encrypt the given content for the given key.
content must be a traditional string. It's up to the caller to
- encode or decode properly. Fingerprint must be hexadecimal string.
+ encode or decode properly.
:param content: the Unicode content to be encrypted.
- :param fingerprint: the OpenPGP key's fingerprint.
+ :param key: the `IPymeKey` to encrypt the content for.
:return: the encrypted content or None if failed.
"""
- def signContent(content, key_fingerprint, password='', mode=None):
- """Signs content with a given GPG fingerprint.
+ def signContent(content, key, password='', mode=None):
+ """Signs content with a given GPG key.
:param content: the content to sign.
- :param key_fingerprint: the fingerprint of the key to use when
- signing the content.
+ :param key: the `IPymeKey` to use when signing the content.
:param password: optional password to the key identified by
key_fingerprint, the default value is '',
:param mode: optional type of GPG signature to produce, the
@@ -386,6 +385,7 @@
"""pyME key model."""
fingerprint = Attribute("Key Fingerprint")
+ key = Attribute("Underlying GpgmeKey object")
algorithm = Attribute("Key Algorithm")
revoked = Attribute("Key Revoked")
expired = Attribute("Key Expired")
=== 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 @@
secret_key = import_secret_test_key(key_name)
content = "abc\n"
signed_content = self.gpg_handler.signContent(
- content, secret_key.fingerprint, password)
+ content, secret_key, password)
signature = self.gpg_handler.getVerifiedSignature(signed_content)
self.assertEqual(content, signature.plain_data)
self.assertEqual(secret_key.fingerprint, signature.fingerprint)
=== 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 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
@@ -182,7 +182,7 @@
"""If the signature is nontrivial future-dated, it's not trusted."""
signing_context = GPGSigningContext(
- import_secret_test_key().fingerprint, password='test')
+ import_secret_test_key(), password='test')
msg = self.factory.makeSignedMessage(signing_context=signing_context)
# It's not trivial to make a gpg signature with a bogus timestamp, so
# let's just treat everything as invalid, and trust that the regular
=== 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 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the SignedMessage class."""
@@ -62,7 +62,7 @@
# Create a signed message for the sender specified with the test
# secret key.
key = import_secret_test_key()
- signing_context = GPGSigningContext(key.fingerprint, password='test')
+ signing_context = GPGSigningContext(key, password='test')
if body is None:
body = dedent("""\
This is a multi-line body.
@@ -136,7 +136,7 @@
gpghandler = getUtility(IGPGHandler)
signature = gpghandler.signContent(
canonicalise_line_endings(body_text.as_string()),
- key.fingerprint, 'test', gpgme.SIG_MODE_DETACH)
+ key, 'test', gpgme.SIG_MODE_DETACH)
attachment = Message()
attachment.set_payload(signature)
=== 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 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -167,8 +167,8 @@
# Encrypt this part's content if requested.
if key.can_encrypt:
gpghandler = getUtility(IGPGHandler)
- token_text = gpghandler.encryptContent(token_text.encode('utf-8'),
- key.fingerprint)
+ token_text = gpghandler.encryptContent(
+ token_text.encode('utf-8'), key)
# In this case, we need to include some clear text instructions
# for people who do not have an MUA that can decrypt the ASCII
# armored text.
=== 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 @@
class GPGSigningContext:
- """A helper object to hold the fingerprint, password and mode."""
+ """A helper object to hold the key, password and mode."""
- def __init__(self, fingerprint, password='', mode=None):
- self.fingerprint = fingerprint
+ def __init__(self, key, password='', mode=None):
+ self.key = key
self.password = password
self.mode = mode
@@ -2158,8 +2158,8 @@
if signing_context is not None:
gpghandler = getUtility(IGPGHandler)
body = gpghandler.signContent(
- body, signing_context.fingerprint,
- signing_context.password, signing_context.mode)
+ body, signing_context.key, signing_context.password,
+ signing_context.mode)
assert body is not None
if attachment_contents is None:
mail.set_payload(body)
Follow ups
