Merge lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18937
Proposed branch: lp:~cjwatson/launchpad/librarian-accept-macaroon
Merge into: lp:launchpad
Diff against target: 1300 lines (+658/-107)
18 files modified
configs/development/launchpad-lazr.conf (+2/-1)
configs/testrunner-appserver/launchpad-lazr.conf (+1/-3)
lib/lp/code/model/codeimportjob.py (+8/-2)
lib/lp/code/model/tests/test_codeimportjob.py (+9/-2)
lib/lp/code/xmlrpc/git.py (+2/-0)
lib/lp/code/xmlrpc/tests/test_git.py (+10/-5)
lib/lp/codehosting/codeimport/tests/test_workermonitor.py (+2/-2)
lib/lp/services/authserver/interfaces.py (+3/-1)
lib/lp/services/authserver/tests/test_authserver.py (+47/-13)
lib/lp/services/authserver/xmlrpc.py (+17/-3)
lib/lp/services/config/schema-lazr.conf (+15/-0)
lib/lp/services/librarianserver/db.py (+68/-20)
lib/lp/services/librarianserver/tests/test_db.py (+104/-6)
lib/lp/services/librarianserver/tests/test_web.py (+128/-46)
lib/lp/services/librarianserver/web.py (+9/-0)
lib/lp/soyuz/configure.zcml (+9/-0)
lib/lp/soyuz/model/binarypackagebuild.py (+96/-2)
lib/lp/soyuz/tests/test_binarypackagebuild.py (+128/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/librarian-accept-macaroon
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+345079@code.launchpad.net

Commit message

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

Description of the change

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 launchpad.internal_macaroon_secret_key and librarian.authentication_endpoint.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

This seems like a massive layering violation. I really think we should just issue macaroons with LFA.id caveats directly, even though that loses us the BuildStatus.BUILDING restriction.

review: Needs Information
Revision history for this message
Colin Watson (cjwatson) wrote :

I don't understand the principle here. Can you explain why it's any more of a layering violation for a BPB issuer to grant permission to access its associated LFAs than it is for a CodeImportJob issuer to grant permission to access its associated GitRepositories?

In general, my intent in designing the IMacaroonIssuer interface was that various subsystems could grant the specific permissions they needed, and the verifying code could then use the getUtility(IMacaroonIssuer, token.identifier) idiom to get the appropriate verifier without having to have specific knowledge of what the verifier in question does. Apart from the security.cfg changes, nothing about the librarian changes here is at all specific to BPBs, and it would be perfectly possible to write a different verifier that grants access based on some other criteria; furthermore, if (hypothetically) a BPB needed access to some other private resource, the same macaroon could be used for both by generalising the verifier.

The only thing here that feels at all like a layering violation to me is that BPBMacaroonIssuer.verifyMacaroon takes an LFA ID rather than a build. But, firstly, that's something that it's free to generalise later without reference to the librarian (although I suppose that would be easier if it took an actual LFA; that would require a little bit of extra work in the librarian to avoid an extra query), and, secondly, it's OK for BPBs to know about LFAs, just not vice versa.

Revision history for this message
William Grant (wgrant) wrote :

On 09/05/18 23:24, Colin Watson wrote:
> I don't understand the principle here. Can you explain why it's any
> more of a layering violation for a BPB issuer to grant permission to
> access its associated LFAs than it is for a CodeImportJob issuer to
> grant permission to access its associated GitRepositories?

It was less obviously wrong there because the GitRepository
authorisation code already touched CodeImport and they were part of the
same Launchpad application. But in this case we have the librarian
touching complex bits of the Soyuz schema, which is ugly from a code
structure perspective and also means we have to be very careful around
synchronising schema changes and librarian deployments.

librarian today only examines non-librarian tables when GC introspects
the schema to work out which FKs there are to LFA.id, and that doesn't
seem like an architectural separation that we should abandon lightly.

> In general, my intent in designing the IMacaroonIssuer interface was
> that various subsystems could grant the specific permissions they
> needed, and the verifying code could then use the
> getUtility(IMacaroonIssuer, token.identifier) idiom to get the
> appropriate verifier without having to have specific knowledge of
> what the verifier in question does. Apart from the security.cfg
> changes, nothing about the librarian changes here is at all specific
> to BPBs, and it would be perfectly possible to write a different
> verifier that grants access based on some other criteria;
> furthermore, if (hypothetically) a BPB needed access to some other
> private resource, the same macaroon could be used for both by
> generalising the verifier.

I agree that sharing macaroons would be ideal, but having the librarian
itself validate macaroons that may require arbitrarily broad and complex
database logic (particularly given the librarian is a Twisted app which
really shouldn't be doing synchronous DB calls at all) doesn't seem right.

Revision history for this message
Colin Watson (cjwatson) wrote :

Would it be better if the librarian talked to the authserver for this?
I'd avoided that approach because it seemed like unnecessary complexity,
but it could easily be done asynchronously and it would allow preserving
the fine-grained permissions, which I'd really like to find a way to do.
We could do it only for the macaroon auth case, which I expect to
continue being comparatively rare.

Revision history for this message
William Grant (wgrant) wrote :

I think that sounds fine. It's a bit of a change from where we are today, but not a huge way off and not clearly in the wrong direction.

Revision history for this message
Colin Watson (cjwatson) wrote :
Revision history for this message
Colin Watson (cjwatson) wrote :

I've updated this to use the authserver. I've also split out https://code.launchpad.net/~cjwatson/launchpad/librarianserver-test-web-requests/+merge/358189, which might be worth reviewing first to make the review diff size here more manageable.

Revision history for this message
Colin Watson (cjwatson) wrote :

OK, that other MP is merged now, so this is a bit more readable.

Revision history for this message
William Grant (wgrant) wrote :

A couple of bits are potentially confusing and would ideally be reworded, otherwise looks good.

review: Approve (code)
Revision history for this message
William Grant (wgrant) wrote :

In fact, the authserver interface is completely ambiguous. It so happens that CodeImportJobMacaroonIssuer.verifyMacaroon will crash if it's given an LFA ID, so it's safe for now, but a giant boobytrap.

review: Needs Fixing
Revision history for this message
Colin Watson (cjwatson) :
Revision history for this message
William Grant (wgrant) :
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2019-04-16 14:30:40 +0000
+++ configs/development/launchpad-lazr.conf 2019-04-23 14:01:24 +0000
@@ -121,7 +121,8 @@
121restricted_upload_port: 58095121restricted_upload_port: 58095
122restricted_download_port: 58085122restricted_download_port: 58085
123restricted_download_url: http://launchpad.dev:58085/123restricted_download_url: http://launchpad.dev:58085/
124use_https = False124use_https: False
125authentication_endpoint: http://xmlrpc-private.launchpad.dev:8087/authserver
125126
126[librarian_server]127[librarian_server]
127root: /var/tmp/fatsam128root: /var/tmp/fatsam
128129
=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf 2018-05-21 20:30:16 +0000
+++ configs/testrunner-appserver/launchpad-lazr.conf 2019-04-23 14:01:24 +0000
@@ -8,14 +8,12 @@
8[codehosting]8[codehosting]
9launch: False9launch: False
1010
11[codeimport]
12macaroon_secret_key: dev-macaroon-secret
13
14[bing_test_service]11[bing_test_service]
15launch: False12launch: False
1613
17[launchpad]14[launchpad]
18openid_provider_root: http://testopenid.dev:8085/15openid_provider_root: http://testopenid.dev:8085/
16internal_macaroon_secret_key: internal-dev-macaroon-secret
1917
20[librarian_server]18[librarian_server]
21launch: False19launch: False
2220
=== 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 2019-04-23 14:01:24 +0000
@@ -413,10 +413,14 @@
413413
414 @property414 @property
415 def _root_secret(self):415 def _root_secret(self):
416 secret = config.codeimport.macaroon_secret_key416 secret = config.launchpad.internal_macaroon_secret_key
417 if not secret:
418 # XXX cjwatson 2018-11-01: Remove this once it is no longer in
419 # production configs.
420 secret = config.codeimport.macaroon_secret_key
417 if not secret:421 if not secret:
418 raise RuntimeError(422 raise RuntimeError(
419 "codeimport.macaroon_secret_key not configured.")423 "launchpad.internal_macaroon_secret_key not configured.")
420 return secret424 return secret
421425
422 def issueMacaroon(self, context):426 def issueMacaroon(self, context):
@@ -442,6 +446,8 @@
442446
443 def verifyMacaroon(self, macaroon, context):447 def verifyMacaroon(self, macaroon, context):
444 """See `IMacaroonIssuer`."""448 """See `IMacaroonIssuer`."""
449 if not ICodeImportJob.providedBy(context):
450 return False
445 if not self.checkMacaroonIssuer(macaroon):451 if not self.checkMacaroonIssuer(macaroon):
446 return False452 return False
447 try:453 try:
448454
=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
--- lib/lp/code/model/tests/test_codeimportjob.py 2018-04-19 00:02:19 +0000
+++ lib/lp/code/model/tests/test_codeimportjob.py 2019-04-23 14:01:24 +0000
@@ -131,7 +131,8 @@
131 'git://git.example.com/project.git']))131 'git://git.example.com/project.git']))
132132
133 def test_git_to_git_arguments(self):133 def test_git_to_git_arguments(self):
134 self.pushConfig('codeimport', macaroon_secret_key='some-secret')134 self.pushConfig(
135 'launchpad', internal_macaroon_secret_key='some-secret')
135 self.useFixture(GitHostingFixture())136 self.useFixture(GitHostingFixture())
136 code_import = self.factory.makeCodeImport(137 code_import = self.factory.makeCodeImport(
137 git_repo_url="git://git.example.com/project.git",138 git_repo_url="git://git.example.com/project.git",
@@ -1271,7 +1272,8 @@
1271 def setUp(self):1272 def setUp(self):
1272 super(TestCodeImportJobMacaroonIssuer, self).setUp()1273 super(TestCodeImportJobMacaroonIssuer, self).setUp()
1273 login_for_code_imports()1274 login_for_code_imports()
1274 self.pushConfig("codeimport", macaroon_secret_key="some-secret")1275 self.pushConfig(
1276 "launchpad", internal_macaroon_secret_key="some-secret")
1275 self.useFixture(GitHostingFixture())1277 self.useFixture(GitHostingFixture())
12761278
1277 def makeJob(self, target_rcs_type=TargetRevisionControlSystems.GIT):1279 def makeJob(self, target_rcs_type=TargetRevisionControlSystems.GIT):
@@ -1296,6 +1298,11 @@
1296 caveat_id="lp.code-import-job %s" % job.id),1298 caveat_id="lp.code-import-job %s" % job.id),
1297 ]))1299 ]))
12981300
1301 def test_issueMacaroon_good_old_config(self):
1302 self.pushConfig("launchpad", internal_macaroon_secret_key="")
1303 self.pushConfig("codeimport", macaroon_secret_key="some-secret")
1304 self.test_issueMacaroon_good()
1305
1299 def test_checkMacaroonIssuer_good(self):1306 def test_checkMacaroonIssuer_good(self):
1300 job = self.makeJob()1307 job = self.makeJob()
1301 issuer = getUtility(IMacaroonIssuer, "code-import-job")1308 issuer = getUtility(IMacaroonIssuer, "code-import-job")
13021309
=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py 2019-04-09 14:19:34 +0000
+++ lib/lp/code/xmlrpc/git.py 2019-04-23 14:01:24 +0000
@@ -83,6 +83,8 @@
83 def _verifyMacaroon(self, macaroon_raw, repository=None):83 def _verifyMacaroon(self, macaroon_raw, repository=None):
84 try:84 try:
85 macaroon = Macaroon.deserialize(macaroon_raw)85 macaroon = Macaroon.deserialize(macaroon_raw)
86 # XXX cjwatson 2019-04-23: Restrict exceptions once
87 # https://github.com/ecordell/pymacaroons/issues/50 is fixed.
86 except Exception:88 except Exception:
87 return False89 return False
88 try:90 try:
8991
=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py 2019-04-09 14:19:34 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py 2019-04-23 14:01:24 +0000
@@ -962,7 +962,8 @@
962 def test_translatePath_code_import(self):962 def test_translatePath_code_import(self):
963 # A code import worker with a suitable macaroon can write to a963 # A code import worker with a suitable macaroon can write to a
964 # repository associated with a running code import job.964 # repository associated with a running code import job.
965 self.pushConfig("codeimport", macaroon_secret_key="some-secret")965 self.pushConfig(
966 "launchpad", internal_macaroon_secret_key="some-secret")
966 machine = self.factory.makeCodeImportMachine(set_online=True)967 machine = self.factory.makeCodeImportMachine(set_online=True)
967 code_imports = [968 code_imports = [
968 self.factory.makeCodeImport(969 self.factory.makeCodeImport(
@@ -998,7 +999,8 @@
998 def test_translatePath_private_code_import(self):999 def test_translatePath_private_code_import(self):
999 # A code import worker with a suitable macaroon can write to a1000 # A code import worker with a suitable macaroon can write to a
1000 # repository associated with a running private code import job.1001 # repository associated with a running private code import job.
1001 self.pushConfig("codeimport", macaroon_secret_key="some-secret")1002 self.pushConfig(
1003 "launchpad", internal_macaroon_secret_key="some-secret")
1002 machine = self.factory.makeCodeImportMachine(set_online=True)1004 machine = self.factory.makeCodeImportMachine(set_online=True)
1003 code_imports = [1005 code_imports = [
1004 self.factory.makeCodeImport(1006 self.factory.makeCodeImport(
@@ -1070,7 +1072,8 @@
1070 faults.Unauthorized)1072 faults.Unauthorized)
10711073
1072 def test_authenticateWithPassword_code_import(self):1074 def test_authenticateWithPassword_code_import(self):
1073 self.pushConfig("codeimport", macaroon_secret_key="some-secret")1075 self.pushConfig(
1076 "launchpad", internal_macaroon_secret_key="some-secret")
1074 code_import = self.factory.makeCodeImport(1077 code_import = self.factory.makeCodeImport(
1075 target_rcs_type=TargetRevisionControlSystems.GIT)1078 target_rcs_type=TargetRevisionControlSystems.GIT)
1076 with celebrity_logged_in("vcs_imports"):1079 with celebrity_logged_in("vcs_imports"):
@@ -1093,7 +1096,8 @@
1093 # A code import worker with a suitable macaroon has repository owner1096 # A code import worker with a suitable macaroon has repository owner
1094 # privileges on a repository associated with a running code import1097 # privileges on a repository associated with a running code import
1095 # job.1098 # job.
1096 self.pushConfig("codeimport", macaroon_secret_key="some-secret")1099 self.pushConfig(
1100 "launchpad", internal_macaroon_secret_key="some-secret")
1097 machine = self.factory.makeCodeImportMachine(set_online=True)1101 machine = self.factory.makeCodeImportMachine(set_online=True)
1098 code_imports = [1102 code_imports = [
1099 self.factory.makeCodeImport(1103 self.factory.makeCodeImport(
@@ -1133,7 +1137,8 @@
1133 # A code import worker with a suitable macaroon has repository owner1137 # A code import worker with a suitable macaroon has repository owner
1134 # privileges on a repository associated with a running private code1138 # privileges on a repository associated with a running private code
1135 # import job.1139 # import job.
1136 self.pushConfig("codeimport", macaroon_secret_key="some-secret")1140 self.pushConfig(
1141 "launchpad", internal_macaroon_secret_key="some-secret")
1137 machine = self.factory.makeCodeImportMachine(set_online=True)1142 machine = self.factory.makeCodeImportMachine(set_online=True)
1138 code_imports = [1143 code_imports = [
1139 self.factory.makeCodeImport(1144 self.factory.makeCodeImport(
11401145
=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2018-03-26 07:37:26 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2019-04-23 14:01:24 +0000
@@ -819,8 +819,8 @@
819 "[codehosting]",819 "[codehosting]",
820 "git_browse_root: %s" % self.target_git_server.get_url(""),820 "git_browse_root: %s" % self.target_git_server.get_url(""),
821 "",821 "",
822 "[codeimport]",822 "[launchpad]",
823 "macaroon_secret_key: some-secret",823 "internal_macaroon_secret_key: some-secret",
824 ]824 ]
825 config_fixture.add_section("\n" + "\n".join(setting_lines))825 config_fixture.add_section("\n" + "\n".join(setting_lines))
826 self.useFixture(ConfigUseFixture(config_name))826 self.useFixture(ConfigUseFixture(config_name))
827827
=== 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-04-23 14:01:24 +0000
@@ -27,10 +27,12 @@
27 person with the given name.27 person with the given name.
28 """28 """
2929
30 def verifyMacaroon(macaroon_raw, context):30 def verifyMacaroon(macaroon_raw, context_type, context):
31 """Verify that `macaroon_raw` grants access to `context`.31 """Verify that `macaroon_raw` grants access to `context`.
3232
33 :param macaroon_raw: A serialised macaroon.33 :param macaroon_raw: A serialised macaroon.
34 :param context_type: A string identifying the type of context to
35 check. Currently only 'LibraryFileAlias' is supported.
34 :param context: The context to check. Note that this is passed over36 :param context: The context to check. Note that this is passed over
35 XML-RPC, so it should be plain data (e.g. an ID) rather than a37 XML-RPC, so it should be plain data (e.g. an ID) rather than a
36 database object.38 database object.
3739
=== modified file 'lib/lp/services/authserver/tests/test_authserver.py'
--- lib/lp/services/authserver/tests/test_authserver.py 2018-05-10 10:05:45 +0000
+++ lib/lp/services/authserver/tests/test_authserver.py 2019-04-23 14:01:24 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the1# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Tests for the internal codehosting API."""4"""Tests for the internal codehosting API."""
@@ -9,6 +9,7 @@
9 Macaroon,9 Macaroon,
10 Verifier,10 Verifier,
11 )11 )
12from storm.sqlobject import SQLObjectNotFound
12from testtools.matchers import Is13from testtools.matchers import Is
13from zope.component import getUtility14from zope.component import getUtility
14from zope.interface import implementer15from zope.interface import implementer
@@ -16,6 +17,10 @@
1617
17from lp.services.authserver.xmlrpc import AuthServerAPIView18from lp.services.authserver.xmlrpc import AuthServerAPIView
18from lp.services.config import config19from lp.services.config import config
20from lp.services.librarian.interfaces import (
21 ILibraryFileAlias,
22 ILibraryFileAliasSet,
23 )
19from lp.services.macaroons.interfaces import IMacaroonIssuer24from lp.services.macaroons.interfaces import IMacaroonIssuer
20from lp.testing import (25from lp.testing import (
21 person_logged_in,26 person_logged_in,
@@ -25,7 +30,7 @@
25from lp.testing.fixture import ZopeUtilityFixture30from lp.testing.fixture import ZopeUtilityFixture
26from lp.testing.layers import (31from lp.testing.layers import (
27 DatabaseFunctionalLayer,32 DatabaseFunctionalLayer,
28 ZopelessLayer,33 ZopelessDatabaseLayer,
29 )34 )
30from lp.xmlrpc import faults35from lp.xmlrpc import faults
31from lp.xmlrpc.interfaces import IPrivateApplication36from lp.xmlrpc.interfaces import IPrivateApplication
@@ -82,7 +87,7 @@
82 macaroon = Macaroon(87 macaroon = Macaroon(
83 location=config.vhost.mainsite.hostname, identifier='test',88 location=config.vhost.mainsite.hostname, identifier='test',
84 key=self._root_secret)89 key=self._root_secret)
85 macaroon.add_first_party_caveat('test %s' % context)90 macaroon.add_first_party_caveat('test %s' % context.id)
86 return macaroon91 return macaroon
8792
88 def checkMacaroonIssuer(self, macaroon):93 def checkMacaroonIssuer(self, macaroon):
@@ -99,11 +104,13 @@
99104
100 def verifyMacaroon(self, macaroon, context):105 def verifyMacaroon(self, macaroon, context):
101 """See `IMacaroonIssuer`."""106 """See `IMacaroonIssuer`."""
107 if not ILibraryFileAlias.providedBy(context):
108 return False
102 if not self.checkMacaroonIssuer(macaroon):109 if not self.checkMacaroonIssuer(macaroon):
103 return False110 return False
104 try:111 try:
105 verifier = Verifier()112 verifier = Verifier()
106 verifier.satisfy_exact('test %s' % context)113 verifier.satisfy_exact('test %s' % context.id)
107 return verifier.verify(macaroon, self._root_secret)114 return verifier.verify(macaroon, self._root_secret)
108 except Exception:115 except Exception:
109 return False116 return False
@@ -111,7 +118,7 @@
111118
112class VerifyMacaroonTests(TestCase):119class VerifyMacaroonTests(TestCase):
113120
114 layer = ZopelessLayer121 layer = ZopelessDatabaseLayer
115122
116 def setUp(self):123 def setUp(self):
117 super(VerifyMacaroonTests, self).setUp()124 super(VerifyMacaroonTests, self).setUp()
@@ -125,7 +132,7 @@
125 def test_nonsense_macaroon(self):132 def test_nonsense_macaroon(self):
126 self.assertEqual(133 self.assertEqual(
127 faults.Unauthorized(),134 faults.Unauthorized(),
128 self.authserver.verifyMacaroon('nonsense', 0))135 self.authserver.verifyMacaroon('nonsense', 'LibraryFileAlias', 1))
129136
130 def test_unknown_issuer(self):137 def test_unknown_issuer(self):
131 macaroon = Macaroon(138 macaroon = Macaroon(
@@ -133,15 +140,42 @@
133 identifier='unknown-issuer', key='test')140 identifier='unknown-issuer', key='test')
134 self.assertEqual(141 self.assertEqual(
135 faults.Unauthorized(),142 faults.Unauthorized(),
136 self.authserver.verifyMacaroon(macaroon.serialize(), 0))143 self.authserver.verifyMacaroon(
144 macaroon.serialize(), 'LibraryFileAlias', 1))
145
146 def test_wrong_context_type(self):
147 lfa = getUtility(ILibraryFileAliasSet)[1]
148 macaroon = self.issuer.issueMacaroon(lfa)
149 self.assertEqual(
150 faults.Unauthorized(),
151 self.authserver.verifyMacaroon(
152 macaroon.serialize(), 'nonsense', lfa.id))
137153
138 def test_wrong_context(self):154 def test_wrong_context(self):
139 macaroon = self.issuer.issueMacaroon(0)155 lfa = getUtility(ILibraryFileAliasSet)[1]
140 self.assertEqual(156 macaroon = self.issuer.issueMacaroon(lfa)
141 faults.Unauthorized(),157 self.assertEqual(
142 self.authserver.verifyMacaroon(macaroon.serialize(), 1))158 faults.Unauthorized(),
159 self.authserver.verifyMacaroon(
160 macaroon.serialize(), 'LibraryFileAlias', 2))
161
162 def test_nonexistent_lfa(self):
163 macaroon = self.issuer.issueMacaroon(
164 getUtility(ILibraryFileAliasSet)[1])
165 # Pick a large ID that doesn't exist in sampledata.
166 lfa_id = 1000000
167 self.assertRaises(
168 SQLObjectNotFound, getUtility(ILibraryFileAliasSet).__getitem__,
169 lfa_id)
170 self.assertEqual(
171 faults.Unauthorized(),
172 self.authserver.verifyMacaroon(
173 macaroon.serialize(), 'LibraryFileAlias', lfa_id))
143174
144 def test_success(self):175 def test_success(self):
145 macaroon = self.issuer.issueMacaroon(0)176 lfa = getUtility(ILibraryFileAliasSet)[1]
177 macaroon = self.issuer.issueMacaroon(lfa)
146 self.assertThat(178 self.assertThat(
147 self.authserver.verifyMacaroon(macaroon.serialize(), 0), Is(True))179 self.authserver.verifyMacaroon(
180 macaroon.serialize(), 'LibraryFileAlias', lfa.id),
181 Is(True))
148182
=== 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-04-23 14:01:24 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the1# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Auth-Server XML-RPC API ."""4"""Auth-Server XML-RPC API ."""
@@ -11,6 +11,7 @@
11 ]11 ]
1212
13from pymacaroons import Macaroon13from pymacaroons import Macaroon
14from storm.sqlobject import SQLObjectNotFound
14from zope.component import (15from zope.component import (
15 ComponentLookupError,16 ComponentLookupError,
16 getUtility,17 getUtility,
@@ -22,6 +23,7 @@
22 IAuthServer,23 IAuthServer,
23 IAuthServerApplication,24 IAuthServerApplication,
24 )25 )
26from lp.services.librarian.interfaces import ILibraryFileAliasSet
25from lp.services.macaroons.interfaces import IMacaroonIssuer27from lp.services.macaroons.interfaces import IMacaroonIssuer
26from lp.services.webapp import LaunchpadXMLRPCView28from lp.services.webapp import LaunchpadXMLRPCView
27from lp.xmlrpc import faults29from lp.xmlrpc import faults
@@ -43,17 +45,29 @@
43 for key in person.sshkeys],45 for key in person.sshkeys],
44 }46 }
4547
46 def verifyMacaroon(self, macaroon_raw, context):48 def verifyMacaroon(self, macaroon_raw, context_type, context):
47 """See `IAuthServer.verifyMacaroon`."""49 """See `IAuthServer.verifyMacaroon`."""
48 try:50 try:
49 macaroon = Macaroon.deserialize(macaroon_raw)51 macaroon = Macaroon.deserialize(macaroon_raw)
52 # XXX cjwatson 2019-04-23: Restrict exceptions once
53 # https://github.com/ecordell/pymacaroons/issues/50 is fixed.
50 except Exception:54 except Exception:
51 return faults.Unauthorized()55 return faults.Unauthorized()
52 try:56 try:
53 issuer = getUtility(IMacaroonIssuer, macaroon.identifier)57 issuer = getUtility(IMacaroonIssuer, macaroon.identifier)
54 except ComponentLookupError:58 except ComponentLookupError:
55 return faults.Unauthorized()59 return faults.Unauthorized()
56 if not issuer.verifyMacaroon(macaroon, context):60 # The context is plain data, since we can't pass general objects over
61 # the XML-RPC interface. Look it up so that we can verify it.
62 if context_type == 'LibraryFileAlias':
63 # The context is a `LibraryFileAlias` ID.
64 try:
65 lfa = getUtility(ILibraryFileAliasSet)[context]
66 except SQLObjectNotFound:
67 return faults.Unauthorized()
68 else:
69 return faults.Unauthorized()
70 if not issuer.verifyMacaroon(macaroon, lfa):
57 return faults.Unauthorized()71 return faults.Unauthorized()
58 return True72 return True
5973
6074
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2019-04-16 14:30:40 +0000
+++ lib/lp/services/config/schema-lazr.conf 2019-04-23 14:01:24 +0000
@@ -431,6 +431,7 @@
431svn_revisions_import_limit: 500431svn_revisions_import_limit: 500
432432
433# Secret key for macaroons used to grant git push permission to workers.433# Secret key for macaroons used to grant git push permission to workers.
434# Deprecated in favour of launchpad.internal_macaroon_secret_key.
434macaroon_secret_key: none435macaroon_secret_key: none
435436
436[codeimportdispatcher]437[codeimportdispatcher]
@@ -1114,6 +1115,10 @@
1114# Minimum account age in days that indicates a legitimate user.1115# Minimum account age in days that indicates a legitimate user.
1115min_legitimate_account_age: 01116min_legitimate_account_age: 0
11161117
1118# Secret key for macaroons used to grant permissions to various internal
1119# components of Launchpad.
1120internal_macaroon_secret_key: none
1121
1117[launchpad_session]1122[launchpad_session]
1118# The database connection string.1123# The database connection string.
1119# datatype: pgconnection1124# datatype: pgconnection
@@ -1196,6 +1201,16 @@
11961201
1197use_https = True1202use_https = True
11981203
1204# The URL of the XML-RPC endpoint that handles verifying macaroons. This
1205# should implement IAuthServer.
1206#
1207# datatype: string
1208authentication_endpoint: none
1209
1210# The time in seconds that the librarian will wait for a reply from the
1211# authserver.
1212authentication_timeout: 5
1213
11991214
1200[librarian_gc]1215[librarian_gc]
1201# The database user which will be used by this process.1216# The database user which will be used by this process.
12021217
=== 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 2019-04-23 14:01:24 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Database access layer for the Librarian."""4"""Database access layer for the Librarian."""
@@ -11,11 +11,20 @@
11import hashlib11import hashlib
12import urllib12import urllib
1313
14from pymacaroons import Macaroon
15from six.moves.xmlrpc_client import Fault
14from storm.expr import (16from storm.expr import (
15 And,17 And,
16 SQL,18 SQL,
17 )19 )
20from twisted.internet import (
21 defer,
22 reactor as default_reactor,
23 threads,
24 )
25from twisted.web import xmlrpc
1826
27from lp.services.config import config
19from lp.services.database.interfaces import IStore28from lp.services.database.interfaces import IStore
20from lp.services.database.sqlbase import session_store29from lp.services.database.sqlbase import session_store
21from lp.services.librarian.model import (30from lp.services.librarian.model import (
@@ -23,6 +32,8 @@
23 LibraryFileContent,32 LibraryFileContent,
24 TimeLimitedToken,33 TimeLimitedToken,
25 )34 )
35from lp.services.twistedsupport import cancel_on_timeout
36from lp.xmlrpc import faults
2637
2738
28class Library:39class Library:
@@ -36,18 +47,47 @@
36 Files created in this library will marked as restricted.47 Files created in this library will marked as restricted.
37 """48 """
38 self.restricted = restricted49 self.restricted = restricted
50 self._authserver = xmlrpc.Proxy(
51 config.librarian.authentication_endpoint,
52 connectTimeout=config.librarian.authentication_timeout)
3953
40 # The following methods are read-only queries.54 # The following methods are read-only queries.
4155
42 def lookupBySHA1(self, digest):56 def lookupBySHA1(self, digest):
43 return [fc.id for fc in LibraryFileContent.selectBy(sha1=digest)]57 return [fc.id for fc in LibraryFileContent.selectBy(sha1=digest)]
4458
59 @defer.inlineCallbacks
60 def _verifyMacaroon(self, macaroon, aliasid):
61 """Verify an LFA-authorising macaroon with the authserver.
62
63 This must be called in the reactor thread.
64
65 :param macaroon: A `Macaroon`.
66 :param aliasid: A `LibraryFileAlias` ID.
67 :return: True if the authserver reports that `macaroon` authorises
68 access to `aliasid`; False if it reports that it does not.
69 :raises Fault: if the authserver request fails.
70 """
71 try:
72 yield cancel_on_timeout(
73 self._authserver.callRemote(
74 "verifyMacaroon", macaroon.serialize(), "LibraryFileAlias",
75 aliasid),
76 config.librarian.authentication_timeout)
77 defer.returnValue(True)
78 except Fault as fault:
79 if fault.faultCode == faults.Unauthorized.error_code:
80 defer.returnValue(False)
81 else:
82 raise
83
45 def getAlias(self, aliasid, token, path):84 def getAlias(self, aliasid, token, path):
46 """Returns a LibraryFileAlias, or raises LookupError.85 """Returns a LibraryFileAlias, or raises LookupError.
4786
48 A LookupError is raised if no record with the given ID exists87 A LookupError is raised if no record with the given ID exists
49 or if not related LibraryFileContent exists.88 or if not related LibraryFileContent exists.
5089
90 :param aliasid: A `LibraryFileAlias` ID.
51 :param token: The token for the file. If None no token is present.91 :param token: The token for the file. If None no token is present.
52 When a token is supplied, it is looked up with path.92 When a token is supplied, it is looked up with path.
53 :param path: The path the request is for, unused unless a token93 :param path: The path the request is for, unused unless a token
@@ -59,26 +99,34 @@
59 if token and path:99 if token and path:
60 # With a token and a path we may be able to serve restricted files100 # With a token and a path we may be able to serve restricted files
61 # on the public port.101 # on the public port.
62 #102 if isinstance(token, Macaroon):
63 # The URL-encoding of the path may have changed somewhere103 # Macaroons have enough other constraints that they don't
64 # along the line, so reencode it canonically. LFA.filename104 # need to be path-specific; it's simpler and faster to just
65 # can't contain slashes, so they're safe to leave unencoded.105 # check the alias ID.
66 # And urllib.quote erroneously excludes ~ from its safe set,106 token_ok = threads.blockingCallFromThread(
67 # while RFC 3986 says it should be unescaped and Chromium107 default_reactor, self._verifyMacaroon, token, aliasid)
68 # forcibly decodes it in any URL that it sees.108 else:
69 #109 # The URL-encoding of the path may have changed somewhere
70 # This needs to match url_path_quote.110 # along the line, so reencode it canonically. LFA.filename
71 normalised_path = urllib.quote(urllib.unquote(path), safe='/~+')111 # can't contain slashes, so they're safe to leave unencoded.
72 store = session_store()112 # And urllib.quote erroneously excludes ~ from its safe set,
73 token_found = store.find(TimeLimitedToken,113 # while RFC 3986 says it should be unescaped and Chromium
74 SQL("age(created) < interval '1 day'"),114 # forcibly decodes it in any URL that it sees.
75 TimeLimitedToken.token == hashlib.sha256(token).hexdigest(),115 #
76 TimeLimitedToken.path == normalised_path).is_empty()116 # This needs to match url_path_quote.
77 store.reset()117 normalised_path = urllib.quote(
78 if token_found:118 urllib.unquote(path), safe='/~+')
119 store = session_store()
120 token_ok = not store.find(TimeLimitedToken,
121 SQL("age(created) < interval '1 day'"),
122 TimeLimitedToken.token ==
123 hashlib.sha256(token).hexdigest(),
124 TimeLimitedToken.path == normalised_path).is_empty()
125 store.reset()
126 if token_ok:
127 restricted = True
128 else:
79 raise LookupError("Token stale/pruned/path mismatch")129 raise LookupError("Token stale/pruned/path mismatch")
80 else:
81 restricted = True
82 alias = LibraryFileAlias.selectOne(And(130 alias = LibraryFileAlias.selectOne(And(
83 LibraryFileAlias.id == aliasid,131 LibraryFileAlias.id == aliasid,
84 LibraryFileAlias.contentID == LibraryFileContent.q.id,132 LibraryFileAlias.contentID == LibraryFileContent.q.id,
85133
=== modified file 'lib/lp/services/librarianserver/tests/test_db.py'
--- lib/lp/services/librarianserver/tests/test_db.py 2013-06-20 05:50:00 +0000
+++ lib/lp/services/librarianserver/tests/test_db.py 2019-04-23 14:01:24 +0000
@@ -1,21 +1,37 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4import unittest4__metaclass__ = type
55
6from fixtures import MockPatchObject
7from pymacaroons import Macaroon
8from testtools.testcase import ExpectedException
9from testtools.twistedsupport import AsynchronousDeferredRunTest
6import transaction10import transaction
11from twisted.internet import (
12 defer,
13 reactor,
14 )
15from twisted.internet.threads import deferToThread
16from twisted.web import (
17 server,
18 xmlrpc,
19 )
720
8from lp.services.database.interfaces import IStore21from lp.services.database.interfaces import IStore
9from lp.services.librarian.model import LibraryFileContent22from lp.services.librarian.model import LibraryFileContent
10from lp.services.librarianserver import db23from lp.services.librarianserver import db
24from lp.testing import TestCase
11from lp.testing.dbuser import switch_dbuser25from lp.testing.dbuser import switch_dbuser
12from lp.testing.layers import LaunchpadZopelessLayer26from lp.testing.layers import LaunchpadZopelessLayer
1327from lp.xmlrpc import faults
1428
15class DBTestCase(unittest.TestCase):29
30class DBTestCase(TestCase):
16 layer = LaunchpadZopelessLayer31 layer = LaunchpadZopelessLayer
1732
18 def setUp(self):33 def setUp(self):
34 super(DBTestCase, self).setUp()
19 switch_dbuser('librarian')35 switch_dbuser('librarian')
2036
21 def test_lookupByDigest(self):37 def test_lookupByDigest(self):
@@ -43,12 +59,36 @@
43 self.assertEqual('text/unknown', alias.mimetype)59 self.assertEqual('text/unknown', alias.mimetype)
4460
4561
46class TestLibrarianStuff(unittest.TestCase):62class FakeAuthServer(xmlrpc.XMLRPC):
63 """A fake authserver.
64
65 This saves us from needing to start an appserver in tests.
66 """
67
68 def __init__(self):
69 xmlrpc.XMLRPC.__init__(self)
70 self.macaroons = set()
71
72 def registerMacaroon(self, macaroon, context):
73 self.macaroons.add((macaroon.serialize(), context))
74
75 def xmlrpc_verifyMacaroon(self, macaroon_raw, context_type, context):
76 if context_type != 'LibraryFileAlias':
77 return faults.Unauthorized()
78 if (macaroon_raw, context) in self.macaroons:
79 return True
80 else:
81 return faults.Unauthorized()
82
83
84class TestLibrarianStuff(TestCase):
47 """Tests for the librarian."""85 """Tests for the librarian."""
4886
49 layer = LaunchpadZopelessLayer87 layer = LaunchpadZopelessLayer
88 run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
5089
51 def setUp(self):90 def setUp(self):
91 super(TestLibrarianStuff, self).setUp()
52 switch_dbuser('librarian')92 switch_dbuser('librarian')
53 self.store = IStore(LibraryFileContent)93 self.store = IStore(LibraryFileContent)
54 self.content_id = db.Library().add('deadbeef', 1234, 'abababab', 'ba')94 self.content_id = db.Library().add('deadbeef', 1234, 'abababab', 'ba')
@@ -102,6 +142,64 @@
102 self.assertRaises(142 self.assertRaises(
103 LookupError, unrestricted_library.getAlias, 1, None, '/')143 LookupError, unrestricted_library.getAlias, 1, None, '/')
104144
145 def setUpAuthServer(self):
146 authserver = FakeAuthServer()
147 authserver_listener = reactor.listenTCP(0, server.Site(authserver))
148 self.addCleanup(authserver_listener.stopListening)
149 authserver_port = authserver_listener.getHost().port
150 authserver_url = 'http://localhost:%d/' % authserver_port
151 self.pushConfig('librarian', authentication_endpoint=authserver_url)
152 return authserver
153
154 @defer.inlineCallbacks
155 def test_getAlias_with_macaroon(self):
156 # Library.getAlias() uses the authserver to verify macaroons.
157 authserver = self.setUpAuthServer()
158 unrestricted_library = db.Library(restricted=False)
159 alias = unrestricted_library.getAlias(1, None, '/')
160 alias.restricted = True
161 transaction.commit()
162 restricted_library = db.Library(restricted=True)
163 macaroon = Macaroon()
164 with ExpectedException(LookupError):
165 yield deferToThread(restricted_library.getAlias, 1, macaroon, '/')
166 authserver.registerMacaroon(macaroon, 1)
167 alias = yield deferToThread(
168 restricted_library.getAlias, 1, macaroon, '/')
169 self.assertEqual(1, alias.id)
170
171 @defer.inlineCallbacks
172 def test_getAlias_with_wrong_macaroon(self):
173 # A macaroon for a different LFA doesn't work.
174 authserver = self.setUpAuthServer()
175 unrestricted_library = db.Library(restricted=False)
176 alias = unrestricted_library.getAlias(1, None, '/')
177 alias.restricted = True
178 transaction.commit()
179 macaroon = Macaroon()
180 authserver.registerMacaroon(macaroon, 2)
181 restricted_library = db.Library(restricted=True)
182 with ExpectedException(LookupError):
183 yield deferToThread(restricted_library.getAlias, 1, macaroon, '/')
184
185 @defer.inlineCallbacks
186 def test_getAlias_with_macaroon_timeout(self):
187 # The authserver call is cancelled after a timeout period.
188 unrestricted_library = db.Library(restricted=False)
189 alias = unrestricted_library.getAlias(1, None, '/')
190 alias.restricted = True
191 transaction.commit()
192 macaroon = Macaroon()
193 restricted_library = db.Library(restricted=True)
194 self.useFixture(MockPatchObject(
195 restricted_library._authserver, 'callRemote',
196 return_value=defer.Deferred()))
197 # XXX cjwatson 2018-11-01: We should use a Clock instead, but I had
198 # trouble getting that working in conjunction with deferToThread.
199 self.pushConfig('librarian', authentication_timeout=1)
200 with ExpectedException(defer.CancelledError):
201 yield deferToThread(restricted_library.getAlias, 1, macaroon, '/')
202
105 def test_getAliases(self):203 def test_getAliases(self):
106 # Library.getAliases() returns a sequence204 # Library.getAliases() returns a sequence
107 # [(LFA.id, LFA.filename, LFA.mimetype), ...] where LFA are205 # [(LFA.id, LFA.filename, LFA.mimetype), ...] where LFA are
108206
=== modified file 'lib/lp/services/librarianserver/tests/test_web.py'
--- lib/lp/services/librarianserver/tests/test_web.py 2018-11-02 10:14:23 +0000
+++ lib/lp/services/librarianserver/tests/test_web.py 2019-04-23 14:01:24 +0000
@@ -1,6 +1,8 @@
1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type
5
4from datetime import datetime6from datetime import datetime
5from gzip import GzipFile7from gzip import GzipFile
6import hashlib8import hashlib
@@ -14,12 +16,14 @@
14import pytz16import pytz
15import requests17import requests
16from storm.expr import SQL18from storm.expr import SQL
17import testtools
18from testtools.matchers import EndsWith19from testtools.matchers import EndsWith
19import transaction20import transaction
20from zope.component import getUtility21from zope.component import getUtility
22from zope.security.proxy import removeSecurityProxy
2123
24from lp.buildmaster.enums import BuildStatus
22from lp.services.config import config25from lp.services.config import config
26from lp.services.config.fixture import ConfigUseFixture
23from lp.services.database.interfaces import IMasterStore27from lp.services.database.interfaces import IMasterStore
24from lp.services.database.sqlbase import (28from lp.services.database.sqlbase import (
25 cursor,29 cursor,
@@ -37,10 +41,17 @@
37 TimeLimitedToken,41 TimeLimitedToken,
38 )42 )
39from lp.services.librarianserver.storage import LibrarianStorage43from lp.services.librarianserver.storage import LibrarianStorage
40from lp.testing.dbuser import switch_dbuser44from lp.services.macaroons.interfaces import IMacaroonIssuer
45from lp.testing import TestCaseWithFactory
46from lp.testing.dbuser import (
47 dbuser,
48 switch_dbuser,
49 )
41from lp.testing.layers import (50from lp.testing.layers import (
51 AppServerLayer,
42 LaunchpadFunctionalLayer,52 LaunchpadFunctionalLayer,
43 LaunchpadZopelessLayer,53 LaunchpadZopelessLayer,
54 ZopelessAppServerLayer,
44 )55 )
4556
4657
@@ -50,19 +61,62 @@
50 return str(parsed.replace(path=parsed.path.replace(old, new)))61 return str(parsed.replace(path=parsed.path.replace(old, new)))
5162
5263
53class LibrarianWebTestCase(testtools.TestCase):64class LibrarianWebTestMixin:
65
66 layer = LaunchpadFunctionalLayer
67
68 def commit(self):
69 """Synchronize database state."""
70 flush_database_updates()
71 transaction.commit()
72
73 def get_restricted_file_and_public_url(self, filename='sample'):
74 # Use a regular LibrarianClient to ensure we speak to the
75 # nonrestricted port on the librarian which is where secured
76 # restricted files are served from.
77 client = LibrarianClient()
78 fileAlias = client.addFile(
79 filename, 12, BytesIO(b'a' * 12), contentType='text/plain')
80 # Note: We're deliberately using the wrong url here: we should be
81 # passing secure=True to getURLForAlias, but to use the returned URL
82 # we would need a wildcard DNS facility patched into requests; instead
83 # we use the *deliberate* choice of having the path of secure and
84 # insecure urls be the same, so that we can test it: the server code
85 # doesn't need to know about the fancy wildcard domains.
86 url = client.getURLForAlias(fileAlias)
87 # Now that we have a url which talks to the public librarian, make the
88 # file restricted.
89 IMasterStore(LibraryFileAlias).find(
90 LibraryFileAlias, LibraryFileAlias.id == fileAlias).set(
91 restricted=True)
92 self.commit()
93 return fileAlias, url
94
95 def require404(self, url, **kwargs):
96 """Assert that opening `url` raises a 404."""
97 response = requests.get(url, **kwargs)
98 self.assertEqual(404, response.status_code)
99
100
101class LibrarianZopelessWebTestMixin(LibrarianWebTestMixin):
102
103 layer = LaunchpadZopelessLayer
104
105 def setUp(self):
106 super(LibrarianZopelessWebTestMixin, self).setUp()
107 switch_dbuser(config.librarian.dbuser)
108
109 def commit(self):
110 LaunchpadZopelessLayer.commit()
111
112
113class LibrarianWebTestCase(LibrarianWebTestMixin, TestCaseWithFactory):
54 """Test the librarian's web interface."""114 """Test the librarian's web interface."""
55 layer = LaunchpadFunctionalLayer
56115
57 # Add stuff to a librarian via the upload port, then check that it's116 # Add stuff to a librarian via the upload port, then check that it's
58 # immediately visible on the web interface. (in an attempt to test ddaa's117 # immediately visible on the web interface. (in an attempt to test ddaa's
59 # 500-error issue).118 # 500-error issue).
60119
61 def commit(self):
62 """Synchronize database state."""
63 flush_database_updates()
64 transaction.commit()
65
66 def test_uploadThenDownload(self):120 def test_uploadThenDownload(self):
67 client = LibrarianClient()121 client = LibrarianClient()
68122
@@ -286,28 +340,6 @@
286 self.assertNotIn('Last-Modified', response.headers)340 self.assertNotIn('Last-Modified', response.headers)
287 self.assertNotIn('Cache-Control', response.headers)341 self.assertNotIn('Cache-Control', response.headers)
288342
289 def get_restricted_file_and_public_url(self, filename='sample'):
290 # Use a regular LibrarianClient to ensure we speak to the
291 # nonrestricted port on the librarian which is where secured
292 # restricted files are served from.
293 client = LibrarianClient()
294 fileAlias = client.addFile(
295 filename, 12, BytesIO(b'a' * 12), contentType='text/plain')
296 # Note: We're deliberately using the wrong url here: we should be
297 # passing secure=True to getURLForAlias, but to use the returned URL
298 # we would need a wildcard DNS facility patched into requests; instead
299 # we use the *deliberate* choice of having the path of secure and
300 # insecure urls be the same, so that we can test it: the server code
301 # doesn't need to know about the fancy wildcard domains.
302 url = client.getURLForAlias(fileAlias)
303 # Now that we have a url which talks to the public librarian, make the
304 # file restricted.
305 IMasterStore(LibraryFileAlias).find(
306 LibraryFileAlias, LibraryFileAlias.id == fileAlias).set(
307 restricted=True)
308 self.commit()
309 return fileAlias, url
310
311 def test_restricted_subdomain_must_match_file_alias(self):343 def test_restricted_subdomain_must_match_file_alias(self):
312 # IFF there is a .restricted. in the host, then the library file alias344 # IFF there is a .restricted. in the host, then the library file alias
313 # in the subdomain must match that in the path.345 # in the subdomain must match that in the path.
@@ -436,21 +468,9 @@
436 last_modified_header, 'Tue, 30 Jan 2001 13:45:59 GMT')468 last_modified_header, 'Tue, 30 Jan 2001 13:45:59 GMT')
437 # Perhaps we should also set Expires to the Last-Modified.469 # Perhaps we should also set Expires to the Last-Modified.
438470
439 def require404(self, url, **kwargs):471
440 """Assert that opening `url` raises a 404."""472class LibrarianZopelessWebTestCase(
441 response = requests.get(url, **kwargs)473 LibrarianZopelessWebTestMixin, LibrarianWebTestCase):
442 self.assertEqual(404, response.status_code)
443
444
445class LibrarianZopelessWebTestCase(LibrarianWebTestCase):
446 layer = LaunchpadZopelessLayer
447
448 def setUp(self):
449 super(LibrarianZopelessWebTestCase, self).setUp()
450 switch_dbuser(config.librarian.dbuser)
451
452 def commit(self):
453 LaunchpadZopelessLayer.commit()
454474
455 def test_getURLForAliasObject(self):475 def test_getURLForAliasObject(self):
456 # getURLForAliasObject returns the same URL as getURLForAlias.476 # getURLForAliasObject returns the same URL as getURLForAlias.
@@ -467,6 +487,68 @@
467 client.getURLForAliasObject(alias))487 client.getURLForAliasObject(alias))
468488
469489
490class LibrarianWebMacaroonTestCase(LibrarianWebTestMixin, TestCaseWithFactory):
491
492 layer = AppServerLayer
493
494 def setUp(self):
495 super(LibrarianWebMacaroonTestCase, self).setUp()
496 # Copy launchpad.internal_macaroon_secret_key from the appserver
497 # config so that we can issue macaroons using it.
498 with ConfigUseFixture(self.layer.appserver_config_name):
499 key = config.launchpad.internal_macaroon_secret_key
500 self.pushConfig("launchpad", internal_macaroon_secret_key=key)
501
502 def test_restricted_with_macaroon(self):
503 fileAlias, url = self.get_restricted_file_and_public_url()
504 lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
505 with dbuser('testadmin'):
506 build = self.factory.makeBinaryPackageBuild(
507 archive=self.factory.makeArchive(private=True))
508 naked_build = removeSecurityProxy(build)
509 self.factory.makeSourcePackageReleaseFile(
510 sourcepackagerelease=naked_build.source_package_release,
511 library_file=lfa)
512 naked_build.updateStatus(BuildStatus.BUILDING)
513 issuer = getUtility(IMacaroonIssuer, "binary-package-build")
514 macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
515 self.commit()
516 response = requests.get(url, auth=("", macaroon.serialize()))
517 response.raise_for_status()
518 self.assertEqual(b"a" * 12, response.content)
519
520 def test_restricted_with_invalid_macaroon(self):
521 fileAlias, url = self.get_restricted_file_and_public_url()
522 lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
523 with dbuser('testadmin'):
524 build = self.factory.makeBinaryPackageBuild(
525 archive=self.factory.makeArchive(private=True))
526 naked_build = removeSecurityProxy(build)
527 self.factory.makeSourcePackageReleaseFile(
528 sourcepackagerelease=naked_build.source_package_release,
529 library_file=lfa)
530 naked_build.updateStatus(BuildStatus.BUILDING)
531 self.commit()
532 self.require404(url, auth=("", "not-a-macaroon"))
533
534 def test_restricted_with_unverifiable_macaroon(self):
535 fileAlias, url = self.get_restricted_file_and_public_url()
536 with dbuser('testadmin'):
537 build = self.factory.makeBinaryPackageBuild(
538 archive=self.factory.makeArchive(private=True))
539 removeSecurityProxy(build).updateStatus(BuildStatus.BUILDING)
540 issuer = getUtility(IMacaroonIssuer, "binary-package-build")
541 macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
542 self.commit()
543 self.require404(url, auth=("", macaroon.serialize()))
544
545
546class LibrarianZopelessWebMacaroonTestCase(
547 LibrarianZopelessWebTestMixin, LibrarianWebMacaroonTestCase):
548
549 layer = ZopelessAppServerLayer
550
551
470class DeletedContentTestCase(unittest.TestCase):552class DeletedContentTestCase(unittest.TestCase):
471553
472 layer = LaunchpadZopelessLayer554 layer = LaunchpadZopelessLayer
473555
=== modified file 'lib/lp/services/librarianserver/web.py'
--- lib/lp/services/librarianserver/web.py 2019-01-05 09:54:44 +0000
+++ lib/lp/services/librarianserver/web.py 2019-04-23 14:01:24 +0000
@@ -7,6 +7,7 @@
7import time7import time
8from urlparse import urlparse8from urlparse import urlparse
99
10from pymacaroons import Macaroon
10from storm.exceptions import DisconnectionError11from storm.exceptions import DisconnectionError
11from twisted.internet import (12from twisted.internet import (
12 abstract,13 abstract,
@@ -126,6 +127,14 @@
126 return fourOhFour127 return fourOhFour
127128
128 token = request.args.get('token', [None])[0]129 token = request.args.get('token', [None])[0]
130 if token is None:
131 if not request.getUser() and request.getPassword():
132 try:
133 token = Macaroon.deserialize(request.getPassword())
134 # XXX cjwatson 2019-04-23: Restrict exceptions once
135 # https://github.com/ecordell/pymacaroons/issues/50 is fixed.
136 except Exception:
137 pass
129 path = request.path138 path = request.path
130 deferred = deferToThread(139 deferred = deferToThread(
131 self._getFileAlias, self.aliasID, token, path)140 self._getFileAlias, self.aliasID, token, path)
132141
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2019-03-08 16:53:33 +0000
+++ lib/lp/soyuz/configure.zcml 2019-04-23 14:01:24 +0000
@@ -483,6 +483,15 @@
483 <allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource"/>483 <allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource"/>
484 </securedutility>484 </securedutility>
485485
486 <!-- BinaryPackageBuildMacaroonIssuer -->
487
488 <securedutility
489 class="lp.soyuz.model.binarypackagebuild.BinaryPackageBuildMacaroonIssuer"
490 provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
491 name="binary-package-build">
492 <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic"/>
493 </securedutility>
494
486 <!-- DistroArchSeriesBinaryPackage -->495 <!-- DistroArchSeriesBinaryPackage -->
487496
488 <class497 <class
489498
=== 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 2019-04-23 14:01:24 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2017 Canonical Ltd. This software is licensed under the1# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type4__metaclass__ = type
@@ -21,6 +21,10 @@
2121
22import apt_pkg22import apt_pkg
23from debian.deb822 import PkgRelation23from debian.deb822 import PkgRelation
24from pymacaroons import (
25 Macaroon,
26 Verifier,
27 )
24import pytz28import pytz
25from sqlobject import SQLObjectNotFound29from sqlobject import SQLObjectNotFound
26from storm.expr import (30from storm.expr import (
@@ -77,10 +81,12 @@
77 sqlvalues,81 sqlvalues,
78 )82 )
79from lp.services.librarian.browser import ProxiedLibraryFileAlias83from lp.services.librarian.browser import ProxiedLibraryFileAlias
84from lp.services.librarian.interfaces import ILibraryFileAlias
80from lp.services.librarian.model import (85from lp.services.librarian.model import (
81 LibraryFileAlias,86 LibraryFileAlias,
82 LibraryFileContent,87 LibraryFileContent,
83 )88 )
89from lp.services.macaroons.interfaces import IMacaroonIssuer
84from lp.soyuz.adapters.buildarch import determine_architectures_to_build90from lp.soyuz.adapters.buildarch import determine_architectures_to_build
85from lp.soyuz.enums import (91from lp.soyuz.enums import (
86 ArchivePurpose,92 ArchivePurpose,
@@ -102,7 +108,10 @@
102from lp.soyuz.mail.binarypackagebuild import BinaryPackageBuildMailer108from lp.soyuz.mail.binarypackagebuild import BinaryPackageBuildMailer
103from lp.soyuz.model.binarypackagename import BinaryPackageName109from lp.soyuz.model.binarypackagename import BinaryPackageName
104from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease110from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
105from lp.soyuz.model.files import BinaryPackageFile111from lp.soyuz.model.files import (
112 BinaryPackageFile,
113 SourcePackageReleaseFile,
114 )
106from lp.soyuz.model.packageset import Packageset115from lp.soyuz.model.packageset import Packageset
107from lp.soyuz.model.queue import (116from lp.soyuz.model.queue import (
108 PackageUpload,117 PackageUpload,
@@ -1362,3 +1371,88 @@
1362 % (build.title, build.id, build.archive.displayname,1371 % (build.title, build.id, build.archive.displayname,
1363 build_queue.lastscore))1372 build_queue.lastscore))
1364 return new_builds1373 return new_builds
1374
1375
1376@implementer(IMacaroonIssuer)
1377class BinaryPackageBuildMacaroonIssuer:
1378
1379 @property
1380 def _root_secret(self):
1381 secret = config.launchpad.internal_macaroon_secret_key
1382 if not secret:
1383 raise RuntimeError(
1384 "launchpad.internal_macaroon_secret_key not configured.")
1385 return secret
1386
1387 def issueMacaroon(self, context):
1388 """See `IMacaroonIssuer`.
1389
1390 For issuing, the context is an `IBinaryPackageBuild`.
1391 """
1392 if not removeSecurityProxy(context).archive.private:
1393 raise ValueError("Refusing to issue macaroon for public build.")
1394 macaroon = Macaroon(
1395 location=config.vhost.mainsite.hostname,
1396 identifier="binary-package-build", key=self._root_secret)
1397 # The "lp.principal" prefix indicates that this caveat constrains
1398 # the macaroon to access only resources that should be accessible
1399 # when acting on behalf of the named build, rather than to access
1400 # the named build directly.
1401 macaroon.add_first_party_caveat(
1402 "lp.principal.binary-package-build %s" %
1403 removeSecurityProxy(context).id)
1404 return macaroon
1405
1406 def checkMacaroonIssuer(self, macaroon):
1407 """See `IMacaroonIssuer`."""
1408 if macaroon.location != config.vhost.mainsite.hostname:
1409 return False
1410 try:
1411 verifier = Verifier()
1412 verifier.satisfy_general(
1413 lambda caveat: caveat.startswith(
1414 "lp.principal.binary-package-build "))
1415 return verifier.verify(macaroon, self._root_secret)
1416 except Exception:
1417 return False
1418
1419 def verifyMacaroon(self, macaroon, context):
1420 """See `IMacaroonIssuer`.
1421
1422 For verification, the context is a `LibraryFileAlias`. We check
1423 that the file is one of those required to build the
1424 `IBinaryPackageBuild` that is the context of the macaroon, and that
1425 the context build is currently building.
1426 """
1427 # Circular import.
1428 from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
1429
1430 if not ILibraryFileAlias.providedBy(context):
1431 return False
1432 if not self.checkMacaroonIssuer(macaroon):
1433 return False
1434
1435 def verify_build(caveat):
1436 prefix = "lp.principal.binary-package-build "
1437 if not caveat.startswith(prefix):
1438 return False
1439 try:
1440 build_id = int(caveat[len(prefix):])
1441 except ValueError:
1442 return False
1443 return not IStore(BinaryPackageBuild).find(
1444 BinaryPackageBuild,
1445 BinaryPackageBuild.id == build_id,
1446 BinaryPackageBuild.source_package_release_id ==
1447 SourcePackageRelease.id,
1448 SourcePackageReleaseFile.sourcepackagereleaseID ==
1449 SourcePackageRelease.id,
1450 SourcePackageReleaseFile.libraryfile == context,
1451 BinaryPackageBuild.status == BuildStatus.BUILDING).is_empty()
1452
1453 try:
1454 verifier = Verifier()
1455 verifier.satisfy_general(verify_build)
1456 return verifier.verify(macaroon, self._root_secret)
1457 except Exception:
1458 return False
13651459
=== 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 2019-04-23 14:01:24 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009-2018 Canonical Ltd. This software is licensed under the1# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Test Build features."""4"""Test Build features."""
@@ -10,8 +10,13 @@
10 timedelta,10 timedelta,
11 )11 )
1212
13from pymacaroons import Macaroon
13import pytz14import pytz
14from simplejson import dumps15from simplejson import dumps
16from testtools.matchers import (
17 MatchesListwise,
18 MatchesStructure,
19 )
15from zope.component import getUtility20from zope.component import getUtility
16from zope.security.proxy import removeSecurityProxy21from zope.security.proxy import removeSecurityProxy
1722
@@ -22,7 +27,9 @@
22from lp.registry.interfaces.pocket import PackagePublishingPocket27from lp.registry.interfaces.pocket import PackagePublishingPocket
23from lp.registry.interfaces.series import SeriesStatus28from lp.registry.interfaces.series import SeriesStatus
24from lp.registry.interfaces.sourcepackage import SourcePackageUrgency29from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
30from lp.services.config import config
25from lp.services.log.logger import DevNullLogger31from lp.services.log.logger import DevNullLogger
32from lp.services.macaroons.interfaces import IMacaroonIssuer
26from lp.services.webapp.interaction import ANONYMOUS33from lp.services.webapp.interaction import ANONYMOUS
27from lp.services.webapp.interfaces import OAuthPermission34from lp.services.webapp.interfaces import OAuthPermission
28from lp.soyuz.enums import (35from lp.soyuz.enums import (
@@ -897,3 +904,123 @@
897 with anonymous_logged_in():904 with anonymous_logged_in():
898 self.assertScoreWriteableByTeam(905 self.assertScoreWriteableByTeam(
899 archive, getUtility(ILaunchpadCelebrities).ppa_admin)906 archive, getUtility(ILaunchpadCelebrities).ppa_admin)
907
908
909class TestBinaryPackageBuildMacaroonIssuer(TestCaseWithFactory):
910 """Test BinaryPackageBuild macaroon issuing and verification."""
911
912 layer = LaunchpadZopelessLayer
913
914 def setUp(self):
915 super(TestBinaryPackageBuildMacaroonIssuer, self).setUp()
916 self.pushConfig(
917 "launchpad", internal_macaroon_secret_key="some-secret")
918
919 def test_issueMacaroon_refuses_public_archive(self):
920 build = self.factory.makeBinaryPackageBuild()
921 issuer = getUtility(IMacaroonIssuer, "binary-package-build")
922 self.assertRaises(
923 ValueError, removeSecurityProxy(issuer).issueMacaroon, build)
924
925 def test_issueMacaroon_good(self):
926 build = self.factory.makeBinaryPackageBuild(
927 archive=self.factory.makeArchive(private=True))
928 issuer = getUtility(IMacaroonIssuer, "binary-package-build")
929 macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
930 self.assertEqual("launchpad.dev", macaroon.location)
931 self.assertEqual("binary-package-build", macaroon.identifier)
932 self.assertThat(macaroon.caveats, MatchesListwise([
933 MatchesStructure.byEquality(
934 caveat_id="lp.principal.binary-package-build %s" % build.id),
935 ]))
936
937 def test_checkMacaroonIssuer_good(self):
938 build = self.factory.makeBinaryPackageBuild(
939 archive=self.factory.makeArchive(private=True))
940 issuer = getUtility(IMacaroonIssuer, "binary-package-build")
941 macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
942 self.assertTrue(issuer.checkMacaroonIssuer(macaroon))
943
944 def test_checkMacaroonIssuer_wrong_location(self):
945 issuer = getUtility(IMacaroonIssuer, "binary-package-build")
946 macaroon = Macaroon(
947 location="another-location",
948 key=removeSecurityProxy(issuer)._root_secret)
949 self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
950
951 def test_checkMacaroonIssuer_wrong_key(self):
952 issuer = getUtility(IMacaroonIssuer, "binary-package-build")
953 macaroon = Macaroon(
954 location=config.vhost.mainsite.hostname, key="another-secret")
955 self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
956
957 def test_verifyMacaroon_good(self):
958 build = self.factory.makeBinaryPackageBuild(
959 archive=self.factory.makeArchive(private=True))
960 sprf = self.factory.makeSourcePackageReleaseFile(
961 sourcepackagerelease=build.source_package_release)
962 build.updateStatus(BuildStatus.BUILDING)
963 issuer = removeSecurityProxy(
964 getUtility(IMacaroonIssuer, "binary-package-build"))
965 macaroon = issuer.issueMacaroon(build)
966 self.assertTrue(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
967
968 def test_verifyMacaroon_wrong_location(self):
969 build = self.factory.makeBinaryPackageBuild(
970 archive=self.factory.makeArchive(private=True))
971 sprf = self.factory.makeSourcePackageReleaseFile(
972 sourcepackagerelease=build.source_package_release)
973 build.updateStatus(BuildStatus.BUILDING)
974 issuer = removeSecurityProxy(
975 getUtility(IMacaroonIssuer, "binary-package-build"))
976 macaroon = Macaroon(
977 location="another-location", key=issuer._root_secret)
978 self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
979
980 def test_verifyMacaroon_wrong_key(self):
981 build = self.factory.makeBinaryPackageBuild(
982 archive=self.factory.makeArchive(private=True))
983 sprf = self.factory.makeSourcePackageReleaseFile(
984 sourcepackagerelease=build.source_package_release)
985 build.updateStatus(BuildStatus.BUILDING)
986 issuer = removeSecurityProxy(
987 getUtility(IMacaroonIssuer, "binary-package-build"))
988 macaroon = Macaroon(
989 location=config.vhost.mainsite.hostname, key="another-secret")
990 self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
991
992 def test_verifyMacaroon_not_building(self):
993 build = self.factory.makeBinaryPackageBuild(
994 archive=self.factory.makeArchive(private=True))
995 sprf = self.factory.makeSourcePackageReleaseFile(
996 sourcepackagerelease=build.source_package_release)
997 issuer = removeSecurityProxy(
998 getUtility(IMacaroonIssuer, "binary-package-build"))
999 macaroon = issuer.issueMacaroon(build)
1000 self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
1001
1002 def test_verifyMacaroon_wrong_build(self):
1003 build = self.factory.makeBinaryPackageBuild(
1004 archive=self.factory.makeArchive(private=True))
1005 sprf = self.factory.makeSourcePackageReleaseFile(
1006 sourcepackagerelease=build.source_package_release)
1007 build.updateStatus(BuildStatus.BUILDING)
1008 other_build = self.factory.makeBinaryPackageBuild(
1009 archive=self.factory.makeArchive(private=True))
1010 other_build.updateStatus(BuildStatus.BUILDING)
1011 issuer = removeSecurityProxy(
1012 getUtility(IMacaroonIssuer, "binary-package-build"))
1013 macaroon = issuer.issueMacaroon(other_build)
1014 self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
1015
1016 def test_verifyMacaroon_wrong_file(self):
1017 build = self.factory.makeBinaryPackageBuild(
1018 archive=self.factory.makeArchive(private=True))
1019 self.factory.makeSourcePackageReleaseFile(
1020 sourcepackagerelease=build.source_package_release)
1021 lfa = self.factory.makeLibraryFileAlias()
1022 build.updateStatus(BuildStatus.BUILDING)
1023 issuer = removeSecurityProxy(
1024 getUtility(IMacaroonIssuer, "binary-package-build"))
1025 macaroon = issuer.issueMacaroon(build)
1026 self.assertFalse(issuer.verifyMacaroon(macaroon, lfa))