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))
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
