Merge ~pappacena/launchpad:inject-lp-signing-generated-keys into launchpad:master

[Thiago F. Pappacena]

Adding the possibility to inject into lp-signing the locally generated signing keys.

It is possible to control which key types to inject when auto-generating them by setting the feature flag `archivepublisher.signing_service.injection.enabled` with a list of key types (separated by spaces). Eg.: "KMOD UEFI".

[Colin Watson (cjwatson)]

review: Needs Information

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes. Let me know if you need another round of review, cjwatson.

[Colin Watson (cjwatson)]

review: Needs Fixing

[Thiago F. Pappacena (pappacena)]

cjwatson, I'll push some of the requested changes, but I would like further clarification on some of your previous comments.

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes. It might be good to have another round of review on the duplicated ArchiveSigningKey check.

[Colin Watson (cjwatson)]

review: Needs Fixing

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes (with some comments, cjwatson).

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/archivepublisher/signing.py b/lib/lp/archivepublisher/signing.py
index 9a175db..1eca0a4 100644
--- a/lib/lp/archivepublisher/signing.py
+++ b/lib/lp/archivepublisher/signing.py
@@ -18,6 +18,7 @@ __all__ = [
     "UefiUpload",
     ]
 
+from datetime import datetime
 from functools import partial
 import os
 import shutil
@@ -27,6 +28,7 @@ import tarfile
 import tempfile
 import textwrap
 
+from pytz import utc
 import scandir
 from zope.component import getUtility
 
@@ -42,6 +44,8 @@ from lp.soyuz.interfaces.queue import CustomUploadError
 
 PUBLISHER_USES_SIGNING_SERVICE = (
     'archivepublisher.signing_service.enabled')
+PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS = (
+    'archivepublisher.signing_service.injection.enabled')
 
 
 class SigningUploadPackError(CustomUploadError):
@@ -59,6 +63,10 @@ class SigningServiceError(Exception):
     pass
 
 
+class SigningKeyConflict(Exception):
+    pass
+
+
 class SigningUpload(CustomUpload):
     """Signing custom upload.
 
@@ -260,7 +268,7 @@ class SigningUpload(CustomUpload):
             # tend to make the publisher rather upset.
             if self.logger is not None:
                 self.logger.warning("%s Failed (cmd='%s')" %
-                    (description, " ".join(cmdl)))
+                                    (description, " ".join(cmdl)))
         return status
 
     def findSigningHandlers(self):
@@ -421,6 +429,56 @@ class SigningUpload(CustomUpload):
             return [None for k in keynames]
         return keynames
 
+    def injectIntoSigningService(
+            self, key_type, private_key_file, public_key_file):
+        """Injects the given key pair into signing service for current
+        archive.
+
+        Note that this injection should only be used for freshly
+        autogenerated keys, always injecting the key for the archive in
+        general (not setting earliest_distro_series).
+        """
+        if key_type not in SigningKeyType:
+            raise ValueError("%s is not a valid key type to inject" % key_type)
+
+        feature_flag = (
+            getFeatureFlag(PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS) or '')
+        key_types_to_inject = [i.strip() for i in feature_flag.split()]
+
+        if key_type.name not in key_types_to_inject:
+            if self.logger:
+                self.logger.info(
+                    "Skipping injection for key type %s: not in %s",
+                    key_type, key_types_to_inject)
+            return
+
+        key_set = getUtility(IArchiveSigningKeySet)
+        current_key = key_set.getSigningKey(
+            key_type, self.archive, None, exact_match=True)
+        if current_key is not None:
+            self.logger.info("Skipping injection for key type %s: archive "
+                             "already has a key on lp-signing.", key_type)
+            raise SigningKeyConflict(
+                "Archive %s already has a signing key type %s on lp-signing."
+                % (self.archive.reference, key_type))
+
+        if self.logger:
+            self.logger.info(
+                "Injecting key_type %s for archive %s into signing service",
+                key_type, self.archive.name)
+
+        with open(private_key_file, 'rb') as fd:
+            private_key = fd.read()
+        with open(public_key_file, 'rb') as fd:
+            public_key = fd.read()
+
+        now = datetime.now().replace(tzinfo=utc)
+        description = (
+                u"%s key for %s" % (key_type.name, self.archive.reference))
+        key_set.inject(
+            key_type, private_key, public_key,
+            description, now, self.archive, earliest_distro_series=None)
+
     def generateKeyCommonName(self, owner, archive, suffix=''):
         # PPA <owner> <archive> <suffix>
         # truncate <owner> <archive> to ensure the overall form is shorter
@@ -454,6 +512,15 @@ class SigningUpload(CustomUpload):
         if os.path.exists(cert_filename):
             os.chmod(cert_filename, 0o644)
 
+            signing_key_type = getattr(SigningKeyType, key_type.upper())
+            try:
+                self.injectIntoSigningService(
+                    signing_key_type, key_filename, cert_filename)
+            except SigningKeyConflict:
+                os.unlink(key_filename)
+                os.unlink(cert_filename)
+                raise
+
     def generateUefiKeys(self):
         """Generate new UEFI Keys for this archive."""
         self.generateKeyCrtPair("UEFI", self.uefi_key, self.uefi_cert)
@@ -541,6 +608,15 @@ class SigningUpload(CustomUpload):
         if os.path.exists(x509_filename):
             os.chmod(x509_filename, 0o644)
 
+            signing_key_type = getattr(SigningKeyType, key_type.upper())
+            try:
+                self.injectIntoSigningService(
+                    signing_key_type, pem_filename, x509_filename)
+            except SigningKeyConflict:
+                os.unlink(pem_filename)
+                os.unlink(x509_filename)
+                raise
+
     def generateKmodKeys(self):
         """Generate new Kernel Signing Keys for this archive."""
         config = self.generateOpensslConfig("Kmod", self.openssl_config_kmod)
diff --git a/lib/lp/archivepublisher/tests/test_signing.py b/lib/lp/archivepublisher/tests/test_signing.py
index 87cf391..e51bb25 100644
--- a/lib/lp/archivepublisher/tests/test_signing.py
+++ b/lib/lp/archivepublisher/tests/test_signing.py
@@ -7,14 +7,19 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 
+from datetime import datetime
 import os
 import re
 import shutil
 import stat
 import tarfile
 
-from fixtures import MonkeyPatch
+from fixtures import (
+    MockPatch,
+    MonkeyPatch,
+    )
 from mock import call
+from pytz import utc
 import scandir
 from testtools.matchers import (
     Contains,
@@ -42,12 +47,15 @@ from lp.archivepublisher.interfaces.archivegpgsigningkey import (
     )
 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
 from lp.archivepublisher.signing import (
+    PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS,
     PUBLISHER_USES_SIGNING_SERVICE,
+    SigningKeyConflict,
     SigningUpload,
     UefiUpload,
     )
 from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
 from lp.services.features.testing import FeatureFixture
+from lp.services.log.logger import BufferLogger
 from lp.services.osutils import write_file
 from lp.services.signing.enums import SigningMode
 from lp.services.signing.proxy import SigningKeyType
@@ -1851,7 +1859,8 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
 
         self.openArchive("test", "1.0", "amd64")
         for filename in filenames:
-            self.tarfile.add_file(filename, b"data - %s" % filename)
+            self.tarfile.add_file(
+                filename, b"data - %s" % filename.encode("UTF-8"))
 
         self.tarfile.close()
         self.buffer.close()
@@ -1965,7 +1974,8 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
 
         self.openArchive("test", "1.0", "amd64")
         for filename in filenames:
-            self.tarfile.add_file(filename, b"data - %s" % filename)
+            self.tarfile.add_file(
+                filename, b"data - %s" % filename.encode("UTF-8"))
 
         self.tarfile.close()
         self.buffer.close()
@@ -2009,3 +2019,101 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
         self.assertEqual(
             [(os.path.join(upload.tmpdir_used, "1.0/empty.efi"),)],
             upload.signUefi.extract_args())
+
+    def test_fallback_injects_key(self):
+        self.useFixture(FeatureFixture({PUBLISHER_USES_SIGNING_SERVICE: ''}))
+        self.useFixture(FeatureFixture({
+            PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS: 'SIPL OPAL'}))
+
+        now = datetime.now()
+        mock_datetime = self.useFixture(MockPatch(
+            'lp.archivepublisher.signing.datetime')).mock
+        mock_datetime.now = lambda: now
+
+        logger = BufferLogger()
+        upload = SigningUpload(logger=logger)
+
+        # Setup PPA to ensure it auto-generates keys.
+        self.setUpPPA()
+
+        filenames = ["1.0/empty.efi", "1.0/empty.opal"]
+
+        self.openArchive("test", "1.0", "amd64")
+        for filename in filenames:
+            self.tarfile.add_file(
+                filename, b"data - %s" % filename.encode("UTF-8"))
+        self.tarfile.close()
+        self.buffer.close()
+
+        upload.process(self.archive, self.path, self.suite)
+        self.assertTrue(upload.autokey)
+
+        # Read the key file content
+        with open(upload.opal_pem, 'rb') as fd:
+            private_key = fd.read()
+        with open(upload.opal_x509, 'rb') as fd:
+            public_key = fd.read()
+
+        # Check if we called lp-signing's /inject endpoint correctly
+        self.assertEqual(1, self.signing_service_client.inject.call_count)
+        self.assertEqual(
+            (SigningKeyType.OPAL, private_key, public_key,
+             u"OPAL key for %s" % self.archive.reference,
+             now.replace(tzinfo=utc)),
+            self.signing_service_client.inject.call_args[0])
+
+        log_content = logger.content.as_text()
+        self.assertIn(
+            "INFO Injecting key_type OPAL for archive %s into signing "
+            "service" % (self.archive.name),
+            log_content)
+
+        self.assertIn(
+            "INFO Skipping injection for key type UEFI: "
+            "not in [u'SIPL', u'OPAL']",
+            log_content)
+
+    def test_fallback_skips_key_injection_for_existing_keys(self):
+        self.useFixture(FeatureFixture({PUBLISHER_USES_SIGNING_SERVICE: ''}))
+        self.useFixture(FeatureFixture({
+            PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS: 'UEFI'}))
+
+        now = datetime.now()
+        mock_datetime = self.useFixture(MockPatch(
+            'lp.archivepublisher.signing.datetime')).mock
+        mock_datetime.now = lambda: now
+
+        # Setup PPA to ensure it auto-generates keys.
+        self.setUpPPA()
+
+        signing_key = self.factory.makeSigningKey(key_type=SigningKeyType.UEFI)
+        self.factory.makeArchiveSigningKey(
+            archive=self.archive, signing_key=signing_key)
+
+        logger = BufferLogger()
+        upload = SigningUpload(logger=logger)
+
+        filenames = ["1.0/empty.efi"]
+
+        self.openArchive("test", "1.0", "amd64")
+        for filename in filenames:
+            self.tarfile.add_file(
+                filename, b"data - %s" % filename.encode("UTF-8"))
+        self.tarfile.close()
+        self.buffer.close()
+
+        self.assertRaises(SigningKeyConflict,
+                          upload.process, self.archive, self.path, self.suite)
+        self.assertTrue(upload.autokey)
+
+        # Make sure we deleted the locally generated keys.
+        self.assertFalse(os.path.exists(upload.uefi_cert))
+        self.assertFalse(os.path.exists(upload.uefi_key))
+
+        # Make sure we didn't call lp-signing's /inject endpoint
+        self.assertEqual(0, self.signing_service_client.inject.call_count)
+        log_content = logger.content.as_text()
+        self.assertIn(
+            "INFO Skipping injection for key type %s: archive "
+            "already has a key on lp-signing." % SigningKeyType.UEFI,
+            log_content)
diff --git a/lib/lp/services/signing/interfaces/signingkey.py b/lib/lp/services/signing/interfaces/signingkey.py
index 587bbb0..eccf2a0 100644
--- a/lib/lp/services/signing/interfaces/signingkey.py
+++ b/lib/lp/services/signing/interfaces/signingkey.py
@@ -117,10 +117,13 @@ class IArchiveSigningKeySet(Interface):
                  False if it was updated).
         """
 
-    def getSigningKey(key_type, archive, distro_series):
+    def getSigningKey(key_type, archive, distro_series, exact_match=False):
         """Get the most suitable key for a given archive / distro series
         pair.
 
+        :param exact_match: If True, returns the ArchiveSigningKey matching
+                            exactly the given key_type, archive and
+                            distro_series. If False, gets the best match.
         :return: The most suitable key
         """
 
diff --git a/lib/lp/services/signing/model/signingkey.py b/lib/lp/services/signing/model/signingkey.py
index 0d5e137..0479b61 100644
--- a/lib/lp/services/signing/model/signingkey.py
+++ b/lib/lp/services/signing/model/signingkey.py
@@ -167,7 +167,8 @@ class ArchiveSigningKeySet:
         return obj
 
     @classmethod
-    def getSigningKey(cls, key_type, archive, distro_series):
+    def getSigningKey(cls, key_type, archive, distro_series,
+                      exact_match=False):
         store = IStore(ArchiveSigningKey)
         # Gets all the keys of the given key_type available for the archive
         rs = store.find(ArchiveSigningKey,
@@ -176,6 +177,10 @@ class ArchiveSigningKeySet:
                 ArchiveSigningKey.key_type == key_type,
                 ArchiveSigningKey.archive == archive)
 
