Merge lp:~cjwatson/launchpad/authserver-issue-macaroon into lp:launchpad

[Colin Watson]

Add an authserver method to issue macaroons (currently only snap-build).

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py	2019-04-24 12:50:20 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py	2019-04-25 09:56:06 +0000
@@ -25,6 +25,7 @@
     )
 import transaction
 from zope.component import getUtility
+from zope.publisher.xmlrpc import TestRequest
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
@@ -53,6 +54,7 @@
     make_running_import,
     )
 from lp.code.tests.helpers import GitHostingFixture
+from lp.services.authserver.xmlrpc import AuthServerAPIView
 from lp.services.config import config
 from lp.services.database.constants import UTC_NOW
 from lp.services.database.interfaces import IStore
@@ -78,6 +80,8 @@
     LaunchpadFunctionalLayer,
     )
 from lp.testing.pages import get_feedback_messages
+from lp.xmlrpc import faults
+from lp.xmlrpc.interfaces import IPrivateApplication
 
 
 def login_for_code_imports():
@@ -1306,6 +1310,14 @@
         self.pushConfig("codeimport", macaroon_secret_key="some-secret")
         self.test_issueMacaroon_good()
 
+    def test_issueMacaroon_not_via_authserver(self):
+        job = self.makeJob()
+        private_root = getUtility(IPrivateApplication)
+        authserver = AuthServerAPIView(private_root.authserver, TestRequest())
+        self.assertEqual(
+            faults.PermissionDenied(),
+            authserver.issueMacaroon("code-import-job", "CodeImportJob", job))
+
     def test_verifyMacaroon_good(self):
         machine = self.factory.makeCodeImportMachine(set_online=True)
         job = self.makeJob()

=== modified file 'lib/lp/services/authserver/interfaces.py'
--- lib/lp/services/authserver/interfaces.py	2019-04-23 13:50:35 +0000
+++ lib/lp/services/authserver/interfaces.py	2019-04-25 09:56:06 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interface for the XML-RPC authentication server."""
@@ -27,12 +27,27 @@
             person with the given name.
         """
 
