launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad.

Commit message:
Allow macaroon authentication to the librarian for BPBs' source files.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/librarian-accept-macaroon/+merge/345079

This will later allow us to dispatch private builds immediately, rather than having to wait for the source to be published first.

In this case we accept the macaroon as the password part of basic authentication with an empty username, similar to the git authentication method used by code import jobs.  This is a bit weird, but it avoids needing to change launchpad-buildd, and we could always switch to a more conventional "Authorization: Macaroon ..." scheme later.  (Alternatively, I suppose we could put the macaroon in a query parameter instead?)

The only things I can see that we'll need to do deployment-wise are to set up *.restricted.dogfood.launchpadlibrarian.net (both DNS and listening to it on a different IP address from the public librarian) and to configure librarian.macaroon_secret_key.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad.

=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf	2018-03-16 21:04:45 +0000
+++ configs/testrunner-appserver/launchpad-lazr.conf	2018-05-04 10:59:37 +0000
@@ -9,7 +9,7 @@
 launch: False
 
 [codeimport]
-macaroon_secret_key: dev-macaroon-secret
+macaroon_secret_key: codeimport-dev-macaroon-secret
 
 [google_test_service]
 launch: False

=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf	2018-03-16 21:04:45 +0000
+++ configs/testrunner/launchpad-lazr.conf	2018-05-04 10:59:37 +0000
@@ -129,6 +129,7 @@
 restricted_download_url: http://localhost:58005/
 restricted_upload_port: 59095
 restricted_download_port: 58005
+macaroon_secret_key: librarian-dev-macaroon-secret
 
 [librarian_server]
 root: /var/tmp/fatsam.test

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2018-04-10 13:07:25 +0000
+++ database/schema/security.cfg	2018-05-04 10:59:37 +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).
 #
 # Possible permissions: SELECT, INSERT, UPDATE, EXECUTE
@@ -392,8 +392,11 @@
 type=user
 
 [librarian]
+public.binarypackagebuild               = SELECT
 public.libraryfilealias                 = SELECT, INSERT, UPDATE
 public.libraryfilecontent               = SELECT, INSERT
+public.sourcepackagerelease             = SELECT
+public.sourcepackagereleasefile         = SELECT
 type=user
 
 [librarianfeedswift]

=== modified file 'lib/lp/code/model/codeimportjob.py'
--- lib/lp/code/model/codeimportjob.py	2018-03-15 20:44:04 +0000
+++ lib/lp/code/model/codeimportjob.py	2018-05-04 10:59:37 +0000
@@ -442,6 +442,8 @@
 
     def verifyMacaroon(self, macaroon, context):
         """See `IMacaroonIssuer`."""
+        if not ICodeImportJob.providedBy(context):
+            return False
         if not self.checkMacaroonIssuer(macaroon):
             return False
         try:

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2018-04-12 19:59:21 +0000
+++ lib/lp/services/config/schema-lazr.conf	2018-05-04 10:59:37 +0000
@@ -1194,6 +1194,9 @@
 
 use_https = True
 
+# Secret key for macaroons used to grant library file download permissions.
+macaroon_secret_key: none
+
 
 [librarian_gc]
 # The database user which will be used by this process.

=== modified file 'lib/lp/services/librarianserver/db.py'
--- lib/lp/services/librarianserver/db.py	2016-01-10 22:37:40 +0000
+++ lib/lp/services/librarianserver/db.py	2018-05-04 10:59:37 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 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).
 
 """Database access layer for the Librarian."""
@@ -11,10 +11,13 @@
 import hashlib
 import urllib
 
+from pymacaroons import Macaroon
 from storm.expr import (
     And,
     SQL,
     )
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import session_store
@@ -23,6 +26,7 @@
     LibraryFileContent,
     TimeLimitedToken,
     )
+from lp.services.macaroons.interfaces import IMacaroonIssuer
 
 
 class Library:
@@ -69,16 +73,26 @@
             #
             # This needs to match url_path_quote.
             normalised_path = urllib.quote(urllib.unquote(path), safe='/~+')
-            store = session_store()
-            token_found = store.find(TimeLimitedToken,
-                SQL("age(created) < interval '1 day'"),
-                TimeLimitedToken.token == hashlib.sha256(token).hexdigest(),
-                TimeLimitedToken.path == normalised_path).is_empty()
-            store.reset()
-            if token_found:
+            if isinstance(token, Macaroon):
+                # We have no Zope interaction, so must remove the proxy.
+                issuer = removeSecurityProxy(
+                    getUtility(IMacaroonIssuer, token.identifier))
+                # Macaroons have enough other constraints that they don't
+                # need to be path-specific; it's simpler and faster to just
+                # check the alias ID.
+                token_ok = issuer.verifyMacaroon(token, aliasid)
+            else:
+                store = session_store()
+                token_ok = not store.find(TimeLimitedToken,
+                    SQL("age(created) < interval '1 day'"),
+                    TimeLimitedToken.token ==
+                        hashlib.sha256(token).hexdigest(),
+                    TimeLimitedToken.path == normalised_path).is_empty()
+                store.reset()
+            if token_ok:
+                restricted = True
+            else:
                 raise LookupError("Token stale/pruned/path mismatch")
-            else:
-                restricted = True
         alias = LibraryFileAlias.selectOne(And(
             LibraryFileAlias.id == aliasid,
             LibraryFileAlias.contentID == LibraryFileContent.q.id,

=== modified file 'lib/lp/services/librarianserver/tests/test_web.py'
--- lib/lp/services/librarianserver/tests/test_web.py	2018-01-02 10:54:31 +0000
+++ lib/lp/services/librarianserver/tests/test_web.py	2018-05-04 10:59:37 +0000
@@ -1,26 +1,25 @@
-# Copyright 2009-2016 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).
 
-from cStringIO import StringIO
 from datetime import datetime
+from gzip import GzipFile
 import hashlib
 import httplib
+from io import BytesIO
 import os
 import unittest
-from urllib2 import (
-    HTTPError,
-    urlopen,
-    )
 from urlparse import urlparse
 
 from lazr.uri import URI
 import pytz
+import requests
 from storm.expr import SQL
-import testtools
 from testtools.matchers import EndsWith
 import transaction
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
+from lp.buildmaster.enums import BuildStatus
 from lp.services.config import config
 from lp.services.database.interfaces import IMasterStore
 from lp.services.database.sqlbase import (
@@ -39,7 +38,12 @@
     TimeLimitedToken,
     )
 from lp.services.librarianserver.storage import LibrarianStorage
-from lp.testing.dbuser import switch_dbuser
+from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.testing import TestCaseWithFactory
+from lp.testing.dbuser import (
+    dbuser,
+    switch_dbuser,
+    )
 from lp.testing.layers import (
     LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,
@@ -52,10 +56,9 @@
     return str(parsed.replace(path=parsed.path.replace(old, new)))
 
 
-class LibrarianWebTestCase(testtools.TestCase):
+class LibrarianWebTestCase(TestCaseWithFactory):
     """Test the librarian's web interface."""
     layer = LaunchpadFunctionalLayer
-    dbuser = 'librarian'
 
     # Add stuff to a librarian via the upload port, then check that it's
     # immediately visible on the web interface. (in an attempt to test ddaa's
@@ -75,9 +78,9 @@
         for count in range(10):
             # Upload a file.  This should work without any exceptions being
             # thrown.
-            sampleData = 'x' + ('blah' * (count % 5))
+            sampleData = b'x' + (b'blah' * (count % 5))
             fileAlias = client.addFile('sample', len(sampleData),
-                                                 StringIO(sampleData),
+                                                 BytesIO(sampleData),
                                                  contentType='text/plain')
 
             # Make sure we can get its URL
@@ -98,9 +101,9 @@
             fileObj.close()
 
             # And make sure the URL works too
-            fileObj = urlopen(url)
-            self.assertEqual(sampleData, fileObj.read())
-            fileObj.close()
+            response = requests.get(url)
+            response.raise_for_status()
+            self.assertEqual(sampleData, response.content)
 
     def test_checkGzipEncoding(self):
         # Files that end in ".txt.gz" are treated special and are returned
@@ -108,29 +111,34 @@
         # displaying Ubuntu build logs in the browser.  The mimetype should be
         # "text/plain" for these files.
         client = LibrarianClient()
-        contents = 'Build log...'
-        build_log = StringIO(contents)
+        contents = b'Build log...'
+        build_log = BytesIO()
+        with GzipFile(mode='wb', fileobj=build_log) as f:
+            f.write(contents)
+        build_log.seek(0)
         alias_id = client.addFile(name="build_log.txt.gz",
-                                  size=len(contents),
+                                  size=len(build_log.getvalue()),
                                   file=build_log,
                                   contentType="text/plain")
 
         self.commit()
 
         url = client.getURLForAlias(alias_id)
-        fileObj = urlopen(url)
-        mimetype = fileObj.headers['content-type']
-        encoding = fileObj.headers['content-encoding']
+        response = requests.get(url)
+        response.raise_for_status()
+        mimetype = response.headers['content-type']
+        encoding = response.headers['content-encoding']
         self.assertTrue(mimetype == "text/plain; charset=utf-8",
                         "Wrong mimetype. %s != 'text/plain'." % mimetype)
         self.assertTrue(encoding == "gzip",
                         "Wrong encoding. %s != 'gzip'." % encoding)
+        self.assertEqual(contents, response.content)
 
     def test_checkNoEncoding(self):
         # Other files should have no encoding.
         client = LibrarianClient()
-        contents = 'Build log...'
-        build_log = StringIO(contents)
+        contents = b'Build log...'
+        build_log = BytesIO(contents)
         alias_id = client.addFile(name="build_log.tgz",
                                   size=len(contents),
                                   file=build_log,
@@ -139,10 +147,10 @@
         self.commit()
 
         url = client.getURLForAlias(alias_id)
-        fileObj = urlopen(url)
-        mimetype = fileObj.headers['content-type']
-        self.assertRaises(KeyError, fileObj.headers.__getitem__,
-                          'content-encoding')
+        response = requests.get(url)
+        response.raise_for_status()
+        mimetype = response.headers['content-type']
+        self.assertNotIn('content-encoding', response.headers)
         self.assertTrue(
             mimetype == "application/x-tar",
             "Wrong mimetype. %s != 'application/x-tar'." % mimetype)
@@ -157,13 +165,17 @@
         # ignored
         client = LibrarianClient()
         filename = 'sample.txt'
-        aid = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
+        aid = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
         self.commit()
         url = client.getURLForAlias(aid)
-        self.assertEqual(urlopen(url).read(), 'sample')
+        response = requests.get(url)
+        response.raise_for_status()
+        self.assertEqual(response.content, b'sample')
 
         old_url = uri_path_replace(url, str(aid), '42/%d' % aid)
-        self.assertEqual(urlopen(old_url).read(), 'sample')
+        response = requests.get(url)
+        response.raise_for_status()
+        self.assertEqual(response.content, b'sample')
 
         # If the content and alias IDs are not integers, a 404 is raised
         old_url = uri_path_replace(url, str(aid), 'foo/%d' % aid)
@@ -174,10 +186,12 @@
     def test_404(self):
         client = LibrarianClient()
         filename = 'sample.txt'
-        aid = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
+        aid = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
         self.commit()
         url = client.getURLForAlias(aid)
-        self.assertEqual(urlopen(url).read(), 'sample')
+        response = requests.get(url)
+        response.raise_for_status()
+        self.assertEqual(response.content, b'sample')
 
         # Change the aliasid and assert we get a 404
         self.assertIn(str(aid), url)
@@ -192,29 +206,30 @@
     def test_duplicateuploads(self):
         client = LibrarianClient()
         filename = 'sample.txt'
-        id1 = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
-        id2 = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
+        id1 = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
+        id2 = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
 
         self.assertNotEqual(id1, id2, 'Got allocated the same id!')
 
         self.commit()
 
-        self.assertEqual(client.getFileByAlias(id1).read(), 'sample')
-        self.assertEqual(client.getFileByAlias(id2).read(), 'sample')
+        self.assertEqual(client.getFileByAlias(id1).read(), b'sample')
+        self.assertEqual(client.getFileByAlias(id2).read(), b'sample')
 
     def test_robotsTxt(self):
         url = 'http://%s:%d/robots.txt' % (
             config.librarian.download_host, config.librarian.download_port)
-        f = urlopen(url)
-        self.assertIn('Disallow: /', f.read())
+        response = requests.get(url)
+        response.raise_for_status()
+        self.assertIn('Disallow: /', response.text)
 
     def test_headers(self):
         client = LibrarianClient()
 
         # Upload a file so we can retrieve it.
-        sample_data = 'blah'
+        sample_data = b'blah'
         file_alias_id = client.addFile(
-            'sample', len(sample_data), StringIO(sample_data),
+            'sample', len(sample_data), BytesIO(sample_data),
             contentType='text/plain')
         url = client.getURLForAlias(file_alias_id)
 
@@ -229,9 +244,10 @@
         self.commit()
 
         # Fetch the file via HTTP, recording the interesting headers
-        result = urlopen(url)
-        last_modified_header = result.info()['Last-Modified']
-        cache_control_header = result.info()['Cache-Control']
+        response = requests.get(url)
+        response.raise_for_status()
+        last_modified_header = response.headers['Last-Modified']
+        cache_control_header = response.headers['Cache-Control']
 
         # URLs point to the same content for ever, so we have a hardcoded
         # 1 year max-age cache policy.
@@ -247,9 +263,9 @@
         client = LibrarianClient()
 
         # Upload a file so we can retrieve it.
-        sample_data = 'blah'
+        sample_data = b'blah'
         file_alias_id = client.addFile(
-            'sample', len(sample_data), StringIO(sample_data),
+            'sample', len(sample_data), BytesIO(sample_data),
             contentType='text/plain')
         url = client.getURLForAlias(file_alias_id)
 
@@ -262,18 +278,19 @@
         self.commit()
 
         # Fetch the file via HTTP.
-        urlopen(url)
+        response = requests.get(url)
+        response.raise_for_status()
 
         # Delete the on-disk file.
         storage = LibrarianStorage(config.librarian_server.root, None)
         os.remove(storage._fileLocation(file_alias.contentID))
 
         # The URL now 500s, since the DB says it should exist.
-        exception = self.assertRaises(HTTPError, urlopen, url)
-        self.assertEqual(500, exception.code)
-        self.assertIn('Server', exception.info())
-        self.assertNotIn('Last-Modified', exception.info())
-        self.assertNotIn('Cache-Control', exception.info())
+        response = requests.get(url)
+        self.assertEqual(500, response.status_code)
+        self.assertIn('Server', response.headers)
+        self.assertNotIn('Last-Modified', response.headers)
+        self.assertNotIn('Cache-Control', response.headers)
 
     def get_restricted_file_and_public_url(self, filename='sample'):
         # Use a regular LibrarianClient to ensure we speak to the
@@ -281,10 +298,10 @@
         # restricted files are served from.
         client = LibrarianClient()
         fileAlias = client.addFile(
-            filename, 12, StringIO('a' * 12), contentType='text/plain')
+            filename, 12, BytesIO(b'a' * 12), contentType='text/plain')
         # Note: We're deliberately using the wrong url here: we should be
         # passing secure=True to getURLForAlias, but to use the returned URL
-        # we would need a wildcard DNS facility patched into urlopen; instead
+        # we would need a wildcard DNS facility patched into requests; instead
         # we use the *deliberate* choice of having the path of secure and
         # insecure urls be the same, so that we can test it: the server code
         # doesn't need to know about the fancy wildcard domains.
@@ -301,9 +318,9 @@
         # IFF there is a .restricted. in the host, then the library file alias
         # in the subdomain must match that in the path.
         client = LibrarianClient()
-        fileAlias = client.addFile('sample', 12, StringIO('a' * 12),
+        fileAlias = client.addFile('sample', 12, BytesIO(b'a' * 12),
             contentType='text/plain')
-        fileAlias2 = client.addFile('sample', 12, StringIO('b' * 12),
+        fileAlias2 = client.addFile('sample', 12, BytesIO(b'b' * 12),
             contentType='text/plain')
         self.commit()
         url = client.getURLForAlias(fileAlias)
@@ -313,7 +330,8 @@
         template_host = 'i%%d.restricted.%s' % download_host
         path = get_libraryfilealias_download_path(fileAlias, 'sample')
         # The basic URL must work.
-        urlopen(url)
+        response = requests.get(url)
+        response.raise_for_status()
         # Use the network level protocol because DNS resolution won't work
         # here (no wildcard support)
         connection = httplib.HTTPConnection(
@@ -356,20 +374,17 @@
         fileAlias, url = self.get_restricted_file_and_public_url()
         # The file should not be able to be opened - the token supplied
         # is not one we issued.
-        self.require404(url + '?token=haxx0r')
+        self.require404(url, params={"token": "haxx0r"})
 
     def test_restricted_with_token(self):
         fileAlias, url = self.get_restricted_file_and_public_url()
         # We have the base url for a restricted file; grant access to it
         # for a short time.
         token = TimeLimitedToken.allocate(url)
-        url = url + "?token=%s" % token
         # Now we should be able to access the file.
-        fileObj = urlopen(url)
-        try:
-            self.assertEqual("a" * 12, fileObj.read())
-        finally:
-            fileObj.close()
+        response = requests.get(url, params={"token": token})
+        response.raise_for_status()
+        self.assertEqual(b"a" * 12, response.content)
 
     def test_restricted_with_token_encoding(self):
         fileAlias, url = self.get_restricted_file_and_public_url('foo~%')
@@ -380,20 +395,16 @@
         token = TimeLimitedToken.allocate(url)
 
         # Now we should be able to access the file.
-        fileObj = urlopen(url + "?token=%s" % token)
-        try:
-            self.assertEqual("a" * 12, fileObj.read())
-        finally:
-            fileObj.close()
+        response = requests.get(url, params={"token": token})
+        response.raise_for_status()
+        self.assertEqual(b"a" * 12, response.content)
 
         # The token is valid even if the filename is encoded differently.
         mangled_url = url.replace('~', '%7E')
         self.assertNotEqual(mangled_url, url)
-        fileObj = urlopen(mangled_url + "?token=%s" % token)
-        try:
-            self.assertEqual("a" * 12, fileObj.read())
-        finally:
-            fileObj.close()
+        response = requests.get(url, params={"token": token})
+        response.raise_for_status()
+        self.assertEqual(b"a" * 12, response.content)
 
     def test_restricted_with_expired_token(self):
         fileAlias, url = self.get_restricted_file_and_public_url()
@@ -407,14 +418,55 @@
             TimeLimitedToken.token == hashlib.sha256(token).hexdigest())
         tokens.set(
             TimeLimitedToken.created == SQL("created - interval '1 week'"))
-        url = url + "?token=%s" % token
         # Now, as per test_restricted_no_token we should get a 404.
-        self.require404(url)
+        self.require404(url, params={"token": token})
+
+    def test_restricted_with_macaroon(self):
+        fileAlias, url = self.get_restricted_file_and_public_url()
+        lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
+        with dbuser('testadmin'):
+            build = self.factory.makeBinaryPackageBuild(
+                archive=self.factory.makeArchive(private=True))
+            naked_build = removeSecurityProxy(build)
+            self.factory.makeSourcePackageReleaseFile(
+                sourcepackagerelease=naked_build.source_package_release,
+                library_file=lfa)
+            naked_build.updateStatus(BuildStatus.BUILDING)
+            issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+            macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+        self.commit()
+        response = requests.get(url, auth=("", macaroon.serialize()))
+        response.raise_for_status()
+        self.assertEqual(b"a" * 12, response.content)
+
+    def test_restricted_with_invalid_macaroon(self):
+        fileAlias, url = self.get_restricted_file_and_public_url()
+        lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
+        with dbuser('testadmin'):
+            build = self.factory.makeBinaryPackageBuild(
+                archive=self.factory.makeArchive(private=True))
+            naked_build = removeSecurityProxy(build)
+            self.factory.makeSourcePackageReleaseFile(
+                sourcepackagerelease=naked_build.source_package_release,
+                library_file=lfa)
+            naked_build.updateStatus(BuildStatus.BUILDING)
+        self.commit()
+        self.require404(url, auth=("", "not-a-macaroon"))
+
+    def test_restricted_with_unverifiable_macaroon(self):
+        fileAlias, url = self.get_restricted_file_and_public_url()
+        with dbuser('testadmin'):
+            build = self.factory.makeBinaryPackageBuild(
+                archive=self.factory.makeArchive(private=True))
+            removeSecurityProxy(build).updateStatus(BuildStatus.BUILDING)
+            issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+            macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+        self.commit()
+        self.require404(url, auth=("", macaroon.serialize()))
 
     def test_restricted_file_headers(self):
         fileAlias, url = self.get_restricted_file_and_public_url()
         token = TimeLimitedToken.allocate(url)
-        url = url + "?token=%s" % token
         # Change the date_created to a known value for testing.
         file_alias = IMasterStore(LibraryFileAlias).get(
             LibraryFileAlias, fileAlias)
@@ -423,9 +475,9 @@
         # Commit the update.
         self.commit()
         # Fetch the file via HTTP, recording the interesting headers
-        result = urlopen(url)
-        last_modified_header = result.info()['Last-Modified']
-        cache_control_header = result.info()['Cache-Control']
+        response = requests.get(url, params={"token": token})
+        last_modified_header = response.headers['Last-Modified']
+        cache_control_header = response.headers['Cache-Control']
         # No caching for restricted files.
         self.assertEqual(cache_control_header, 'max-age=0, private')
         # And we should have a correct Last-Modified header too.
@@ -433,13 +485,10 @@
             last_modified_header, 'Tue, 30 Jan 2001 13:45:59 GMT')
         # Perhaps we should also set Expires to the Last-Modified.
 
-    def require404(self, url):
+    def require404(self, url, **kwargs):
         """Assert that opening `url` raises a 404."""
-        try:
-            urlopen(url)
-            self.fail('404 not raised')
-        except HTTPError as e:
-            self.assertEqual(e.code, 404)
+        response = requests.get(url, **kwargs)
+        self.assertEqual(404, response.status_code)
 
 
 class LibrarianZopelessWebTestCase(LibrarianWebTestCase):
@@ -455,9 +504,9 @@
     def test_getURLForAliasObject(self):
         # getURLForAliasObject returns the same URL as getURLForAlias.
         client = LibrarianClient()
-        content = "Test content"
+        content = b"Test content"
         alias_id = client.addFile(
-            'test.txt', len(content), StringIO(content),
+            'test.txt', len(content), BytesIO(content),
             contentType='text/plain')
         self.commit()
 
@@ -481,7 +530,7 @@
         switch_dbuser('testadmin')
 
         alias = getUtility(ILibraryFileAliasSet).create(
-                'whatever', 8, StringIO('xxx\nxxx\n'), 'text/plain')
+                'whatever', 8, BytesIO(b'xxx\nxxx\n'), 'text/plain')
         alias_id = alias.id
         transaction.commit()
 
@@ -493,8 +542,9 @@
 
         # And it can be retrieved via the web
         url = alias.http_url
-        retrieved_content = urlopen(url).read()
-        self.assertEqual(retrieved_content, 'xxx\nxxx\n')
+        response = requests.get(url)
+        response.raise_for_status()
+        self.assertEqual(response.content, b'xxx\nxxx\n')
 
         # But when we flag the content as deleted
         cur = cursor()
@@ -508,8 +558,5 @@
         self.assertRaises(DownloadFailed, alias.open)
 
         # And people see a 404 page
-        try:
-            urlopen(url)
-            self.fail('404 not raised')
-        except HTTPError as x:
-            self.assertEqual(x.code, 404)
+        response = requests.get(url)
+        self.assertEqual(404, response.status_code)

=== modified file 'lib/lp/services/librarianserver/web.py'
--- lib/lp/services/librarianserver/web.py	2018-01-01 17:40:03 +0000
+++ lib/lp/services/librarianserver/web.py	2018-05-04 10:59:37 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 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).
 
 __metaclass__ = type
@@ -7,6 +7,7 @@
 import time
 from urlparse import urlparse
 
+from pymacaroons import Macaroon
 from storm.exceptions import DisconnectionError
 from twisted.internet import defer
 from twisted.internet.threads import deferToThread
@@ -116,6 +117,12 @@
                 return fourOhFour
 
         token = request.args.get('token', [None])[0]
+        if token is None:
+            if not request.getUser() and request.getPassword():
+                try:
+                    token = Macaroon.deserialize(request.getPassword())
+                except Exception:
+                    pass
         path = request.path
         deferred = deferToThread(
             self._getFileAlias, self.aliasID, token, path)

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2017-07-18 16:22:03 +0000
+++ lib/lp/soyuz/configure.zcml	2018-05-04 10:59:37 +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).
 -->
 
@@ -480,6 +480,15 @@
         <allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource"/>
     </securedutility>
 
+    <!-- BinaryPackageBuildMacaroonIssuer -->
+
+    <securedutility
+        class="lp.soyuz.model.binarypackagebuild.BinaryPackageBuildMacaroonIssuer"
+        provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
+        name="binary-package-build">
+        <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic"/>
+    </securedutility>
+
     <!-- DistroArchSeriesBinaryPackage -->
 
     <class

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2017-03-29 09:28:09 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2018-05-04 10:59:37 +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).
 
 __metaclass__ = type
