Merge lp:~cjwatson/launchpad-buildd/add-trusted-keys into lp:launchpad-buildd

Proposed by Colin Watson
Status: Merged
Merged at revision: 217
Proposed branch: lp:~cjwatson/launchpad-buildd/add-trusted-keys
Merge into: lp:launchpad-buildd
Diff against target: 544 lines (+381/-5)
10 files modified
MANIFEST.in (+1/-0)
Makefile (+1/-0)
add-trusted-keys (+24/-0)
debian/changelog (+6/-0)
debian/launchpad-buildd.install (+1/-0)
lpbuildd/debian.py (+34/-0)
lpbuildd/slave.py (+15/-4)
lpbuildd/tests/fakeslave.py (+2/-1)
lpbuildd/tests/test_debian.py (+294/-0)
sbuildrc (+3/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad-buildd/add-trusted-keys
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+323228@code.launchpad.net

Commit message

Write out trusted keys sent by buildd-manager (LP: #1626739).

Description of the change

Historically, we've relied (at least for PPAs) on builders having a secure internal transport to the archives they use as build-dependencies, even though this isn't good policy in general. However, we now need to do better since snapcraft sometimes does its own repository fetching and doesn't provide a way to disable authentication.

The other side of this is in Launchpad and is rather more involved, but it's safe to deploy this side of it in the meantime.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'MANIFEST.in'
2--- MANIFEST.in 2015-11-04 14:10:28 +0000
3+++ MANIFEST.in 2017-04-26 12:31:52 +0000
4@@ -1,5 +1,6 @@
5 include LICENSE
6 include Makefile
7+include add-trusted-keys
8 include buildd-genconfig
9 include buildd-slave.tac
10 include buildlivefs
11
12=== modified file 'Makefile'
13--- Makefile 2015-07-31 11:54:07 +0000
14+++ Makefile 2017-04-26 12:31:52 +0000
15@@ -31,6 +31,7 @@
16 lpbuildd.tests.test_buildd_slave \
17 lpbuildd.tests.test_buildrecipe \
18 lpbuildd.tests.test_check_implicit_pointer_functions \
19+ lpbuildd.tests.test_debian \
20 lpbuildd.tests.test_harness \
21 lpbuildd.tests.test_livefs \
22 lpbuildd.tests.test_snap \
23
24=== added file 'add-trusted-keys'
25--- add-trusted-keys 1970-01-01 00:00:00 +0000
26+++ add-trusted-keys 2017-04-26 12:31:52 +0000
27@@ -0,0 +1,24 @@
28+#!/bin/sh
29+#
30+# Copyright 2017 Canonical Ltd. This software is licensed under the
31+# GNU Affero General Public License version 3 (see the file LICENSE).
32+
33+# Write out new trusted keys in the chroot.
34+
35+# Expects the build id as the first argument, and key material on stdin.
36+
37+set -e
38+
39+SUDO=/usr/bin/sudo
40+CHROOT=/usr/sbin/chroot
41+
42+BUILDID="$1"
43+ROOT="$HOME/build-$BUILDID/chroot-autobuild"
44+
45+cd $HOME
46+cd "$ROOT/etc/apt"
47+
48+echo "Adding trusted keys to build-$BUILDID"
49+
50+$SUDO $CHROOT $ROOT apt-key add -
51+$SUDO $CHROOT $ROOT apt-key list
52
53=== modified file 'debian/changelog'
54--- debian/changelog 2017-02-10 14:55:42 +0000
55+++ debian/changelog 2017-04-26 12:31:52 +0000
56@@ -1,3 +1,9 @@
57+launchpad-buildd (143) UNRELEASED; urgency=medium
58+
59+ * Write out trusted keys sent by buildd-manager (LP: #1626739).
60+
61+ -- Colin Watson <cjwatson@ubuntu.com> Wed, 19 Apr 2017 11:23:20 +0100
62+
63 launchpad-buildd (142) trusty; urgency=medium
64
65 * lpbuildd.binarypackage: Pass DEB_BUILD_OPTIONS=noautodbgsym if we have
66
67=== modified file 'debian/launchpad-buildd.install'
68--- debian/launchpad-buildd.install 2015-08-25 09:23:24 +0000
69+++ debian/launchpad-buildd.install 2017-04-26 12:31:52 +0000
70@@ -3,6 +3,7 @@
71 debian/upgrade-config usr/share/launchpad-buildd
72 sbuildrc usr/share/launchpad-buildd
73 template-buildd-slave.conf usr/share/launchpad-buildd
74+add-trusted-keys usr/share/launchpad-buildd/slavebin
75 buildlivefs usr/share/launchpad-buildd/slavebin
76 buildrecipe usr/share/launchpad-buildd/slavebin
77 buildsnap usr/share/launchpad-buildd/slavebin
78
79=== modified file 'lpbuildd/debian.py'
80--- lpbuildd/debian.py 2016-02-18 14:44:09 +0000
81+++ lpbuildd/debian.py 2017-04-26 12:31:52 +0000
82@@ -8,6 +8,7 @@
83
84 __metaclass__ = type
85
86+import base64
87 import os
88 import re
89 import signal
90@@ -25,6 +26,7 @@
91 UNPACK = "UNPACK"
92 MOUNT = "MOUNT"
93 SOURCES = "SOURCES"
94+ KEYS = "KEYS"
95 UPDATE = "UPDATE"
96 UMOUNT = "UMOUNT"
97 CLEANUP = "CLEANUP"
98@@ -38,6 +40,7 @@
99 self._updatepath = os.path.join(self._slavebin, "update-debian-chroot")
100 self._sourcespath = os.path.join(
101 self._slavebin, "override-sources-list")
102+ self._keyspath = os.path.join(self._slavebin, "add-trusted-keys")
103 self._cachepath = slave._config.get("slave", "filecache")
104 self._state = DebianBuildState.INIT
105 slave.emptyLog()
106@@ -52,6 +55,7 @@
107
108 self.arch_tag = extra_args.get('arch_tag', self._slave.getArch())
109 self.sources_list = extra_args.get('archives')
110+ self.trusted_keys = extra_args.get('trusted_keys')
111
112 BuildManager.initiate(self, files, chroot, extra_args)
113
114@@ -64,6 +68,14 @@
115 args.extend(self.sources_list)
116 self.runSubProcess(self._sourcespath, args)
117
118+ def doTrustedKeys(self):
119+ """Add trusted keys."""
120+ trusted_keys = b"".join(
121+ base64.b64decode(key) for key in self.trusted_keys)
122+ self.runSubProcess(
123+ self._keyspath, ["add-trusted-keys", self._buildid],
124+ stdin=trusted_keys)
125+
126 def doUpdateChroot(self):
127 """Perform the chroot upgrade."""
128 self.runSubProcess(
129@@ -177,6 +189,9 @@
130 if self.sources_list is not None:
131 self._state = DebianBuildState.SOURCES
132 self.doSourcesList()
133+ elif self.trusted_keys:
134+ self._state = DebianBuildState.KEYS
135+ self.doTrustedKeys()
136 else:
137 self._state = DebianBuildState.UPDATE
138 self.doUpdateChroot()
139@@ -229,6 +244,9 @@
140 self._slave.chrootFail()
141 self.alreadyfailed = True
142 self.doReapProcesses(self._state)
143+ elif self.trusted_keys:
144+ self._state = DebianBuildState.KEYS
145+ self.doTrustedKeys()
146 else:
147 self._state = DebianBuildState.UPDATE
148 self.doUpdateChroot()
149@@ -238,6 +256,22 @@
150 self._state = DebianBuildState.UMOUNT
151 self.doUnmounting()
152
153+ def iterate_KEYS(self, success):
154+ """Just finished adding trusted keys."""
155+ if success != 0:
156+ if not self.alreadyfailed:
157+ self._slave.chrootFail()
158+ self.alreadyfailed = True
159+ self.doReapProcesses(self._state)
160+ else:
161+ self._state = DebianBuildState.UPDATE
162+ self.doUpdateChroot()
163+
164+ def iterateReap_KEYS(self, success):
165+ """Just finished reaping after failure to add trusted keys."""
166+ self._state = DebianBuildState.UMOUNT
167+ self.doUnmounting()
168+
169 def iterate_UPDATE(self, success):
170 """Just finished updating the chroot."""
171 if success != 0:
172
173=== modified file 'lpbuildd/slave.py'
174--- lpbuildd/slave.py 2016-12-09 18:04:00 +0000
175+++ lpbuildd/slave.py 2017-04-26 12:31:52 +0000
176@@ -52,12 +52,19 @@
177 class RunCapture(protocol.ProcessProtocol):
178 """Run a command and capture its output to a slave's log"""
179
180- def __init__(self, slave, callback):
181+ def __init__(self, slave, callback, stdin=None):
182 self.slave = slave
183 self.notify = callback
184+ self.stdin = stdin
185 self.builderFailCall = None
186 self.ignore = False
187
188+ def connectionMade(self):
189+ """Write any stdin data."""
190+ if self.stdin is not None:
191+ self.transport.write(self.stdin)
192+ self.transport.closeStdin()
193+
194 def outReceived(self, data):
195 """Pass on stdout data to the log."""
196 self.slave.log(data)
197@@ -121,13 +128,17 @@
198 def needs_sanitized_logs(self):
199 return self.is_archive_private
200
201- def runSubProcess(self, command, args, iterate=None, env=None):
202+ def runSubProcess(self, command, args, iterate=None, stdin=None, env=None):
203 """Run a sub process capturing the results in the log."""
204 if iterate is None:
205 iterate = self.iterate
206- self._subprocess = RunCapture(self._slave, iterate)
207+ self._subprocess = RunCapture(self._slave, iterate, stdin=stdin)
208 self._slave.log("RUN: %s %r\n" % (command, args))
209- childfds = {0: devnull.fileno(), 1: "r", 2: "r"}
210+ childfds = {
211+ 0: devnull.fileno() if stdin is None else "w",
212+ 1: "r",
213+ 2: "r",
214+ }
215 self._reactor.spawnProcess(
216 self._subprocess, command, args, env=env,
217 path=self.home, childFDs=childfds)
218
219=== modified file 'lpbuildd/tests/fakeslave.py'
220--- lpbuildd/tests/fakeslave.py 2013-09-27 10:34:13 +0000
221+++ lpbuildd/tests/fakeslave.py 2017-04-26 12:31:52 +0000
222@@ -77,7 +77,8 @@
223 self.waitingfiles = {}
224 for fake_method in (
225 "storeFile", "addWaitingFile", "emptyLog", "log",
226- "chrootFail", "buildFail", "builderFail", "depFail",
227+ "chrootFail", "buildFail", "builderFail", "depFail", "buildOK",
228+ "buildComplete",
229 ):
230 setattr(self, fake_method, FakeMethod())
231
232
233=== added file 'lpbuildd/tests/test_debian.py'
234--- lpbuildd/tests/test_debian.py 1970-01-01 00:00:00 +0000
235+++ lpbuildd/tests/test_debian.py 2017-04-26 12:31:52 +0000
236@@ -0,0 +1,294 @@
237+# Copyright 2017 Canonical Ltd. This software is licensed under the
238+# GNU Affero General Public License version 3 (see the file LICENSE).
239+
240+__metaclass__ = type
241+
242+import base64
243+import os.path
244+import shutil
245+import tempfile
246+
247+from testtools import TestCase
248+from twisted.internet.task import Clock
249+
250+from lpbuildd.debian import (
251+ DebianBuildManager,
252+ DebianBuildState,
253+ )
254+from lpbuildd.tests.fakeslave import FakeSlave
255+
256+
257+class MockBuildState(DebianBuildState):
258+
259+ MAIN = "MAIN"
260+
261+
262+class MockBuildManager(DebianBuildManager):
263+
264+ initial_build_state = MockBuildState.MAIN
265+
266+ def __init__(self, *args, **kwargs):
267+ super(MockBuildManager, self).__init__(*args, **kwargs)
268+ self.commands = []
269+ self.iterators = []
270+ self.arch_indep = False
271+
272+ def runSubProcess(self, path, command, iterate=None, stdin=None):
273+ self.commands.append(([path] + command, stdin))
274+ if iterate is None:
275+ iterate = self.iterate
276+ self.iterators.append(iterate)
277+ return 0
278+
279+ def doRunBuild(self):
280+ self.runSubProcess('/bin/true', ['true'])
281+
282+ def iterate_MAIN(self, success):
283+ if success != 0:
284+ if not self.alreadyfailed:
285+ self._slave.buildFail()
286+ self.alreadyfailed = True
287+ self.doReapProcesses(self._state)
288+
289+ def iterateReap_MAIN(self, success):
290+ self._state = DebianBuildState.UMOUNT
291+ self.doUnmounting()
292+
293+
294+class TestDebianBuildManagerIteration(TestCase):
295+ """Run a generic DebianBuildManager through its iteration steps."""
296+
297+ def setUp(self):
298+ super(TestDebianBuildManagerIteration, self).setUp()
299+ self.working_dir = tempfile.mkdtemp()
300+ self.addCleanup(lambda: shutil.rmtree(self.working_dir))
301+ slave_dir = os.path.join(self.working_dir, 'slave')
302+ home_dir = os.path.join(self.working_dir, 'home')
303+ for dir in (slave_dir, home_dir):
304+ os.mkdir(dir)
305+ self.slave = FakeSlave(slave_dir)
306+ self.buildid = '123'
307+ self.clock = Clock()
308+ self.buildmanager = MockBuildManager(
309+ self.slave, self.buildid, reactor=self.clock)
310+ self.buildmanager.home = home_dir
311+ self.buildmanager._cachepath = self.slave._cachepath
312+ self.chrootdir = os.path.join(
313+ home_dir, 'build-%s' % self.buildid, 'chroot-autobuild')
314+
315+ def getState(self):
316+ """Retrieve build manager's state."""
317+ return self.buildmanager._state
318+
319+ def startBuild(self, extra_args):
320+ # The build manager's iterate() kicks off the consecutive states
321+ # after INIT.
322+ self.buildmanager.initiate({}, 'chroot.tar.gz', extra_args)
323+ self.assertEqual(DebianBuildState.INIT, self.getState())
324+ self.assertEqual(
325+ (['sharepath/slavebin/slave-prep', 'slave-prep'], None),
326+ self.buildmanager.commands[-1])
327+ self.assertEqual(
328+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
329+
330+ def test_iterate(self):
331+ # The build manager iterates a normal build from start to finish.
332+ extra_args = {
333+ 'arch_tag': 'amd64',
334+ 'archives': [
335+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main',
336+ ],
337+ }
338+ self.startBuild(extra_args)
339+
340+ self.buildmanager.iterate(0)
341+ self.assertEqual(DebianBuildState.UNPACK, self.getState())
342+ self.assertEqual(
343+ (['sharepath/slavebin/unpack-chroot', 'unpack-chroot',
344+ self.buildid,
345+ os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
346+ None),
347+ self.buildmanager.commands[-1])
348+ self.assertEqual(
349+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
350+
351+ self.buildmanager.iterate(0)
352+ self.assertEqual(DebianBuildState.MOUNT, self.getState())
353+ self.assertEqual(
354+ (['sharepath/slavebin/mount-chroot', 'mount-chroot', self.buildid],
355+ None),
356+ self.buildmanager.commands[-1])
357+ self.assertEqual(
358+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
359+
360+ self.buildmanager.iterate(0)
361+ self.assertEqual(DebianBuildState.SOURCES, self.getState())
362+ self.assertEqual(
363+ (['sharepath/slavebin/override-sources-list',
364+ 'override-sources-list', self.buildid,
365+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main'],
366+ None),
367+ self.buildmanager.commands[-1])
368+ self.assertEqual(
369+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
370+
371+ self.buildmanager.iterate(0)
372+ self.assertEqual(DebianBuildState.UPDATE, self.getState())
373+ self.assertEqual(
374+ (['sharepath/slavebin/update-debian-chroot',
375+ 'update-debian-chroot', self.buildid, 'amd64'],
376+ None),
377+ self.buildmanager.commands[-1])
378+ self.assertEqual(
379+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
380+
381+ self.buildmanager.iterate(0)
382+ self.assertEqual(MockBuildState.MAIN, self.getState())
383+ self.assertEqual(
384+ (['/bin/true', 'true'], None), self.buildmanager.commands[-1])
385+ self.assertEqual(
386+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
387+
388+ self.buildmanager.iterate(0)
389+ self.assertEqual(MockBuildState.MAIN, self.getState())
390+ self.assertEqual(
391+ (['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
392+ self.buildid],
393+ None),
394+ self.buildmanager.commands[-1])
395+ self.assertNotEqual(
396+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
397+
398+ self.buildmanager.iterateReap(self.getState(), 0)
399+ self.assertEqual(DebianBuildState.UMOUNT, self.getState())
400+ self.assertEqual(
401+ (['sharepath/slavebin/umount-chroot', 'umount-chroot',
402+ self.buildid],
403+ None),
404+ self.buildmanager.commands[-1])
405+ self.assertEqual(
406+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
407+
408+ self.buildmanager.iterate(0)
409+ self.assertEqual(DebianBuildState.CLEANUP, self.getState())
410+ self.assertEqual(
411+ (['sharepath/slavebin/remove-build', 'remove-build', self.buildid],
412+ None),
413+ self.buildmanager.commands[-1])
414+ self.assertEqual(
415+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
416+
417+ self.buildmanager.iterate(0)
418+ self.assertFalse(self.slave.wasCalled('builderFail'))
419+ self.assertFalse(self.slave.wasCalled('chrootFail'))
420+ self.assertFalse(self.slave.wasCalled('buildFail'))
421+ self.assertFalse(self.slave.wasCalled('depFail'))
422+ self.assertTrue(self.slave.wasCalled('buildOK'))
423+ self.assertTrue(self.slave.wasCalled('buildComplete'))
424+
425+ def test_iterate_trusted_keys(self):
426+ # The build manager iterates a build with trusted keys from start to
427+ # finish.
428+ extra_args = {
429+ 'arch_tag': 'amd64',
430+ 'archives': [
431+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main',
432+ ],
433+ 'trusted_keys': [base64.b64encode(b'key material')],
434+ }
435+ self.startBuild(extra_args)
436+
437+ self.buildmanager.iterate(0)
438+ self.assertEqual(DebianBuildState.UNPACK, self.getState())
439+ self.assertEqual(
440+ (['sharepath/slavebin/unpack-chroot', 'unpack-chroot',
441+ self.buildid,
442+ os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
443+ None),
444+ self.buildmanager.commands[-1])
445+ self.assertEqual(
446+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
447+
448+ self.buildmanager.iterate(0)
449+ self.assertEqual(DebianBuildState.MOUNT, self.getState())
450+ self.assertEqual(
451+ (['sharepath/slavebin/mount-chroot', 'mount-chroot', self.buildid],
452+ None),
453+ self.buildmanager.commands[-1])
454+ self.assertEqual(
455+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
456+
457+ self.buildmanager.iterate(0)
458+ self.assertEqual(DebianBuildState.SOURCES, self.getState())
459+ self.assertEqual(
460+ (['sharepath/slavebin/override-sources-list',
461+ 'override-sources-list', self.buildid,
462+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main'],
463+ None),
464+ self.buildmanager.commands[-1])
465+ self.assertEqual(
466+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
467+
468+ self.buildmanager.iterate(0)
469+ self.assertEqual(DebianBuildState.KEYS, self.getState())
470+ self.assertEqual(
471+ (['sharepath/slavebin/add-trusted-keys', 'add-trusted-keys',
472+ self.buildid],
473+ b'key material'),
474+ self.buildmanager.commands[-1])
475+ self.assertEqual(
476+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
477+
478+ self.buildmanager.iterate(0)
479+ self.assertEqual(DebianBuildState.UPDATE, self.getState())
480+ self.assertEqual(
481+ (['sharepath/slavebin/update-debian-chroot',
482+ 'update-debian-chroot', self.buildid, 'amd64'],
483+ None),
484+ self.buildmanager.commands[-1])
485+ self.assertEqual(
486+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
487+
488+ self.buildmanager.iterate(0)
489+ self.assertEqual(MockBuildState.MAIN, self.getState())
490+ self.assertEqual(
491+ (['/bin/true', 'true'], None), self.buildmanager.commands[-1])
492+ self.assertEqual(
493+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
494+
495+ self.buildmanager.iterate(0)
496+ self.assertEqual(MockBuildState.MAIN, self.getState())
497+ self.assertEqual(
498+ (['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
499+ self.buildid],
500+ None),
501+ self.buildmanager.commands[-1])
502+ self.assertNotEqual(
503+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
504+
505+ self.buildmanager.iterateReap(self.getState(), 0)
506+ self.assertEqual(DebianBuildState.UMOUNT, self.getState())
507+ self.assertEqual(
508+ (['sharepath/slavebin/umount-chroot', 'umount-chroot',
509+ self.buildid],
510+ None),
511+ self.buildmanager.commands[-1])
512+ self.assertEqual(
513+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
514+
515+ self.buildmanager.iterate(0)
516+ self.assertEqual(DebianBuildState.CLEANUP, self.getState())
517+ self.assertEqual(
518+ (['sharepath/slavebin/remove-build', 'remove-build', self.buildid],
519+ None),
520+ self.buildmanager.commands[-1])
521+ self.assertEqual(
522+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
523+
524+ self.buildmanager.iterate(0)
525+ self.assertFalse(self.slave.wasCalled('builderFail'))
526+ self.assertFalse(self.slave.wasCalled('chrootFail'))
527+ self.assertFalse(self.slave.wasCalled('buildFail'))
528+ self.assertFalse(self.slave.wasCalled('depFail'))
529+ self.assertTrue(self.slave.wasCalled('buildOK'))
530+ self.assertTrue(self.slave.wasCalled('buildComplete'))
531
532=== modified file 'sbuildrc'
533--- sbuildrc 2015-07-05 12:43:54 +0000
534+++ sbuildrc 2017-04-26 12:31:52 +0000
535@@ -9,6 +9,9 @@
536 # launchpad-buildd does this before sbuild.
537 $apt_update = 0;
538 $apt_distupgrade = 0;
539+# XXX cjwatson 2017-04-26: We should drop this (or at least make it
540+# conditional so that it only applies in development setups) once the
541+# trusted keys logic is robust.
542 $apt_allow_unauthenticated = 1;
543
544 $resolve_alternatives = 1;

Subscribers

People subscribed via source and target branches

to all changes: