launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25263
[Merge] ~cjwatson/launchpad:archive-gpg-sign-using-signing-service into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:archive-gpg-sign-using-signing-service into launchpad:master.
Commit message:
Allow signing archives using the signing service
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/390410
This is guarded by the feature rule 'archivepublisher.gpg.signing_service.enabled'.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:archive-gpg-sign-using-signing-service into launchpad:master.
diff --git a/lib/lp/archivepublisher/archivegpgsigningkey.py b/lib/lp/archivepublisher/archivegpgsigningkey.py
index fead6c5..a342332 100644
--- a/lib/lp/archivepublisher/archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/archivegpgsigningkey.py
@@ -28,6 +28,7 @@ from lp.archivepublisher.interfaces.archivegpgsigningkey import (
CannotSignArchive,
IArchiveGPGSigningKey,
ISignableArchive,
+ PUBLISHER_GPG_USES_SIGNING_SERVICE,
)
from lp.archivepublisher.run_parts import (
find_run_parts_dir,
@@ -35,10 +36,18 @@ from lp.archivepublisher.run_parts import (
)
from lp.registry.interfaces.gpg import IGPGKeySet
from lp.services.config import config
+from lp.services.features import getFeatureFlag
from lp.services.gpg.interfaces import IGPGHandler
from lp.services.osutils import remove_if_exists
-from lp.services.propertycache import get_property_cache
-from lp.services.signing.enums import SigningMode
+from lp.services.propertycache import (
+ cachedproperty,
+ get_property_cache,
+ )
+from lp.services.signing.enums import (
+ SigningKeyType,
+ SigningMode,
+ )
+from lp.services.signing.interfaces.signingkey import ISigningKeySet
@implementer(ISignableArchive)
@@ -54,13 +63,41 @@ class SignableArchive:
self.archive = archive
self.pubconf = getPubConfig(self.archive)
+ @cachedproperty
+ def _run_parts_dir(self):
+ """This distribution's sign.d run-parts directory, if any."""
+ return find_run_parts_dir(self.archive.distribution.name, "sign.d")
+
@property
def can_sign(self):
"""See `ISignableArchive`."""
return (
self.archive.signing_key is not None or
- find_run_parts_dir(
- self.archive.distribution.name, "sign.d") is not None)
+ self._run_parts_dir is not None)
+
+ @cachedproperty
+ def _signing_key(self):
+ """This archive's signing key on the signing service, if any."""
+ if not getFeatureFlag(PUBLISHER_GPG_USES_SIGNING_SERVICE):
+ return None
+ elif self.archive.signing_key_fingerprint is not None:
+ return getUtility(ISigningKeySet).get(
+ SigningKeyType.OPENPGP, self.archive.signing_key_fingerprint)
+ else:
+ return None
+
+ @cachedproperty
+ def _secret_key(self):
+ """This archive's signing key as a local GPG key."""
+ if self.archive.signing_key is not None:
+ secret_key_path = self.getPathForSecretKey(
+ self.archive.signing_key)
+ with open(secret_key_path, "rb") as secret_key_file:
+ secret_key_export = secret_key_file.read()
+ gpghandler = getUtility(IGPGHandler)
+ return gpghandler.importSecretKey(secret_key_export)
+ else:
+ return None
def _makeSignatures(self, signatures, log=None):
"""Make a sequence of signatures.
@@ -79,28 +116,38 @@ class SignableArchive:
raise CannotSignArchive(
"No signing key available for %s" % self.archive.displayname)
- if self.archive.signing_key is not None:
- secret_key_path = self.getPathForSecretKey(
- self.archive.signing_key)
- with open(secret_key_path, "rb") as secret_key_file:
- secret_key_export = secret_key_file.read()
- gpghandler = getUtility(IGPGHandler)
- secret_key = gpghandler.importSecretKey(secret_key_export)
-
output_paths = []
for input_path, output_path, mode, suite in signatures:
if mode not in {SigningMode.DETACHED, SigningMode.CLEAR}:
raise ValueError('Invalid signature mode for GPG: %s' % mode)
- if self.archive.signing_key is not None:
+ signed = False
+
+ if self._signing_key is not None or self._secret_key is not None:
with open(input_path, "rb") as input_file:
input_content = input_file.read()
- signature = gpghandler.signContent(
- input_content, secret_key, mode=self.gpgme_modes[mode])
- with open(output_path, "wb") as output_file:
- output_file.write(signature)
- output_paths.append(output_path)
- elif find_run_parts_dir(
- self.archive.distribution.name, "sign.d") is not None:
+ if self._signing_key is not None:
+ try:
+ signature = self._signing_key.sign(
+ input_content, os.path.basename(input_path),
+ mode=mode)
+ signed = True
+ except Exception:
+ if log is not None:
+ log.exception(
+ "Failed to sign archive using signing "
+ "service; falling back to local key")
+ get_property_cache(self)._signing_key = None
+ if not signed and self._secret_key is not None:
+ signature = getUtility(IGPGHandler).signContent(
+ input_content, self._secret_key,
+ mode=self.gpgme_modes[mode])
+ signed = True
+ if signed:
+ with open(output_path, "wb") as output_file:
+ output_file.write(signature)
+ output_paths.append(output_path)
+
+ if not signed and self._run_parts_dir is not None:
remove_if_exists(output_path)
env = {
"ARCHIVEROOT": self.pubconf.archiveroot,
@@ -113,9 +160,11 @@ class SignableArchive:
run_parts(
self.archive.distribution.name, "sign.d",
log=log, env=env)
+ signed = True
if os.path.exists(output_path):
output_paths.append(output_path)
- else:
+
+ if not signed:
raise AssertionError(
"No signing key available for %s" %
self.archive.displayname)
diff --git a/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py b/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py
index 2efffb6..3aa6d91 100644
--- a/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/interfaces/archivegpgsigningkey.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""ArchiveGPGSigningKey interface."""
@@ -9,6 +9,7 @@ __all__ = [
'CannotSignArchive',
'IArchiveGPGSigningKey',
'ISignableArchive',
+ 'PUBLISHER_GPG_USES_SIGNING_SERVICE',
]
from zope.interface import (
@@ -21,6 +22,10 @@ from lp import _
from lp.soyuz.interfaces.archive import IArchive
+PUBLISHER_GPG_USES_SIGNING_SERVICE = (
+ 'archivepublisher.gpg.signing_service.enabled')
+
+
class CannotSignArchive(Exception):
"""An archive is not set up for signing."""
diff --git a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
index 47600b6..87359e8 100644
--- a/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
+++ b/lib/lp/archivepublisher/tests/test_archivegpgsigningkey.py
@@ -1,4 +1,4 @@
-# Copyright 2016-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2016-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test ArchiveGPGSigningKey."""
@@ -10,7 +10,10 @@ __metaclass__ = type
import os
from textwrap import dedent
-from testtools.matchers import FileContains
+from testtools.matchers import (
+ FileContains,
+ StartsWith,
+ )
from testtools.twistedsupport import AsynchronousDeferredRunTest
from twisted.internet import defer
from zope.component import getUtility
@@ -19,10 +22,19 @@ from lp.archivepublisher.config import getPubConfig
from lp.archivepublisher.interfaces.archivegpgsigningkey import (
IArchiveGPGSigningKey,
ISignableArchive,
+ PUBLISHER_GPG_USES_SIGNING_SERVICE,
)
from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
+from lp.services.compat import mock
+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 (
+ SigningKeyType,
+ SigningMode,
+ )
+from lp.services.signing.tests.helpers import SigningServiceClientFixture
from lp.soyuz.enums import ArchivePurpose
from lp.testing import TestCaseWithFactory
from lp.testing.gpgkeys import gpgkeysdir
@@ -94,6 +106,80 @@ class TestSignableArchiveWithSigningKey(TestCaseWithFactory):
self.assertRaises(
AssertionError, signer.signFile, self.suite, filename_relative)
+ def test_signRepository_uses_signing_service(self):
+ # If the appropriate feature rule is true, then we use the signing
+ # service to sign files.
+ self.useFixture(
+ FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"}))
+ signing_service_client = self.useFixture(
+ SigningServiceClientFixture(self.factory))
+ self.factory.makeSigningKey(
+ key_type=SigningKeyType.OPENPGP,
+ fingerprint=self.archive.signing_key_fingerprint)
+ logger = BufferLogger()
+
+ suite_dir = os.path.join(self.archive_root, "dists", self.suite)
+ release_path = os.path.join(suite_dir, "Release")
+ write_file(release_path, b"Release contents")
+
+ signer = ISignableArchive(self.archive)
+ self.assertTrue(signer.can_sign)
+ self.assertContentEqual(
+ ["Release.gpg", "InRelease"],
+ signer.signRepository(self.suite, log=logger))
+ self.assertEqual("", logger.getLogBuffer())
+ signing_service_client.sign.assert_has_calls([
+ mock.call(
+ SigningKeyType.OPENPGP, self.archive.signing_key_fingerprint,
+ "Release", b"Release contents", SigningMode.DETACHED),
+ mock.call(
+ SigningKeyType.OPENPGP, self.archive.signing_key_fingerprint,
+ "Release", b"Release contents", SigningMode.CLEAR),
+ ])
+ self.assertThat(
+ os.path.join(suite_dir, "Release.gpg"),
+ FileContains("signed with key_type=OPENPGP mode=DETACHED"))
+ self.assertThat(
+ os.path.join(suite_dir, "InRelease"),
+ FileContains("signed with key_type=OPENPGP mode=CLEAR"))
+
+ def test_signRepository_falls_back_from_signing_service(self):
+ # If the signing service fails to sign a file, we fall back to
+ # making local signatures if possible.
+ self.useFixture(
+ FeatureFixture({PUBLISHER_GPG_USES_SIGNING_SERVICE: "on"}))
+ signing_service_client = self.useFixture(
+ SigningServiceClientFixture(self.factory))
+ self.factory.makeSigningKey(
+ key_type=SigningKeyType.OPENPGP,
+ fingerprint=self.archive.signing_key_fingerprint)
+ logger = BufferLogger()
+
+ suite_dir = os.path.join(self.archive_root, "dists", self.suite)
+ release_path = os.path.join(suite_dir, "Release")
+ write_file(release_path, b"Release contents")
+
+ signer = ISignableArchive(self.archive)
+ self.assertTrue(signer.can_sign)
+ signing_service_client.sign.side_effect = Exception("boom")
+ self.assertContentEqual(
+ ["Release.gpg", "InRelease"],
+ signer.signRepository(self.suite, log=logger))
+ self.assertEqual(
+ "ERROR Failed to sign archive using signing service; falling back "
+ "to local key\n", logger.getLogBuffer())
+ signing_service_client.sign.assert_called_once_with(
+ SigningKeyType.OPENPGP, self.archive.signing_key_fingerprint,
+ "Release", b"Release contents", SigningMode.DETACHED)
+ self.assertThat(
+ os.path.join(suite_dir, "Release.gpg"),
+ FileContains(
+ matcher=StartsWith("-----BEGIN PGP SIGNATURE-----\n")))
+ self.assertThat(
+ os.path.join(suite_dir, "InRelease"),
+ FileContains(
+ matcher=StartsWith("-----BEGIN PGP SIGNED MESSAGE-----\n")))
+
class TestSignableArchiveWithRunParts(RunPartsMixin, TestCaseWithFactory):
diff --git a/lib/lp/archivepublisher/tests/test_signing.py b/lib/lp/archivepublisher/tests/test_signing.py
index 6c70073..32f1df8 100644
--- a/lib/lp/archivepublisher/tests/test_signing.py
+++ b/lib/lp/archivepublisher/tests/test_signing.py
@@ -57,8 +57,10 @@ 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
+from lp.services.signing.enums import (
+ SigningKeyType,
+ SigningMode,
+ )
from lp.services.signing.tests.helpers import SigningServiceClientFixture
from lp.services.tarfile_helpers import LaunchpadWriteTarFile
from lp.soyuz.enums import ArchivePurpose
@@ -1918,8 +1920,16 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
key_types = (
SigningKeyType.UEFI, SigningKeyType.KMOD, SigningKeyType.OPAL,
SigningKeyType.SIPL, SigningKeyType.FIT)
+ modes = {
+ SigningKeyType.UEFI: SigningMode.ATTACHED,
+ SigningKeyType.KMOD: SigningMode.DETACHED,
+ SigningKeyType.OPAL: SigningMode.DETACHED,
+ SigningKeyType.SIPL: SigningMode.DETACHED,
+ SigningKeyType.FIT: SigningMode.ATTACHED,
+ }
expected_signed_contents = [
- b"signed with key_type=%s" % k.name for k in key_types]
+ b"signed with key_type=%s mode=%s" % (k.name, modes[k].name)
+ for k in key_types]
self.assertItemsEqual(expected_signed_contents, contents)
# Checks that all public keys ended up in the 1.0/control/xxx files
diff --git a/lib/lp/services/signing/interfaces/signingkey.py b/lib/lp/services/signing/interfaces/signingkey.py
index 1708ba2..dadabd6 100644
--- a/lib/lp/services/signing/interfaces/signingkey.py
+++ b/lib/lp/services/signing/interfaces/signingkey.py
@@ -47,11 +47,14 @@ class ISigningKey(Interface):
date_created = Datetime(
title=_('When this key was created'), required=True, readonly=True)
- def sign(message, message_name):
+ def sign(message, message_name, mode=None):
"""Sign the given message using this key
:param message: The message to be signed.
:param message_name: A name for the message being signed.
+ :param mode: A `SigningMode` specifying how the message is to be
+ signed. Defaults to `SigningMode.ATTACHED` for UEFI and FIT
+ keys, and `SigningMode.DETACHED` for other key types.
"""
@@ -59,6 +62,14 @@ class ISigningKeySet(Interface):
"""Interface to deal with the collection of signing keys
"""
+ def get(key_type, fingerprint):
+ """Get a signing key by key type and fingerprint.
+
+ :param key_type: A `SigningKeyType`.
+ :param fingerprint: The key's fingerprint.
+ :return: A `SigningKey`, or None.
+ """
+
def generate(key_type, description,
openpgp_key_algorithm=None, length=None):
"""Generates a new signing key on lp-signing and stores it in LP's
diff --git a/lib/lp/services/signing/model/signingkey.py b/lib/lp/services/signing/model/signingkey.py
index 4ecbed9..5dd21f5 100644
--- a/lib/lp/services/signing/model/signingkey.py
+++ b/lib/lp/services/signing/model/signingkey.py
@@ -87,6 +87,12 @@ class SigningKey(StormBase):
self.date_created = date_created
@classmethod
+ def get(cls, key_type, fingerprint):
+ """See `ISigningKeySet`."""
+ return IStore(SigningKey).find(
+ SigningKey, key_type=key_type, fingerprint=fingerprint).one()
+
+ @classmethod
def generate(cls, key_type, description,
openpgp_key_algorithm=None, length=None):
signing_service = getUtility(ISigningServiceClient)
@@ -123,11 +129,12 @@ class SigningKey(StormBase):
store.add(db_key)
return db_key
- def sign(self, message, message_name):
- if self.key_type in (SigningKeyType.UEFI, SigningKeyType.FIT):
- mode = SigningMode.ATTACHED
- else:
- mode = SigningMode.DETACHED
+ def sign(self, message, message_name, mode=None):
+ if mode is None:
+ if self.key_type in (SigningKeyType.UEFI, SigningKeyType.FIT):
+ mode = SigningMode.ATTACHED
+ else:
+ mode = SigningMode.DETACHED
signing_service = getUtility(ISigningServiceClient)
signed = signing_service.sign(
self.key_type, self.fingerprint, message_name, message, mode)
diff --git a/lib/lp/services/signing/proxy.py b/lib/lp/services/signing/proxy.py
index 932a483..390cca9 100644
--- a/lib/lp/services/signing/proxy.py
+++ b/lib/lp/services/signing/proxy.py
@@ -176,7 +176,10 @@ class SigningServiceClient:
"public-key": base64.b64decode(ret["public-key"])}
def sign(self, key_type, fingerprint, message_name, message, mode):
- if mode not in {SigningMode.ATTACHED, SigningMode.DETACHED}:
+ valid_modes = {SigningMode.ATTACHED, SigningMode.DETACHED}
+ if key_type == SigningKeyType.OPENPGP:
+ valid_modes.add(SigningMode.CLEAR)
+ if mode not in valid_modes:
raise ValueError("%s is not a valid mode" % mode)
if key_type not in SigningKeyType.items:
raise ValueError("%s is not a valid key type" % key_type)
diff --git a/lib/lp/services/signing/tests/helpers.py b/lib/lp/services/signing/tests/helpers.py
index 339c4f9..f819745 100644
--- a/lib/lp/services/signing/tests/helpers.py
+++ b/lib/lp/services/signing/tests/helpers.py
@@ -58,7 +58,8 @@ class SigningServiceClientFixture(fixtures.Fixture):
def _sign(self, key_type, fingerprint, message_name, message, mode):
key = bytes(PrivateKey.generate().public_key)
signed_msg = (
- "signed with key_type={}".format(key_type.name).encode("UTF-8"))
+ "signed with key_type={} mode={}".format(
+ key_type.name, mode.name).encode("UTF-8"))
data = {
'public-key': key,
'signed-message': signed_msg,
diff --git a/lib/lp/services/signing/tests/test_signingkey.py b/lib/lp/services/signing/tests/test_signingkey.py
index a7d7360..0259946 100644
--- a/lib/lp/services/signing/tests/test_signingkey.py
+++ b/lib/lp/services/signing/tests/test_signingkey.py
@@ -11,13 +11,24 @@ from nacl.public import PrivateKey
from pytz import utc
import responses
from storm.store import Store
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+ AfterPreprocessing,
+ Equals,
+ MatchesDict,
+ MatchesStructure,
+ )
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from lp.services.database.interfaces import IMasterStore
-from lp.services.signing.enums import SigningKeyType
-from lp.services.signing.interfaces.signingkey import IArchiveSigningKeySet
+from lp.services.signing.enums import (
+ SigningKeyType,
+ SigningMode,
+ )
+from lp.services.signing.interfaces.signingkey import (
+ IArchiveSigningKeySet,
+ ISigningKeySet,
+ )
from lp.services.signing.interfaces.signingserviceclient import (
ISigningServiceClient,
)
@@ -41,6 +52,24 @@ class TestSigningKey(TestCaseWithFactory, TestWithFixtures):
client = removeSecurityProxy(getUtility(ISigningServiceClient))
self.addCleanup(client._cleanCaches)
+ def test_get(self):
+ signing_keys = [
+ self.factory.makeSigningKey(key_type=key_type)
+ for key_type in (SigningKeyType.UEFI, SigningKeyType.OPENPGP)]
+ fingerprints = [
+ signing_key.fingerprint for signing_key in signing_keys]
+ signing_key_set = getUtility(ISigningKeySet)
+ self.assertEqual(
+ signing_keys[0],
+ signing_key_set.get(SigningKeyType.UEFI, fingerprints[0]))
+ self.assertEqual(
+ signing_keys[1],
+ signing_key_set.get(SigningKeyType.OPENPGP, fingerprints[1]))
+ self.assertIsNone(
+ signing_key_set.get(SigningKeyType.UEFI, fingerprints[1]))
+ self.assertIsNone(
+ signing_key_set.get(SigningKeyType.OPENPGP, fingerprints[0]))
+
@responses.activate
def test_generate_signing_key_saves_correctly(self):
self.signing_service.addResponses(self)
@@ -116,20 +145,78 @@ class TestSigningKey(TestCaseWithFactory, TestWithFixtures):
self.assertEqual(5, len(responses.calls))
@responses.activate
- def test_sign_some_data(self):
+ def test_sign(self):
self.signing_service.addResponses(self)
s = SigningKey(
SigningKeyType.UEFI, u"a fingerprint",
bytes(self.signing_service.generated_public_key),
description=u"This is my key!")
- signed = s.sign("secure message", "message_name")
+ signed = s.sign(b"secure message", "message_name")
# Checks if the returned value is actually the returning value from
# HTTP POST /sign call to lp-signing service
self.assertEqual(3, len(responses.calls))
+ self.assertThat(
+ responses.calls[2].request,
+ MatchesStructure(
+ url=Equals(self.signing_service.getUrl("/sign")),
+ body=AfterPreprocessing(
+ self.signing_service._decryptPayload,
+ MatchesDict({
+ "key-type": Equals("UEFI"),
+ "fingerprint": Equals(u"a fingerprint"),
+ "message-name": Equals("message_name"),
+ "message": Equals(
+ base64.b64encode(
+ b"secure message").decode("UTF-8")),
+ "mode": Equals("ATTACHED"),
+ }))))
self.assertEqual(self.signing_service.getAPISignedContent(), signed)
+ @responses.activate
+ def test_sign_openpgp_modes(self):
+ self.signing_service.addResponses(self)
+
+ s = SigningKey(
+ SigningKeyType.OPENPGP, u"a fingerprint",
+ bytes(self.signing_service.generated_public_key),
+ description=u"This is my key!")
+ s.sign(b"secure message", "message_name")
+ s.sign(b"another message", "another_name", mode=SigningMode.CLEAR)
+
+ self.assertEqual(5, len(responses.calls))
+ self.assertThat(
+ responses.calls[2].request,
+ MatchesStructure(
+ url=Equals(self.signing_service.getUrl("/sign")),
+ body=AfterPreprocessing(
+ self.signing_service._decryptPayload,
+ MatchesDict({
+ "key-type": Equals("OPENPGP"),
+ "fingerprint": Equals(u"a fingerprint"),
+ "message-name": Equals("message_name"),
+ "message": Equals(
+ base64.b64encode(
+ b"secure message").decode("UTF-8")),
+ "mode": Equals("DETACHED"),
+ }))))
+ self.assertThat(
+ responses.calls[4].request,
+ MatchesStructure(
+ url=Equals(self.signing_service.getUrl("/sign")),
+ body=AfterPreprocessing(
+ self.signing_service._decryptPayload,
+ MatchesDict({
+ "key-type": Equals("OPENPGP"),
+ "fingerprint": Equals(u"a fingerprint"),
+ "message-name": Equals("another_name"),
+ "message": Equals(
+ base64.b64encode(
+ b"another message").decode("UTF-8")),
+ "mode": Equals("CLEAR"),
+ }))))
+
class TestArchiveSigningKey(TestCaseWithFactory):
layer = ZopelessDatabaseLayer
