launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/archive-get-signing-key-data into lp:launchpad.

Commit message:
Add Archive.getSigningKeyData, currently just proxying through to the keyserver.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1667725 in Launchpad itself: "[feature request] make full ppa signing public key available over https"
  https://bugs.launchpad.net/launchpad/+bug/1667725

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/archive-get-signing-key-data/+merge/350431

GPGHandler.retrieveKey uses urlfetch and thus honours the webapp request timeout, so this shouldn't kill the webapp even if the keyserver is timing out.

At the moment, the only use for this will be making it possible to add a PPA from a restricted network with one fewer firewall egress rule.  If we go ahead with the plan to store PPA signing keys in the LP database, it will become more useful as we'll be able to remove the keyserver as a point of unreliability.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/archive-get-signing-key-data into lp:launchpad.

=== modified file 'lib/lp/services/gpg/interfaces.py'
--- lib/lp/services/gpg/interfaces.py	2018-03-02 16:17:35 +0000
+++ lib/lp/services/gpg/interfaces.py	2018-07-22 20:52:48 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 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).
 
 __all__ = [
@@ -21,12 +21,14 @@
     'valid_keyid',
     ]
 
+import httplib
 import re
 
 from lazr.enum import (
     DBEnumeratedType,
     DBItem,
     )