+    def issueMacaroon(issuer_name, context_type, context):
+        """Issue a macaroon of type `issuer_name` for `context`.
+
+        :param issuer_name: An `IMacaroonIssuer` name.  Only issuers where
+            `issuable_via_authserver` is True are permitted.
+        :param context_type: A string identifying the type of context for
+            which to issue the macaroon.  Currently only 'LibraryFileAlias'
+            and 'SnapBuild' are supported.
+        :param context: The context for which to issue the macaroon.  Note
+            that this is passed over XML-RPC, so it should be plain data
+            (e.g. an ID) rather than a database object.
+        :return: A serialised macaroon or a fault.
+        """
+
     def verifyMacaroon(macaroon_raw, context_type, context):
         """Verify that `macaroon_raw` grants access to `context`.
 
         :param macaroon_raw: A serialised macaroon.
         :param context_type: A string identifying the type of context to
-            check.  Currently only 'LibraryFileAlias' is supported.
+            check.  Currently only 'LibraryFileAlias' and 'SnapBuild' are
+            supported.
         :param context: The context to check.  Note that this is passed over
             XML-RPC, so it should be plain data (e.g. an ID) rather than a
             database object.

=== modified file 'lib/lp/services/authserver/tests/test_authserver.py'
--- lib/lp/services/authserver/tests/test_authserver.py	2019-04-24 12:50:20 +0000
+++ lib/lp/services/authserver/tests/test_authserver.py	2019-04-25 09:56:06 +0000
@@ -7,7 +7,12 @@
 
 from pymacaroons import Macaroon
 from storm.sqlobject import SQLObjectNotFound
-from testtools.matchers import Is
+from testtools.matchers import (
+    Equals,
+    Is,
+    MatchesListwise,
+    MatchesStructure,
+    )
 from zope.component import getUtility
 from zope.interface import implementer
 from zope.publisher.xmlrpc import TestRequest
@@ -25,7 +30,6 @@
 from lp.services.macaroons.model import MacaroonIssuerBase
 from lp.testing import (
     person_logged_in,
-    TestCase,
     TestCaseWithFactory,
     )
 from lp.testing.fixture import ZopeUtilityFixture
@@ -82,6 +86,7 @@
 class DummyMacaroonIssuer(MacaroonIssuerBase):
 
     identifier = 'test'
+    issuable_via_authserver = True
     _root_secret = 'test'
 
     def checkIssuingContext(self, context):
@@ -101,12 +106,12 @@
         return caveat_value == str(context.id)
 
 
-class VerifyMacaroonTests(TestCase):
+class MacaroonTests(TestCaseWithFactory):
 
     layer = ZopelessDatabaseLayer
 
     def setUp(self):
-        super(VerifyMacaroonTests, self).setUp()
+        super(MacaroonTests, self).setUp()
         self.issuer = DummyMacaroonIssuer()
         self.useFixture(ZopeUtilityFixture(
             self.issuer, IMacaroonIssuer, name='test'))
@@ -114,12 +119,46 @@
         self.authserver = AuthServerAPIView(
             private_root.authserver, TestRequest())
 
-    def test_nonsense_macaroon(self):
+    def test_issue_unknown_issuer(self):
+        self.assertEqual(
+            faults.PermissionDenied(),
+            self.authserver.issueMacaroon(
+                'unknown-issuer', 'LibraryFileAlias', 1))
+
+    def test_issue_wrong_context_type(self):
+        self.assertEqual(
+            faults.PermissionDenied(),
+            self.authserver.issueMacaroon(
+                'unknown-issuer', 'nonsense', 1))
+
+    def test_issue_not_issuable_via_authserver(self):
+        self.issuer.issuable_via_authserver = False
+        self.assertEqual(
+            faults.PermissionDenied(),
+            self.authserver.issueMacaroon('test', 'LibraryFileAlias', 1))
+
+    def test_issue_bad_context(self):
+        build = self.factory.makeSnapBuild()
+        self.assertEqual(
+            faults.PermissionDenied(),
+            self.authserver.issueMacaroon('test', 'SnapBuild', build.id))
+
+    def test_issue_success(self):
+        macaroon = Macaroon.deserialize(
+            self.authserver.issueMacaroon('test', 'LibraryFileAlias', 1))
+        self.assertThat(macaroon, MatchesStructure(
+            location=Equals(config.vhost.mainsite.hostname),
+            identifier=Equals('test'),
+            caveats=MatchesListwise([
+                MatchesStructure.byEquality(caveat_id='lp.test 1'),
+                ])))
+
+    def test_verify_nonsense_macaroon(self):
         self.assertEqual(
             faults.Unauthorized(),
             self.authserver.verifyMacaroon('nonsense', 'LibraryFileAlias', 1))
 
-    def test_unknown_issuer(self):
+    def test_verify_unknown_issuer(self):
         macaroon = Macaroon(
             location=config.vhost.mainsite.hostname,
             identifier='unknown-issuer', key='test')
@@ -128,7 +167,7 @@
             self.authserver.verifyMacaroon(
                 macaroon.serialize(), 'LibraryFileAlias', 1))
 
-    def test_wrong_context_type(self):
+    def test_verify_wrong_context_type(self):
         lfa = getUtility(ILibraryFileAliasSet)[1]
         macaroon = self.issuer.issueMacaroon(lfa)
         self.assertEqual(
@@ -136,7 +175,7 @@
             self.authserver.verifyMacaroon(
                 macaroon.serialize(), 'nonsense', lfa.id))
 
-    def test_wrong_context(self):
+    def test_verify_wrong_context(self):
         lfa = getUtility(ILibraryFileAliasSet)[1]
         macaroon = self.issuer.issueMacaroon(lfa)
         self.assertEqual(
@@ -144,7 +183,7 @@
             self.authserver.verifyMacaroon(
                 macaroon.serialize(), 'LibraryFileAlias', 2))
 
-    def test_nonexistent_lfa(self):
+    def test_verify_nonexistent_lfa(self):
         macaroon = self.issuer.issueMacaroon(
             getUtility(ILibraryFileAliasSet)[1])
         # Pick a large ID that doesn't exist in sampledata.
@@ -157,7 +196,7 @@
             self.authserver.verifyMacaroon(
                 macaroon.serialize(), 'LibraryFileAlias', lfa_id))
 
-    def test_success(self):
+    def test_verify_success(self):
         lfa = getUtility(ILibraryFileAliasSet)[1]
         macaroon = self.issuer.issueMacaroon(lfa)
         self.assertThat(

=== modified file 'lib/lp/services/authserver/xmlrpc.py'
--- lib/lp/services/authserver/xmlrpc.py	2019-04-23 13:50:35 +0000
+++ lib/lp/services/authserver/xmlrpc.py	2019-04-25 09:56:06 +0000
@@ -17,6 +17,7 @@
     getUtility,
     )
 from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
 
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.authserver.interfaces import (
@@ -24,8 +25,12 @@
     IAuthServerApplication,
     )
 from lp.services.librarian.interfaces import ILibraryFileAliasSet
-from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.services.macaroons.interfaces import (
+    BadMacaroonContext,
+    IMacaroonIssuer,
+    )
 from lp.services.webapp import LaunchpadXMLRPCView
+from lp.snappy.interfaces.snapbuild import ISnapBuildSet
 from lp.xmlrpc import faults
 
 
@@ -45,6 +50,53 @@
                      for key in person.sshkeys],
             }
 
+    def _resolveContext(self, context_type, context):
+        """Resolve a serialised context.
+
+        :param context_type: A string identifying the type of context.
+            Currently only 'LibraryFileAlias' and 'SnapBuild' are supported.
+        :param context: The context as plain data (e.g. an ID).
+        :return: The resolved context, or None.
+        """
+        if context_type == 'LibraryFileAlias':
+            # The context is a `LibraryFileAlias` ID.
+            try:
+                return getUtility(ILibraryFileAliasSet)[context]
+            except SQLObjectNotFound:
+                return None
+        elif context_type == 'SnapBuild':
+            # The context is a `SnapBuild` ID.
+            return getUtility(ISnapBuildSet).getByID(context)
+        else:
+            return None
+
+    def issueMacaroon(self, issuer_name, context_type, context):
+        """See `IAuthServer.issueMacaroon`."""
+        try:
+            issuer = getUtility(IMacaroonIssuer, issuer_name)
+        except ComponentLookupError:
+            return faults.PermissionDenied()
+        # Only permit issuers that have been specifically designed for use
+        # with the authserver: they must need to be issued by parts of
+        # Launchpad other than appservers but be verified by appservers,
+        # they must take parameters that can be passed over XML-RPC, and
+        # they must issue macaroons with carefully-designed constraints to
+        # minimise privilege-escalation attacks.
+        if not issuer.issuable_via_authserver:
+            return faults.PermissionDenied()
+        # The context is plain data, since we can't pass general objects over
+        # the XML-RPC interface.  Look it up so that we can verify it.
+        context = self._resolveContext(context_type, context)
+        if context is None:
+            return faults.PermissionDenied()
+        try:
+            # issueMacaroon isn't normally public, but we clearly need it
+            # here.
+            macaroon = removeSecurityProxy(issuer).issueMacaroon(context)
+        except BadMacaroonContext:
+            return faults.PermissionDenied()
+        return macaroon.serialize()
+
     def verifyMacaroon(self, macaroon_raw, context_type, context):
         """See `IAuthServer.verifyMacaroon`."""
         try:
@@ -59,15 +111,10 @@
             return faults.Unauthorized()
         # The context is plain data, since we can't pass general objects over
         # the XML-RPC interface.  Look it up so that we can verify it.
-        if context_type == 'LibraryFileAlias':
-            # The context is a `LibraryFileAlias` ID.
-            try:
-                lfa = getUtility(ILibraryFileAliasSet)[context]
-            except SQLObjectNotFound:
-                return faults.Unauthorized()
-        else:
+        context = self._resolveContext(context_type, context)
+        if context is None:
             return faults.Unauthorized()
-        if not issuer.verifyMacaroon(macaroon, lfa):
+        if not issuer.verifyMacaroon(macaroon, context):
             return faults.Unauthorized()
         return True
 

=== modified file 'lib/lp/services/macaroons/interfaces.py'
--- lib/lp/services/macaroons/interfaces.py	2019-04-24 12:50:20 +0000
+++ lib/lp/services/macaroons/interfaces.py	2019-04-25 09:56:06 +0000
@@ -12,6 +12,7 @@
     ]
 
 from zope.interface import Interface
+from zope.schema import Bool
 
 
 class BadMacaroonContext(Exception):
@@ -27,6 +28,9 @@
 class IMacaroonIssuerPublic(Interface):
     """Public interface to a policy for verifying macaroons."""
 
+    issuable_via_authserver = Bool(
+        "Does this issuer allow issuing macaroons via the authserver?")
+
     def verifyMacaroon(macaroon, context, require_context=True):
         """Verify that `macaroon` is valid for `context`.
 
