launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:verify-archive-macaroons into launchpad:master with ~cjwatson/launchpad:livefsbuild-macaroons as a prerequisite.

Commit message:
Extend ArchiveAPI to verify buildd macaroons

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400788

This should allow issuing build-scoped tokens rather than relying on the buildd secret, provided that the WSGI authenticator for private PPAs is in use.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:verify-archive-macaroons into launchpad:master.

diff --git a/lib/lp/soyuz/xmlrpc/archive.py b/lib/lp/soyuz/xmlrpc/archive.py
index 6050c8c..5acf7f3 100644
--- a/lib/lp/soyuz/xmlrpc/archive.py
+++ b/lib/lp/soyuz/xmlrpc/archive.py
@@ -10,10 +10,18 @@ __all__ = [
     'ArchiveAPI',
     ]
 
-from zope.component import getUtility
+from pymacaroons import Macaroon
+from zope.component import (
+    ComponentLookupError,
+    getUtility,
+    )
 from zope.interface import implementer
 from zope.security.proxy import removeSecurityProxy
 
+from lp.services.macaroons.interfaces import (
+    IMacaroonIssuer,
+    NO_USER,
+    )
 from lp.services.webapp import LaunchpadXMLRPCView
 from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.interfaces.archiveapi import IArchiveAPI
@@ -29,6 +37,24 @@ BUILDD_USER_NAME = "buildd"
 class ArchiveAPI(LaunchpadXMLRPCView):
     """See `IArchiveAPI`."""
 
+    def _verifyMacaroon(self, archive, password):
+        try:
+            macaroon = Macaroon.deserialize(password)
+        # XXX cjwatson 2021-03-31: Restrict exceptions once
+        # https://github.com/ecordell/pymacaroons/issues/50 is fixed.
+        except Exception:
+            return False
+        try:
+            issuer = getUtility(IMacaroonIssuer, macaroon.identifier)
+        except ComponentLookupError:
+            return False
+        verified = issuer.verifyMacaroon(macaroon, archive)
+        if verified and verified.user != NO_USER:
+            # We currently only permit verifying standalone macaroons, not
+            # ones issued on behalf of a particular user.
+            return False
+        return verified
+
     @return_fault
     def _checkArchiveAuthToken(self, archive_reference, username, password):
         archive = getUtility(IArchiveSet).getByReference(archive_reference)
@@ -37,6 +63,15 @@ class ArchiveAPI(LaunchpadXMLRPCView):
                 message="No archive found for '%s'." % archive_reference)
         archive = removeSecurityProxy(archive)
         token_set = getUtility(IArchiveAuthTokenSet)
+
+        # If the password is a serialized macaroon for the buildd user, then
+        # try macaroon authentication.
+        if (username == BUILDD_USER_NAME and
+                self._verifyMacaroon(archive, password)):
+            # Success.
+            return
+
+        # Fall back to checking archive auth tokens.
         if username == BUILDD_USER_NAME:
             secret = archive.buildd_secret
         else:
diff --git a/lib/lp/soyuz/xmlrpc/tests/test_archive.py b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
index 71c9150..3007668 100644
--- a/lib/lp/soyuz/xmlrpc/tests/test_archive.py
+++ b/lib/lp/soyuz/xmlrpc/tests/test_archive.py
@@ -7,9 +7,12 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 
+from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.buildmaster.enums import BuildStatus
 from lp.services.features.testing import FeatureFixture
+from lp.services.macaroons.interfaces import IMacaroonIssuer
 from lp.soyuz.interfaces.archive import NAMED_AUTH_TOKEN_FEATURE_FLAG
 from lp.soyuz.xmlrpc.archive import ArchiveAPI
 from lp.testing import TestCaseWithFactory
@@ -25,6 +28,8 @@ class TestArchiveAPI(TestCaseWithFactory):
         super(TestArchiveAPI, self).setUp()
         self.useFixture(FeatureFixture({NAMED_AUTH_TOKEN_FEATURE_FLAG: "on"}))
         self.archive_api = ArchiveAPI(None, None)
+        self.pushConfig(
+            "launchpad", internal_macaroon_secret_key="some-secret")
 
     def assertNotFound(self, archive_reference, username, password, message):
         """Assert that an archive auth token check returns NotFound."""
@@ -65,6 +70,48 @@ class TestArchiveAPI(TestCaseWithFactory):
         self.assertIsNone(self.archive_api.checkArchiveAuthToken(
             archive.reference, "buildd", archive.buildd_secret))
 
+    def test_checkArchiveAuthToken_buildd_macaroon_wrong_archive(self):
+        archive = self.factory.makeArchive(private=True)
+        build = self.factory.makeBinaryPackageBuild(archive=archive)
+        other_archive = self.factory.makeArchive(
+            distribution=archive.distribution, private=True)
+        removeSecurityProxy(build).updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(build)
+        self.assertUnauthorized(
+            other_archive.reference, "buildd", macaroon.serialize())
+
+    def test_checkArchiveAuthToken_buildd_macaroon_not_building(self):
+        archive = self.factory.makeArchive(private=True)
+        build = self.factory.makeBinaryPackageBuild(archive=archive)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(build)
+        self.assertUnauthorized(
+            archive.reference, "buildd", macaroon.serialize())
+
+    def test_checkArchiveAuthToken_buildd_macaroon_wrong_user(self):
+        archive = self.factory.makeArchive(private=True)
+        build = self.factory.makeBinaryPackageBuild(archive=archive)
+        removeSecurityProxy(build).updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(build)
+        self.assertNotFound(
+            archive.reference, "another-user", macaroon.serialize(),
+            "No valid tokens for 'another-user' in '%s'." % archive.reference)
+
+    def test_checkArchiveAuthToken_buildd_macaroon_correct(self):
+        archive = self.factory.makeArchive(private=True)
+        build = self.factory.makeBinaryPackageBuild(archive=archive)
+        removeSecurityProxy(build).updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(build)
+        self.assertIsNone(self.archive_api.checkArchiveAuthToken(
+            archive.reference, "buildd", macaroon.serialize()))
+
     def test_checkArchiveAuthToken_named_token_wrong_password(self):
         archive = removeSecurityProxy(self.factory.makeArchive(private=True))
         token = archive.newNamedAuthToken("special")