Launchpad itself

Merge lp:~jtv/launchpad/bug-544237 into lp:launchpad

  1. bug-544237
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~jtv/launchpad/bug-544237
Merge into: lp:launchpad
Diff against target: 888 lines (+177/-326)
19 files modified
lib/lp/buildmaster/doc/builder.txt (+6/-6)
lib/lp/buildmaster/doc/buildfarmjobbehavior.txt (+2/-2)
lib/lp/buildmaster/interfaces/builder.py (+4/-4)
lib/lp/buildmaster/interfaces/buildfarmjob.py (+10/-0)
lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py (+6/-5)
lib/lp/buildmaster/model/builder.py (+6/-5)
lib/lp/buildmaster/model/buildfarmjob.py (+24/-0)
lib/lp/buildmaster/model/buildfarmjobbehavior.py (+8/-105)
lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py (+76/-39)
lib/lp/buildmaster/tests/test_manager.py (+1/-1)
lib/lp/code/model/recipebuilder.py (+2/-10)
lib/lp/code/model/sourcepackagerecipebuild.py (+3/-0)
lib/lp/code/tests/test_recipebuilder.py (+10/-25)
lib/lp/soyuz/doc/buildd-slavescanner.txt (+7/-1)
lib/lp/soyuz/model/binarypackagebuildbehavior.py (+5/-3)
lib/lp/soyuz/tests/soyuzbuilddhelpers.py (+3/-2)
lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py (+0/-66)
lib/lp/translations/model/translationtemplatesbuildbehavior.py (+4/-19)
lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py (+0/-33)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-544237
Reviewer Review Type Date Requested Status
Eleanor Berger (community) code Approve
Review via email: mp+23214@code.launchpad.net

Commit message

Slave build cookies.

Description of the change

= Bug 544237 =

This is an oversized branch. I'm sorry for that; I really am. A lot of the diff is caused by the renaming of an exception class.

The build farm dispatches build jobs to slave machines. To avoid confusing one job running on a slave with another, the slave receives a "build id" that proves to the master that it's been working on that particular job. It's not technically an id; it's not even necessarily associated with a build. It's just there for verification. There's also a security aspect: slaves can be compromised, and in that state, probably should not be able to substitute their own output for that of a legitimate job running on another slave.

A method in IBuildFarmJobBehavior, which was to be overridden in each implementation, tried to verify a slave's build id against expectations. This was fairly complex without offering much security.

In this branch I replace the build id with something that's simpler and more secure at the same time: a build cookie. It hashes a bunch of values that are associated with a job running on a slave, and which don't change as a job progresses. Put together and hashed, these values are not very easily predictable. (The hashing helps by obscuring from the slave various values that it could otherwise use as starting points for its guesses). I removed all attempts to parse and analyze the cookie. All that's really needed for verification is to re-generate the cookie and compare the outcome to the version that the slave provided. The two should be equal. This does away with the multiple implementations of the verification logic.

We've discussed these changes on the mailing list and on IRC, in particular with bigjools, jml, and wgrant. A few design considerations:

 * We haven't actually identified any real risks even if a compromised slave does forge another slave's build cookie. But the purpose of this check has always been a bit obscure so we're just not taking any chances—especially with future code changes that might otherwise rely on false security. If there is a security risk, this branch can be shown to reduce it. If there is none, this branch merely simplifies the code.

 * For now I included the original slave build id, as generated by getName, in the hashed cookie. This means that the cookie will be at least as hard to guess as what we had before.

 * William and I analyzed failures that might conceivably occur while verifying a cookie, and considered re-raising them as verification failures. But all failures we could imagine would be bugs. So the appropriate response was always to let the exception propagate as-is.

 * There are probably "safer" hash algorithms out there than SHA1. But the code never hashes output from the slave, so there is no question of a compromised slave appending arbitrary data to its output in order to get a desired hash value.

 * It's probably still not that hard for a compromised slave, with knowledge of ballpark figures for some of the inputs, to guess the values that went into its own cookie, which it could then use as a starting point for guessing other cookies. We considered including values like the job's start date or builder id. But that may make it easier for future code changes to break the algorithm. For properly secure cookies, we'd need some salt.

There's a bunch of lint output, mostly the usual "unable to import." Also some lint in places I didn't touch; this branch being the size it is I'd rather not mess with that (and risk causing more conflicts for others).

For testing, I ran all Soyuz tests and all tests with "build" in them. But be aware that includes windmill tests, and that when using lucid, you may get "unknown error code" errors from pygpgme. Jelmer has a pygpgme patch underway that fixes those.

To Q/A, verify that Soyuz builds still succeed on dogfood.

Jeroen