+from lazr.restful.declarations import error_status
 from zope.interface import (
     Attribute,
     Interface,
@@ -112,6 +114,7 @@
         super(GPGKeyNotFoundError, self).__init__(message)
 
 
+@error_status(httplib.INTERNAL_SERVER_ERROR)
 class GPGKeyTemporarilyNotFoundError(GPGKeyNotFoundError):
     """The GPG key with the given fingerprint was not found on the keyserver.
 
@@ -126,6 +129,7 @@
             fingerprint, message)
 
 
+@error_status(httplib.NOT_FOUND)
 class GPGKeyDoesNotExistOnServer(GPGKeyNotFoundError):
     """The GPG key with the given fingerprint was not found on the keyserver.
 

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2018-06-27 00:27:02 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2018-07-22 20:52:48 +0000
@@ -462,7 +462,18 @@
     series_with_sources = Attribute(
         "DistroSeries to which this archive has published sources")
     signing_key = Object(
-        title=_('Repository sigining key.'), required=False, schema=IGPGKey)
+        title=_('Repository signing key.'), required=False, schema=IGPGKey)
+
+    @export_read_operation()
+    @operation_for_version("devel")
+    def getSigningKeyData():
+        """Get the public key used to sign this repository.
+
+        Returns the public key material as a byte string, None if the
+        repository has no signing key, or an HTTP status code if the
+        repository has a signing key but it cannot be retrieved from the
+        keyserver.
+        """
 
     def getAuthToken(person):
         """Returns an IArchiveAuthToken for the archive in question for

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2018-06-27 00:27:02 +0000
+++ lib/lp/soyuz/model/archive.py	2018-07-22 20:52:48 +0000
@@ -120,6 +120,7 @@
     )
 from lp.services.database.stormexpr import BulkUpdate
 from lp.services.features import getFeatureFlag
+from lp.services.gpg.interfaces import IGPGHandler
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.model import (
     LibraryFileAlias,
@@ -409,6 +410,14 @@
             return getUtility(IGPGKeySet).getByFingerprint(
                 self.signing_key_fingerprint)
 
+    def getSigningKeyData(self):
+        """See `IArchive`."""
+        if self.signing_key_fingerprint is not None:
+            # This may raise GPGKeyNotFoundError, which we allow to
+            # propagate as an HTTP error.
+            return getUtility(IGPGHandler).retrieveKey(
+                self.signing_key_fingerprint).export()
+
     @property
     def is_ppa(self):
         """See `IArchive`."""

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2018-05-04 21:59:32 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2018-07-22 20:52:48 +0000
@@ -11,9 +11,11 @@
     timedelta,
     )
 import doctest
+import httplib
 import os.path
 
 from pytz import UTC
+import responses
 import six
 from testtools.matchers import (
     AllMatch,
@@ -55,11 +57,17 @@
 from lp.services.database.sqlbase import sqlvalues
 from lp.services.features import getFeatureFlag
 from lp.services.features.testing import FeatureFixture
+from lp.services.gpg.interfaces import (
+    GPGKeyDoesNotExistOnServer,
+    GPGKeyTemporarilyNotFoundError,
+    IGPGHandler,
+    )
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.propertycache import (
     clear_property_cache,
     get_property_cache,
     )
+from lp.services.timeout import default_timeout
 from lp.services.webapp.interfaces import OAuthPermission
 from lp.services.worlddata.interfaces.country import ICountrySet
 from lp.soyuz.adapters.archivedependencies import (
@@ -138,6 +146,7 @@
     )
 from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import webservice_for_person
+from lp.testing.views import create_webservice_error_view
 
 
 class TestGetPublicationsInArchive(TestCaseWithFactory):
@@ -3789,6 +3798,69 @@
         self.assertEqual(person.gpg_keys[0], ppa_with_key.signing_key)
 
 
+class TestGetSigningKeyData(TestCaseWithFactory):
+    """Test `Archive.getSigningKeyData`.
+
+    We just use `responses` to mock the keyserver here; the details of its
+    implementation aren't especially important, we can't use
+    `InProcessKeyServerFixture` because the keyserver operations are
+    synchronous, and `responses` is much faster than `KeyServerTac`.
+    """
+
+    layer = DatabaseFunctionalLayer
+    run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
+
+    def test_getSigningKeyData_no_fingerprint(self):
+        ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        self.assertIsNone(ppa.getSigningKeyData())
+
+    @responses.activate
+    def test_getSigningKeyData_keyserver_success(self):
+        ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        key_path = os.path.join(gpgkeysdir, "ppa-sample@xxxxxxxxxxxxxxxxx")
+        gpghandler = getUtility(IGPGHandler)
+        with open(key_path, "rb") as key_file:
+            secret_key = gpghandler.importSecretKey(key_file.read())
+        public_key = gpghandler.retrieveKey(secret_key.fingerprint)
+        public_key_data = public_key.export()
+        removeSecurityProxy(ppa).signing_key_fingerprint = (
+            public_key.fingerprint)
+        key_url = gpghandler.getURLForKeyInServer(
+            public_key.fingerprint, action="get")
+        responses.add("GET", key_url, body=public_key_data)
+        gpghandler.resetLocalState()
+        with default_timeout(5.0):
+            self.assertEqual(public_key_data, ppa.getSigningKeyData())
+
+    @responses.activate
+    def test_getSigningKeyData_not_found_on_keyserver(self):
+        ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        gpghandler = getUtility(IGPGHandler)
+        removeSecurityProxy(ppa).signing_key_fingerprint = "dummy-fp"
+        key_url = gpghandler.getURLForKeyInServer("dummy-fp", action="get")
+        responses.add(
+            "GET", key_url, status=404,
+            body="No results found: No keys found")
+        with default_timeout(5.0):
+            error = self.assertRaises(
+                GPGKeyDoesNotExistOnServer, ppa.getSigningKeyData)
+        error_view = create_webservice_error_view(error)
+        self.assertEqual(httplib.NOT_FOUND, error_view.status)
+
+    @responses.activate
+    def test_getSigningKeyData_keyserver_failure(self):
+        ppa = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        gpghandler = getUtility(IGPGHandler)
+        removeSecurityProxy(ppa).signing_key_fingerprint = "dummy-fp"
+        key_url = gpghandler.getURLForKeyInServer("dummy-fp", action="get")
+        responses.add("GET", key_url, status=500)
+        with default_timeout(5.0):
+            error = self.assertRaises(
+                GPGKeyTemporarilyNotFoundError, ppa.getSigningKeyData)
+        error_view = create_webservice_error_view(error)
+        self.assertEqual(httplib.INTERNAL_SERVER_ERROR, error_view.status)
+
+
 class TestCountersAndSummaries(TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer