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

Proposed by Colin Watson on 2018-05-04
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 2018-05-04 Approve on 2019-04-23
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.
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
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.

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.

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.

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.

18632. By Colin Watson on 2018-10-31

Merge devel.

18633. By Colin Watson on 2018-11-01

Generalise codeimport.macaroon_secret_key to launchpad.internal_macaroon_secret_key so that it can be used for some other macaroons.

18634. By Colin Watson on 2018-11-01

Make the librarian verify macaroons using the authserver rather than directly.

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.

18635. By Colin Watson on 2018-11-02

Merge devel.

Colin Watson (cjwatson) wrote :

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

18636. By Colin Watson on 2019-03-07

Raise ValueError rather than AssertionError when given a public build.

18637. By Colin Watson on 2019-03-12

Merge devel.

William Grant (wgrant) wrote :

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

review: Approve (code)
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
18638. By Colin Watson on 2019-04-23

Move path re-encoding into TLT-handling block.

18639. By Colin Watson on 2019-04-23

Add XXX comments for overly-general pymacaroons exception catching.

18640. By Colin Watson on 2019-04-23

Make BinaryPackageBuildMacaroonIssuer.verifyMacaroon take an ILibraryFileAlias rather than a bare ID.

Colin Watson (cjwatson) :
William Grant (wgrant) :
18641. By Colin Watson on 2019-04-23

Merge devel.

18642. By Colin Watson on 2019-04-23

Add a context_type parameter to make IAuthServer.verifyMacaroon safer.

18643. By Colin Watson on 2019-04-23

Tweak BinaryPackageBuildMacaroonIssuer caveat name to try to clarify its purpose.

William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configs/development/launchpad-lazr.conf'
2--- configs/development/launchpad-lazr.conf 2019-04-16 14:30:40 +0000
3+++ configs/development/launchpad-lazr.conf 2019-04-23 14:01:24 +0000
4@@ -121,7 +121,8 @@
5 restricted_upload_port: 58095
6 restricted_download_port: 58085
7 restricted_download_url: http://launchpad.dev:58085/
8-use_https = False
9+use_https: False
10+authentication_endpoint: http://xmlrpc-private.launchpad.dev:8087/authserver
11
12 [librarian_server]
13 root: /var/tmp/fatsam
14
15=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
16--- configs/testrunner-appserver/launchpad-lazr.conf 2018-05-21 20:30:16 +0000
17+++ configs/testrunner-appserver/launchpad-lazr.conf 2019-04-23 14:01:24 +0000
18@@ -8,14 +8,12 @@
19 [codehosting]
20 launch: False
21
22-[codeimport]
23-macaroon_secret_key: dev-macaroon-secret
24-
25 [bing_test_service]
26 launch: False
27
28 [launchpad]
29 openid_provider_root: http://testopenid.dev:8085/
30+internal_macaroon_secret_key: internal-dev-macaroon-secret
31
32 [librarian_server]
33 launch: False
34
35=== modified file 'lib/lp/code/model/codeimportjob.py'
36--- lib/lp/code/model/codeimportjob.py 2018-03-15 20:44:04 +0000
37+++ lib/lp/code/model/codeimportjob.py 2019-04-23 14:01:24 +0000
38@@ -413,10 +413,14 @@
39
40 @property
41 def _root_secret(self):
42- secret = config.codeimport.macaroon_secret_key
43+ secret = config.launchpad.internal_macaroon_secret_key
44+ if not secret:
45+ # XXX cjwatson 2018-11-01: Remove this once it is no longer in
46+ # production configs.
47+ secret = config.codeimport.macaroon_secret_key
48 if not secret:
49 raise RuntimeError(
50- "codeimport.macaroon_secret_key not configured.")
51+ "launchpad.internal_macaroon_secret_key not configured.")
52 return secret
53
54 def issueMacaroon(self, context):
55@@ -442,6 +446,8 @@
56
57 def verifyMacaroon(self, macaroon, context):
58 """See `IMacaroonIssuer`."""
59+ if not ICodeImportJob.providedBy(context):
60+ return False
61 if not self.checkMacaroonIssuer(macaroon):
62 return False
63 try:
64
65=== modified file 'lib/lp/code/model/tests/test_codeimportjob.py'
66--- lib/lp/code/model/tests/test_codeimportjob.py 2018-04-19 00:02:19 +0000
67+++ lib/lp/code/model/tests/test_codeimportjob.py 2019-04-23 14:01:24 +0000
68@@ -131,7 +131,8 @@
69 'git://git.example.com/project.git']))
70
71 def test_git_to_git_arguments(self):
72- self.pushConfig('codeimport', macaroon_secret_key='some-secret')
73+ self.pushConfig(
74+ 'launchpad', internal_macaroon_secret_key='some-secret')
75 self.useFixture(GitHostingFixture())
76 code_import = self.factory.makeCodeImport(
77 git_repo_url="git://git.example.com/project.git",
78@@ -1271,7 +1272,8 @@
79 def setUp(self):
80 super(TestCodeImportJobMacaroonIssuer, self).setUp()
81 login_for_code_imports()
82- self.pushConfig("codeimport", macaroon_secret_key="some-secret")
83+ self.pushConfig(
84+ "launchpad", internal_macaroon_secret_key="some-secret")
85 self.useFixture(GitHostingFixture())
86
87 def makeJob(self, target_rcs_type=TargetRevisionControlSystems.GIT):
88@@ -1296,6 +1298,11 @@
89 caveat_id="lp.code-import-job %s" % job.id),
90 ]))
91
92+ def test_issueMacaroon_good_old_config(self):
93+ self.pushConfig("launchpad", internal_macaroon_secret_key="")
94+ self.pushConfig("codeimport", macaroon_secret_key="some-secret")
95+ self.test_issueMacaroon_good()
96+
97 def test_checkMacaroonIssuer_good(self):
98 job = self.makeJob()
99 issuer = getUtility(IMacaroonIssuer, "code-import-job")
100
101=== modified file 'lib/lp/code/xmlrpc/git.py'
102--- lib/lp/code/xmlrpc/git.py 2019-04-09 14:19:34 +0000
103+++ lib/lp/code/xmlrpc/git.py 2019-04-23 14:01:24 +0000
104@@ -83,6 +83,8 @@
105 def _verifyMacaroon(self, macaroon_raw, repository=None):
106 try:
107 macaroon = Macaroon.deserialize(macaroon_raw)
108+ # XXX cjwatson 2019-04-23: Restrict exceptions once
109+ # https://github.com/ecordell/pymacaroons/issues/50 is fixed.
110 except Exception:
111 return False
112 try:
113
114=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
115--- lib/lp/code/xmlrpc/tests/test_git.py 2019-04-09 14:19:34 +0000
116+++ lib/lp/code/xmlrpc/tests/test_git.py 2019-04-23 14:01:24 +0000
117@@ -962,7 +962,8 @@
118 def test_translatePath_code_import(self):
119 # A code import worker with a suitable macaroon can write to a
120 # repository associated with a running code import job.
121- self.pushConfig("codeimport", macaroon_secret_key="some-secret")
122+ self.pushConfig(
123+ "launchpad", internal_macaroon_secret_key="some-secret")
124 machine = self.factory.makeCodeImportMachine(set_online=True)
125 code_imports = [
126 self.factory.makeCodeImport(
127@@ -998,7 +999,8 @@
128 def test_translatePath_private_code_import(self):
129 # A code import worker with a suitable macaroon can write to a
130 # repository associated with a running private code import job.
131- self.pushConfig("codeimport", macaroon_secret_key="some-secret")
132+ self.pushConfig(
133+ "launchpad", internal_macaroon_secret_key="some-secret")
134 machine = self.factory.makeCodeImportMachine(set_online=True)
135 code_imports = [
136 self.factory.makeCodeImport(
137@@ -1070,7 +1072,8 @@
138 faults.Unauthorized)
139
140 def test_authenticateWithPassword_code_import(self):
141- self.pushConfig("codeimport", macaroon_secret_key="some-secret")
142+ self.pushConfig(
143+ "launchpad", internal_macaroon_secret_key="some-secret")
144 code_import = self.factory.makeCodeImport(
145 target_rcs_type=TargetRevisionControlSystems.GIT)
146 with celebrity_logged_in("vcs_imports"):
147@@ -1093,7 +1096,8 @@
148 # A code import worker with a suitable macaroon has repository owner
149 # privileges on a repository associated with a running code import
150 # job.
151- self.pushConfig("codeimport", macaroon_secret_key="some-secret")
152+ self.pushConfig(
153+ "launchpad", internal_macaroon_secret_key="some-secret")
154 machine = self.factory.makeCodeImportMachine(set_online=True)
155 code_imports = [
156 self.factory.makeCodeImport(
157@@ -1133,7 +1137,8 @@
158 # A code import worker with a suitable macaroon has repository owner
159 # privileges on a repository associated with a running private code
160 # import job.
161- self.pushConfig("codeimport", macaroon_secret_key="some-secret")
162+ self.pushConfig(
163+ "launchpad", internal_macaroon_secret_key="some-secret")
164 machine = self.factory.makeCodeImportMachine(set_online=True)
165 code_imports = [
166 self.factory.makeCodeImport(
167
168=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
169--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2018-03-26 07:37:26 +0000
170+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py 2019-04-23 14:01:24 +0000
171@@ -819,8 +819,8 @@
172 "[codehosting]",
173 "git_browse_root: %s" % self.target_git_server.get_url(""),
174 "",
175- "[codeimport]",
176- "macaroon_secret_key: some-secret",
177+ "[launchpad]",
178+ "internal_macaroon_secret_key: some-secret",
179 ]
180 config_fixture.add_section("\n" + "\n".join(setting_lines))
181 self.useFixture(ConfigUseFixture(config_name))
182
183=== modified file 'lib/lp/services/authserver/interfaces.py'
184--- lib/lp/services/authserver/interfaces.py 2018-05-10 10:05:45 +0000
185+++ lib/lp/services/authserver/interfaces.py 2019-04-23 14:01:24 +0000
186@@ -27,10 +27,12 @@
187 person with the given name.
188 """
189
190- def verifyMacaroon(macaroon_raw, context):
191+ def verifyMacaroon(macaroon_raw, context_type, context):
192 """Verify that `macaroon_raw` grants access to `context`.
193
194 :param macaroon_raw: A serialised macaroon.
195+ :param context_type: A string identifying the type of context to
196+ check. Currently only 'LibraryFileAlias' is supported.
197 :param context: The context to check. Note that this is passed over
198 XML-RPC, so it should be plain data (e.g. an ID) rather than a
199 database object.
200
201=== modified file 'lib/lp/services/authserver/tests/test_authserver.py'
202--- lib/lp/services/authserver/tests/test_authserver.py 2018-05-10 10:05:45 +0000
203+++ lib/lp/services/authserver/tests/test_authserver.py 2019-04-23 14:01:24 +0000
204@@ -1,4 +1,4 @@
205-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
206+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
207 # GNU Affero General Public License version 3 (see the file LICENSE).
208
209 """Tests for the internal codehosting API."""
210@@ -9,6 +9,7 @@
211 Macaroon,
212 Verifier,
213 )
214+from storm.sqlobject import SQLObjectNotFound
215 from testtools.matchers import Is
216 from zope.component import getUtility
217 from zope.interface import implementer
218@@ -16,6 +17,10 @@
219
220 from lp.services.authserver.xmlrpc import AuthServerAPIView
221 from lp.services.config import config
222+from lp.services.librarian.interfaces import (
223+ ILibraryFileAlias,
224+ ILibraryFileAliasSet,
225+ )
226 from lp.services.macaroons.interfaces import IMacaroonIssuer
227 from lp.testing import (
228 person_logged_in,
229@@ -25,7 +30,7 @@
230 from lp.testing.fixture import ZopeUtilityFixture
231 from lp.testing.layers import (
232 DatabaseFunctionalLayer,
233- ZopelessLayer,
234+ ZopelessDatabaseLayer,
235 )
236 from lp.xmlrpc import faults
237 from lp.xmlrpc.interfaces import IPrivateApplication
238@@ -82,7 +87,7 @@
239 macaroon = Macaroon(
240 location=config.vhost.mainsite.hostname, identifier='test',
241 key=self._root_secret)
242- macaroon.add_first_party_caveat('test %s' % context)
243+ macaroon.add_first_party_caveat('test %s' % context.id)
244 return macaroon
245
246 def checkMacaroonIssuer(self, macaroon):
247@@ -99,11 +104,13 @@
248
249 def verifyMacaroon(self, macaroon, context):
250 """See `IMacaroonIssuer`."""
251+ if not ILibraryFileAlias.providedBy(context):
252+ return False
253 if not self.checkMacaroonIssuer(macaroon):
254 return False
255 try:
256 verifier = Verifier()
257- verifier.satisfy_exact('test %s' % context)
258+ verifier.satisfy_exact('test %s' % context.id)
259 return verifier.verify(macaroon, self._root_secret)
260 except Exception:
261 return False
262@@ -111,7 +118,7 @@
263
264 class VerifyMacaroonTests(TestCase):
265
266- layer = ZopelessLayer
267+ layer = ZopelessDatabaseLayer
268
269 def setUp(self):
270 super(VerifyMacaroonTests, self).setUp()
271@@ -125,7 +132,7 @@
272 def test_nonsense_macaroon(self):
273 self.assertEqual(
274 faults.Unauthorized(),
275- self.authserver.verifyMacaroon('nonsense', 0))
276+ self.authserver.verifyMacaroon('nonsense', 'LibraryFileAlias', 1))
277
278 def test_unknown_issuer(self):
279 macaroon = Macaroon(
280@@ -133,15 +140,42 @@
281 identifier='unknown-issuer', key='test')
282 self.assertEqual(
283 faults.Unauthorized(),
284- self.authserver.verifyMacaroon(macaroon.serialize(), 0))
285+ self.authserver.verifyMacaroon(
286+ macaroon.serialize(), 'LibraryFileAlias', 1))
287+
288+ def test_wrong_context_type(self):
289+ lfa = getUtility(ILibraryFileAliasSet)[1]
290+ macaroon = self.issuer.issueMacaroon(lfa)
291+ self.assertEqual(
292+ faults.Unauthorized(),
293+ self.authserver.verifyMacaroon(
294+ macaroon.serialize(), 'nonsense', lfa.id))
295
296 def test_wrong_context(self):
297- macaroon = self.issuer.issueMacaroon(0)
298- self.assertEqual(
299- faults.Unauthorized(),
300- self.authserver.verifyMacaroon(macaroon.serialize(), 1))
301+ lfa = getUtility(ILibraryFileAliasSet)[1]
302+ macaroon = self.issuer.issueMacaroon(lfa)
303+ self.assertEqual(
304+ faults.Unauthorized(),
305+ self.authserver.verifyMacaroon(
306+ macaroon.serialize(), 'LibraryFileAlias', 2))
307+
308+ def test_nonexistent_lfa(self):
309+ macaroon = self.issuer.issueMacaroon(
310+ getUtility(ILibraryFileAliasSet)[1])
311+ # Pick a large ID that doesn't exist in sampledata.
312+ lfa_id = 1000000
313+ self.assertRaises(
314+ SQLObjectNotFound, getUtility(ILibraryFileAliasSet).__getitem__,
315+ lfa_id)
316+ self.assertEqual(
317+ faults.Unauthorized(),
318+ self.authserver.verifyMacaroon(
319+ macaroon.serialize(), 'LibraryFileAlias', lfa_id))
320
321 def test_success(self):
322- macaroon = self.issuer.issueMacaroon(0)
323+ lfa = getUtility(ILibraryFileAliasSet)[1]
324+ macaroon = self.issuer.issueMacaroon(lfa)
325 self.assertThat(
326- self.authserver.verifyMacaroon(macaroon.serialize(), 0), Is(True))
327+ self.authserver.verifyMacaroon(
328+ macaroon.serialize(), 'LibraryFileAlias', lfa.id),
329+ Is(True))
330
331=== modified file 'lib/lp/services/authserver/xmlrpc.py'
332--- lib/lp/services/authserver/xmlrpc.py 2018-05-10 10:05:45 +0000
333+++ lib/lp/services/authserver/xmlrpc.py 2019-04-23 14:01:24 +0000
334@@ -1,4 +1,4 @@
335-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
336+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
337 # GNU Affero General Public License version 3 (see the file LICENSE).
338
339 """Auth-Server XML-RPC API ."""
340@@ -11,6 +11,7 @@
341 ]
342
343 from pymacaroons import Macaroon
344+from storm.sqlobject import SQLObjectNotFound
345 from zope.component import (
346 ComponentLookupError,
347 getUtility,
348@@ -22,6 +23,7 @@
349 IAuthServer,
350 IAuthServerApplication,
351 )
352+from lp.services.librarian.interfaces import ILibraryFileAliasSet
353 from lp.services.macaroons.interfaces import IMacaroonIssuer
354 from lp.services.webapp import LaunchpadXMLRPCView
355 from lp.xmlrpc import faults
356@@ -43,17 +45,29 @@
357 for key in person.sshkeys],
358 }
359
360- def verifyMacaroon(self, macaroon_raw, context):
361+ def verifyMacaroon(self, macaroon_raw, context_type, context):
362 """See `IAuthServer.verifyMacaroon`."""
363 try:
364 macaroon = Macaroon.deserialize(macaroon_raw)
365+ # XXX cjwatson 2019-04-23: Restrict exceptions once
366+ # https://github.com/ecordell/pymacaroons/issues/50 is fixed.
367 except Exception:
368 return faults.Unauthorized()
369 try:
370 issuer = getUtility(IMacaroonIssuer, macaroon.identifier)
371 except ComponentLookupError:
372 return faults.Unauthorized()
373- if not issuer.verifyMacaroon(macaroon, context):
374+ # The context is plain data, since we can't pass general objects over
375+ # the XML-RPC interface. Look it up so that we can verify it.
376+ if context_type == 'LibraryFileAlias':
377+ # The context is a `LibraryFileAlias` ID.
378+ try:
379+ lfa = getUtility(ILibraryFileAliasSet)[context]
380+ except SQLObjectNotFound:
381+ return faults.Unauthorized()
382+ else:
383+ return faults.Unauthorized()
384+ if not issuer.verifyMacaroon(macaroon, lfa):
385 return faults.Unauthorized()
386 return True
387
388
389=== modified file 'lib/lp/services/config/schema-lazr.conf'
390--- lib/lp/services/config/schema-lazr.conf 2019-04-16 14:30:40 +0000
391+++ lib/lp/services/config/schema-lazr.conf 2019-04-23 14:01:24 +0000
392@@ -431,6 +431,7 @@
393 svn_revisions_import_limit: 500
394
395 # Secret key for macaroons used to grant git push permission to workers.
396+# Deprecated in favour of launchpad.internal_macaroon_secret_key.
397 macaroon_secret_key: none
398
399 [codeimportdispatcher]
400@@ -1114,6 +1115,10 @@
401 # Minimum account age in days that indicates a legitimate user.
402 min_legitimate_account_age: 0
403
404+# Secret key for macaroons used to grant permissions to various internal
405+# components of Launchpad.
406+internal_macaroon_secret_key: none
407+
408 [launchpad_session]
409 # The database connection string.
410 # datatype: pgconnection
411@@ -1196,6 +1201,16 @@
412
413 use_https = True
414
415+# The URL of the XML-RPC endpoint that handles verifying macaroons. This
416+# should implement IAuthServer.
417+#
418+# datatype: string
419+authentication_endpoint: none
420+
421+# The time in seconds that the librarian will wait for a reply from the
422+# authserver.
423+authentication_timeout: 5
424+
425
426 [librarian_gc]
427 # The database user which will be used by this process.
428
429=== modified file 'lib/lp/services/librarianserver/db.py'
430--- lib/lp/services/librarianserver/db.py 2016-01-10 22:37:40 +0000
431+++ lib/lp/services/librarianserver/db.py 2019-04-23 14:01:24 +0000
432@@ -1,4 +1,4 @@
433-# Copyright 2009 Canonical Ltd. This software is licensed under the
434+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
435 # GNU Affero General Public License version 3 (see the file LICENSE).
436
437 """Database access layer for the Librarian."""
438@@ -11,11 +11,20 @@
439 import hashlib
440 import urllib
441
442+from pymacaroons import Macaroon
443+from six.moves.xmlrpc_client import Fault
444 from storm.expr import (
445 And,
446 SQL,
447 )
448+from twisted.internet import (
449+ defer,
450+ reactor as default_reactor,
451+ threads,
452+ )
453+from twisted.web import xmlrpc
454
455+from lp.services.config import config
456 from lp.services.database.interfaces import IStore
457 from lp.services.database.sqlbase import session_store
458 from lp.services.librarian.model import (
459@@ -23,6 +32,8 @@
460 LibraryFileContent,
461 TimeLimitedToken,
462 )
463+from lp.services.twistedsupport import cancel_on_timeout
464+from lp.xmlrpc import faults
465
466
467 class Library:
468@@ -36,18 +47,47 @@
469 Files created in this library will marked as restricted.
470 """
471 self.restricted = restricted
472+ self._authserver = xmlrpc.Proxy(
473+ config.librarian.authentication_endpoint,
474+ connectTimeout=config.librarian.authentication_timeout)
475
476 # The following methods are read-only queries.
477
478 def lookupBySHA1(self, digest):
479 return [fc.id for fc in LibraryFileContent.selectBy(sha1=digest)]
480
481+ @defer.inlineCallbacks
482+ def _verifyMacaroon(self, macaroon, aliasid):
483+ """Verify an LFA-authorising macaroon with the authserver.
484+
485+ This must be called in the reactor thread.
486+
487+ :param macaroon: A `Macaroon`.
488+ :param aliasid: A `LibraryFileAlias` ID.
489+ :return: True if the authserver reports that `macaroon` authorises
490+ access to `aliasid`; False if it reports that it does not.
491+ :raises Fault: if the authserver request fails.
492+ """
493+ try:
494+ yield cancel_on_timeout(
495+ self._authserver.callRemote(
496+ "verifyMacaroon", macaroon.serialize(), "LibraryFileAlias",
497+ aliasid),
498+ config.librarian.authentication_timeout)
499+ defer.returnValue(True)
500+ except Fault as fault:
501+ if fault.faultCode == faults.Unauthorized.error_code:
502+ defer.returnValue(False)
503+ else:
504+ raise
505+
506 def getAlias(self, aliasid, token, path):
507 """Returns a LibraryFileAlias, or raises LookupError.
508
509 A LookupError is raised if no record with the given ID exists
510 or if not related LibraryFileContent exists.
511
512+ :param aliasid: A `LibraryFileAlias` ID.
513 :param token: The token for the file. If None no token is present.
514 When a token is supplied, it is looked up with path.
515 :param path: The path the request is for, unused unless a token
516@@ -59,26 +99,34 @@
517 if token and path:
518 # With a token and a path we may be able to serve restricted files
519 # on the public port.
520- #
521- # The URL-encoding of the path may have changed somewhere
522- # along the line, so reencode it canonically. LFA.filename
523- # can't contain slashes, so they're safe to leave unencoded.
524- # And urllib.quote erroneously excludes ~ from its safe set,
525- # while RFC 3986 says it should be unescaped and Chromium
526- # forcibly decodes it in any URL that it sees.
527- #
528- # This needs to match url_path_quote.
529- normalised_path = urllib.quote(urllib.unquote(path), safe='/~+')
530- store = session_store()
531- token_found = store.find(TimeLimitedToken,
532- SQL("age(created) < interval '1 day'"),
533- TimeLimitedToken.token == hashlib.sha256(token).hexdigest(),
534- TimeLimitedToken.path == normalised_path).is_empty()
535- store.reset()
536- if token_found:
537+ if isinstance(token, Macaroon):
538+ # Macaroons have enough other constraints that they don't
539+ # need to be path-specific; it's simpler and faster to just
540+ # check the alias ID.
541+ token_ok = threads.blockingCallFromThread(
542+ default_reactor, self._verifyMacaroon, token, aliasid)
543+ else:
544+ # The URL-encoding of the path may have changed somewhere
545+ # along the line, so reencode it canonically. LFA.filename
546+ # can't contain slashes, so they're safe to leave unencoded.
547+ # And urllib.quote erroneously excludes ~ from its safe set,
548+ # while RFC 3986 says it should be unescaped and Chromium
549+ # forcibly decodes it in any URL that it sees.
550+ #
551+ # This needs to match url_path_quote.
552+ normalised_path = urllib.quote(
553+ urllib.unquote(path), safe='/~+')
554+ store = session_store()
555+ token_ok = not store.find(TimeLimitedToken,
556+ SQL("age(created) < interval '1 day'"),
557+ TimeLimitedToken.token ==
558+ hashlib.sha256(token).hexdigest(),
559+ TimeLimitedToken.path == normalised_path).is_empty()
560+ store.reset()
561+ if token_ok:
562+ restricted = True
563+ else:
564 raise LookupError("Token stale/pruned/path mismatch")
565- else:
566- restricted = True
567 alias = LibraryFileAlias.selectOne(And(
568 LibraryFileAlias.id == aliasid,
569 LibraryFileAlias.contentID == LibraryFileContent.q.id,
570
571=== modified file 'lib/lp/services/librarianserver/tests/test_db.py'
572--- lib/lp/services/librarianserver/tests/test_db.py 2013-06-20 05:50:00 +0000
573+++ lib/lp/services/librarianserver/tests/test_db.py 2019-04-23 14:01:24 +0000
574@@ -1,21 +1,37 @@
575-# Copyright 2009 Canonical Ltd. This software is licensed under the
576+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
577 # GNU Affero General Public License version 3 (see the file LICENSE).
578
579-import unittest
580+__metaclass__ = type
581
582+from fixtures import MockPatchObject
583+from pymacaroons import Macaroon
584+from testtools.testcase import ExpectedException
585+from testtools.twistedsupport import AsynchronousDeferredRunTest
586 import transaction
587+from twisted.internet import (
588+ defer,
589+ reactor,
590+ )
591+from twisted.internet.threads import deferToThread
592+from twisted.web import (
593+ server,
594+ xmlrpc,
595+ )
596
597 from lp.services.database.interfaces import IStore
598 from lp.services.librarian.model import LibraryFileContent
599 from lp.services.librarianserver import db
600+from lp.testing import TestCase
601 from lp.testing.dbuser import switch_dbuser
602 from lp.testing.layers import LaunchpadZopelessLayer
603-
604-
605-class DBTestCase(unittest.TestCase):
606+from lp.xmlrpc import faults
607+
608+
609+class DBTestCase(TestCase):
610 layer = LaunchpadZopelessLayer
611
612 def setUp(self):
613+ super(DBTestCase, self).setUp()
614 switch_dbuser('librarian')
615
616 def test_lookupByDigest(self):
617@@ -43,12 +59,36 @@
618 self.assertEqual('text/unknown', alias.mimetype)
619
620
621-class TestLibrarianStuff(unittest.TestCase):
622+class FakeAuthServer(xmlrpc.XMLRPC):
623+ """A fake authserver.
624+
625+ This saves us from needing to start an appserver in tests.
626+ """
627+
628+ def __init__(self):
629+ xmlrpc.XMLRPC.__init__(self)
630+ self.macaroons = set()
631+
632+ def registerMacaroon(self, macaroon, context):
633+ self.macaroons.add((macaroon.serialize(), context))
634+
635+ def xmlrpc_verifyMacaroon(self, macaroon_raw, context_type, context):
636+ if context_type != 'LibraryFileAlias':
637+ return faults.Unauthorized()
638+ if (macaroon_raw, context) in self.macaroons:
639+ return True
640+ else:
641+ return faults.Unauthorized()
642+
643+
644+class TestLibrarianStuff(TestCase):
645 """Tests for the librarian."""
646
647 layer = LaunchpadZopelessLayer
648+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
649
650 def setUp(self):
651+ super(TestLibrarianStuff, self).setUp()
652 switch_dbuser('librarian')
653 self.store = IStore(LibraryFileContent)
654 self.content_id = db.Library().add('deadbeef', 1234, 'abababab', 'ba')
655@@ -102,6 +142,64 @@
656 self.assertRaises(
657 LookupError, unrestricted_library.getAlias, 1, None, '/')
658
659+ def setUpAuthServer(self):
660+ authserver = FakeAuthServer()
661+ authserver_listener = reactor.listenTCP(0, server.Site(authserver))
662+ self.addCleanup(authserver_listener.stopListening)
663+ authserver_port = authserver_listener.getHost().port
664+ authserver_url = 'http://localhost:%d/' % authserver_port
665+ self.pushConfig('librarian', authentication_endpoint=authserver_url)
666+ return authserver
667+
668+ @defer.inlineCallbacks
669+ def test_getAlias_with_macaroon(self):
670+ # Library.getAlias() uses the authserver to verify macaroons.
671+ authserver = self.setUpAuthServer()
672+ unrestricted_library = db.Library(restricted=False)
673+ alias = unrestricted_library.getAlias(1, None, '/')
674+ alias.restricted = True
675+ transaction.commit()
676+ restricted_library = db.Library(restricted=True)
677+ macaroon = Macaroon()
678+ with ExpectedException(LookupError):
679+ yield deferToThread(restricted_library.getAlias, 1, macaroon, '/')
680+ authserver.registerMacaroon(macaroon, 1)
681+ alias = yield deferToThread(
682+ restricted_library.getAlias, 1, macaroon, '/')
683+ self.assertEqual(1, alias.id)
684+
685+ @defer.inlineCallbacks
686+ def test_getAlias_with_wrong_macaroon(self):
687+ # A macaroon for a different LFA doesn't work.
688+ authserver = self.setUpAuthServer()
689+ unrestricted_library = db.Library(restricted=False)
690+ alias = unrestricted_library.getAlias(1, None, '/')
691+ alias.restricted = True
692+ transaction.commit()
693+ macaroon = Macaroon()
694+ authserver.registerMacaroon(macaroon, 2)
695+ restricted_library = db.Library(restricted=True)
696+ with ExpectedException(LookupError):
697+ yield deferToThread(restricted_library.getAlias, 1, macaroon, '/')
698+
699+ @defer.inlineCallbacks
700+ def test_getAlias_with_macaroon_timeout(self):
701+ # The authserver call is cancelled after a timeout period.
702+ unrestricted_library = db.Library(restricted=False)
703+ alias = unrestricted_library.getAlias(1, None, '/')
704+ alias.restricted = True
705+ transaction.commit()
706+ macaroon = Macaroon()
707+ restricted_library = db.Library(restricted=True)
708+ self.useFixture(MockPatchObject(
709+ restricted_library._authserver, 'callRemote',
710+ return_value=defer.Deferred()))
711+ # XXX cjwatson 2018-11-01: We should use a Clock instead, but I had
712+ # trouble getting that working in conjunction with deferToThread.
713+ self.pushConfig('librarian', authentication_timeout=1)
714+ with ExpectedException(defer.CancelledError):
715+ yield deferToThread(restricted_library.getAlias, 1, macaroon, '/')
716+
717 def test_getAliases(self):
718 # Library.getAliases() returns a sequence
719 # [(LFA.id, LFA.filename, LFA.mimetype), ...] where LFA are
720
721=== modified file 'lib/lp/services/librarianserver/tests/test_web.py'
722--- lib/lp/services/librarianserver/tests/test_web.py 2018-11-02 10:14:23 +0000
723+++ lib/lp/services/librarianserver/tests/test_web.py 2019-04-23 14:01:24 +0000
724@@ -1,6 +1,8 @@
725 # Copyright 2009-2018 Canonical Ltd. This software is licensed under the
726 # GNU Affero General Public License version 3 (see the file LICENSE).
727
728+__metaclass__ = type
729+
730 from datetime import datetime
731 from gzip import GzipFile
732 import hashlib
733@@ -14,12 +16,14 @@
734 import pytz
735 import requests
736 from storm.expr import SQL
737-import testtools
738 from testtools.matchers import EndsWith
739 import transaction
740 from zope.component import getUtility
741+from zope.security.proxy import removeSecurityProxy
742
743+from lp.buildmaster.enums import BuildStatus
744 from lp.services.config import config
745+from lp.services.config.fixture import ConfigUseFixture
746 from lp.services.database.interfaces import IMasterStore
747 from lp.services.database.sqlbase import (
748 cursor,
749@@ -37,10 +41,17 @@
750 TimeLimitedToken,
751 )
752 from lp.services.librarianserver.storage import LibrarianStorage
753-from lp.testing.dbuser import switch_dbuser
754+from lp.services.macaroons.interfaces import IMacaroonIssuer
755+from lp.testing import TestCaseWithFactory
756+from lp.testing.dbuser import (
757+ dbuser,
758+ switch_dbuser,
759+ )
760 from lp.testing.layers import (
761+ AppServerLayer,
762 LaunchpadFunctionalLayer,
763 LaunchpadZopelessLayer,
764+ ZopelessAppServerLayer,
765 )
766
767
768@@ -50,19 +61,62 @@
769 return str(parsed.replace(path=parsed.path.replace(old, new)))
770
771
772-class LibrarianWebTestCase(testtools.TestCase):
773+class LibrarianWebTestMixin:
774+
775+ layer = LaunchpadFunctionalLayer
776+
777+ def commit(self):
778+ """Synchronize database state."""
779+ flush_database_updates()
780+ transaction.commit()
781+
782+ def get_restricted_file_and_public_url(self, filename='sample'):
783+ # Use a regular LibrarianClient to ensure we speak to the
784+ # nonrestricted port on the librarian which is where secured
785+ # restricted files are served from.
786+ client = LibrarianClient()
787+ fileAlias = client.addFile(
788+ filename, 12, BytesIO(b'a' * 12), contentType='text/plain')
789+ # Note: We're deliberately using the wrong url here: we should be
790+ # passing secure=True to getURLForAlias, but to use the returned URL
791+ # we would need a wildcard DNS facility patched into requests; instead
792+ # we use the *deliberate* choice of having the path of secure and
793+ # insecure urls be the same, so that we can test it: the server code
794+ # doesn't need to know about the fancy wildcard domains.
795+ url = client.getURLForAlias(fileAlias)
796+ # Now that we have a url which talks to the public librarian, make the
797+ # file restricted.
798+ IMasterStore(LibraryFileAlias).find(
799+ LibraryFileAlias, LibraryFileAlias.id == fileAlias).set(
800+ restricted=True)
801+ self.commit()
802+ return fileAlias, url
803+
804+ def require404(self, url, **kwargs):
805+ """Assert that opening `url` raises a 404."""
806+ response = requests.get(url, **kwargs)
807+ self.assertEqual(404, response.status_code)
808+
809+
810+class LibrarianZopelessWebTestMixin(LibrarianWebTestMixin):
811+
812+ layer = LaunchpadZopelessLayer
813+
814+ def setUp(self):
815+ super(LibrarianZopelessWebTestMixin, self).setUp()
816+ switch_dbuser(config.librarian.dbuser)
817+
818+ def commit(self):
819+ LaunchpadZopelessLayer.commit()
820+
821+
822+class LibrarianWebTestCase(LibrarianWebTestMixin, TestCaseWithFactory):
823 """Test the librarian's web interface."""
824- layer = LaunchpadFunctionalLayer
825
826 # Add stuff to a librarian via the upload port, then check that it's
827 # immediately visible on the web interface. (in an attempt to test ddaa's
828 # 500-error issue).
829
830- def commit(self):
831- """Synchronize database state."""
832- flush_database_updates()
833- transaction.commit()
834-
835 def test_uploadThenDownload(self):
836 client = LibrarianClient()
837
838@@ -286,28 +340,6 @@
839 self.assertNotIn('Last-Modified', response.headers)
840 self.assertNotIn('Cache-Control', response.headers)
841
842- def get_restricted_file_and_public_url(self, filename='sample'):
843- # Use a regular LibrarianClient to ensure we speak to the
844- # nonrestricted port on the librarian which is where secured
845- # restricted files are served from.
846- client = LibrarianClient()
847- fileAlias = client.addFile(
848- filename, 12, BytesIO(b'a' * 12), contentType='text/plain')
849- # Note: We're deliberately using the wrong url here: we should be
850- # passing secure=True to getURLForAlias, but to use the returned URL
851- # we would need a wildcard DNS facility patched into requests; instead
852- # we use the *deliberate* choice of having the path of secure and
853- # insecure urls be the same, so that we can test it: the server code
854- # doesn't need to know about the fancy wildcard domains.
855- url = client.getURLForAlias(fileAlias)
856- # Now that we have a url which talks to the public librarian, make the
857- # file restricted.
858- IMasterStore(LibraryFileAlias).find(
859- LibraryFileAlias, LibraryFileAlias.id == fileAlias).set(
860- restricted=True)
861- self.commit()
862- return fileAlias, url
863-
864 def test_restricted_subdomain_must_match_file_alias(self):
865 # IFF there is a .restricted. in the host, then the library file alias
866 # in the subdomain must match that in the path.
867@@ -436,21 +468,9 @@
868 last_modified_header, 'Tue, 30 Jan 2001 13:45:59 GMT')
869 # Perhaps we should also set Expires to the Last-Modified.
870
871- def require404(self, url, **kwargs):
872- """Assert that opening `url` raises a 404."""
873- response = requests.get(url, **kwargs)
874- self.assertEqual(404, response.status_code)
875-
876-
877-class LibrarianZopelessWebTestCase(LibrarianWebTestCase):
878- layer = LaunchpadZopelessLayer
879-
880- def setUp(self):
881- super(LibrarianZopelessWebTestCase, self).setUp()
882- switch_dbuser(config.librarian.dbuser)
883-
884- def commit(self):
885- LaunchpadZopelessLayer.commit()
886+
887+class LibrarianZopelessWebTestCase(
888+ LibrarianZopelessWebTestMixin, LibrarianWebTestCase):
889
890 def test_getURLForAliasObject(self):
891 # getURLForAliasObject returns the same URL as getURLForAlias.
892@@ -467,6 +487,68 @@
893 client.getURLForAliasObject(alias))
894
895
896+class LibrarianWebMacaroonTestCase(LibrarianWebTestMixin, TestCaseWithFactory):
897+
898+ layer = AppServerLayer
899+
900+ def setUp(self):
901+ super(LibrarianWebMacaroonTestCase, self).setUp()
902+ # Copy launchpad.internal_macaroon_secret_key from the appserver
903+ # config so that we can issue macaroons using it.
904+ with ConfigUseFixture(self.layer.appserver_config_name):
905+ key = config.launchpad.internal_macaroon_secret_key
906+ self.pushConfig("launchpad", internal_macaroon_secret_key=key)
907+
908+ def test_restricted_with_macaroon(self):
909+ fileAlias, url = self.get_restricted_file_and_public_url()
910+ lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
911+ with dbuser('testadmin'):
912+ build = self.factory.makeBinaryPackageBuild(
913+ archive=self.factory.makeArchive(private=True))
914+ naked_build = removeSecurityProxy(build)
915+ self.factory.makeSourcePackageReleaseFile(
916+ sourcepackagerelease=naked_build.source_package_release,
917+ library_file=lfa)
918+ naked_build.updateStatus(BuildStatus.BUILDING)
919+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
920+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
921+ self.commit()
922+ response = requests.get(url, auth=("", macaroon.serialize()))
923+ response.raise_for_status()
924+ self.assertEqual(b"a" * 12, response.content)
925+
926+ def test_restricted_with_invalid_macaroon(self):
927+ fileAlias, url = self.get_restricted_file_and_public_url()
928+ lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
929+ with dbuser('testadmin'):
930+ build = self.factory.makeBinaryPackageBuild(
931+ archive=self.factory.makeArchive(private=True))
932+ naked_build = removeSecurityProxy(build)
933+ self.factory.makeSourcePackageReleaseFile(
934+ sourcepackagerelease=naked_build.source_package_release,
935+ library_file=lfa)
936+ naked_build.updateStatus(BuildStatus.BUILDING)
937+ self.commit()
938+ self.require404(url, auth=("", "not-a-macaroon"))
939+
940+ def test_restricted_with_unverifiable_macaroon(self):
941+ fileAlias, url = self.get_restricted_file_and_public_url()
942+ with dbuser('testadmin'):
943+ build = self.factory.makeBinaryPackageBuild(
944+ archive=self.factory.makeArchive(private=True))
945+ removeSecurityProxy(build).updateStatus(BuildStatus.BUILDING)
946+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
947+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
948+ self.commit()
949+ self.require404(url, auth=("", macaroon.serialize()))
950+
951+
952+class LibrarianZopelessWebMacaroonTestCase(
953+ LibrarianZopelessWebTestMixin, LibrarianWebMacaroonTestCase):
954+
955+ layer = ZopelessAppServerLayer
956+
957+
958 class DeletedContentTestCase(unittest.TestCase):
959
960 layer = LaunchpadZopelessLayer
961
962=== modified file 'lib/lp/services/librarianserver/web.py'
963--- lib/lp/services/librarianserver/web.py 2019-01-05 09:54:44 +0000
964+++ lib/lp/services/librarianserver/web.py 2019-04-23 14:01:24 +0000
965@@ -7,6 +7,7 @@
966 import time
967 from urlparse import urlparse
968
969+from pymacaroons import Macaroon
970 from storm.exceptions import DisconnectionError
971 from twisted.internet import (
972 abstract,
973@@ -126,6 +127,14 @@
974 return fourOhFour
975
976 token = request.args.get('token', [None])[0]
977+ if token is None:
978+ if not request.getUser() and request.getPassword():
979+ try:
980+ token = Macaroon.deserialize(request.getPassword())
981+ # XXX cjwatson 2019-04-23: Restrict exceptions once
982+ # https://github.com/ecordell/pymacaroons/issues/50 is fixed.
983+ except Exception:
984+ pass
985 path = request.path
986 deferred = deferToThread(
987 self._getFileAlias, self.aliasID, token, path)
988
989=== modified file 'lib/lp/soyuz/configure.zcml'
990--- lib/lp/soyuz/configure.zcml 2019-03-08 16:53:33 +0000
991+++ lib/lp/soyuz/configure.zcml 2019-04-23 14:01:24 +0000
992@@ -483,6 +483,15 @@
993 <allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource"/>
994 </securedutility>
995
996+ <!-- BinaryPackageBuildMacaroonIssuer -->
997+
998+ <securedutility
999+ class="lp.soyuz.model.binarypackagebuild.BinaryPackageBuildMacaroonIssuer"
1000+ provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
1001+ name="binary-package-build">
1002+ <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic"/>
1003+ </securedutility>
1004+
1005 <!-- DistroArchSeriesBinaryPackage -->
1006
1007 <class
1008
1009=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
1010--- lib/lp/soyuz/model/binarypackagebuild.py 2017-03-29 09:28:09 +0000
1011+++ lib/lp/soyuz/model/binarypackagebuild.py 2019-04-23 14:01:24 +0000
1012@@ -1,4 +1,4 @@
1013-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
1014+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
1015 # GNU Affero General Public License version 3 (see the file LICENSE).
1016
1017 __metaclass__ = type
1018@@ -21,6 +21,10 @@
1019
1020 import apt_pkg
1021 from debian.deb822 import PkgRelation
1022+from pymacaroons import (
1023+ Macaroon,
1024+ Verifier,
1025+ )
1026 import pytz
1027 from sqlobject import SQLObjectNotFound
1028 from storm.expr import (
1029@@ -77,10 +81,12 @@
1030 sqlvalues,
1031 )
1032 from lp.services.librarian.browser import ProxiedLibraryFileAlias
1033+from lp.services.librarian.interfaces import ILibraryFileAlias
1034 from lp.services.librarian.model import (
1035 LibraryFileAlias,
1036 LibraryFileContent,
1037 )
1038+from lp.services.macaroons.interfaces import IMacaroonIssuer
1039 from lp.soyuz.adapters.buildarch import determine_architectures_to_build
1040 from lp.soyuz.enums import (
1041 ArchivePurpose,
1042@@ -102,7 +108,10 @@
1043 from lp.soyuz.mail.binarypackagebuild import BinaryPackageBuildMailer
1044 from lp.soyuz.model.binarypackagename import BinaryPackageName
1045 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
1046-from lp.soyuz.model.files import BinaryPackageFile
1047+from lp.soyuz.model.files import (
1048+ BinaryPackageFile,
1049+ SourcePackageReleaseFile,
1050+ )
1051 from lp.soyuz.model.packageset import Packageset
1052 from lp.soyuz.model.queue import (
1053 PackageUpload,
1054@@ -1362,3 +1371,88 @@
1055 % (build.title, build.id, build.archive.displayname,
1056 build_queue.lastscore))
1057 return new_builds
1058+
1059+
1060+@implementer(IMacaroonIssuer)
1061+class BinaryPackageBuildMacaroonIssuer:
1062+
1063+ @property
1064+ def _root_secret(self):
1065+ secret = config.launchpad.internal_macaroon_secret_key
1066+ if not secret:
1067+ raise RuntimeError(
1068+ "launchpad.internal_macaroon_secret_key not configured.")
1069+ return secret
1070+
1071+ def issueMacaroon(self, context):
1072+ """See `IMacaroonIssuer`.
1073+
1074+ For issuing, the context is an `IBinaryPackageBuild`.
1075+ """
1076+ if not removeSecurityProxy(context).archive.private:
1077+ raise ValueError("Refusing to issue macaroon for public build.")
1078+ macaroon = Macaroon(
1079+ location=config.vhost.mainsite.hostname,
1080+ identifier="binary-package-build", key=self._root_secret)
1081+ # The "lp.principal" prefix indicates that this caveat constrains
1082+ # the macaroon to access only resources that should be accessible
1083+ # when acting on behalf of the named build, rather than to access
1084+ # the named build directly.
1085+ macaroon.add_first_party_caveat(
1086+ "lp.principal.binary-package-build %s" %
1087+ removeSecurityProxy(context).id)
1088+ return macaroon
1089+
1090+ def checkMacaroonIssuer(self, macaroon):
1091+ """See `IMacaroonIssuer`."""
1092+ if macaroon.location != config.vhost.mainsite.hostname:
1093+ return False
1094+ try:
1095+ verifier = Verifier()
1096+ verifier.satisfy_general(
1097+ lambda caveat: caveat.startswith(
1098+ "lp.principal.binary-package-build "))
1099+ return verifier.verify(macaroon, self._root_secret)
1100+ except Exception:
1101+ return False
1102+
1103+ def verifyMacaroon(self, macaroon, context):
1104+ """See `IMacaroonIssuer`.
1105+
1106+ For verification, the context is a `LibraryFileAlias`. We check
1107+ that the file is one of those required to build the
1108+ `IBinaryPackageBuild` that is the context of the macaroon, and that
1109+ the context build is currently building.
1110+ """
1111+ # Circular import.
1112+ from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
1113+
1114+ if not ILibraryFileAlias.providedBy(context):
1115+ return False
1116+ if not self.checkMacaroonIssuer(macaroon):
1117+ return False
1118+
1119+ def verify_build(caveat):
1120+ prefix = "lp.principal.binary-package-build "
1121+ if not caveat.startswith(prefix):
1122+ return False
1123+ try:
1124+ build_id = int(caveat[len(prefix):])
1125+ except ValueError:
1126+ return False
1127+ return not IStore(BinaryPackageBuild).find(
1128+ BinaryPackageBuild,
1129+ BinaryPackageBuild.id == build_id,
1130+ BinaryPackageBuild.source_package_release_id ==
1131+ SourcePackageRelease.id,
1132+ SourcePackageReleaseFile.sourcepackagereleaseID ==
1133+ SourcePackageRelease.id,
1134+ SourcePackageReleaseFile.libraryfile == context,
1135+ BinaryPackageBuild.status == BuildStatus.BUILDING).is_empty()
1136+
1137+ try:
1138+ verifier = Verifier()
1139+ verifier.satisfy_general(verify_build)
1140+ return verifier.verify(macaroon, self._root_secret)
1141+ except Exception:
1142+ return False
1143
1144=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
1145--- lib/lp/soyuz/tests/test_binarypackagebuild.py 2018-02-09 14:56:43 +0000
1146+++ lib/lp/soyuz/tests/test_binarypackagebuild.py 2019-04-23 14:01:24 +0000
1147@@ -1,4 +1,4 @@
1148-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
1149+# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
1150 # GNU Affero General Public License version 3 (see the file LICENSE).
1151
1152 """Test Build features."""
1153@@ -10,8 +10,13 @@
1154 timedelta,
1155 )
1156
1157+from pymacaroons import Macaroon
1158 import pytz
1159 from simplejson import dumps
1160+from testtools.matchers import (
1161+ MatchesListwise,
1162+ MatchesStructure,
1163+ )
1164 from zope.component import getUtility
1165 from zope.security.proxy import removeSecurityProxy
1166
1167@@ -22,7 +27,9 @@
1168 from lp.registry.interfaces.pocket import PackagePublishingPocket
1169 from lp.registry.interfaces.series import SeriesStatus
1170 from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
1171+from lp.services.config import config
1172 from lp.services.log.logger import DevNullLogger
1173+from lp.services.macaroons.interfaces import IMacaroonIssuer
1174 from lp.services.webapp.interaction import ANONYMOUS
1175 from lp.services.webapp.interfaces import OAuthPermission
1176 from lp.soyuz.enums import (
1177@@ -897,3 +904,123 @@
1178 with anonymous_logged_in():
1179 self.assertScoreWriteableByTeam(
1180 archive, getUtility(ILaunchpadCelebrities).ppa_admin)
1181+
1182+
1183+class TestBinaryPackageBuildMacaroonIssuer(TestCaseWithFactory):
1184+ """Test BinaryPackageBuild macaroon issuing and verification."""
1185+
1186+ layer = LaunchpadZopelessLayer
1187+
1188+ def setUp(self):
1189+ super(TestBinaryPackageBuildMacaroonIssuer, self).setUp()
1190+ self.pushConfig(
1191+ "launchpad", internal_macaroon_secret_key="some-secret")
1192+
1193+ def test_issueMacaroon_refuses_public_archive(self):
1194+ build = self.factory.makeBinaryPackageBuild()
1195+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
1196+ self.assertRaises(
1197+ ValueError, removeSecurityProxy(issuer).issueMacaroon, build)
1198+
1199+ def test_issueMacaroon_good(self):
1200+ build = self.factory.makeBinaryPackageBuild(
1201+ archive=self.factory.makeArchive(private=True))
1202+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
1203+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
1204+ self.assertEqual("launchpad.dev", macaroon.location)
1205+ self.assertEqual("binary-package-build", macaroon.identifier)
1206+ self.assertThat(macaroon.caveats, MatchesListwise([
1207+ MatchesStructure.byEquality(
1208+ caveat_id="lp.principal.binary-package-build %s" % build.id),
1209+ ]))
1210+
1211+ def test_checkMacaroonIssuer_good(self):
1212+ build = self.factory.makeBinaryPackageBuild(
1213+ archive=self.factory.makeArchive(private=True))
1214+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
1215+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
1216+ self.assertTrue(issuer.checkMacaroonIssuer(macaroon))
1217+
1218+ def test_checkMacaroonIssuer_wrong_location(self):
1219+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
1220+ macaroon = Macaroon(
1221+ location="another-location",
1222+ key=removeSecurityProxy(issuer)._root_secret)
1223+ self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
1224+
1225+ def test_checkMacaroonIssuer_wrong_key(self):
1226+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
1227+ macaroon = Macaroon(
1228+ location=config.vhost.mainsite.hostname, key="another-secret")
1229+ self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
1230+
1231+ def test_verifyMacaroon_good(self):
1232+ build = self.factory.makeBinaryPackageBuild(
1233+ archive=self.factory.makeArchive(private=True))
1234+ sprf = self.factory.makeSourcePackageReleaseFile(
1235+ sourcepackagerelease=build.source_package_release)
1236+ build.updateStatus(BuildStatus.BUILDING)
1237+ issuer = removeSecurityProxy(
1238+ getUtility(IMacaroonIssuer, "binary-package-build"))
1239+ macaroon = issuer.issueMacaroon(build)
1240+ self.assertTrue(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
1241+
1242+ def test_verifyMacaroon_wrong_location(self):
1243+ build = self.factory.makeBinaryPackageBuild(
1244+ archive=self.factory.makeArchive(private=True))
1245+ sprf = self.factory.makeSourcePackageReleaseFile(
1246+ sourcepackagerelease=build.source_package_release)
1247+ build.updateStatus(BuildStatus.BUILDING)
1248+ issuer = removeSecurityProxy(
1249+ getUtility(IMacaroonIssuer, "binary-package-build"))
1250+ macaroon = Macaroon(
1251+ location="another-location", key=issuer._root_secret)
1252+ self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
1253+
1254+ def test_verifyMacaroon_wrong_key(self):
1255+ build = self.factory.makeBinaryPackageBuild(
1256+ archive=self.factory.makeArchive(private=True))
1257+ sprf = self.factory.makeSourcePackageReleaseFile(
1258+ sourcepackagerelease=build.source_package_release)
1259+ build.updateStatus(BuildStatus.BUILDING)
1260+ issuer = removeSecurityProxy(
1261+ getUtility(IMacaroonIssuer, "binary-package-build"))
1262+ macaroon = Macaroon(
1263+ location=config.vhost.mainsite.hostname, key="another-secret")
1264+ self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
1265+
1266+ def test_verifyMacaroon_not_building(self):
1267+ build = self.factory.makeBinaryPackageBuild(
1268+ archive=self.factory.makeArchive(private=True))
1269+ sprf = self.factory.makeSourcePackageReleaseFile(
1270+ sourcepackagerelease=build.source_package_release)
1271+ issuer = removeSecurityProxy(
1272+ getUtility(IMacaroonIssuer, "binary-package-build"))
1273+ macaroon = issuer.issueMacaroon(build)
1274+ self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
1275+
1276+ def test_verifyMacaroon_wrong_build(self):
1277+ build = self.factory.makeBinaryPackageBuild(
1278+ archive=self.factory.makeArchive(private=True))
1279+ sprf = self.factory.makeSourcePackageReleaseFile(
1280+ sourcepackagerelease=build.source_package_release)
1281+ build.updateStatus(BuildStatus.BUILDING)
1282+ other_build = self.factory.makeBinaryPackageBuild(
1283+ archive=self.factory.makeArchive(private=True))
1284+ other_build.updateStatus(BuildStatus.BUILDING)
1285+ issuer = removeSecurityProxy(
1286+ getUtility(IMacaroonIssuer, "binary-package-build"))
1287+ macaroon = issuer.issueMacaroon(other_build)
1288+ self.assertFalse(issuer.verifyMacaroon(macaroon, sprf.libraryfile))
1289+
1290+ def test_verifyMacaroon_wrong_file(self):
1291+ build = self.factory.makeBinaryPackageBuild(
1292+ archive=self.factory.makeArchive(private=True))
1293+ self.factory.makeSourcePackageReleaseFile(
1294+ sourcepackagerelease=build.source_package_release)
1295+ lfa = self.factory.makeLibraryFileAlias()
1296+ build.updateStatus(BuildStatus.BUILDING)
1297+ issuer = removeSecurityProxy(
1298+ getUtility(IMacaroonIssuer, "binary-package-build"))
1299+ macaroon = issuer.issueMacaroon(build)
1300+ self.assertFalse(issuer.verifyMacaroon(macaroon, lfa))