@@ -49,6 +53,6 @@
 
         :param context: The context that the returned macaroon should relate
             to.
-        :raises ValueError: if the context is unsuitable.
+        :raises BadMacaroonContext: if the context is unsuitable.
         :return: A macaroon.
         """

=== modified file 'lib/lp/services/macaroons/model.py'
--- lib/lp/services/macaroons/model.py	2019-04-24 16:15:47 +0000
+++ lib/lp/services/macaroons/model.py	2019-04-25 09:56:06 +0000
@@ -23,6 +23,8 @@
 class MacaroonIssuerBase:
     """See `IMacaroonIssuer`."""
 
+    issuable_via_authserver = False
+
     @property
     def identifier(self):
         """An identifying name for this issuer."""

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2019-04-24 12:50:20 +0000
+++ lib/lp/snappy/model/snapbuild.py	2019-04-25 09:56:06 +0000
@@ -597,17 +597,14 @@
 class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
 
     identifier = "snap-build"
+    issuable_via_authserver = True
 
     def checkIssuingContext(self, context):
         """See `MacaroonIssuerBase`.
 
-        For issuing, the context is an `ISnapBuild` or its ID.
+        For issuing, the context is an `ISnapBuild`.
         """
-        if ISnapBuild.providedBy(context):
-            pass
-        elif isinstance(context, int):
-            context = getUtility(ISnapBuildSet).getByID(context)
-        else:
+        if not ISnapBuild.providedBy(context):
             raise BadMacaroonContext(context)
         if not removeSecurityProxy(context).is_private:
             raise BadMacaroonContext(

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2019-04-24 12:50:20 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2019-04-25 09:56:06 +0000
@@ -26,6 +26,7 @@
     MatchesStructure,
     )
 from zope.component import getUtility
+from zope.publisher.xmlrpc import TestRequest
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.enums import InformationType
@@ -36,6 +37,7 @@
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.registry.enums import PersonVisibility
+from lp.services.authserver.xmlrpc import AuthServerAPIView
 from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
 from lp.services.job.interfaces.job import JobStatus
@@ -73,6 +75,7 @@
 from lp.testing.mail_helpers import pop_notifications
 from lp.testing.matchers import HasQueryCount
 from lp.testing.pages import webservice_for_person
+from lp.xmlrpc.interfaces import IPrivateApplication
 
 
 expected_body = """\
