launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25274
[Merge] ~cjwatson/launchpad:archive-gpg-generate-using-signing-service into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:archive-gpg-generate-using-signing-service into launchpad:master.
Commit message:
Generate archive signing keys using the signing service
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/390668
This is guarded by the feature rule 'archivepublisher.gpg.signing_service.enabled'.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:archive-gpg-generate-using-signing-service into launchpad:master.
diff --git a/lib/lp/archivepublisher/archivegpgsigningkey.py b/lib/lp/archivepublisher/archivegpgsigningkey.py
index a342332..20a4d79 100644
--- a/lib/lp/archivepublisher/archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/archivegpgsigningkey.py
@@ -37,17 +37,24 @@ from lp.archivepublisher.run_parts import (
from lp.registry.interfaces.gpg import IGPGKeySet
from lp.services.config import config
from lp.services.features import getFeatureFlag
-from lp.services.gpg.interfaces import IGPGHandler
+from lp.services.gpg.interfaces import (
+ IGPGHandler,
+ IPymeKey,
+ )
from lp.services.osutils import remove_if_exists
from lp.services.propertycache import (
cachedproperty,
get_property_cache,
)
from lp.services.signing.enums import (
+ OpenPGPKeyAlgorithm,
SigningKeyType,
SigningMode,
)
-from lp.services.signing.interfaces.signingkey import ISigningKeySet
+from lp.services.signing.interfaces.signingkey import (
+ ISigningKey,
+ ISigningKeySet,
+ )
@implementer(ISignableArchive)
@@ -72,7 +79,7 @@ class SignableArchive:
def can_sign(self):
"""See `ISignableArchive`."""
return (
- self.archive.signing_key is not None or
+ self.archive.signing_key_fingerprint is not None or
self._run_parts_dir is not None)
@cachedproperty
@@ -237,9 +244,9 @@ class ArchiveGPGSigningKey(SignableArchive):
with open(export_path, 'wb') as export_file:
export_file.write(key.export())
- def generateSigningKey(self, log=None):
+ def generateSigningKey(self, log=None, async_keyserver=False):
"""See `IArchiveGPGSigningKey`."""
- assert self.archive.signing_key is None, (
+ assert self.archive.signing_key_fingerprint is None, (
"Cannot override signing_keys.")
# Always generate signing keys for the default PPA, even if it
@@ -257,13 +264,26 @@ class ArchiveGPGSigningKey(SignableArchive):
key_displayname = (
"Launchpad PPA for %s" % self.archive.owner.displayname)
- secret_key = getUtility(IGPGHandler).generateKey(
- key_displayname, logger=log)
- self._setupSigningKey(secret_key)
+ if getFeatureFlag(PUBLISHER_GPG_USES_SIGNING_SERVICE):
+ try:
+ signing_key = getUtility(ISigningKeySet).generate(
+ SigningKeyType.OPENPGP, key_displayname,
+ openpgp_key_algorithm=OpenPGPKeyAlgorithm.RSA, length=4096)
+ except Exception as e:
+ if log is not None:
+ log.exception(
+ "Error generating signing key for %s: %s %s" %
+ (self.archive.reference, e.__class__.__name__, e))
+ raise
+ else:
+ signing_key = getUtility(IGPGHandler).generateKey(
+ key_displayname, logger=log)
+ return self._setupSigningKey(
+ signing_key, async_keyserver=async_keyserver)
def setSigningKey(self, key_path, async_keyserver=False):
"""See `IArchiveGPGSigningKey`."""
- assert self.archive.signing_key is None, (
+ assert self.archive.signing_key_fingerprint is None, (
"Cannot override signing_keys.")
assert os.path.exists(key_path), (
"%s does not exist" % key_path)
@@ -274,34 +294,46 @@ class ArchiveGPGSigningKey(SignableArchive):
return self._setupSigningKey(
secret_key, async_keyserver=async_keyserver)
- def _uploadPublicSigningKey(self, secret_key):
+ def _uploadPublicSigningKey(self, signing_key):
"""Upload the public half of a signing key to the keyserver."""
# The handler's security proxying doesn't protect anything useful
# here, and when we're running in a thread we don't have an
# interaction.
gpghandler = removeSecurityProxy(getUtility(IGPGHandler))
- pub_key = gpghandler.retrieveKey(secret_key.fingerprint)
- gpghandler.uploadPublicKey(pub_key.fingerprint)
- return pub_key
+ if IPymeKey.providedBy(signing_key):
+ pub_key = gpghandler.retrieveKey(signing_key.fingerprint)
+ gpghandler.uploadPublicKey(pub_key.fingerprint)
+ return pub_key
+ else:
+ assert ISigningKey.providedBy(signing_key)
+ gpghandler.submitKey(removeSecurityProxy(signing_key).public_key)
+ return signing_key
def _storeSigningKey(self, pub_key):
"""Store signing key reference in the database."""
key_owner = getUtility(ILaunchpadCelebrities).ppa_key_guard
- key, _ = getUtility(IGPGKeySet).activate(
- key_owner, pub_key, pub_key.can_encrypt)
- self.archive.signing_key_owner = key.owner
+ if IPymeKey.providedBy(pub_key):
+ key, _ = getUtility(IGPGKeySet).activate(
+ key_owner, pub_key, pub_key.can_encrypt)
+ else:
+ assert ISigningKey.providedBy(pub_key)
+ key = pub_key
+ self.archive.signing_key_owner = key_owner
self.archive.signing_key_fingerprint = key.fingerprint
del get_property_cache(self.archive).signing_key
- def _setupSigningKey(self, secret_key, async_keyserver=False):
+ def _setupSigningKey(self, signing_key, async_keyserver=False):
"""Mandatory setup for signing keys.
- * Export the secret key into the protected disk location.
+ * Export the secret key into the protected disk location (for
+ locally-generated keys).
* Upload public key to the keyserver.
- * Store the public GPGKey reference in the database and update
- the context archive.signing_key.
+ * Store the public GPGKey reference in the database (for
+ locally-generated keys) and update the context
+ archive.signing_key.
"""
- self.exportSecretKey(secret_key)
+ if IPymeKey.providedBy(signing_key):
+ self.exportSecretKey(signing_key)
if async_keyserver:
# If we have an asynchronous keyserver running in the current
# thread using Twisted, then we need some contortions to ensure
@@ -310,10 +342,10 @@ class ArchiveGPGSigningKey(SignableArchive):
# Since that thread won't have a Zope interaction, we need to
# unwrap the security proxy for it.
d = deferToThread(
- self._uploadPublicSigningKey, removeSecurityProxy(secret_key))
+ self._uploadPublicSigningKey, removeSecurityProxy(signing_key))
d.addCallback(ProxyFactory)
d.addCallback(self._storeSigningKey)
return d
else:
- pub_key = self._uploadPublicSigningKey(secret_key)
+ pub_key = self._uploadPublicSigningKey(signing_key)
self._storeSigningKey(pub_key)
diff --git a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
index 87359e8..67366bd 100644
--- a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
@@ -14,8 +14,15 @@ from testtools.matchers import (
FileContains,
StartsWith,
)
-from testtools.twistedsupport import AsynchronousDeferredRunTest
-from twisted.internet import defer
+from testtools.twistedsupport import (
+ AsynchronousDeferredRunTest,
+ AsynchronousDeferredRunTestForBrokenTwisted,
+ )
+import treq
+from twisted.internet import (
+ defer,
+ reactor,
+ )
from zope.component import getUtility
from lp.archivepublisher.config import getPubConfig
@@ -26,18 +33,27 @@ from lp.archivepublisher.interfaces.archivegpgsigningkey import (
)
from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
+from lp.registry.interfaces.gpg import IGPGKeySet
from lp.services.compat import mock
from lp.services.features.testing import FeatureFixture
+from lp.services.gpg.interfaces import IGPGHandler
+from lp.services.gpg.tests.test_gpghandler import FakeGenerateKey
from lp.services.log.logger import BufferLogger
from lp.services.osutils import write_file
from lp.services.signing.enums import (
SigningKeyType,
SigningMode,
)
+from lp.services.signing.interfaces.signingkey import ISigningKeySet
from lp.services.signing.tests.helpers import SigningServiceClientFixture
+from lp.services.twistedsupport.testing import TReqFixture
+from lp.services.twistedsupport.treq import check_status
from lp.soyuz.enums import ArchivePurpose
from lp.testing import TestCaseWithFactory
-from lp.testing.gpgkeys import gpgkeysdir
+from lp.testing.gpgkeys import (
+ gpgkeysdir,
+ test_pubkey_from_email,
+ )
from lp.testing.keyserver import InProcessKeyServerFixture
from lp.testing.layers import ZopelessDatabaseLayer
@@ -271,3 +287,89 @@ class TestSignableArchiveWithRunParts(RunPartsMixin, TestCaseWithFactory):
FileContains(
"detached signature of %s (%s, %s/%s)\n" %
(filename, self.archive_root, self.distro.name, self.suite)))
+
+
+class TestArchiveGPGSigningKey(TestCaseWithFactory):
+
+ layer = ZopelessDatabaseLayer
+ # treq.content doesn't close the connection before yielding control back
+ # to the test, so we need to spin the reactor at the end to finish
+ # things off.
+ run_tests_with = AsynchronousDeferredRunTestForBrokenTwisted.make_factory(
+ timeout=10000)
+
+ @defer.inlineCallbacks
+ def setUp(self):
+ super(TestArchiveGPGSigningKey, self).setUp()
+ self.temp_dir = self.makeTemporaryDirectory()
+ self.pushConfig("personalpackagearchive", root=self.temp_dir)
+ self.keyserver = self.useFixture(InProcessKeyServerFixture())
+ yield self.keyserver.start()
+
+ @defer.inlineCallbacks
+ def test_generateSigningKey_local(self):
+ # Generating a signing key locally using GPGHandler stores it in the
+ # database and pushes it to the keyserver.
+ self.useFixture(FakeGenerateKey("ppa-sample@xxxxxxxxxxxxxxxxx"))
+ logger = BufferLogger()
+ # Use a display name that matches the pregenerated sample key.
+ owner = self.factory.makePerson(
+ displayname="Celso \xe1\xe9\xed\xf3\xfa Providelo")
+ archive = self.factory.makeArchive(owner=owner)
+ yield IArchiveGPGSigningKey(archive).generateSigningKey(
+ log=logger, async_keyserver=True)
+ # The key is stored in the database.
+ self.assertIsNotNone(archive.signing_key_owner)
+ self.assertIsNotNone(archive.signing_key_fingerprint)
+ # The key is stored as a GPGKey, not a SigningKey.
+ self.assertIsNotNone(
+ getUtility(IGPGKeySet).getByFingerprint(
+ archive.signing_key_fingerprint))
+ self.assertIsNone(
+ getUtility(ISigningKeySet).get(
+ SigningKeyType.OPENPGP, archive.signing_key_fingerprint))
+ # The key is uploaded to the keyserver.
+ client = self.useFixture(TReqFixture(reactor)).client
+ response = yield client.get(
+ getUtility(IGPGHandler).getURLForKeyInServer(
+ archive.signing_key_fingerprint, "get"))
+ yield check_status(response)
+ content = yield treq.content(response)
+ self.assertIn(b"-----BEGIN PGP PUBLIC KEY BLOCK-----\n", content)
+
+ @defer.inlineCallbacks
+ def test_generateSigningKey_signing_service(self):
+ # Generating a signing key on the signing service stores it in the
+ # database and pushes it to the keyserver.
+ self.useFixture(
+ FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"}))
+ signing_service_client = self.useFixture(
+ SigningServiceClientFixture(self.factory))
+ signing_service_client.generate.side_effect = None
+ test_key = test_pubkey_from_email("ftpmaster@xxxxxxxxxxxxx")
+ signing_service_client.generate.return_value = {
+ "fingerprint": "33C0A61893A5DC5EB325B29E415A12CAC2F30234",
+ "public-key": test_key,
+ }
+ logger = BufferLogger()
+ archive = self.factory.makeArchive()
+ yield IArchiveGPGSigningKey(archive).generateSigningKey(
+ log=logger, async_keyserver=True)
+ # The key is stored in the database.
+ self.assertIsNotNone(archive.signing_key_owner)
+ self.assertIsNotNone(archive.signing_key_fingerprint)
+ # The key is stored as a SigningKey, not a GPGKey.
+ self.assertIsNone(
+ getUtility(IGPGKeySet).getByFingerprint(
+ archive.signing_key_fingerprint))
+ signing_key = getUtility(ISigningKeySet).get(
+ SigningKeyType.OPENPGP, archive.signing_key_fingerprint)
+ self.assertEqual(test_key, signing_key.public_key)
+ # The key is uploaded to the keyserver.
+ client = self.useFixture(TReqFixture(reactor)).client
+ response = yield client.get(
+ getUtility(IGPGHandler).getURLForKeyInServer(
+ archive.signing_key_fingerprint, "get"))
+ yield check_status(response)
+ content = yield treq.content(response)
+ self.assertIn(test_key, content)
diff --git a/lib/lp/services/gpg/handler.py b/lib/lp/services/gpg/handler.py
index b2bcbad..e3eba45 100644
--- a/lib/lp/services/gpg/handler.py
+++ b/lib/lp/services/gpg/handler.py
@@ -489,12 +489,8 @@ class GPGHandler:
raise GPGKeyExpired(key)
return key
- def _submitKey(self, content):
- """Submit an ASCII-armored public key export to the keyserver.
-
- It issues a POST at /pks/add on the keyserver specified in the
- configuration.
- """
+ def submitKey(self, content):
+ """See `IGPGHandler`."""
keyserver_http_url = '%s:%s' % (
config.gpghandler.host, config.gpghandler.port)
@@ -527,7 +523,7 @@ class GPGHandler:
return
pub_key = self.retrieveKey(fingerprint)
- self._submitKey(pub_key.export())
+ self.submitKey(pub_key.export())
def getURLForKeyInServer(self, fingerprint, action='index', public=False):
"""See IGPGHandler"""
diff --git a/lib/lp/services/gpg/interfaces.py b/lib/lp/services/gpg/interfaces.py
index 78b44c8..d6f0f73 100644
--- a/lib/lp/services/gpg/interfaces.py
+++ b/lib/lp/services/gpg/interfaces.py
@@ -357,6 +357,17 @@ class IGPGHandler(Interface):
:return: a `PymeKey`object containing the key information.
"""
+ def submitKey(content):
+ """Submit an ASCII-armored public key export to the keyserver.
+
+ It issues a POST at /pks/add on the keyserver specified in the
+ configuration.
+
+ :param content: The exported public key, as a byte string.
+ :raise GPGUploadFailure: if the keyserver could not be reached.
+ :raise AssertionError: if the POST request failed.
+ """
+
def uploadPublicKey(fingerprint):
"""Upload the specified public key to a keyserver.
@@ -365,8 +376,8 @@ class IGPGHandler(Interface):
:param fingerprint: The key fingerprint, which must be an hexadecimal
string.
- :raise GPGUploadFailure: if the keyserver could not be reaches.
- :raise AssertionError: if the POST request doesn't succeed.
+ :raise GPGUploadFailure: if the keyserver could not be reached.
+ :raise AssertionError: if the POST request failed.
"""
def localKeys(filter=None, secret=False):
diff --git a/lib/lp/services/signing/tests/helpers.py b/lib/lp/services/signing/tests/helpers.py
index f819745..6831edb 100644
--- a/lib/lp/services/signing/tests/helpers.py
+++ b/lib/lp/services/signing/tests/helpers.py
@@ -49,7 +49,7 @@ class SigningServiceClientFixture(fixtures.Fixture):
openpgp_key_algorithm=None, length=None):
key = bytes(PrivateKey.generate().public_key)
data = {
- "fingerprint": self.factory.getUniqueHexString(40),
+ "fingerprint": self.factory.getUniqueHexString(40).upper(),
"public-key": key,
}
self.generate_returns.append((key_type, data))
@@ -69,7 +69,7 @@ class SigningServiceClientFixture(fixtures.Fixture):
def _inject(self, key_type, private_key, public_key, description,
created_at):
- data = {'fingerprint': self.factory.getUniqueHexString(40)}
+ data = {'fingerprint': self.factory.getUniqueHexString(40).upper()}
self.inject_returns.append(data)
return data
diff --git a/lib/lp/soyuz/configure.zcml b/lib/lp/soyuz/configure.zcml
index 2e97958..643a85b 100644
--- a/lib/lp/soyuz/configure.zcml
+++ b/lib/lp/soyuz/configure.zcml
@@ -373,7 +373,6 @@
set_schema="lp.soyuz.interfaces.archive.IArchiveRestricted"/>
<require
permission="launchpad.InternalScriptsOnly"
- attributes="signing_key_owner"
set_attributes="dirty_suites distribution signing_key_owner
signing_key_fingerprint"/>
</class>
diff --git a/lib/lp/soyuz/interfaces/archive.py b/lib/lp/soyuz/interfaces/archive.py
index 407f953..b6cc663 100644
--- a/lib/lp/soyuz/interfaces/archive.py
+++ b/lib/lp/soyuz/interfaces/archive.py
@@ -461,6 +461,8 @@ class IArchiveSubscriberView(Interface):
"explicit publish flag and any other constraints."))
series_with_sources = Attribute(
"DistroSeries to which this archive has published sources")
+ signing_key_owner = Reference(
+ title=_("Archive signing key owner"), required=False, schema=IPerson)
signing_key_fingerprint = exported(
Text(
title=_("Archive signing key fingerprint"), required=False,
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index a37b737..0d58e56 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -466,7 +466,7 @@ class Archive(SQLBase):
return (
not config.personalpackagearchive.require_signing_keys or
not self.is_ppa or
- self.signing_key is not None)
+ self.signing_key_fingerprint is not None)
@property
def reference(self):
@@ -2717,10 +2717,12 @@ class ArchiveSet:
(owner.name, distribution.name, name))
# Signing-key for the default PPA is reused when it's already present.
- signing_key = None
+ signing_key_owner = None
+ signing_key_fingerprint = None
if purpose == ArchivePurpose.PPA:
if owner.archive is not None:
- signing_key = owner.archive.signing_key
+ signing_key_owner = owner.archive.signing_key_owner
+ signing_key_fingerprint = owner.archive.signing_key_fingerprint
else:
# owner.archive is a cached property and we've just cached it.
del get_property_cache(owner).archive
@@ -2729,9 +2731,8 @@ class ArchiveSet:
owner=owner, distribution=distribution, name=name,
displayname=displayname, description=description,
purpose=purpose, publish=publish,
- signing_key_owner=signing_key.owner if signing_key else None,
- signing_key_fingerprint=(
- signing_key.fingerprint if signing_key else None),
+ signing_key_owner=signing_key_owner,
+ signing_key_fingerprint=signing_key_fingerprint,
require_virtualized=require_virtualized)
# Upon creation archives are enabled by default.
diff --git a/lib/lp/soyuz/scripts/ppakeygenerator.py b/lib/lp/soyuz/scripts/ppakeygenerator.py
index 190b4a0..88e1d84 100644
--- a/lib/lp/soyuz/scripts/ppakeygenerator.py
+++ b/lib/lp/soyuz/scripts/ppakeygenerator.py
@@ -34,7 +34,7 @@ class PPAKeyGenerator(LaunchpadCronScript):
(archive.reference, archive.displayname))
archive_signing_key = IArchiveGPGSigningKey(archive)
archive_signing_key.generateSigningKey(log=self.logger)
- self.logger.info("Key %s" % archive.signing_key.fingerprint)
+ self.logger.info("Key %s" % archive.signing_key_fingerprint)
def main(self):
"""Generate signing keys for the selected PPAs."""
@@ -45,11 +45,11 @@ class PPAKeyGenerator(LaunchpadCronScript):
raise LaunchpadScriptFailure(
"No archive named '%s' could be found."
% self.options.archive)
- if archive.signing_key is not None:
+ if archive.signing_key_fingerprint is not None:
raise LaunchpadScriptFailure(
"%s (%s) already has a signing_key (%s)"
% (archive.reference, archive.displayname,
- archive.signing_key.fingerprint))
+ archive.signing_key_fingerprint))
archives = [archive]
else:
archive_set = getUtility(IArchiveSet)
diff --git a/lib/lp/soyuz/scripts/tests/test_ppakeygenerator.py b/lib/lp/soyuz/scripts/tests/test_ppakeygenerator.py
index 56e8710..a5d3caf 100644
--- a/lib/lp/soyuz/scripts/tests/test_ppakeygenerator.py
+++ b/lib/lp/soyuz/scripts/tests/test_ppakeygenerator.py
@@ -83,7 +83,7 @@ class TestPPAKeyGenerator(TestCase):
LaunchpadScriptFailure,
("~cprov/ubuntu/ppa (PPA for Celso Providelo) already has a "
"signing_key (%s)" %
- cprov.archive.signing_key.fingerprint),
+ cprov.archive.signing_key_fingerprint),
key_generator.main)
def testGenerateKeyForASinglePPA(self):
@@ -95,14 +95,14 @@ class TestPPAKeyGenerator(TestCase):
cprov = getUtility(IPersonSet).getByName('cprov')
self._fixArchiveForKeyGeneration(cprov.archive)
- self.assertTrue(cprov.archive.signing_key is None)
+ self.assertIsNone(cprov.archive.signing_key_fingerprint)
txn = FakeTransaction()
key_generator = self._getKeyGenerator(
archive_reference='~cprov/ubuntutest/ppa', txn=txn)
key_generator.main()
- self.assertTrue(cprov.archive.signing_key is not None)
+ self.assertIsNotNone(cprov.archive.signing_key_fingerprint)
self.assertEqual(txn.commit_count, 1)
def testGenerateKeyForAllPPA(self):
@@ -115,13 +115,13 @@ class TestPPAKeyGenerator(TestCase):
for archive in archives:
self._fixArchiveForKeyGeneration(archive)
- self.assertTrue(archive.signing_key is None)
+ self.assertIsNone(archive.signing_key_fingerprint)
txn = FakeTransaction()
key_generator = self._getKeyGenerator(txn=txn)
key_generator.main()
for archive in archives:
- self.assertTrue(archive.signing_key is not None)
+ self.assertIsNotNone(archive.signing_key_fingerprint)
self.assertEqual(txn.commit_count, len(archives))
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index 91acf53..5c3f3b6 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -4046,7 +4046,7 @@ class TestSigningKeyPropagation(TestCaseWithFactory):
def test_ppa_created_with_no_signing_key(self):
ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
- self.assertIsNone(ppa.signing_key)
+ self.assertIsNone(ppa.signing_key_fingerprint)
def test_default_signing_key_propagated_to_new_ppa(self):
person = self.factory.makePerson()
