launchpad-reviewers team mailing list archive

[Guruprasad <mp+466937@xxxxxxxxxxxxxxxxxx>]

Guruprasad has proposed merging ~lgp171188/launchpad:new-ppa-propagate-only-the-secure-signing-key-from-default-ppa into launchpad:master.

Commit message:
Propagate only the 4096-bit RSA signing key from the default PPA

If the default archive of a person has more than one signing key, for
example during the 1024-bit RSA signing key to 4096-bit RSA signing key
migration, propagate only the 4096-bit RSA signing key to the new PPAs
of the same person.


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/466937
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:new-ppa-propagate-only-the-secure-signing-key-from-default-ppa into launchpad:master.

diff --git a/lib/lp/soyuz/model/archive.py b/lib/lp/soyuz/model/archive.py
index 3e23a51..12fa3ac 100644
--- a/lib/lp/soyuz/model/archive.py
+++ b/lib/lp/soyuz/model/archive.py
@@ -98,7 +98,10 @@ from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.model import LibraryFileAlias, LibraryFileContent
 from lp.services.propertycache import cachedproperty, get_property_cache
 from lp.services.signing.enums import SigningKeyType
-from lp.services.signing.interfaces.signingkey import ISigningKeySet
+from lp.services.signing.interfaces.signingkey import (
+    IArchiveSigningKeySet,
+    ISigningKeySet,
+)
 from lp.services.signing.model.signingkey import ArchiveSigningKey, SigningKey
 from lp.services.tokens import create_token
 from lp.services.webapp.authorization import check_permission
@@ -3532,7 +3535,21 @@ class ArchiveSet:
         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
+                # Check if the archive has a replacement 4096-bit RSA key
+                # generated and propagate that, if it exists. There is no
+                # need to populate the 'archivesigningkey' table here because
+                # the new archive will only have a single secure signing key
+                # and all such archives can be migrated en masse to the
+                # 'archivesigningkey' table later.
+                signing_key = getUtility(
+                    IArchiveSigningKeySet
+                ).get4096BitRSASigningKey(owner.archive)
+                if signing_key:
+                    signing_key_fingerprint = signing_key.fingerprint
+                else:
+                    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
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index 01e0708..e5bb461 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -65,6 +65,7 @@ from lp.services.job.interfaces.job import JobStatus
 from lp.services.macaroons.testing import MacaroonVerifies
 from lp.services.propertycache import clear_property_cache, get_property_cache
 from lp.services.signing.enums import SigningKeyType
+from lp.services.signing.interfaces.signingkey import IArchiveSigningKeySet
 from lp.services.timeout import default_timeout
 from lp.services.webapp.authorization import check_permission
 from lp.services.webapp.interfaces import OAuthPermission
@@ -6149,6 +6150,58 @@ class TestSigningKeyPropagation(TestCaseWithFactory):
         )
         self.assertEqual(person.gpg_keys[0], ppa_with_key.signing_key)
 
+    def test_secure_default_signing_key_propagated_to_new_ppa(self):
+        # When a default PPA has more than one signing key, for example during
+        # the 1024-bit RSA signing key to 4096-bit RSA signing key migration,
+        # only the secure key is propagated to the new PPAs of the same
+        # person.
+        person = self.factory.makePerson()
+        default_ppa = self.factory.makeArchive(
+            owner=person, purpose=ArchivePurpose.PPA, name="ppa"
+        )
+        self.assertEqual(default_ppa, person.archive)
+        fingerprint_1024R = self.factory.getUniqueHexString(digits=40).upper()
+        signing_key_1024R = self.factory.makeSigningKey(
+            key_type=SigningKeyType.OPENPGP, fingerprint=fingerprint_1024R
+        )
+        self.factory.makeGPGKey(
+            owner=person,
+            keyid=fingerprint_1024R[-8:],
+            fingerprint=fingerprint_1024R,
+            keysize=1024,
+        )
+        removeSecurityProxy(person.archive).signing_key_owner = person
+        removeSecurityProxy(person.archive).signing_key_fingerprint = (
+            fingerprint_1024R
+        )
+        del get_property_cache(person.archive).signing_key
+        getUtility(IArchiveSigningKeySet).create(
+            default_ppa,
+            None,
+            signing_key_1024R,
+        )
+        fingerprint_4096R = self.factory.getUniqueHexString(digits=40).upper()
+        signing_key_4096R = self.factory.makeSigningKey(
+            key_type=SigningKeyType.OPENPGP, fingerprint=fingerprint_4096R
+        )
+        self.factory.makeGPGKey(
+            owner=person,
+            keyid=fingerprint_4096R[-8:],
+            fingerprint=fingerprint_4096R,
+            keysize=4096,
+        )
+        getUtility(IArchiveSigningKeySet).create(
+            default_ppa,
+            None,
+            signing_key_4096R,
+        )
+        another_ppa = self.factory.makeArchive(
+            owner=person, purpose=ArchivePurpose.PPA
+        )
+        self.assertEqual(
+            another_ppa.signing_key_fingerprint, fingerprint_4096R
+        )
+
 
 class TestGetSigningKeyData(TestCaseWithFactory):
     """Test `Archive.getSigningKeyData`.