launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/refactor-archive-signing into lp:launchpad with lp:~cjwatson/launchpad/archive-signing-run-parts as a prerequisite.

Commit message:
Extend custom uploads and Release file signing to use the new ISignableArchive interface.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/refactor-archive-signing/+merge/336377

This replaces ubuntu-archive-publishing/publish-distro.d/10-sign-releases.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/refactor-archive-signing into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/archivesigningkey.py'
--- lib/lp/archivepublisher/archivesigningkey.py	2018-01-19 17:56:02 +0000
+++ lib/lp/archivepublisher/archivesigningkey.py	2018-01-19 17:56:02 +0000
@@ -119,19 +119,21 @@
                     "No signing key available for %s" %
                     self.archive.displayname)
 
-    def signRepository(self, suite, log=None):
+    def signRepository(self, suite, suffix='', log=None):
         """See `ISignableArchive`."""
         suite_path = os.path.join(self._archive_root_path, 'dists', suite)
-        release_file_path = os.path.join(suite_path, 'Release')
+        release_file_path = os.path.join(suite_path, 'Release' + suffix)
         if not os.path.exists(release_file_path):
             raise AssertionError(
                 "Release file doesn't exist in the repository: %s" %
                 release_file_path)
 
         self._makeSignatures([
-            (release_file_path, os.path.join(suite_path, 'Release.gpg'),
+            (release_file_path,
+             os.path.join(suite_path, 'Release.gpg' + suffix),
              SigningMode.DETACHED, suite),
-            (release_file_path, os.path.join(suite_path, 'InRelease'),
+            (release_file_path,
+             os.path.join(suite_path, 'InRelease' + suffix),
              SigningMode.CLEAR, suite),
             ], log=log)
 

=== modified file 'lib/lp/archivepublisher/customupload.py'
--- lib/lp/archivepublisher/customupload.py	2018-01-19 17:56:02 +0000
+++ lib/lp/archivepublisher/customupload.py	2018-01-19 17:56:02 +0000
@@ -26,6 +26,7 @@
     Version as make_version,
     VersionError,
     )
+from lp.archivepublisher.interfaces.archivesigningkey import ISignableArchive
 from lp.services.librarian.utils import copy_and_close
 from lp.soyuz.interfaces.queue import (
     CustomUploadError,
@@ -268,6 +269,20 @@
         if not os.path.isdir(parentdir):
             os.makedirs(parentdir, 0o755)
 
+    def shouldSign(self, filename):
+        """Returns True if the given filename should be signed."""
+        return False
+
+    def sign(self, archive, suite, filename):
+        """Sign a file.
+
+        For now, we always write a detached signature to the input file name
+        plus ".gpg".
+        """
+        signable_archive = ISignableArchive(archive)
+        if signable_archive.can_sign:
+            signable_archive.signFile(suite, filename, log=self.logger)
+
     def installFiles(self, archive, suite):
         """Install the files from the custom upload to the archive."""
         assert self.tmpdir is not None, "Must extract tarfile first"
@@ -316,6 +331,8 @@
                 else:
                     shutil.copy(sourcepath, destpath)
                     os.chmod(destpath, 0o644)
+                    if self.shouldSign(destpath):
+                        self.sign(archive, suite, destpath)
 
                 extracted = True
 

=== modified file 'lib/lp/archivepublisher/debian_installer.py'
--- lib/lp/archivepublisher/debian_installer.py	2016-06-07 17:07:35 +0000
+++ lib/lp/archivepublisher/debian_installer.py	2018-01-19 17:56:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """The processing of debian installer tarballs."""
@@ -76,3 +76,7 @@
 
     def shouldInstall(self, filename):
         return filename.startswith('%s/' % self.version)
+
+    def shouldSign(self, filename):
+        """Sign checksums files."""
+        return filename.endswith('SUMS')

=== modified file 'lib/lp/archivepublisher/dist_upgrader.py'
--- lib/lp/archivepublisher/dist_upgrader.py	2016-06-07 17:07:35 +0000
+++ lib/lp/archivepublisher/dist_upgrader.py	2018-01-19 17:56:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """The processing of dist-upgrader tarballs."""
@@ -81,7 +81,7 @@
             return None
 
     def shouldInstall(self, filename):
-        """ Install files from a dist-upgrader tarball.
+        """Install files from a dist-upgrader tarball.
 
         It raises DistUpgraderBadVersion if if finds a directory name that
         could not be treated as a valid Debian version.
@@ -100,3 +100,7 @@
         except BadUpstreamError as exc:
             raise DistUpgraderBadVersion(self.tarfile_path, exc)
         return version and not filename.startswith('current')
+
+    def shouldSign(self, filename):
+        """Sign *.tar.gz files."""
+        return filename.endswith('.tar.gz')

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2018-01-19 17:56:02 +0000
+++ lib/lp/archivepublisher/publishing.py	2018-01-19 17:56:02 +0000
@@ -57,9 +57,7 @@
     build_source_stanza_fields,
     build_translations_stanza_fields,
     )
-from lp.archivepublisher.interfaces.archivesigningkey import (
-    IArchiveSigningKey,
-    )
+from lp.archivepublisher.interfaces.archivesigningkey import ISignableArchive
 from lp.archivepublisher.model.ftparchive import FTPArchiveHandler
 from lp.archivepublisher.utils import (
     get_ppa_reference,
@@ -1245,20 +1243,23 @@
         if distroseries.publish_by_hash:
             self._updateByHash(suite, "Release.new")
 
-        os.rename(
-            os.path.join(suite_dir, "Release.new"),
-            os.path.join(suite_dir, "Release"))
-
-        if self.archive.signing_key is not None:
+        signable_archive = ISignableArchive(self.archive)
+        if signable_archive.can_sign:
             # Sign the repository.
             self.log.debug("Signing Release file for %s" % suite)
-            IArchiveSigningKey(self.archive).signRepository(suite)
+            signable_archive.signRepository(suite, suffix=".new", log=self.log)
             core_files.add("Release.gpg")
             core_files.add("InRelease")
         else:
-            # Skip signature if the archive signing key is undefined.
+            # Skip signature if the archive is not set up for signing.
             self.log.debug("No signing key available, skipping signature.")
 
+        for name in ("Release", "Release.gpg", "InRelease"):
+            if name in core_files:
+                os.rename(
+                    os.path.join(suite_dir, "%s.new" % name),
+                    os.path.join(suite_dir, name))
+
         # Make sure all the timestamps match, to make it easier to insert
         # caching headers on mirrors.
         self._syncTimestamps(suite, core_files)

=== modified file 'lib/lp/archivepublisher/signing.py'
--- lib/lp/archivepublisher/signing.py	2018-01-19 17:56:02 +0000
+++ lib/lp/archivepublisher/signing.py	2018-01-19 17:56:02 +0000
@@ -28,9 +28,6 @@
 
 from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.customupload import CustomUpload
-from lp.archivepublisher.interfaces.archivesigningkey import (
-    IArchiveSigningKey,
-    )
 from lp.services.osutils import remove_if_exists
 from lp.soyuz.interfaces.queue import CustomUploadError
 
@@ -379,12 +376,15 @@
         with DirectoryHash(versiondir, self.temproot) as hasher:
             hasher.add_dir(versiondir)
         for checksum_path in hasher.checksum_paths:
-            if archive.signing_key is not None:
-                IArchiveSigningKey(archive).signFile(suite, checksum_path)
+            if self.shouldSign(checksum_path):
+                self.sign(archive, suite, checksum_path)
 
     def shouldInstall(self, filename):
         return filename.startswith("%s/" % self.version)
 
+    def shouldSign(self, filename):
+        return filename.endswith("SUMS")
+
 
 class UefiUpload(SigningUpload):
     """Legacy UEFI Signing custom upload.

=== modified file 'lib/lp/archivepublisher/tests/test_customupload.py'
--- lib/lp/archivepublisher/tests/test_customupload.py	2012-05-28 13:13:53 +0000
+++ lib/lp/archivepublisher/tests/test_customupload.py	2018-01-19 17:56:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for `CustomUploads`."""
@@ -13,13 +13,40 @@
 import tempfile
 import unittest
 
+from fixtures import MonkeyPatch
+from testtools.deferredruntest import AsynchronousDeferredRunTest
+from testtools.matchers import (
+    Equals,
+    MatchesDict,
+    Not,
+    PathExists,
+    )
+from twisted.internet import defer
+from zope.component import getUtility
+
+from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.customupload import (
     CustomUpload,
     CustomUploadTarballBadFile,
     CustomUploadTarballBadSymLink,
     CustomUploadTarballInvalidFileType,
     )
-from lp.testing import TestCase
+from lp.archivepublisher.interfaces.archivesigningkey import (
+    IArchiveSigningKey,
+    )
+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
+from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
+from lp.services.gpg.interfaces import IGPGHandler
+from lp.services.osutils import write_file
+from lp.soyuz.enums import ArchivePurpose
+from lp.testing import (
+    TestCase,
+    TestCaseWithFactory,
+    )
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.gpgkeys import gpgkeysdir
+from lp.testing.keyserver import InProcessKeyServerFixture
+from lp.testing.layers import LaunchpadZopelessLayer
 
 
 class TestCustomUpload(unittest.TestCase):
@@ -198,3 +225,68 @@
                 self.custom_processor.extract)
         finally:
             shutil.rmtree(self.tarfile_path)
+
+
+class TestSigning(TestCaseWithFactory, RunPartsMixin):
+
+    layer = LaunchpadZopelessLayer
+    run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
+
+    def setUp(self):
+        super(TestSigning, self).setUp()
+        self.temp_dir = self.makeTemporaryDirectory()
+        self.distro = self.factory.makeDistribution()
+        db_pubconf = getUtility(IPublisherConfigSet).getByDistribution(
+            self.distro)
+        db_pubconf.root_dir = unicode(self.temp_dir)
+        self.archive = self.factory.makeArchive(
+            distribution=self.distro, purpose=ArchivePurpose.PRIMARY)
+
+    def test_sign_without_signing_key(self):
+        filename = os.path.join(
+            getPubConfig(self.archive).archiveroot, "file")
+        self.assertIsNone(self.archive.signing_key)
+        custom_processor = CustomUpload()
+        custom_processor.sign(self.archive, "suite", filename)
+        self.assertThat("%s.gpg" % filename, Not(PathExists()))
+
+    @defer.inlineCallbacks
+    def test_sign_with_signing_key(self):
+        filename = os.path.join(
+            getPubConfig(self.archive).archiveroot, "file")
+        write_file(filename, "contents")
+        self.assertIsNone(self.archive.signing_key)
+        self.useFixture(InProcessKeyServerFixture()).start()
+        key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
+        yield IArchiveSigningKey(self.archive).setSigningKey(
+            key_path, async_keyserver=True)
+        self.assertIsNotNone(self.archive.signing_key)
+        custom_processor = CustomUpload()
+        custom_processor.sign(self.archive, "suite", filename)
+        with open(filename) as cleartext_file:
+            cleartext = cleartext_file.read()
+            with open("%s.gpg" % filename) as signature_file:
+                signature = getUtility(IGPGHandler).getVerifiedSignature(
+                    cleartext, signature_file.read())
+        self.assertEqual(
+            self.archive.signing_key.fingerprint, signature.fingerprint)
+
+    def test_sign_with_external_run_parts(self):
+        self.enableRunParts(distribution_name=self.distro.name)
+        filename = os.path.join(
+            getPubConfig(self.archive).archiveroot, "file")
+        write_file(filename, "contents")
+        self.assertIsNone(self.archive.signing_key)
+        run_parts_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.archivesigningkey.run_parts", FakeMethod()))
+        custom_processor = CustomUpload()
+        custom_processor.sign(self.archive, "suite", filename)
+        args, kwargs = run_parts_fixture.new_value.calls[0]
+        self.assertEqual((self.distro.name, "sign.d"), args)
+        self.assertThat(kwargs["env"], MatchesDict({
+            "INPUT_PATH": Equals(filename),
+            "OUTPUT_PATH": Equals("%s.gpg" % filename),
+            "MODE": Equals("detached"),
+            "DISTRIBUTION": Equals(self.distro.name),
+            "SUITE": Equals("suite"),
+            }))

=== modified file 'lib/lp/archivepublisher/tests/test_debian_installer.py'
--- lib/lp/archivepublisher/tests/test_debian_installer.py	2016-06-07 17:07:35 +0000
+++ lib/lp/archivepublisher/tests/test_debian_installer.py	2018-01-19 17:56:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test debian-installer custom uploads.
@@ -8,7 +8,9 @@
 """
 
 import os
+from textwrap import dedent
 
+from testtools.matchers import DirContains
 from zope.component import getUtility
 
 from lp.archivepublisher.config import getPubConfig
@@ -161,6 +163,26 @@
         self.assertEqual(
             0o755, os.stat(self.getInstallerPath(directory)).st_mode & 0o777)
 
+    def test_sign_with_external_run_parts(self):
+        parts_directory = self.makeTemporaryDirectory()
+        sign_directory = os.path.join(
+            parts_directory, self.distro.name, "sign.d")
+        os.makedirs(sign_directory)
+        with open(os.path.join(sign_directory, "10-sign"), "w") as f:
+            f.write(dedent("""\
+                #! /bin/sh
+                touch "$OUTPUT_PATH"
+                """))
+            os.fchmod(f.fileno(), 0o755)
+        self.pushConfig("archivepublisher", run_parts_location=parts_directory)
+        self.openArchive()
+        self.addFile("images/list", "a list")
+        self.addFile("images/SHA256SUMS", "a checksum")
+        self.process()
+        self.assertThat(
+            self.getInstallerPath("images"),
+            DirContains(["list", "SHA256SUMS", "SHA256SUMS.gpg"]))
+
     def test_getSeriesKey_extracts_architecture(self):
         # getSeriesKey extracts the architecture from an upload's filename.
         self.openArchive()

=== modified file 'lib/lp/archivepublisher/tests/test_dist_upgrader.py'
--- lib/lp/archivepublisher/tests/test_dist_upgrader.py	2016-06-07 17:07:35 +0000
+++ lib/lp/archivepublisher/tests/test_dist_upgrader.py	2018-01-19 17:56:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test dist-upgrader custom uploads.
@@ -8,7 +8,9 @@
 """
 
 import os
+from textwrap import dedent
 
+from testtools.matchers import DirContains
 from zope.component import getUtility
 
 from lp.archivepublisher.config import getPubConfig
@@ -109,6 +111,26 @@
         self.tarfile.add_file("foobar/foobar/dapper.tar.gz", "")
         self.assertRaises(DistUpgraderBadVersion, self.process)
 
+    def test_sign_with_external_run_parts(self):
+        parts_directory = self.makeTemporaryDirectory()
+        sign_directory = os.path.join(
+            parts_directory, self.distro.name, "sign.d")
+        os.makedirs(sign_directory)
+        with open(os.path.join(sign_directory, "10-sign"), "w") as f:
+            f.write(dedent("""\
+                #! /bin/sh
+                touch "$OUTPUT_PATH"
+                """))
+            os.fchmod(f.fileno(), 0o755)
+        self.pushConfig("archivepublisher", run_parts_location=parts_directory)
+        self.openArchive("20060302.0120")
+        self.tarfile.add_file("20060302.0120/list", "a list")
+        self.tarfile.add_file("20060302.0120/foo.tar.gz", "a tarball")
+        self.process()
+        self.assertThat(
+            os.path.join(self.getUpgraderPath(), "20060302.0120"),
+            DirContains(["list", "foo.tar.gz", "foo.tar.gz.gpg"]))
+
     def test_getSeriesKey_extracts_architecture(self):
         # getSeriesKey extracts the architecture from an upload's filename.
         self.openArchive("20060302.0120")

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2018-01-19 17:56:02 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2018-01-19 17:56:02 +0000
@@ -67,6 +67,7 @@
     I18nIndex,
     Publisher,
     )
+from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
 from lp.archivepublisher.utils import RepositoryIndexFile
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.distroseries import IDistroSeries
@@ -2925,7 +2926,7 @@
             os.rename(temporary_dists, original_dists)
 
 
-class TestPublisherRepositorySignatures(TestPublisherBase):
+class TestPublisherRepositorySignatures(RunPartsMixin, TestPublisherBase):
     """Testing `Publisher` signature behaviour."""
 
     run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
@@ -3065,6 +3066,54 @@
         self.assertThat(
             sync_args[1], ContainsAll(['Release', 'Release.gpg', 'InRelease']))
 
+    def testRepositorySignatureWithExternalRunParts(self):
+        """Check publisher behaviour when signing repositories.
+
+        When a 'sign.d' run-parts directory is configured for the archive,
+        it is used to sign the Release file.
+        """
+        cprov = getUtility(IPersonSet).getByName('cprov')
+        self.assertIsNone(cprov.archive.signing_key)
+        self.enableRunParts(distribution_name=cprov.archive.distribution.name)
+        sign_directory = os.path.join(
+            self.parts_directory, cprov.archive.distribution.name, 'sign.d')
+        with open(os.path.join(sign_directory, '10-sign'), 'w') as sign_script:
+            sign_script.write(dedent("""\
+                #! /bin/sh
+                echo "$MODE signature of $INPUT_PATH ($DISTRIBUTION/$SUITE)" \\
+                    >"$OUTPUT_PATH"
+                """))
+            os.fchmod(sign_script.fileno(), 0o755)
+
+        self.setupPublisher(cprov.archive)
+        self.archive_publisher._syncTimestamps = FakeMethod()
+
+        self._publishArchive(cprov.archive)
+
+        # Release exists.
+        self.assertThat(self.release_file_path, PathExists())
+
+        # Release.gpg and InRelease exist with suitable fake signatures.
+        # Note that the signatures are made before Release.new is renamed to
+        # to Release.
+        self.assertThat(
+            self.release_file_signature_path,
+            FileContains(
+                "detached signature of %s.new (%s/breezy-autotest)\n" %
+                (self.release_file_path, cprov.archive.distribution.name)))
+        self.assertThat(
+            self.inline_release_file_path,
+            FileContains(
+                "clear signature of %s.new (%s/breezy-autotest)\n" %
+                (self.release_file_path, cprov.archive.distribution.name)))
+
+        # The publisher synchronises the various Release file timestamps.
+        self.assertEqual(1, self.archive_publisher._syncTimestamps.call_count)
+        sync_args = self.archive_publisher._syncTimestamps.extract_args()[0]
+        self.assertEqual(self.distroseries.name, sync_args[0])
+        self.assertThat(
+            sync_args[1], ContainsAll(['Release', 'Release.gpg', 'InRelease']))
+
 
 class TestPublisherLite(TestCaseWithFactory):
     """Lightweight unit tests for the publisher."""