+        if exact_match:
+            rs = rs.find(
+                ArchiveSigningKey.earliest_distro_series == distro_series)
+
         # prefetch related signing keys to avoid extra queries.
         signing_keys = store.find(SigningKey, [
             SigningKey.id.is_in([i.signing_key_id for i in rs])])
diff --git a/lib/lp/services/signing/tests/helpers.py b/lib/lp/services/signing/tests/helpers.py
index e01730f..5675bc6 100644
--- a/lib/lp/services/signing/tests/helpers.py
+++ b/lib/lp/services/signing/tests/helpers.py
@@ -39,8 +39,12 @@ class SigningServiceClientFixture(fixtures.Fixture):
         self.sign = mock.Mock()
         self.sign.side_effect = self._sign
 
+        self.inject = mock.Mock()
+        self.inject.side_effect = self._inject
+
         self.generate_returns = []
         self.sign_returns = []
+        self.inject_returns = []
 
     def _generate(self, key_type, description):
         key = bytes(PrivateKey.generate().public_key)
@@ -59,6 +63,12 @@ class SigningServiceClientFixture(fixtures.Fixture):
         self.sign_returns.append((key_type, data))
         return data
 
+    def _inject(self, key_type, private_key, public_key, description,
+                created_at):
+        data = {'fingerprint': text_type(self.factory.getUniqueHexString(40))}
+        self.inject_returns.append(data)
+        return data
+
     def _setUp(self):
         self.useFixture(ZopeUtilityFixture(self, ISigningServiceClient))
 
diff --git a/lib/lp/services/signing/tests/test_signingkey.py b/lib/lp/services/signing/tests/test_signingkey.py
index 3610303..8613dd7 100644
--- a/lib/lp/services/signing/tests/test_signingkey.py
+++ b/lib/lp/services/signing/tests/test_signingkey.py
@@ -244,6 +244,53 @@ class TestArchiveSigningKey(TestCaseWithFactory):
             arch_kmod_key.signing_key,
             arch_signing_key_set.getSigningKey(KMOD, archive, distro_series))
 
+    def test_get_signing_key_exact_match(self):
+        UEFI = SigningKeyType.UEFI
+        KMOD = SigningKeyType.KMOD
+
+        archive = self.factory.makeArchive()
+        distro_series1 = archive.distribution.series[0]
+        distro_series2 = archive.distribution.series[1]
+        uefi_key = self.factory.makeSigningKey(
+            key_type=SigningKeyType.UEFI)
+        kmod_key = self.factory.makeSigningKey(
+            key_type=SigningKeyType.KMOD)
+
+        arch_signing_key_set = getUtility(IArchiveSigningKeySet)
+
+        # Create a key for the first distro series
+        series1_uefi_key = arch_signing_key_set.create(
+            archive, distro_series1, uefi_key)
+
+        # Create a key for the archive
+        arch_kmod_key = arch_signing_key_set.create(
+            archive, None, kmod_key)
+
+        # Should get the UEFI key for distro_series1
+        self.assertEqual(
+            series1_uefi_key.signing_key,
+            arch_signing_key_set.getSigningKey(
+                UEFI, archive, distro_series1, exact_match=True)
+        )
+        # Should get the archive's KMOD key.
+        self.assertEqual(
+            arch_kmod_key.signing_key,
+            arch_signing_key_set.getSigningKey(
+                KMOD, archive, None, exact_match=True)
+        )
+        # distro_series1 has no KMOD key.
+        self.assertEqual(
+            None,
+            arch_signing_key_set.getSigningKey(
+                KMOD, archive, distro_series1, exact_match=True)
+        )
+        # distro_series2 has no key at all.
+        self.assertEqual(
+            None,
+            arch_signing_key_set.getSigningKey(
+                KMOD, archive, distro_series2, exact_match=True)
+        )
+
     def test_get_signing_keys_with_distro_series_configured(self):
         UEFI = SigningKeyType.UEFI
         KMOD = SigningKeyType.KMOD