launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21523
[Merge] lp:~cjwatson/launchpad-buildd/add-trusted-keys into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/add-trusted-keys into lp:launchpad-buildd.
Commit message:
Write out trusted keys sent by buildd-manager (LP: #1626739).
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1626739 in launchpad-buildd: "Snapcraft build failing in Yakkety for unauthenticated stage-packages"
https://bugs.launchpad.net/launchpad-buildd/+bug/1626739
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/add-trusted-keys/+merge/323228
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/add-trusted-keys into lp:launchpad-buildd.
=== modified file 'MANIFEST.in'
--- MANIFEST.in 2015-11-04 14:10:28 +0000
+++ MANIFEST.in 2017-04-26 12:28:05 +0000
@@ -1,5 +1,6 @@
include LICENSE
include Makefile
+include add-trusted-keys
include buildd-genconfig
include buildd-slave.tac
include buildlivefs
=== modified file 'Makefile'
--- Makefile 2015-07-31 11:54:07 +0000
+++ Makefile 2017-04-26 12:28:05 +0000
@@ -31,6 +31,7 @@
lpbuildd.tests.test_buildd_slave \
lpbuildd.tests.test_buildrecipe \
lpbuildd.tests.test_check_implicit_pointer_functions \
+ lpbuildd.tests.test_debian \
lpbuildd.tests.test_harness \
lpbuildd.tests.test_livefs \
lpbuildd.tests.test_snap \
=== added file 'add-trusted-keys'
--- add-trusted-keys 1970-01-01 00:00:00 +0000
+++ add-trusted-keys 2017-04-26 12:28:05 +0000
@@ -0,0 +1,24 @@
+#!/bin/sh
+#
+# Copyright 2017 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+# Write out new trusted keys in the chroot.
+
+# Expects the build id as the first argument, and key material on stdin.
+
+set -e
+
+SUDO=/usr/bin/sudo
+CHROOT=/usr/sbin/chroot
+
+BUILDID="$1"
+ROOT="$HOME/build-$BUILDID/chroot-autobuild"
+
+cd $HOME
+cd "$ROOT/etc/apt"
+
+echo "Adding trusted keys to build-$BUILDID"
+
+$SUDO $CHROOT $ROOT apt-key add -
+$SUDO $CHROOT $ROOT apt-key list
=== modified file 'debian/changelog'
--- debian/changelog 2017-02-10 14:55:42 +0000
+++ debian/changelog 2017-04-26 12:28:05 +0000
@@ -1,3 +1,9 @@
+launchpad-buildd (143) UNRELEASED; urgency=medium
+
+ * Write out trusted keys sent by buildd-manager (LP: #1626739).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx> Wed, 19 Apr 2017 11:23:20 +0100
+
launchpad-buildd (142) trusty; urgency=medium
* lpbuildd.binarypackage: Pass DEB_BUILD_OPTIONS=noautodbgsym if we have
=== modified file 'debian/launchpad-buildd.install'
--- debian/launchpad-buildd.install 2015-08-25 09:23:24 +0000
+++ debian/launchpad-buildd.install 2017-04-26 12:28:05 +0000
@@ -3,6 +3,7 @@
debian/upgrade-config usr/share/launchpad-buildd
sbuildrc usr/share/launchpad-buildd
template-buildd-slave.conf usr/share/launchpad-buildd
+add-trusted-keys usr/share/launchpad-buildd/slavebin
buildlivefs usr/share/launchpad-buildd/slavebin
buildrecipe usr/share/launchpad-buildd/slavebin
buildsnap usr/share/launchpad-buildd/slavebin
=== modified file 'lpbuildd/debian.py'
--- lpbuildd/debian.py 2016-02-18 14:44:09 +0000
+++ lpbuildd/debian.py 2017-04-26 12:28:05 +0000
@@ -8,6 +8,7 @@
__metaclass__ = type
+import base64
import os
import re
import signal
@@ -25,6 +26,7 @@
UNPACK = "UNPACK"
MOUNT = "MOUNT"
SOURCES = "SOURCES"
+ KEYS = "KEYS"
UPDATE = "UPDATE"
UMOUNT = "UMOUNT"
CLEANUP = "CLEANUP"
@@ -38,6 +40,7 @@
self._updatepath = os.path.join(self._slavebin, "update-debian-chroot")
self._sourcespath = os.path.join(
self._slavebin, "override-sources-list")
+ self._keyspath = os.path.join(self._slavebin, "add-trusted-keys")
self._cachepath = slave._config.get("slave", "filecache")
self._state = DebianBuildState.INIT
slave.emptyLog()
@@ -52,6 +55,7 @@
self.arch_tag = extra_args.get('arch_tag', self._slave.getArch())
self.sources_list = extra_args.get('archives')
+ self.trusted_keys = extra_args.get('trusted_keys')
BuildManager.initiate(self, files, chroot, extra_args)
@@ -64,6 +68,14 @@
args.extend(self.sources_list)
self.runSubProcess(self._sourcespath, args)
+ def doTrustedKeys(self):
+ """Add trusted keys."""
+ trusted_keys = b"".join(
+ base64.b64decode(key) for key in self.trusted_keys)
+ self.runSubProcess(
+ self._keyspath, ["add-trusted-keys", self._buildid],
+ stdin=trusted_keys)
+
def doUpdateChroot(self):
"""Perform the chroot upgrade."""
self.runSubProcess(
@@ -177,6 +189,9 @@
if self.sources_list is not None:
self._state = DebianBuildState.SOURCES
self.doSourcesList()
+ elif self.trusted_keys:
+ self._state = DebianBuildState.KEYS
+ self.doTrustedKeys()
else:
self._state = DebianBuildState.UPDATE
self.doUpdateChroot()
@@ -229,6 +244,9 @@
self._slave.chrootFail()
self.alreadyfailed = True
self.doReapProcesses(self._state)
+ elif self.trusted_keys:
+ self._state = DebianBuildState.KEYS
+ self.doTrustedKeys()
else:
self._state = DebianBuildState.UPDATE
self.doUpdateChroot()
@@ -238,6 +256,22 @@
self._state = DebianBuildState.UMOUNT
self.doUnmounting()
+ def iterate_KEYS(self, success):
+ """Just finished adding trusted keys."""
+ if success != 0:
+ if not self.alreadyfailed:
+ self._slave.chrootFail()
+ self.alreadyfailed = True
+ self.doReapProcesses(self._state)
+ else:
+ self._state = DebianBuildState.UPDATE
+ self.doUpdateChroot()
+
+ def iterateReap_KEYS(self, success):
+ """Just finished reaping after failure to add trusted keys."""
+ self._state = DebianBuildState.UMOUNT
+ self.doUnmounting()
+
def iterate_UPDATE(self, success):
"""Just finished updating the chroot."""
if success != 0:
=== modified file 'lpbuildd/slave.py'
--- lpbuildd/slave.py 2016-12-09 18:04:00 +0000
+++ lpbuildd/slave.py 2017-04-26 12:28:05 +0000
@@ -52,12 +52,19 @@
class RunCapture(protocol.ProcessProtocol):
"""Run a command and capture its output to a slave's log"""
- def __init__(self, slave, callback):
+ def __init__(self, slave, callback, stdin=None):
self.slave = slave
self.notify = callback
+ self.stdin = stdin
self.builderFailCall = None
self.ignore = False
+ def connectionMade(self):
+ """Write any stdin data."""
+ if self.stdin is not None:
+ self.transport.write(self.stdin)
+ self.transport.closeStdin()
+
def outReceived(self, data):
"""Pass on stdout data to the log."""
self.slave.log(data)
@@ -121,13 +128,17 @@
def needs_sanitized_logs(self):
return self.is_archive_private
- def runSubProcess(self, command, args, iterate=None, env=None):
+ def runSubProcess(self, command, args, iterate=None, stdin=None, env=None):
"""Run a sub process capturing the results in the log."""
if iterate is None:
iterate = self.iterate
- self._subprocess = RunCapture(self._slave, iterate)
+ self._subprocess = RunCapture(self._slave, iterate, stdin=stdin)
self._slave.log("RUN: %s %r\n" % (command, args))
- childfds = {0: devnull.fileno(), 1: "r", 2: "r"}
+ childfds = {
+ 0: devnull.fileno() if stdin is None else "w",
+ 1: "r",
+ 2: "r",
+ }
self._reactor.spawnProcess(
self._subprocess, command, args, env=env,
path=self.home, childFDs=childfds)
=== modified file 'lpbuildd/tests/fakeslave.py'
--- lpbuildd/tests/fakeslave.py 2013-09-27 10:34:13 +0000
+++ lpbuildd/tests/fakeslave.py 2017-04-26 12:28:05 +0000
@@ -77,7 +77,8 @@
self.waitingfiles = {}
for fake_method in (
"storeFile", "addWaitingFile", "emptyLog", "log",
- "chrootFail", "buildFail", "builderFail", "depFail",
+ "chrootFail", "buildFail", "builderFail", "depFail", "buildOK",
+ "buildComplete",
):
setattr(self, fake_method, FakeMethod())
=== added file 'lpbuildd/tests/test_debian.py'
--- lpbuildd/tests/test_debian.py 1970-01-01 00:00:00 +0000
+++ lpbuildd/tests/test_debian.py 2017-04-26 12:28:05 +0000
@@ -0,0 +1,294 @@
+# Copyright 2017 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import base64
+import os.path
+import shutil
+import tempfile
+
+from testtools import TestCase
+from twisted.internet.task import Clock
+
+from lpbuildd.debian import (
+ DebianBuildManager,
+ DebianBuildState,
+ )
+from lpbuildd.tests.fakeslave import FakeSlave
+
+
+class MockBuildState(DebianBuildState):
+
+ MAIN = "MAIN"
+
+
+class MockBuildManager(DebianBuildManager):
+
+ initial_build_state = MockBuildState.MAIN
+
+ def __init__(self, *args, **kwargs):
+ super(MockBuildManager, self).__init__(*args, **kwargs)
+ self.commands = []
+ self.iterators = []
+ self.arch_indep = False
+
+ def runSubProcess(self, path, command, iterate=None, stdin=None):
+ self.commands.append(([path] + command, stdin))
+ if iterate is None:
+ iterate = self.iterate
+ self.iterators.append(iterate)
+ return 0
+
+ def doRunBuild(self):
+ self.runSubProcess('/bin/true', ['true'])
+
+ def iterate_MAIN(self, success):
+ if success != 0:
+ if not self.alreadyfailed:
+ self._slave.buildFail()
+ self.alreadyfailed = True
+ self.doReapProcesses(self._state)
+
+ def iterateReap_MAIN(self, success):
+ self._state = DebianBuildState.UMOUNT
+ self.doUnmounting()
+
+
+class TestDebianBuildManagerIteration(TestCase):
+ """Run a generic DebianBuildManager through its iteration steps."""
+
+ def setUp(self):
+ super(TestDebianBuildManagerIteration, self).setUp()
+ self.working_dir = tempfile.mkdtemp()
+ self.addCleanup(lambda: shutil.rmtree(self.working_dir))
+ slave_dir = os.path.join(self.working_dir, 'slave')
+ home_dir = os.path.join(self.working_dir, 'home')
+ for dir in (slave_dir, home_dir):
+ os.mkdir(dir)
+ self.slave = FakeSlave(slave_dir)
+ self.buildid = '123'
+ self.clock = Clock()
+ self.buildmanager = MockBuildManager(
+ self.slave, self.buildid, reactor=self.clock)
+ self.buildmanager.home = home_dir
+ self.buildmanager._cachepath = self.slave._cachepath
+ self.chrootdir = os.path.join(
+ home_dir, 'build-%s' % self.buildid, 'chroot-autobuild')
+
+ def getState(self):
+ """Retrieve build manager's state."""
+ return self.buildmanager._state
+
+ def startBuild(self, extra_args):
+ # The build manager's iterate() kicks off the consecutive states
+ # after INIT.
+ self.buildmanager.initiate({}, 'chroot.tar.gz', extra_args)
+ self.assertEqual(DebianBuildState.INIT, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/slave-prep', 'slave-prep'], None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ def test_iterate(self):
+ # The build manager iterates a normal build from start to finish.
+ extra_args = {
+ 'arch_tag': 'amd64',
+ 'archives': [
+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main',
+ ],
+ }
+ self.startBuild(extra_args)
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.UNPACK, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/unpack-chroot', 'unpack-chroot',
+ self.buildid,
+ os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.MOUNT, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/mount-chroot', 'mount-chroot', self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.SOURCES, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/override-sources-list',
+ 'override-sources-list', self.buildid,
+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main'],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.UPDATE, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/update-debian-chroot',
+ 'update-debian-chroot', self.buildid, 'amd64'],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(MockBuildState.MAIN, self.getState())
+ self.assertEqual(
+ (['/bin/true', 'true'], None), self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(MockBuildState.MAIN, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertNotEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterateReap(self.getState(), 0)
+ self.assertEqual(DebianBuildState.UMOUNT, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/umount-chroot', 'umount-chroot',
+ self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.CLEANUP, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/remove-build', 'remove-build', self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertFalse(self.slave.wasCalled('builderFail'))
+ self.assertFalse(self.slave.wasCalled('chrootFail'))
+ self.assertFalse(self.slave.wasCalled('buildFail'))
+ self.assertFalse(self.slave.wasCalled('depFail'))
+ self.assertTrue(self.slave.wasCalled('buildOK'))
+ self.assertTrue(self.slave.wasCalled('buildComplete'))
+
+ def test_iterate_trusted_keys(self):
+ # The build manager iterates a build with trusted keys from start to
+ # finish.
+ extra_args = {
+ 'arch_tag': 'amd64',
+ 'archives': [
+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main',
+ ],
+ 'trusted_keys': [base64.b64encode(b'key material')],
+ }
+ self.startBuild(extra_args)
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.UNPACK, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/unpack-chroot', 'unpack-chroot',
+ self.buildid,
+ os.path.join(self.buildmanager._cachepath, 'chroot.tar.gz')],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.MOUNT, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/mount-chroot', 'mount-chroot', self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.SOURCES, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/override-sources-list',
+ 'override-sources-list', self.buildid,
+ 'deb http://ppa.launchpad.dev/owner/name/ubuntu xenial main'],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.KEYS, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/add-trusted-keys', 'add-trusted-keys',
+ self.buildid],
+ b'key material'),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.UPDATE, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/update-debian-chroot',
+ 'update-debian-chroot', self.buildid, 'amd64'],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(MockBuildState.MAIN, self.getState())
+ self.assertEqual(
+ (['/bin/true', 'true'], None), self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(MockBuildState.MAIN, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/scan-for-processes', 'scan-for-processes',
+ self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertNotEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterateReap(self.getState(), 0)
+ self.assertEqual(DebianBuildState.UMOUNT, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/umount-chroot', 'umount-chroot',
+ self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertEqual(DebianBuildState.CLEANUP, self.getState())
+ self.assertEqual(
+ (['sharepath/slavebin/remove-build', 'remove-build', self.buildid],
+ None),
+ self.buildmanager.commands[-1])
+ self.assertEqual(
+ self.buildmanager.iterate, self.buildmanager.iterators[-1])
+
+ self.buildmanager.iterate(0)
+ self.assertFalse(self.slave.wasCalled('builderFail'))
+ self.assertFalse(self.slave.wasCalled('chrootFail'))
+ self.assertFalse(self.slave.wasCalled('buildFail'))
+ self.assertFalse(self.slave.wasCalled('depFail'))
+ self.assertTrue(self.slave.wasCalled('buildOK'))
+ self.assertTrue(self.slave.wasCalled('buildComplete'))
=== modified file 'sbuildrc'
--- sbuildrc 2015-07-05 12:43:54 +0000
+++ sbuildrc 2017-04-26 12:28:05 +0000
@@ -9,6 +9,9 @@
# launchpad-buildd does this before sbuild.
$apt_update = 0;
$apt_distupgrade = 0;
+# XXX cjwatson 2017-04-26: We should drop this (or at least make it
+# conditional so that it only applies in development setups) once the
+# trusted keys logic is robust.
$apt_allow_unauthenticated = 1;
$resolve_alternatives = 1;