To post a comment you must log in.
Revision history for this message
Eleanor Berger (intellectronica) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/doc/builder.txt'
2--- lib/lp/buildmaster/doc/builder.txt 2010-04-06 22:33:44 +0000
3+++ lib/lp/buildmaster/doc/builder.txt 2010-04-12 06:43:20 +0000
4@@ -266,18 +266,18 @@
5 against the database. If it isn't building what we think it should be,
6 the current build will be aborted and the slave cleaned in preparation
7 for a new task. The decision about the slave's correctness is left up
8-to IBuildFarmJobBehavior.verifySlaveBuildID -- for these examples we
9-will use a special behavior that just checks if the slave ID is 'good'.
10+to IBuildFarmJobBehavior.verifySlaveBuildCookie -- for these examples we
11+will use a special behavior that just checks if the cookie reads 'good'.
12
13 >>> import logging
14- >>> from lp.buildmaster.interfaces.builder import CorruptBuildID
15+ >>> from lp.buildmaster.interfaces.builder import CorruptBuildCookie
16 >>> from lp.soyuz.tests.soyuzbuilddhelpers import (
17 ... BuildingSlave, MockBuilder, OkSlave, WaitingSlave)
18
19 >>> class TestBuildBehavior:
20- ... def verifySlaveBuildID(self, build_id):
21- ... if build_id != 'good':
22- ... raise CorruptBuildID('Bad value')
23+ ... def verifySlaveBuildCookie(self, cookie):
24+ ... if cookie != 'good':
25+ ... raise CorruptBuildCookie('Bad value')
26
27 >>> def rescue_slave_if_lost(slave):
28 ... builder = MockBuilder('mock', slave, TestBuildBehavior())
29
30=== modified file 'lib/lp/buildmaster/doc/buildfarmjobbehavior.txt'
31--- lib/lp/buildmaster/doc/buildfarmjobbehavior.txt 2010-03-25 04:31:10 +0000
32+++ lib/lp/buildmaster/doc/buildfarmjobbehavior.txt 2010-04-12 06:43:20 +0000
33@@ -115,7 +115,7 @@
34 If a slave is working on a job while we think it is idle, it will always be
35 aborted.
36
37- >>> bob.current_build_behavior.verifySlaveBuildID('foo')
38+ >>> bob.current_build_behavior.verifySlaveBuildCookie('foo')
39 Traceback (most recent call last):
40 ...
41- CorruptBuildID: No job assigned to builder
42+ CorruptBuildCookie: No job assigned to builder
43
44=== modified file 'lib/lp/buildmaster/interfaces/builder.py'
45--- lib/lp/buildmaster/interfaces/builder.py 2010-03-24 04:58:35 +0000
46+++ lib/lp/buildmaster/interfaces/builder.py 2010-04-12 06:43:20 +0000
47@@ -9,7 +9,7 @@
48
49 __all__ = [
50 'BuildDaemonError',
51- 'CorruptBuildID',
52+ 'CorruptBuildCookie',
53 'BuildSlaveFailure',
54 'CannotBuild',
55 'CannotFetchFile',
56@@ -46,7 +46,7 @@
57 """The build slave had a protocol version. This is a serious error."""
58
59
60-class CorruptBuildID(BuildDaemonError):
61+class CorruptBuildCookie(BuildDaemonError):
62 """The build slave is working with mismatched information.
63
64 It needs to be rescued.
65@@ -229,8 +229,8 @@
66 the status.
67 """
68
69- def verifySlaveBuildID(slave_build_id):
70- """Verify that a slave's build ID is consistent.
71+ def verifySlaveBuildCookie(slave_build_id):
72+ """Verify that a slave's build cookie is consistent.
73
74 This should delegate to the current `IBuildFarmJobBehavior`.
75 """
76
77=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
78--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-03-16 05:08:47 +0000
79+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2010-04-12 06:43:20 +0000
80@@ -57,6 +57,16 @@
81 class IBuildFarmJob(Interface):
82 """Operations that jobs for the build farm must implement."""
83
84+ def generateSlaveBuildCookie():
85+ """Produce a cookie for the slave as a token of the job it's doing.
86+
87+ The cookie need not be unique, but should be hard for a
88+ compromised slave to guess.
89+
90+ :return: a hard-to-guess ASCII string that can be reproduced
91+ accurately based on this job's properties.
92+ """
93+
94 def score():
95 """Calculate a job score appropriate for the job type in question."""
96
97
98=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py'
99--- lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2010-03-31 02:15:04 +0000
100+++ lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2010-04-12 06:43:20 +0000
101@@ -59,12 +59,13 @@
102 added to it.
103 """
104
105- def verifySlaveBuildID(slave_build_id):
106- """Verify that a slave's build ID shows no signs of corruption.
107+ def verifySlaveBuildCookie(slave_build_cookie):
108+ """Verify that a slave's build cookie shows no signs of corruption.
109
110- :param slave_build_id: The slave's build ID, as specified in
111- dispatchBuildToSlave.
112- :raises CorruptBuildID: if the build ID is determined to be corrupt.
113+ :param slave_build_cookie: The slave's build cookie, as specified in
114+ `dispatchBuildToSlave`.
115+ :raises CorruptBuildCookie: if the build cookie isn't what it's
116+ supposed to be.
117 """
118
119 def updateBuild(queueItem):
120
121=== modified file 'lib/lp/buildmaster/model/builder.py'
122--- lib/lp/buildmaster/model/builder.py 2010-04-08 15:29:39 +0000
123+++ lib/lp/buildmaster/model/builder.py 2010-04-12 06:43:20 +0000
124@@ -42,7 +42,7 @@
125 from lp.buildmaster.interfaces.buildbase import BuildStatus
126 from lp.buildmaster.interfaces.builder import (
127 BuildDaemonError, BuildSlaveFailure, CannotBuild, CannotFetchFile,
128- CannotResumeHost, CorruptBuildID, IBuilder, IBuilderSet,
129+ CannotResumeHost, CorruptBuildCookie, IBuilder, IBuilderSet,
130 ProtocolVersionMismatch)
131 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
132 BuildBehaviorMismatch)
133@@ -182,8 +182,8 @@
134 slave_build_id = status_sentence[ident_position[status]]
135
136 try:
137- builder.verifySlaveBuildID(slave_build_id)
138- except CorruptBuildID, reason:
139+ builder.verifySlaveBuildCookie(slave_build_id)
140+ except CorruptBuildCookie, reason:
141 if status == 'BuilderStatus.WAITING':
142 builder.cleanSlave()
143 else:
144@@ -454,9 +454,10 @@
145 """See IBuilder."""
146 return self.slave.status()
147
148- def verifySlaveBuildID(self, slave_build_id):
149+ def verifySlaveBuildCookie(self, slave_build_id):
150 """See `IBuilder`."""
151- return self.current_build_behavior.verifySlaveBuildID(slave_build_id)
152+ return self.current_build_behavior.verifySlaveBuildCookie(
153+ slave_build_id)
154
155 def updateBuild(self, queueItem):
156 """See `IBuilder`."""
157
158=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
159--- lib/lp/buildmaster/model/buildfarmjob.py 2010-03-16 05:08:47 +0000
160+++ lib/lp/buildmaster/model/buildfarmjob.py 2010-04-12 06:43:20 +0000
161@@ -5,14 +5,18 @@
162 __all__ = ['BuildFarmJob']
163
164
165+import hashlib
166+
167 from zope.component import getUtility
168 from zope.interface import classProvides, implements
169+from zope.security.proxy import removeSecurityProxy
170
171 from canonical.launchpad.webapp.interfaces import (
172 DEFAULT_FLAVOR, IStoreSelector, MAIN_STORE)
173 from lp.buildmaster.interfaces.buildfarmjob import (
174 IBuildFarmJob, IBuildFarmCandidateJobSelection,
175 ISpecificBuildFarmJobClass)
176+from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
177
178
179 class BuildFarmJob:
180@@ -21,6 +25,26 @@
181 classProvides(
182 IBuildFarmCandidateJobSelection, ISpecificBuildFarmJobClass)
183
184+ def generateSlaveBuildCookie(self):
185+ """See `IBuildFarmJob`."""
186+ buildqueue = getUtility(IBuildQueueSet).getByJob(self.job)
187+
188+ if buildqueue.processor is None:
189+ processor = '*'
190+ else:
191+ processor = repr(buildqueue.processor.id)
192+
193+ contents = ';'.join([
194+ repr(removeSecurityProxy(self.job).id),
195+ self.job.date_created.isoformat(),
196+ repr(buildqueue.id),
197+ buildqueue.job_type.name,
198+ processor,
199+ self.getName(),
200+ ])
201+
202+ return hashlib.sha1(contents).hexdigest()
203+
204 def score(self):
205 """See `IBuildFarmJob`."""
206 raise NotImplementedError
207
208=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
209--- lib/lp/buildmaster/model/buildfarmjobbehavior.py 2010-03-31 04:08:34 +0000
210+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py 2010-04-12 06:43:20 +0000
211@@ -16,23 +16,17 @@
212 import socket
213 import xmlrpclib
214
215-from sqlobject import SQLObjectNotFound
216-
217 from zope.component import getUtility
218 from zope.interface import implements
219-from zope.security.proxy import removeSecurityProxy, isinstance as zisinstance
220+from zope.security.proxy import removeSecurityProxy
221
222 from canonical import encoding
223 from canonical.librarian.interfaces import ILibrarianClient
224
225-from canonical.launchpad.webapp.interfaces import NotFoundError
226-from lp.buildmaster.interfaces.builder import CorruptBuildID
227+from lp.buildmaster.interfaces.builder import CorruptBuildCookie
228 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
229 BuildBehaviorMismatch, IBuildFarmJobBehavior)
230-from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
231-from lp.buildmaster.model.buildqueue import BuildQueue
232 from lp.services.job.interfaces.job import JobStatus
233-from lp.soyuz.interfaces.build import IBuildSet
234
235
236 class BuildFarmJobBehaviorBase:
237@@ -64,102 +58,11 @@
238 The default behavior is that we don't add any extra values."""
239 pass
240
241- def _helpVerifyBuildIDComponent(self, raw_id, item_type, finder):
242- """Helper for verifying parts of a `BuildFarmJob` name.
243-
244- Different `IBuildFarmJob` implementations can have different
245- ways of constructing their identifying names. The names are
246- produced by `IBuildFarmJob.getName` and verified by
247- `IBuildFarmJobBehavior.verifySlaveBuildID`.
248-
249- This little helper makes it easier to verify an object id
250- embedded in that name, check that it's a valid number, and
251- retrieve the associated database object.
252-
253- :param raw_id: An unverified id string as extracted from the
254- build name. The method will verify that it is a number, and
255- try to retrieve the associated object.
256- :param item_type: The type of object this id represents. Should
257- be a class.
258- :param finder: A function that, given an integral id, finds the
259- associated object of type `item_type`.
260- :raise CorruptBuildID: If `raw_id` is malformed in some way or
261- the associated `item_type` object is not found.
262- :return: An object that is an instance of `item_type`.
263- """
264- type_name = item_type.__name__
265- try:
266- numeric_id = int(raw_id)
267- except ValueError:
268- raise CorruptBuildID(
269- "%s ID is not a number: '%s'" % (type_name, raw_id))
270-
271- try:
272- item = finder(numeric_id)
273- except (NotFoundError, SQLObjectNotFound), reason:
274- raise CorruptBuildID(
275- "%s %d is not available: %s" % (
276- type_name, numeric_id, reason))
277- except Exception, reason:
278- raise CorruptBuildID(
279- "Error while looking up %s %d: %s" % (
280- type_name, numeric_id, reason))
281-
282- if item is None:
283- raise CorruptBuildID("There is no %s with id %d." % (
284- type_name, numeric_id))
285-
286- assert zisinstance(item, item_type), (
287- "Looked for %s, but found %s." % (type_name, repr(item)))
288-
289- return item
290-
291- def getVerifiedBuild(self, raw_id):
292- """Verify and retrieve the `Build` component of a slave build id.
293-
294- This does part of the verification for `verifySlaveBuildID`.
295-
296- By default, a `BuildFarmJob` has an identifying name of the form
297- "b-q", where b is the id of its `Build` and q is the id of its
298- `BuildQueue` record.
299-
300- Use `getVerifiedBuild` to verify the "b" part, and retrieve the
301- associated `Build`.
302- """
303- # Avoid circular import.
304- from lp.soyuz.model.build import Build
305-
306- return self._helpVerifyBuildIDComponent(
307- raw_id, Build, getUtility(IBuildSet).getByBuildID)
308-
309- def getVerifiedBuildQueue(self, raw_id):
310- """Verify and retrieve the `BuildQueue` component of a slave build id.
311-
312- This does part of the verification for `verifySlaveBuildID`.
313-
314- By default, a `BuildFarmJob` has an identifying name of the form
315- "b-q", where b is the id of its `Build` and q is the id of its
316- `BuildQueue` record.
317-
318- Use `getVerifiedBuildQueue` to verify the "q" part, and retrieve
319- the associated `BuildQueue` object.
320- """
321- return self._helpVerifyBuildIDComponent(
322- raw_id, BuildQueue, getUtility(IBuildQueueSet).get)
323-
324- def verifySlaveBuildID(self, slave_build_id):
325+ def verifySlaveBuildCookie(self, slave_build_cookie):
326 """See `IBuildFarmJobBehavior`."""
327- # Extract information from the identifier.
328- try:
329- build_id, queue_item_id = slave_build_id.split('-')
330- except ValueError:
331- raise CorruptBuildID('Malformed build ID')
332-
333- build = self.getVerifiedBuild(build_id)
334- queue_item = self.getVerifiedBuildQueue(queue_item_id)
335-
336- if build != queue_item.specific_job.build:
337- raise CorruptBuildID('Job build entry mismatch')
338+ expected_cookie = self.buildfarmjob.generateSlaveBuildCookie()
339+ if slave_build_cookie != expected_cookie:
340+ raise CorruptBuildCookie("Invalid slave build cookie.")
341
342 def updateBuild(self, queueItem):
343 """See `IBuildFarmJobBehavior`."""
344@@ -310,6 +213,6 @@
345 """See `IBuildFarmJobBehavior`."""
346 return "Idle"
347
348- def verifySlaveBuildID(self, slave_build_id):
349+ def verifySlaveBuildCookie(self, slave_build_id):
350 """See `IBuildFarmJobBehavior`."""
351- raise CorruptBuildID('No job assigned to builder')
352+ raise CorruptBuildCookie('No job assigned to builder')
353
354=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
355--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2010-03-12 20:24:53 +0000
356+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2010-04-12 06:43:20 +0000
357@@ -6,12 +6,14 @@
358 from unittest import TestLoader
359
360 from zope.component import getUtility
361+from zope.security.proxy import removeSecurityProxy
362
363 from canonical.testing.layers import ZopelessDatabaseLayer
364 from lp.testing import TestCaseWithFactory
365+from lp.testing.fakemethod import FakeMethod
366
367 from lp.buildmaster.interfaces.buildbase import BuildStatus
368-from lp.buildmaster.interfaces.builder import CorruptBuildID
369+from lp.buildmaster.interfaces.builder import CorruptBuildCookie
370 from lp.buildmaster.model.buildfarmjobbehavior import BuildFarmJobBehaviorBase
371 from lp.registry.interfaces.pocket import PackagePublishingPocket
372 from lp.soyuz.interfaces.processor import IProcessorFamilySet
373@@ -31,8 +33,15 @@
374 """Create a `BuildFarmJobBehaviorBase`."""
375 if buildfarmjob is None:
376 buildfarmjob = FakeBuildFarmJob()
377+ else:
378+ buildfarmjob = removeSecurityProxy(buildfarmjob)
379 return BuildFarmJobBehaviorBase(buildfarmjob)
380
381+ def _changeBuildFarmJobName(self, buildfarmjob):
382+ """Manipulate `buildfarmjob` so that its `getName` changes."""
383+ name = buildfarmjob.getName() + 'x'
384+ removeSecurityProxy(buildfarmjob).getName = FakeMethod(result=name)
385+
386 def _makeBuild(self):
387 """Create a `Build` object."""
388 x86 = getUtility(IProcessorFamilySet).getByName('x86')
389@@ -69,44 +78,72 @@
390 self.assertRaises(
391 AssertionError, behavior.extractBuildStatus, slave_status)
392
393- def test_getVerifiedBuild_success(self):
394- build = self._makeBuild()
395- behavior = self._makeBehavior()
396- raw_id = str(build.id)
397-
398- self.assertEqual(build, behavior.getVerifiedBuild(raw_id))
399-
400- def test_getVerifiedBuild_malformed(self):
401- behavior = self._makeBehavior()
402- self.assertRaises(CorruptBuildID, behavior.getVerifiedBuild, 'hi!')
403-
404- def test_getVerifiedBuild_notfound(self):
405- build = self._makeBuild()
406- behavior = self._makeBehavior()
407- nonexistent_id = str(build.id + 99)
408-
409- self.assertRaises(
410- CorruptBuildID, behavior.getVerifiedBuild, nonexistent_id)
411-
412- def test_getVerifiedBuildQueue_success(self):
413- buildqueue = self._makeBuildQueue()
414- behavior = self._makeBehavior()
415- raw_id = str(buildqueue.id)
416-
417- self.assertEqual(buildqueue, behavior.getVerifiedBuildQueue(raw_id))
418-
419- def test_getVerifiedBuildQueue_malformed(self):
420- behavior = self._makeBehavior()
421- self.assertRaises(
422- CorruptBuildID, behavior.getVerifiedBuildQueue, 'bye!')
423-
424- def test_getVerifiedBuildQueue_notfound(self):
425- buildqueue = self._makeBuildQueue()
426- behavior = self._makeBehavior()
427- nonexistent_id = str(buildqueue.id + 99)
428-
429- self.assertRaises(
430- CorruptBuildID, behavior.getVerifiedBuildQueue, nonexistent_id)
431+ def test_cookie_baseline(self):
432+ buildfarmjob = self.factory.makeTranslationTemplatesBuildJob()
433+
434+ cookie = buildfarmjob.generateSlaveBuildCookie()
435+
436+ self.assertNotEqual(None, cookie)
437+ self.assertNotEqual(0, len(cookie))
438+ self.assertTrue(len(cookie) > 10)
439+
440+ self.assertEqual(cookie, buildfarmjob.generateSlaveBuildCookie())
441+
442+ def test_verifySlaveBuildCookie_good(self):
443+ buildfarmjob = self.factory.makeTranslationTemplatesBuildJob()
444+ behavior = self._makeBehavior(buildfarmjob)
445+
446+ cookie = buildfarmjob.generateSlaveBuildCookie()
447+
448+ # The correct cookie validates successfully.
449+ behavior.verifySlaveBuildCookie(cookie)
450+
451+ def test_verifySlaveBuildCookie_bad(self):
452+ buildfarmjob = self.factory.makeTranslationTemplatesBuildJob()
453+ behavior = self._makeBehavior(buildfarmjob)
454+
455+ cookie = buildfarmjob.generateSlaveBuildCookie()
456+
457+ self.assertRaises(
458+ CorruptBuildCookie,
459+ behavior.verifySlaveBuildCookie,
460+ cookie + 'x')
461+
462+ def test_cookie_includes_job_name(self):
463+ # The cookie is a hash that includes the job's name.
464+ buildfarmjob = self.factory.makeTranslationTemplatesBuildJob()
465+ buildfarmjob = removeSecurityProxy(buildfarmjob)
466+ behavior = self._makeBehavior(buildfarmjob)
467+ cookie = buildfarmjob.generateSlaveBuildCookie()
468+
469+ self._changeBuildFarmJobName(buildfarmjob)
470+
471+ self.assertRaises(
472+ CorruptBuildCookie,
473+ behavior.verifySlaveBuildCookie,
474+ cookie)
475+
476+ # However, the name is not included in plaintext so as not to
477+ # provide a compromised slave a starting point for guessing
478+ # another slave's cookie.
479+ self.assertNotIn(buildfarmjob.getName(), cookie)
480+
481+ def test_cookie_includes_more_than_name(self):
482+ # Two build jobs with the same name still get different cookies.
483+ buildfarmjob1 = self.factory.makeTranslationTemplatesBuildJob()
484+ buildfarmjob1 = removeSecurityProxy(buildfarmjob1)
485+ buildfarmjob2 = self.factory.makeTranslationTemplatesBuildJob(
486+ branch=buildfarmjob1.branch)
487+ buildfarmjob2 = removeSecurityProxy(buildfarmjob2)
488+
489+ name_factory = FakeMethod(result="same-name")
490+ buildfarmjob1.getName = name_factory
491+ buildfarmjob2.getName = name_factory
492+
493+ self.assertEqual(buildfarmjob1.getName(), buildfarmjob2.getName())
494+ self.assertNotEqual(
495+ buildfarmjob1.generateSlaveBuildCookie(),
496+ buildfarmjob2.generateSlaveBuildCookie())
497
498
499 def test_suite():
500
501=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
502--- lib/lp/buildmaster/tests/test_manager.py 2010-04-09 00:00:26 +0000
503+++ lib/lp/buildmaster/tests/test_manager.py 2010-04-12 06:43:20 +0000
504@@ -552,7 +552,7 @@
505 'http://localhost:58000/43/alsa-utils_1.0.9a-4ubuntu1.dsc',
506 '', '')),
507 ('build',
508- ('11-2',
509+ ('6358a89e2215e19b02bf91e2e4d009640fae5cf8',
510 'binarypackage', '0feca720e2c29dafb2c900713ba560e03b758711',
511 {'alsa-utils_1.0.9a-4ubuntu1.dsc':
512 '4e3961baf4f56fdbc95d0dd47f3c5bc275da8a33'},
513
514=== modified file 'lib/lp/code/model/recipebuilder.py'
515--- lib/lp/code/model/recipebuilder.py 2010-03-31 02:15:04 +0000
516+++ lib/lp/code/model/recipebuilder.py 2010-04-12 06:43:20 +0000
517@@ -96,13 +96,14 @@
518 # results so we know we are referring to the right database object in
519 # subsequent runs.
520 buildid = "%s-%s" % (self.build.id, build_queue_id)
521+ cookie = self.buildfarmjob.generateSlaveBuildCookie()
522 chroot_sha1 = chroot.content.sha1
523 logger.debug(
524 "Initiating build %s on %s" % (buildid, self._builder.url))
525
526 args = self._extraBuildArgs(distroarchseries)
527 status, info = self._builder.slave.build(
528- buildid, "sourcepackagerecipe", chroot_sha1, {}, args)
529+ cookie, "sourcepackagerecipe", chroot_sha1, {}, args)
530 message = """%s (%s):
531 ***** RESULT *****
532 %s
533@@ -156,12 +157,3 @@
534 status['build_status'] in build_status_with_files):
535 status['filemap'] = raw_slave_status[3]
536 status['dependencies'] = raw_slave_status[4]
537-
538-
539- def getVerifiedBuild(self, raw_id):
540- """See `IBuildFarmJobBehavior`."""
541- # This type of job has a build that is of type BuildBase but not
542- # actually a Build.
543- return self._helpVerifyBuildIDComponent(
544- raw_id, SourcePackageRecipeBuild,
545- getUtility(ISourcePackageRecipeBuildSource).getById)
546
547=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
548--- lib/lp/code/model/sourcepackagerecipebuild.py 2010-04-06 20:17:04 +0000
549+++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-04-12 06:43:20 +0000
550@@ -216,3 +216,6 @@
551 store = IMasterStore(SourcePackageRecipeBuildJob)
552 store.add(specific_job)
553 return specific_job
554+
555+ def getName(self):
556+ return "%s-%s" % (self.id, self.build_id)
557
558=== modified file 'lib/lp/code/tests/test_recipebuilder.py'
559--- lib/lp/code/tests/test_recipebuilder.py 2010-04-06 22:33:44 +0000
560+++ lib/lp/code/tests/test_recipebuilder.py 2010-04-12 06:43:20 +0000
561@@ -8,22 +8,25 @@
562 import transaction
563 import unittest
564
565+from zope.security.proxy import removeSecurityProxy
566+
567 from canonical.testing import LaunchpadFunctionalLayer
568 from canonical.launchpad.scripts.logger import BufferLogger
569
570 from lp.buildmaster.interfaces.builder import CannotBuild
571+from lp.buildmaster.interfaces.buildfarmjob import BuildFarmJobType
572 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
573 IBuildFarmJobBehavior)
574 from lp.buildmaster.manager import RecordingSlave
575+from lp.buildmaster.model.buildqueue import BuildQueue
576 from lp.code.model.recipebuilder import RecipeBuildBehavior
577 from lp.code.model.sourcepackagerecipebuild import (
578 SourcePackageRecipeBuild)
579-from lp.soyuz.adapters.archivedependencies import get_sources_list_for_building
580+from lp.soyuz.adapters.archivedependencies import (
581+ get_sources_list_for_building)
582 from lp.soyuz.model.processor import ProcessorFamilySet
583 from lp.soyuz.tests.soyuzbuilddhelpers import (
584 MockBuilder, OkSlave)
585-from lp.soyuz.tests.test_binarypackagebuildbehavior import (
586- BaseTestVerifySlaveBuildID)
587 from lp.soyuz.tests.test_publishing import (
588 SoyuzTestPublisher,)
589 from lp.testing import TestCaseWithFactory
590@@ -63,6 +66,8 @@
591 spb = self.factory.makeSourcePackageRecipeBuild(
592 sourcepackage=sourcepackage, recipe=recipe, requester=requester)
593 job = spb.makeJob()
594+ job_id = removeSecurityProxy(job.job).id
595+ BuildQueue(job_type=BuildFarmJobType.RECIPEBRANCHBUILD, job=job_id)
596 job = IBuildFarmJobBehavior(job)
597 return job
598
599@@ -126,7 +131,8 @@
600 self.assertEquals(["ensurepresent", "build"],
601 [call[0] for call in slave.calls])
602 build_args = slave.calls[1][1]
603- self.assertEquals(build_args[0], "1-someid")
604+ self.assertEquals(
605+ build_args[0], job.buildfarmjob.generateSlaveBuildCookie())
606 self.assertEquals(build_args[1], "sourcepackagerecipe")
607 self.assertEquals(build_args[3], {})
608 distroarchseries = job.build.distroseries.architectures[0]
609@@ -151,26 +157,5 @@
610 job.build, SourcePackageRecipeBuild.getById(job.build.id))
611
612
613-class BaseTestCaseWithBuilds(TestCaseWithFactory):
614- def setUp(self):
615- super(BaseTestCaseWithBuilds, self).setUp()
616-
617- self.builds = []
618-
619- build = self.factory.makeSourcePackageRecipeBuild()
620- build.queueBuild()
621- self.builds.append(build)
622-
623- build = self.factory.makeSourcePackageRecipeBuild()
624- build.queueBuild()
625- self.builds.append(build)
626-
627-
628-class TestVerifySlaveBuildID(BaseTestVerifySlaveBuildID,
629- BaseTestCaseWithBuilds):
630- """Run the tests from BaseTestVerifySlaveBuildID against recipe builds."""
631- pass
632-
633-
634 def test_suite():
635 return unittest.TestLoader().loadTestsFromName(__name__)
636
637=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
638--- lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-04-09 00:00:26 +0000
639+++ lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-04-12 06:43:20 +0000
640@@ -38,8 +38,15 @@
641 Slave-scanner will deactivate a 'lost-building' builder that could not
642 be aborted appropriately.
643
644+ >>> from zope.security.proxy import removeSecurityProxy
645+ >>> from lp.buildmaster.interfaces.builder import CorruptBuildCookie
646+ >>> from lp.testing.fakemethod import FakeMethod
647 >>> lostbuilding_builder = MockBuilder(
648 ... 'Lost Building Broken Slave', LostBuildingBrokenSlave())
649+ >>> behavior = removeSecurityProxy(
650+ ... lostbuilding_builder.current_build_behavior)
651+ >>> behavior.verifySlaveBuildCookie = FakeMethod(
652+ ... failure=CorruptBuildCookie("Hopelessly lost!"))
653
654 >>> lostbuilding_builder.updateStatus(logger)
655 Aborting slave
656@@ -80,7 +87,6 @@
657 To make testing easier we provide a convenience function to put a BuildQueue
658 object into a preset fixed state:
659
660- >>> from zope.security.proxy import removeSecurityProxy
661 >>> default_start = datetime.datetime(2005, 1, 1, 8, 0, 0, tzinfo=UTC)
662 >>> def setupBuildQueue(build_queue, builder):
663 ... build_queue.builder = builder
664
665=== modified file 'lib/lp/soyuz/model/binarypackagebuildbehavior.py'
666--- lib/lp/soyuz/model/binarypackagebuildbehavior.py 2010-04-08 14:48:02 +0000
667+++ lib/lp/soyuz/model/binarypackagebuildbehavior.py 2010-04-12 06:43:20 +0000
668@@ -65,13 +65,14 @@
669 # results so we know we are referring to the right database object in
670 # subsequent runs.
671 buildid = "%s-%s" % (self.build.id, build_queue_id)
672+ cookie = self.buildfarmjob.generateSlaveBuildCookie()
673 chroot_sha1 = chroot.content.sha1
674 logger.debug(
675 "Initiating build %s on %s" % (buildid, self._builder.url))
676
677 args = self._extraBuildArgs(self.build)
678 status, info = self._builder.slave.build(
679- buildid, "binarypackage", chroot_sha1, filemap, args)
680+ cookie, "binarypackage", chroot_sha1, filemap, args)
681 message = """%s (%s):
682 ***** RESULT *****
683 %s
684@@ -122,10 +123,11 @@
685
686 # This should already have been checked earlier, but just check again
687 # here in case of programmer errors.
688- reason = check_upload_to_pocket(build.archive, build.distroseries, build.pocket)
689+ reason = check_upload_to_pocket(
690+ build.archive, build.distroseries, build.pocket)
691 assert reason is None, (
692 "%s (%s) can not be built for pocket %s: invalid pocket due "
693- "to the series status of %s." %
694+ "to the series status of %s." %
695 (build.title, build.id, build.pocket.name,
696 build.distroseries.name))
697
698
699=== modified file 'lib/lp/soyuz/tests/soyuzbuilddhelpers.py'
700--- lib/lp/soyuz/tests/soyuzbuilddhelpers.py 2010-04-03 03:49:52 +0000
701+++ lib/lp/soyuz/tests/soyuzbuilddhelpers.py 2010-04-12 06:43:20 +0000
702@@ -52,8 +52,9 @@
703 def slaveStatusSentence(self):
704 return self.slave.status()
705
706- def verifySlaveBuildID(self, slave_build_id):
707- return self.current_build_behavior.verifySlaveBuildID(slave_build_id)
708+ def verifySlaveBuildCookie(self, slave_build_id):
709+ return self.current_build_behavior.verifySlaveBuildCookie(
710+ slave_build_id)
711
712 def cleanSlave(self):
713 print 'Cleaning slave'
714
715=== removed file 'lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py'
716--- lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py 2010-01-21 05:03:16 +0000
717+++ lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py 1970-01-01 00:00:00 +0000
718@@ -1,66 +0,0 @@
719-# Copyright 2010 Canonical Ltd. This software is licensed under the
720-# GNU Affero General Public License version 3 (see the file LICENSE).
721-
722-"""Test BinaryPackageBuildBehavior functionality."""
723-
724-__metaclass__ = type
725-
726-import unittest
727-
728-from canonical.testing import LaunchpadZopelessLayer
729-from lp.buildmaster.interfaces.buildfarmjobbehavior import (
730- IBuildFarmJobBehavior)
731-from lp.buildmaster.interfaces.builder import CorruptBuildID
732-from lp.soyuz.tests.test_build import BaseTestCaseWithThreeBuilds
733-
734-
735-class BaseTestVerifySlaveBuildID:
736-
737- layer = LaunchpadZopelessLayer
738-
739- def setUp(self):
740- super(BaseTestVerifySlaveBuildID, self).setUp()
741- self.build = self.builds[0]
742- self.other_build = self.builds[1]
743- self.builder = self.factory.makeBuilder(name='builder')
744-
745- def test_consistent_build_id(self):
746- # verifySlaveBuildID returns None if the build and buildqueue
747- # ID pair reported by the slave are associated in the database.
748- buildfarmjob = self.build.buildqueue_record.specific_job
749- behavior = IBuildFarmJobBehavior(buildfarmjob)
750- behavior.verifySlaveBuildID(
751- '%d-%d' % (self.build.id, self.build.buildqueue_record.id))
752-
753- def test_mismatched_build_id(self):
754- # verifySlaveBuildID returns an error if the build and
755- # buildqueue exist, but are not associated in the database.
756- buildfarmjob = self.build.buildqueue_record.specific_job
757- behavior = IBuildFarmJobBehavior(buildfarmjob)
758- self.assertRaises(
759- CorruptBuildID, behavior.verifySlaveBuildID,
760- '%d-%d' % (self.other_build.id, self.build.buildqueue_record.id))
761-
762- def test_build_id_without_separator(self):
763- # verifySlaveBuildID returns an error if the build ID does not
764- # contain a build and build queue ID separated by a hyphen.
765- buildfarmjob = self.build.buildqueue_record.specific_job
766- behavior = IBuildFarmJobBehavior(buildfarmjob)
767- self.assertRaises(
768- CorruptBuildID, behavior.verifySlaveBuildID, 'foo')
769-
770- def test_build_id_with_missing_build(self):
771- # verifySlaveBuildID returns an error if either the build or
772- # build queue specified do not exist.
773- buildfarmjob = self.build.buildqueue_record.specific_job
774- behavior = IBuildFarmJobBehavior(buildfarmjob)
775- self.assertRaises(
776- CorruptBuildID, behavior.verifySlaveBuildID, '98-99')
777-
778-
779-class TestVerifySlaveBuildID(BaseTestVerifySlaveBuildID,
780- BaseTestCaseWithThreeBuilds):
781- pass
782-
783-def test_suite():
784- return unittest.TestLoader().loadTestsFromName(__name__)
785
786=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
787--- lib/lp/translations/model/translationtemplatesbuildbehavior.py 2010-04-08 15:09:15 +0000
788+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py 2010-04-12 06:43:20 +0000
789@@ -17,7 +17,6 @@
790
791 from canonical.launchpad.interfaces import ILaunchpadCelebrities
792
793-from lp.buildmaster.interfaces.builder import CorruptBuildID
794 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
795 IBuildFarmJobBehavior)
796 from lp.buildmaster.model.buildfarmjobbehavior import (
797@@ -42,28 +41,13 @@
798 chroot = self._getChroot()
799 chroot_sha1 = chroot.content.sha1
800 self._builder.slave.cacheFile(logger, chroot)
801- buildid = self.buildfarmjob.getName()
802+ cookie = self.buildfarmjob.generateSlaveBuildCookie()
803
804 args = self.buildfarmjob.metadata
805 filemap = {}
806
807 self._builder.slave.build(
808- buildid, self.build_type, chroot_sha1, filemap, args)
809-
810- def verifySlaveBuildID(self, slave_build_id):
811- """See `IBuildFarmJobBehavior`."""
812- try:
813- branch_name, queue_item_id = slave_build_id.rsplit('-', 1)
814- except ValueError:
815- raise CorruptBuildID(
816- "Malformed translation templates build id: '%s'" % (
817- slave_build_id))
818-
819- buildqueue = self.getVerifiedBuildQueue(queue_item_id)
820- if buildqueue.job != self.buildfarmjob.job:
821- raise CorruptBuildID(
822- "ID mismatch for translation templates build '%s'" % (
823- slave_build_id))
824+ cookie, self.build_type, chroot_sha1, filemap, args)
825
826 def _getChroot(self):
827 ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
828@@ -127,7 +111,8 @@
829 logger.error("Build produced no tarball.")
830 else:
831 logger.debug("Uploading translation templates tarball.")
832- self._uploadTarball(queue_item.specific_job.branch, tarball, logger)
833+ self._uploadTarball(
834+ queue_item.specific_job.branch, tarball, logger)
835 logger.debug("Upload complete.")
836
837 queue_item.builder.cleanSlave()
838
839=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
840--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-04-08 15:09:15 +0000
841+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2010-04-12 06:43:20 +0000
842@@ -20,7 +20,6 @@
843 from lp.buildmaster.interfaces.buildbase import BuildStatus
844 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
845 IBuildFarmJobBehavior)
846-from lp.buildmaster.interfaces.builder import CorruptBuildID
847 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
848 from lp.testing import TestCaseWithFactory
849 from lp.testing.fakemethod import FakeMethod
850@@ -198,38 +197,6 @@
851 self.assertEqual(1, builder.cleanSlave.call_count)
852 self.assertEqual(0, behavior._uploadTarball.call_count)
853
854- def test_verifySlaveBuildID_success(self):
855- # TranslationTemplatesBuildJob.getName generates slave build ids
856- # that TranslationTemplatesBuildBehavior.verifySlaveBuildID
857- # accepts.
858- behavior = self.makeBehavior()
859- buildfarmjob = behavior.buildfarmjob
860- job = buildfarmjob.job
861-
862- # The test is that this not raise CorruptBuildID (or anything
863- # else, for that matter).
864- behavior.verifySlaveBuildID(behavior.buildfarmjob.getName())
865-
866- def test_verifySlaveBuildID_handles_dashes(self):
867- # TranslationTemplatesBuildBehavior.verifySlaveBuildID can deal
868- # with dashes in branch names.
869- behavior = self.makeBehavior()
870- buildfarmjob = behavior.buildfarmjob
871- job = buildfarmjob.job
872- buildfarmjob.branch.name = 'x-y-z--'
873-
874- # The test is that this not raise CorruptBuildID (or anything
875- # else, for that matter).
876- behavior.verifySlaveBuildID(behavior.buildfarmjob.getName())
877-
878- def test_verifySlaveBuildID_malformed(self):
879- behavior = self.makeBehavior()
880- self.assertRaises(CorruptBuildID, behavior.verifySlaveBuildID, 'huh?')
881-
882- def test_verifySlaveBuildID_notfound(self):
883- behavior = self.makeBehavior()
884- self.assertRaises(CorruptBuildID, behavior.verifySlaveBuildID, '1-1')
885-
886
887 class TestTTBuildBehaviorTranslationsQueue(
888 TestCaseWithFactory, MakeBehaviorMixin):