launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging lp:~twom/launchpad/per-series-signing-keys into lp:launchpad.

Commit message:
Allow per series signing keys for UEFI

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/per-series-signing-keys/+merge/371675

If the series has a uefi key created for it, use it. Otherwise walk back up the series list until we find one that exists and use that.
If nothing exists use the root key.
If we're going to create the key if it doesn't exist, just accept the given path for the series.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/per-series-signing-keys into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/signing.py'
--- lib/lp/archivepublisher/signing.py	2019-06-06 10:55:17 +0000
+++ lib/lp/archivepublisher/signing.py	2019-08-22 16:56:22 +0000
@@ -82,6 +82,29 @@
         self.package, self.version, self.arch = self.parsePath(
             tarfile_path)
 
+    def getSeriesPath(self, signingroot, key_name, archive, autokey):
+        # If there are no series, use the one at the root of the filesystem
+        if not archive.distribution.series:
+            return os.path.join(signingroot, key_name)
+        # If we are creating the key, we don't care if it exists
+        if autokey:
+            return os.path.join(
+                signingroot,
+                archive.distribution.series[0].name,
+                key_name,
+                )
+        # Walk the series backwards, looking for a key
+        for series in archive.distribution.series:
+            path = os.path.join(
+                signingroot,
+                series.name,
+                key_name
+            )
+            if os.path.exists(path):
+                return path
+        # If we have exhausted all available series, return the root
+        return os.path.join(signingroot, key_name)
+
     def setTargetDirectory(self, archive, tarfile_path, suite):
         self.archive = archive
         pubconf = getPubConfig(archive)
@@ -101,8 +124,20 @@
             self.fit_cert = None
             self.autokey = False
         else:
-            self.uefi_key = os.path.join(pubconf.signingroot, "uefi.key")
-            self.uefi_cert = os.path.join(pubconf.signingroot, "uefi.crt")
+            # Per series uefi signing keys. This can be walked backwards
+            # in the series list if it doesn't exist in the current series
+            self.uefi_key = self.getSeriesPath(
+                pubconf.signingroot,
+                "uefi.key",
+                archive,
+                pubconf.signingautokey
+                )
+            self.uefi_cert = self.getSeriesPath(
+                pubconf.signingroot,
+                "uefi.crt",
+                archive,
+                pubconf.signingautokey
+                )
             self.kmod_pem = os.path.join(pubconf.signingroot, "kmod.pem")
             self.kmod_x509 = os.path.join(pubconf.signingroot, "kmod.x509")
             self.opal_pem = os.path.join(pubconf.signingroot, "opal.pem")

=== modified file 'lib/lp/archivepublisher/tests/test_signing.py'
--- lib/lp/archivepublisher/tests/test_signing.py	2019-06-06 10:55:17 +0000
+++ lib/lp/archivepublisher/tests/test_signing.py	2019-08-22 16:56:22 +0000
@@ -215,9 +215,13 @@
             yield IArchiveSigningKey(self.archive).setSigningKey(
                 key_path, async_keyserver=True)
 
-    def setUpUefiKeys(self, create=True):
-        self.key = os.path.join(self.signing_dir, "uefi.key")
-        self.cert = os.path.join(self.signing_dir, "uefi.crt")
+    def setUpUefiKeys(self, create=True, series=None):
+        if not series:
+            self.key = os.path.join(self.signing_dir, "uefi.key")
+            self.cert = os.path.join(self.signing_dir, "uefi.crt")
+        else:
+            self.key = os.path.join(self.signing_dir, series.name, "uefi.key")
+            self.cert = os.path.join(self.signing_dir, series.name, "uefi.crt")
         if create:
             write_file(self.key, b"")
             write_file(self.cert, b"")
@@ -970,6 +974,33 @@
         upload = self.process()
         self.assertEqual(1, upload.signUefi.call_count)
 
+    def test_signs_uefi_image_per_series(self):
+        """Check that signing can be per series.
+        This should fall through to the first series,
+        as the second does not have keys.
+        """
+        first_series = self.factory.makeDistroSeries(
+            self.distro,
+            name="existing-keys"
+        )
+        self.factory.makeDistroSeries(
+            self.distro,
+            name="no-keys"
+        )
+        # Each image in the tarball is signed.
+        self.setUpUefiKeys()
+        self.setUpUefiKeys(series=first_series)
+        self.openArchive("test", "1.0", "amd64")
+        self.tarfile.add_file("1.0/empty.efi", b"")
+        upload = self.process_emulate()
+        expected_callers = [
+            ('UEFI signing', 1),
+        ]
+        self.assertContentEqual(expected_callers, upload.callLog.caller_list())
+        # Check the correct series name appears in the call arguments
+        self.assertTrue(
+            "existing-keys" in upload.callLog.extract_args()[0][1][2])
+
     def test_signs_fit_image(self):
         # Each image in the tarball is signed.
         self.setUpFitKeys()
@@ -1351,6 +1382,78 @@
             "SUITE": Equals(self.suite),
             }))
 
+    def test_getSeriesKeyName_no_series(self):
+        upload = SigningUpload()
+        config = getPubConfig(self.archive)
+        result = upload.getSeriesPath(
+            config.signingroot, 'key.key', self.archive, False)
+        expected_path = os.path.join(
+            config.signingroot,
+            'key.key'
+        )
+        self.assertEqual(expected_path, result)
+
+    def test_getSeriesKeyName_autokey(self):
+        self.factory.makeDistroSeries(self.distro, name='newdistro')
+        upload = SigningUpload()
+        config = getPubConfig(self.archive)
+        result = upload.getSeriesPath(
+            config.signingroot, "uefi.key", self.archive, True)
+        expected_path = os.path.join(
+            config.signingroot,
+            "newdistro",
+            "uefi.key"
+            )
+        self.assertEqual(expected_path, result)
+
+    def test_getSeriesKeyName_one_distro(self):
+        self.setUpUefiKeys(
+            series=self.factory.makeDistroSeries(
+                self.distro, name="newdistro"))
+        upload = SigningUpload()
+        config = getPubConfig(self.archive)
+        result = upload.getSeriesPath(
+            config.signingroot, "uefi.key", self.archive, False)
+        expected_path = os.path.join(
+            config.signingroot,
+            "newdistro",
+            "uefi.key",
+            )
+        self.assertEqual(expected_path, result)
+
+    def test_getSeriesKeyName_two_distro(self):
+        self.setUpUefiKeys(
+            series=self.factory.makeDistroSeries(
+                self.distro, name="newdistro"))
+        self.setUpUefiKeys(
+            series=self.factory.makeDistroSeries(
+                self.distro, name="seconddistro"))
+        upload = SigningUpload()
+        config = getPubConfig(self.archive)
+        result = upload.getSeriesPath(
+            config.signingroot, "uefi.key", self.archive, False)
+        expected_path = os.path.join(
+            config.signingroot,
+            "seconddistro",
+            "uefi.key",
+            )
+        self.assertEqual(expected_path, result)
+
+    def test_getSeriesKeyName_two_distro_fallthrough(self):
+        self.setUpUefiKeys(
+            series=self.factory.makeDistroSeries(
+                self.distro, name="newdistro"))
+        upload = SigningUpload()
+        config = getPubConfig(self.archive)
+        result = upload.getSeriesPath(
+            config.signingroot, "uefi.key", self.archive, False)
+        expected_path = os.path.join(
+            config.signingroot,
+            "newdistro",
+            "uefi.key",
+            )
+        self.assertEqual(expected_path, result)
+
 
 class TestUefi(TestSigningHelpers):