@@ -21,6 +21,10 @@
 
 import apt_pkg
 from debian.deb822 import PkgRelation
+from pymacaroons import (
+    Macaroon,
+    Verifier,
+    )
 import pytz
 from sqlobject import SQLObjectNotFound
 from storm.expr import (
@@ -81,6 +85,7 @@
     LibraryFileAlias,
     LibraryFileContent,
     )
+from lp.services.macaroons.interfaces import IMacaroonIssuer
 from lp.soyuz.adapters.buildarch import determine_architectures_to_build
 from lp.soyuz.enums import (
     ArchivePurpose,
@@ -102,7 +107,10 @@
 from lp.soyuz.mail.binarypackagebuild import BinaryPackageBuildMailer
 from lp.soyuz.model.binarypackagename import BinaryPackageName
 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
-from lp.soyuz.model.files import BinaryPackageFile
+from lp.soyuz.model.files import (
+    BinaryPackageFile,
+    SourcePackageReleaseFile,
+    )
 from lp.soyuz.model.packageset import Packageset
 from lp.soyuz.model.queue import (
     PackageUpload,
@@ -1362,3 +1370,82 @@
                     % (build.title, build.id, build.archive.displayname,
                     build_queue.lastscore))
         return new_builds
+
+
+@implementer(IMacaroonIssuer)
+class BinaryPackageBuildMacaroonIssuer:
+
+    @property
+    def _root_secret(self):
+        secret = config.librarian.macaroon_secret_key
+        if not secret:
+            raise RuntimeError("librarian.macaroon_secret_key not configured.")
+        return secret
+
+    def issueMacaroon(self, context):
+        """See `IMacaroonIssuer`.
+
+        For issuing, the context is an `IBinaryPackageBuild`.
+        """
+        if not removeSecurityProxy(context).archive.private:
+            raise AssertionError(
+                "Refusing to issue macaroon for public build.")
+        macaroon = Macaroon(
+            location=config.vhost.mainsite.hostname,
+            identifier="binary-package-build", key=self._root_secret)
+        macaroon.add_first_party_caveat(
+            "lp.binary-package-build %s" % removeSecurityProxy(context).id)
+        return macaroon
+
+    def checkMacaroonIssuer(self, macaroon):
+        """See `IMacaroonIssuer`."""
+        if macaroon.location != config.vhost.mainsite.hostname:
+            return False
+        try:
+            verifier = Verifier()
+            verifier.satisfy_general(
+                lambda caveat: caveat.startswith("lp.binary-package-build "))
+            return verifier.verify(macaroon, self._root_secret)
+        except Exception:
+            return False
+
+    def verifyMacaroon(self, macaroon, context):
+        """See `IMacaroonIssuer`.
+
+        For verification, the context is a `LibraryFileAlias` ID.  We check
+        that the file is one of those required to build the
+        `IBinaryPackageBuild` that is the context of the macaroon, and that
+        the context build is currently building.
+        """
+        # Circular import.
+        from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+
+        if not isinstance(context, int):
+            return False
+        if not self.checkMacaroonIssuer(macaroon):
+            return False
+
+        def verify_build(caveat):
+            prefix = "lp.binary-package-build "
+            if not caveat.startswith(prefix):
+                return False
+            try:
+                build_id = int(caveat[len(prefix):])
+            except ValueError:
+                return False
+            return not IStore(BinaryPackageBuild).find(
+                BinaryPackageBuild,
+                BinaryPackageBuild.id == build_id,
+                BinaryPackageBuild.source_package_release_id ==
+                    SourcePackageRelease.id,
+                SourcePackageReleaseFile.sourcepackagereleaseID ==
+                    SourcePackageRelease.id,
+                SourcePackageReleaseFile.libraryfileID == context,
+                BinaryPackageBuild.status == BuildStatus.BUILDING).is_empty()
+
+        try:
+            verifier = Verifier()
+            verifier.satisfy_general(verify_build)
+            return verifier.verify(macaroon, self._root_secret)
+        except Exception:
+            return False

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2018-02-09 14:56:43 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2018-05-04 10:59:37 +0000
@@ -10,8 +10,13 @@
     timedelta,
     )
 