=== modified file 'lib/lp/archivepublisher/tests/test_signing.py'
--- lib/lp/archivepublisher/tests/test_signing.py	2018-01-19 17:56:02 +0000
+++ lib/lp/archivepublisher/tests/test_signing.py	2018-01-19 17:56:02 +0000
@@ -13,9 +13,11 @@
 from testtools.deferredruntest import AsynchronousDeferredRunTest
 from testtools.matchers import (
     Contains,
+    Equals,
     FileContains,
     Matcher,
     MatchesAll,
+    MatchesDict,
     Mismatch,
     Not,
     StartsWith,
@@ -36,6 +38,7 @@
     SigningUpload,
     UefiUpload,
     )
+from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
 from lp.services.osutils import write_file
 from lp.services.tarfile_helpers import LaunchpadWriteTarFile
 from lp.soyuz.enums import ArchivePurpose
@@ -215,7 +218,7 @@
         return os.path.join(pubconf.archiveroot, "dists", self.suite, "main")
 
 
-class TestSigning(TestSigningHelpers):
+class TestSigning(RunPartsMixin, TestSigningHelpers):
 
     def getSignedPath(self, loader_type, arch):
         return os.path.join(self.getDistsPath(), "signed",
@@ -934,6 +937,36 @@
                   "1.0/signed.tar.gz",
                   ]]))
 
