launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25533
[Merge] ~cjwatson/launchpad:stop-ppa-key-propagation into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:stop-ppa-key-propagation into launchpad:master.
Commit message:
Stop propagating signing keys between an owner's PPAs
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1700167 in Launchpad itself: "new PPAs are re-using old 1024-bit RSA signing keys"
https://bugs.launchpad.net/launchpad/+bug/1700167
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/392627
Things were perhaps different in 2009 when this feature was designed, but add-apt-repository has dealt with fetching keys on a per-archive basis for a long time now, and it makes more sense for keys to be per-archive. This also improves behaviour for users whose default archive was created long enough ago that it has a 1024-bit signing key.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stop-ppa-key-propagation into launchpad:master.
diff --git a/lib/lp/archivepublisher/archivegpgsigningkey.py b/lib/lp/archivepublisher/archivegpgsigningkey.py
index 4c3688d..2c09afa 100644
--- a/lib/lp/archivepublisher/archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/archivegpgsigningkey.py
@@ -14,7 +14,6 @@ __all__ = [
import os
import gpgme
-from twisted.internet import defer
from twisted.internet.threads import deferToThread
from zope.component import getUtility
from zope.interface import implementer
@@ -250,32 +249,6 @@ class ArchiveGPGSigningKey(SignableArchive):
assert self.archive.signing_key_fingerprint is None, (
"Cannot override signing_keys.")
- # Always generate signing keys for the default PPA, even if it
- # was not specifically requested. The default PPA signing key
- # is then propagated to the context named-ppa.
- default_ppa = self.archive.owner.archive
- if self.archive != default_ppa:
- def propagate_key(_):
- self.archive.signing_key_owner = default_ppa.signing_key_owner
- self.archive.signing_key_fingerprint = (
- default_ppa.signing_key_fingerprint)
- del get_property_cache(self.archive).signing_key
-
- if default_ppa.signing_key_fingerprint is None:
- d = IArchiveGPGSigningKey(default_ppa).generateSigningKey(
- log=log, async_keyserver=async_keyserver)
- else:
- d = defer.succeed(None)
- # generateSigningKey is only asynchronous if async_keyserver is
- # true; we need some contortions to keep it synchronous
- # otherwise.
- if async_keyserver:
- d.addCallback(propagate_key)
- return d
- else:
- propagate_key(None)
- return
-
key_displayname = (
"Launchpad PPA for %s" % self.archive.owner.displayname)
if getFeatureFlag(PUBLISHER_GPG_USES_SIGNING_SERVICE):
diff --git a/lib/lp/archivepublisher/tests/archive-signing.txt b/lib/lp/archivepublisher/tests/archive-signing.txt
index a487a3b..7e06104 100644
--- a/lib/lp/archivepublisher/tests/archive-signing.txt
+++ b/lib/lp/archivepublisher/tests/archive-signing.txt
@@ -266,72 +266,24 @@ key is available in the keyserver.
>>> retrieved_key.fingerprint == signing_key.fingerprint
True
-As documented in archive.txt, when a named-ppa is created it is
-already configured to used the same signing-key created for the
-default PPA. We will create a named-ppa for Celso.
+We will create a named PPA for Celso.
>>> from lp.soyuz.enums import ArchivePurpose
>>> named_ppa = getUtility(IArchiveSet).new(
... owner=cprov, purpose=ArchivePurpose.PPA, name='boing')
-As expected it will use the same key used in Celso's default PPA.
+It initially has no signing key.
- >>> print cprov.archive.signing_key.fingerprint
+ >>> print(cprov.archive.signing_key_fingerprint)
0D57E99656BEFB0897606EE9A022DD1F5001B46D
- >>> print named_ppa.signing_key.fingerprint
- 0D57E99656BEFB0897606EE9A022DD1F5001B46D
-
-We will reset the signing key of the just created named PPA,
-simulating the situation when a the default PPA and a named-ppas get
-created within the same cycle of the key-generator process.
-
- >>> from lp.services.propertycache import get_property_cache
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> named_ppa.signing_key_owner = None
- >>> named_ppa.signing_key_fingerprint = None
- >>> del get_property_cache(named_ppa).signing_key
- >>> login(ANONYMOUS)
-
- >>> print named_ppa.signing_key
+ >>> print(named_ppa.signing_key_fingerprint)
None
Default PPAs are always created first and thus get their keys generated
-before the named-ppa for the same owner. We submit the named-ppa to
-the key generation procedure, as it would be normally in production.
-
- >>> named_ppa_signing_key = IArchiveGPGSigningKey(named_ppa)
- >>> named_ppa_signing_key.generateSigningKey()
-
-Instead of generating a new key, the signing key from the default ppa
-(Celso's default PPA) gets reused.
-
- >>> print cprov.archive.signing_key.fingerprint
- 0D57E99656BEFB0897606EE9A022DD1F5001B46D
-
- >>> print named_ppa.signing_key.fingerprint
- 0D57E99656BEFB0897606EE9A022DD1F5001B46D
-
-We will reset the signing-keys for both PPA of Celso.
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
- >>> cprov.archive.signing_key_owner = None
- >>> cprov.archive.signing_key_fingerprint = None
- >>> del get_property_cache(cprov.archive).signing_key
- >>> named_ppa.signing_key_owner = None
- >>> named_ppa.signing_key_fingerprint = None
- >>> del get_property_cache(named_ppa).signing_key
- >>> login(ANONYMOUS)
-
- >>> print cprov.archive.signing_key
- None
-
- >>> print named_ppa.signing_key
- None
-
-Then modify the GPGHandler utility to return a sampledata key instead
-of generating a new one, mainly for running the test faster and for
-printing the context the key is generated.
+before the named PPA for the same owner. As before, generating a real key
+is slow, so we modify the GPGHandler utility to return a sampledata key
+instead.
>>> def mock_key_generator(name, logger=None):
... print 'Generating:', name
@@ -343,19 +295,18 @@ printing the context the key is generated.
>>> real_key_generator = naked_gpghandler.generateKey
>>> naked_gpghandler.generateKey = mock_key_generator
-When the signing key for the named-ppa is requested, it is generated
-in the default PPA context then propagated to the named-ppa. The key is
-named after the user, even if the default PPA name is something different.
+The key is named after the user, even if the default PPA name is something
+different.
>>> cprov.display_name = "Not Celso Providelo"
>>> named_ppa_signing_key = IArchiveGPGSigningKey(named_ppa)
>>> named_ppa_signing_key.generateSigningKey()
Generating: Launchpad PPA for Not Celso Providelo
- >>> print cprov.archive.signing_key.fingerprint
- 447DBF38C4F9C4ED752246B77D88913717B05A8F
+ >>> print(cprov.archive.signing_key_fingerprint)
+ 0D57E99656BEFB0897606EE9A022DD1F5001B46D
- >>> print named_ppa.signing_key.fingerprint
+ >>> print(named_ppa.signing_key_fingerprint)
447DBF38C4F9C4ED752246B77D88913717B05A8F
Restore the original functionality of GPGHandler.
@@ -371,6 +322,7 @@ for archive which already contains a 'signing_key'.
Celso's default PPA will uses the testing signing key.
+ >>> from lp.services.propertycache import get_property_cache
>>> login('foo.bar@xxxxxxxxxxxxx')
>>> cprov.archive.signing_key_owner = signing_key.owner
>>> cprov.archive.signing_key_fingerprint = signing_key.fingerprint
diff --git a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
index b521e85..8fefb07 100644
--- a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
@@ -345,8 +345,9 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
@defer.inlineCallbacks
def test_generateSigningKey_local_non_default_ppa(self):
# Generating a signing key locally using GPGHandler for a
- # non-default PPA generates one for the user's default PPA first and
- # then propagates it.
+ # non-default PPA ignores the user's default PPA. (It used to
+ # generate one for the user's default PPA first and then propagate
+ # it.)
self.useFixture(FakeGenerateKey("ppa-sample@xxxxxxxxxxxxxxxxx"))
logger = BufferLogger()
# Use a display name that matches the pregenerated sample key.
@@ -357,19 +358,19 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
yield IArchiveGPGSigningKey(another_ppa).generateSigningKey(
log=logger, async_keyserver=True)
self.assertThat(default_ppa, MatchesStructure(
+ signing_key=Is(None),
+ signing_key_owner=Is(None),
+ signing_key_fingerprint=Is(None)))
+ self.assertThat(another_ppa, MatchesStructure(
signing_key=Not(Is(None)),
signing_key_owner=Not(Is(None)),
signing_key_fingerprint=Not(Is(None))))
self.assertIsNotNone(
getUtility(IGPGKeySet).getByFingerprint(
- default_ppa.signing_key_fingerprint))
+ another_ppa.signing_key_fingerprint))
self.assertIsNone(
getUtility(ISigningKeySet).get(
- SigningKeyType.OPENPGP, default_ppa.signing_key_fingerprint))
- self.assertThat(another_ppa, MatchesStructure.byEquality(
- signing_key=default_ppa.signing_key,
- signing_key_owner=default_ppa.signing_key_owner,
- signing_key_fingerprint=default_ppa.signing_key_fingerprint))
+ SigningKeyType.OPENPGP, another_ppa.signing_key_fingerprint))
@defer.inlineCallbacks
def test_generateSigningKey_signing_service(self):
@@ -411,8 +412,8 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
@defer.inlineCallbacks
def test_generateSigningKey_signing_service_non_default_ppa(self):
# Generating a signing key on the signing service for a non-default
- # PPA generates one for the user's default PPA first and then
- # propagates it.
+ # PPA ignores the user's default PPA. (It used to generate one for
+ # the user's default PPA first and then propagate it.)
self.useFixture(
FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"}))
signing_service_client = self.useFixture(
@@ -430,16 +431,15 @@ class TestArchiveGPGSigningKey(TestCaseWithFactory):
log=logger, async_keyserver=True)
self.assertThat(default_ppa, MatchesStructure(
signing_key=Is(None),
+ signing_key_owner=Is(None),
+ signing_key_fingerprint=Is(None)))
+ self.assertThat(another_ppa, MatchesStructure(
+ signing_key=Is(None),
signing_key_owner=Not(Is(None)),
signing_key_fingerprint=Not(Is(None))))
self.assertIsNone(
getUtility(IGPGKeySet).getByFingerprint(
- default_ppa.signing_key_fingerprint))
+ another_ppa.signing_key_fingerprint))
signing_key = getUtility(ISigningKeySet).get(
- SigningKeyType.OPENPGP, default_ppa.signing_key_fingerprint)
+ SigningKeyType.OPENPGP, another_ppa.signing_key_fingerprint)
self.assertEqual(test_key, signing_key.public_key)
- self.assertThat(another_ppa, MatchesStructure(
- signing_key=Is(None),
- signing_key_owner=Equals(default_ppa.signing_key_owner),
- signing_key_fingerprint=Equals(
- default_ppa.signing_key_fingerprint)))
diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index bbe2d7c..a23a30d 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -2713,16 +2713,8 @@ class ArchiveSet:
"Person '%s' already has a PPA for %s named '%s'." %
(owner.name, distribution.name, name))
- # Signing-key for the default PPA is reused when it's already present.
signing_key_owner = None
signing_key_fingerprint = None
- if purpose == ArchivePurpose.PPA:
- if owner.archive is not None:
- 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
new_archive = Archive(
owner=owner, distribution=distribution, name=name,
diff --git a/lib/lp/soyuz/stories/webservice/xx-archive.txt b/lib/lp/soyuz/stories/webservice/xx-archive.txt
index 8e2ddc6..74fba4e 100644
--- a/lib/lp/soyuz/stories/webservice/xx-archive.txt
+++ b/lib/lp/soyuz/stories/webservice/xx-archive.txt
@@ -1226,7 +1226,7 @@ the IArchive context, in this case only Celso has it.
require_virtualized: True
resource_type_link: 'http://.../#archive'
self_link: 'http://.../~cprov/+archive/ubuntu/p3a'
- signing_key_fingerprint: 'ABCDEF0123456789ABCDDCBA0000111112345678'
+ signing_key_fingerprint: None
suppress_subscription_notifications: False
web_link: 'http://launchpad.../~cprov/+archive/ubuntu/p3a'
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index 5c3f3b6..a59a3fe 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -64,10 +64,7 @@ from lp.services.gpg.interfaces import (
IGPGHandler,
)
from lp.services.job.interfaces.job import JobStatus
-from lp.services.propertycache import (
- clear_property_cache,
- get_property_cache,
- )
+from lp.services.propertycache import clear_property_cache
from lp.services.timeout import default_timeout
from lp.services.webapp.interfaces import OAuthPermission
from lp.services.worlddata.interfaces.country import ICountrySet
@@ -1869,13 +1866,15 @@ class TestArchiveDependencies(TestCaseWithFactory):
def test_private_sources_list(self):
"""Entries for private dependencies include credentials."""
p3a = self.factory.makeArchive(name='p3a', private=True)
+ dependency = self.factory.makeArchive(
+ name='dependency', private=True, owner=p3a.owner)
with InProcessKeyServerFixture() as keyserver:
yield keyserver.start()
key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
yield IArchiveGPGSigningKey(p3a).setSigningKey(
key_path, async_keyserver=True)
- dependency = self.factory.makeArchive(
- name='dependency', private=True, owner=p3a.owner)
+ yield IArchiveGPGSigningKey(dependency).setSigningKey(
+ key_path, async_keyserver=True)
with person_logged_in(p3a.owner):
bpph = self.factory.makeBinaryPackagePublishingHistory(
archive=dependency, status=PackagePublishingStatus.PUBLISHED,
@@ -4039,31 +4038,6 @@ class TestDisplayName(TestCaseWithFactory):
archive.displayname = "My testing packages"
-class TestSigningKeyPropagation(TestCaseWithFactory):
- """Signing keys are shared between PPAs owned by the same person/team."""
-
- layer = DatabaseFunctionalLayer
-
- def test_ppa_created_with_no_signing_key(self):
- ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
- self.assertIsNone(ppa.signing_key_fingerprint)
-
- def test_default_signing_key_propagated_to_new_ppa(self):
- person = self.factory.makePerson()
- ppa = self.factory.makeArchive(
- owner=person, purpose=ArchivePurpose.PPA, name="ppa")
- self.assertEqual(ppa, person.archive)
- self.factory.makeGPGKey(person)
- key = person.gpg_keys[0]
- removeSecurityProxy(person.archive).signing_key_owner = key.owner
- removeSecurityProxy(person.archive).signing_key_fingerprint = (
- key.fingerprint)
- del get_property_cache(person.archive).signing_key
- ppa_with_key = self.factory.makeArchive(
- owner=person, purpose=ArchivePurpose.PPA)
- self.assertEqual(person.gpg_keys[0], ppa_with_key.signing_key)
-
-
class TestGetSigningKeyData(TestCaseWithFactory):
"""Test `Archive.getSigningKeyData`.
Follow ups
