launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/authserver-issue-macaroon into lp:launchpad with lp:~cjwatson/launchpad/snap-build-macaroon as a prerequisite.

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1639975 in Launchpad itself: "support for building private snaps"
  https://bugs.launchpad.net/launchpad/+bug/1639975

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/authserver-issue-macaroon/+merge/364353
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/authserver-issue-macaroon into lp:launchpad.

=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py	2019-03-12 23:34:10 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py	2019-03-12 23:34:11 +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).
 
 """Unit tests for CodeImportJob and CodeImportJobWorkflow."""
@@ -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
@@ -75,6 +77,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():
@@ -1303,6 +1307,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", job))
+
     def test_checkMacaroonIssuer_good(self):
         job = self.makeJob()
         issuer = getUtility(IMacaroonIssuer, "code-import-job")

=== modified file 'lib/lp/services/authserver/interfaces.py'
--- lib/lp/services/authserver/interfaces.py	2018-05-10 10:05:45 +0000
+++ lib/lp/services/authserver/interfaces.py	2019-03-12 23:34:11 +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,6 +27,17 @@
             person with the given name.
         """
 
+    def issueMacaroon(issuer_name, 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: 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):
         """Verify that `macaroon_raw` grants access to `context`.
 

=== modified file 'lib/lp/services/authserver/tests/test_authserver.py'
--- lib/lp/services/authserver/tests/test_authserver.py	2019-03-12 23:34:10 +0000
+++ lib/lp/services/authserver/tests/test_authserver.py	2019-03-12 23:34:11 +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).
 
 """Tests for the internal codehosting API."""
@@ -6,7 +6,12 @@
 __metaclass__ = type
 
 from pymacaroons import Macaroon
-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
@@ -74,6 +79,7 @@
 class DummyMacaroonIssuer(MacaroonIssuerBase):
 
     identifier = 'test'
+    issuable_via_authserver = True
     _root_secret = 'test'
 
     def issueMacaroon(self, context):
@@ -87,12 +93,12 @@
         return caveat_text == str(context)
 
 
-class VerifyMacaroonTests(TestCase):
+class MacaroonTests(TestCase):
 
     layer = ZopelessLayer
 
     def setUp(self):
-        super(VerifyMacaroonTests, self).setUp()
+        super(MacaroonTests, self).setUp()
         self.issuer = DummyMacaroonIssuer()
         self.useFixture(ZopeUtilityFixture(
             self.issuer, IMacaroonIssuer, name='test'))
@@ -100,12 +106,38 @@
         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', 0))
+
+    def test_issue_bad_context_type(self):
+        self.assertEqual(
+            faults.PermissionDenied(),
+            self.authserver.issueMacaroon('unknown-issuer', ''))
+
+    def test_issue_not_issuable_via_authserver(self):
+        self.issuer.issuable_via_authserver = False
+        self.assertEqual(
+            faults.PermissionDenied(),
+            self.authserver.issueMacaroon('test', 0))
+
+    def test_issue_success(self):
+        macaroon = Macaroon.deserialize(
+            self.authserver.issueMacaroon('test', 0))
+        self.assertThat(macaroon, MatchesStructure(
+            location=Equals(config.vhost.mainsite.hostname),
+            identifier=Equals('test'),
+            caveats=MatchesListwise([
+                MatchesStructure.byEquality(caveat_id='lp.test 0'),
+                ])))
+
+    def test_verify_nonsense_macaroon(self):
         self.assertEqual(
             faults.Unauthorized(),
             self.authserver.verifyMacaroon('nonsense', 0))
 
-    def test_unknown_issuer(self):
+    def test_verify_unknown_issuer(self):
         macaroon = Macaroon(
             location=config.vhost.mainsite.hostname,
             identifier='unknown-issuer', key='test')
@@ -113,13 +145,13 @@
             faults.Unauthorized(),
             self.authserver.verifyMacaroon(macaroon.serialize(), 0))
 
-    def test_wrong_context(self):
+    def test_verify_wrong_context(self):
         macaroon = self.issuer.issueMacaroon(0)
         self.assertEqual(
             faults.Unauthorized(),
             self.authserver.verifyMacaroon(macaroon.serialize(), 1))
 
-    def test_success(self):
+    def test_verify_success(self):
         macaroon = self.issuer.issueMacaroon(0)
         self.assertThat(
             self.authserver.verifyMacaroon(macaroon.serialize(), 0), Is(True))

=== modified file 'lib/lp/services/authserver/xmlrpc.py'
--- lib/lp/services/authserver/xmlrpc.py	2018-05-10 10:05:45 +0000
+++ lib/lp/services/authserver/xmlrpc.py	2019-03-12 23:34:11 +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).
 
 """Auth-Server XML-RPC API ."""
@@ -16,6 +16,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 (
@@ -43,6 +44,28 @@
                      for key in person.sshkeys],
             }
 
+    def issueMacaroon(self, issuer_name, 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()
+        try:
+            # issueMacaroon isn't normally public, but we clearly need it
+            # here.
+            macaroon = removeSecurityProxy(issuer).issueMacaroon(context)
+        except ValueError:
+            return faults.PermissionDenied()
+        return macaroon.serialize()
+
     def verifyMacaroon(self, macaroon_raw, context):
         """See `IAuthServer.verifyMacaroon`."""
         try:

=== modified file 'lib/lp/services/macaroons/interfaces.py'
--- lib/lp/services/macaroons/interfaces.py	2016-10-07 15:50:10 +0000
+++ lib/lp/services/macaroons/interfaces.py	2019-03-12 23:34:11 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interface to a policy for issuing and verifying macaroons."""
@@ -11,11 +11,15 @@
     ]
 
 from zope.interface import Interface
+from zope.schema import Bool
 
 
 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 checkMacaroonIssuer(macaroon):
         """Check that `macaroon` was issued by this issuer.
 

=== modified file 'lib/lp/services/macaroons/model.py'
--- lib/lp/services/macaroons/model.py	2019-03-12 23:34:10 +0000
+++ lib/lp/services/macaroons/model.py	2019-03-12 23:34:11 +0000
@@ -21,6 +21,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-03-12 23:34:10 +0000
+++ lib/lp/snappy/model/snapbuild.py	2019-03-12 23:34:11 +0000
@@ -594,6 +594,7 @@
 class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
 
     identifier = "snap-build"
+    issuable_via_authserver = True
 
     def issueMacaroon(self, context):
         """See `IMacaroonIssuer`.

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2019-03-12 23:34:10 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2019-03-12 23:34:11 +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
@@ -70,6 +72,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 = """\
@@ -809,6 +812,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", 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_checkMacaroonIssuer_good(self):
         build = self.factory.makeSnapBuild(
             snap=self.factory.makeSnap(private=True))

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2019-03-12 23:34:10 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2019-03-12 23:34:11 +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).
 
 """Test Build features."""
@@ -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 IMacaroonIssuer
@@ -64,6 +66,8 @@
     LaunchpadZopelessLayer,
     )
 from lp.testing.pages import webservice_for_person
+from lp.xmlrpc import faults
+from lp.xmlrpc.interfaces import IPrivateApplication
 
 
 class TestBinaryPackageBuild(TestCaseWithFactory):
@@ -934,6 +938,15 @@
                 caveat_id="lp.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", build))
+
     def test_checkMacaroonIssuer_good(self):
         build = self.factory.makeBinaryPackageBuild(
             archive=self.factory.makeArchive(private=True))