+    def test_checksumming_tree_signed_with_external_run_parts(self):
+        # Checksum files can be signed using an external run-parts helper.
+        # We disable subprocess.call because there's just too much going on,
+        # so we can't test this completely, but we can at least test that
+        # run_parts is called.
+        self.enableRunParts(distribution_name=self.distro.name)
+        run_parts_fixture = self.useFixture(MonkeyPatch(
+            "lp.archivepublisher.archivesigningkey.run_parts", FakeMethod()))
+        self.setUpUefiKeys()
+        self.setUpKmodKeys()
+        self.setUpOpalKeys()
+        self.openArchive("test", "1.0", "amd64")
+        self.tarfile.add_file("1.0/empty.efi", "")
+        self.tarfile.add_file("1.0/empty.ko", "")
+        self.tarfile.add_file("1.0/empty.opal", "")
+        self.process_emulate()
+        sha256file = os.path.join(self.getSignedPath("test", "amd64"),
+             "1.0", "SHA256SUMS")
+        self.assertTrue(os.path.exists(sha256file))
+        self.assertEqual(1, run_parts_fixture.new_value.call_count)
+        args, kwargs = run_parts_fixture.new_value.calls[-1]
+        self.assertEqual((self.distro.name, "sign.d"), args)
+        self.assertThat(kwargs["env"], MatchesDict({
+            "INPUT_PATH": Equals(sha256file),
+            "OUTPUT_PATH": Equals("%s.gpg" % sha256file),
+            "MODE": Equals("detached"),
+            "DISTRIBUTION": Equals(self.distro.name),
+            "SUITE": Equals(self.suite),
+            }))
+
 
 class TestUefi(TestSigningHelpers):