@@ -813,6 +816,21 @@
                     caveat_id="lp.snap-build %s" % build.id),
                 ])))
 
+    def test_issueMacaroon_via_authserver(self):
+        build = self.factory.makeSnapBuild(
+            snap=self.factory.makeSnap(private=True))
+        private_root = getUtility(IPrivateApplication)
+        authserver = AuthServerAPIView(private_root.authserver, TestRequest())
+        macaroon = Macaroon.deserialize(
+            authserver.issueMacaroon("snap-build", "SnapBuild", build.id))
+        self.assertThat(macaroon, MatchesStructure(
+            location=Equals("launchpad.dev"),
+            identifier=Equals("snap-build"),
+            caveats=MatchesListwise([
+                MatchesStructure.byEquality(
+                    caveat_id="lp.snap-build %s" % build.id),
+                ])))
+
     def test_verifyMacaroon_good(self):
         [ref] = self.factory.makeGitRefs(
             information_type=InformationType.USERDATA)

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2019-04-24 12:50:20 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2019-04-25 09:56:06 +0000
@@ -18,6 +18,7 @@
     MatchesStructure,
     )
 from zope.component import getUtility
+from zope.publisher.xmlrpc import TestRequest
 from zope.security.proxy import removeSecurityProxy
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
@@ -27,6 +28,7 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
+from lp.services.authserver.xmlrpc import AuthServerAPIView
 from lp.services.config import config
 from lp.services.log.logger import DevNullLogger
 from lp.services.macaroons.interfaces import (
@@ -67,6 +69,8 @@
     LaunchpadZopelessLayer,
     )
 from lp.testing.pages import webservice_for_person
+from lp.xmlrpc import faults
+from lp.xmlrpc.interfaces import IPrivateApplication
 
 
 class TestBinaryPackageBuild(TestCaseWithFactory):
@@ -938,6 +942,16 @@
                 caveat_id="lp.principal.binary-package-build %s" % build.id),
             ]))
 
+    def test_issueMacaroon_not_via_authserver(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        private_root = getUtility(IPrivateApplication)
+        authserver = AuthServerAPIView(private_root.authserver, TestRequest())
+        self.assertEqual(
+            faults.PermissionDenied(),
+            authserver.issueMacaroon(
+                "binary-package-build", "BinaryPackageBuild", build))
+
     def test_verifyMacaroon_good(self):
         build = self.factory.makeBinaryPackageBuild(
             archive=self.factory.makeArchive(private=True))