+from pymacaroons import Macaroon
 import pytz
 from simplejson import dumps
+from testtools.matchers import (
+    MatchesListwise,
+    MatchesStructure,
+    )
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -22,7 +27,9 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
+from lp.services.config import config
 from lp.services.log.logger import DevNullLogger
+from lp.services.macaroons.interfaces import IMacaroonIssuer
 from lp.services.webapp.interaction import ANONYMOUS
 from lp.services.webapp.interfaces import OAuthPermission
 from lp.soyuz.enums import (
@@ -897,3 +904,139 @@
         with anonymous_logged_in():
             self.assertScoreWriteableByTeam(
                 archive, getUtility(ILaunchpadCelebrities).ppa_admin)
+
+
+class TestBinaryPackageBuildMacaroonIssuer(TestCaseWithFactory):
+    """Test BinaryPackageBuild macaroon issuing and verification.
+
+    The librarian server verifies these macaroons, and has no Zope
+    interaction when doing so, so we take care to test that verification
+    works without an interaction.
+    """
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestBinaryPackageBuildMacaroonIssuer, self).setUp()
+        self.pushConfig("librarian", macaroon_secret_key="some-secret")
+
+    def test_issueMacaroon_refuses_public_archive(self):
+        build = self.factory.makeBinaryPackageBuild()
+        issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+        self.assertRaises(
+            AssertionError, removeSecurityProxy(issuer).issueMacaroon, build)
+
+    def test_issueMacaroon_good(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+        macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+        self.assertEqual("launchpad.dev", macaroon.location)
+        self.assertEqual("binary-package-build", macaroon.identifier)
+        self.assertThat(macaroon.caveats, MatchesListwise([
+            MatchesStructure.byEquality(
+                caveat_id="lp.binary-package-build %s" % build.id),
+            ]))
+
+    def test_checkMacaroonIssuer_good(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+        macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+        self.assertTrue(issuer.checkMacaroonIssuer(macaroon))
+
+    def test_checkMacaroonIssuer_wrong_location(self):
+        issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+        macaroon = Macaroon(
+            location="another-location",
+            key=removeSecurityProxy(issuer)._root_secret)
+        self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
+
+    def test_checkMacaroonIssuer_wrong_key(self):
+        issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+        macaroon = Macaroon(
+            location=config.vhost.mainsite.hostname, key="another-secret")
+        self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
+
+    def test_verifyMacaroon_good(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=build.source_package_release)
+        lfa_id = sprf.libraryfile.id
+        build.updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(build)
+        logout()
+        self.assertTrue(issuer.verifyMacaroon(macaroon, lfa_id))
+
+    def test_verifyMacaroon_wrong_location(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=build.source_package_release)
+        lfa_id = sprf.libraryfile.id
+        build.updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = Macaroon(
+            location="another-location", key=issuer._root_secret)
+        logout()
+        self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+    def test_verifyMacaroon_wrong_key(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=build.source_package_release)
+        lfa_id = sprf.libraryfile.id
+        build.updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = Macaroon(
+            location=config.vhost.mainsite.hostname, key="another-secret")
+        logout()
+        self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+    def test_verifyMacaroon_not_building(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=build.source_package_release)
+        lfa_id = sprf.libraryfile.id
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(build)
+        logout()
+        self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+    def test_verifyMacaroon_wrong_build(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        sprf = self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=build.source_package_release)
+        lfa_id = sprf.libraryfile.id
+        build.updateStatus(BuildStatus.BUILDING)
+        other_build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        other_build.updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(other_build)
+        logout()
+        self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+    def test_verifyMacaroon_wrong_file(self):
+        build = self.factory.makeBinaryPackageBuild(
+            archive=self.factory.makeArchive(private=True))
+        self.factory.makeSourcePackageReleaseFile(
+            sourcepackagerelease=build.source_package_release)
+        lfa = self.factory.makeLibraryFileAlias()
+        lfa_id = lfa.id
+        build.updateStatus(BuildStatus.BUILDING)
+        issuer = removeSecurityProxy(
+            getUtility(IMacaroonIssuer, "binary-package-build"))
+        macaroon = issuer.issueMacaroon(build)
+        logout()
+        self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))