Launchpad itself

Merge lp:~apw/launchpad/uefi-auto-key into lp:launchpad

  1. uefi-auto-key
  2. Merge into devel
Proposed by Andy Whitcroft
Status: Merged
Approved by: Colin Watson
Approved revision: no longer in the source branch.
Merged at revision: 18015
Proposed branch: lp:~apw/launchpad/uefi-auto-key
Merge into: lp:launchpad
Diff against target: 443 lines (+221/-50)
4 files modified
lib/lp/archivepublisher/config.py (+3/-0)
lib/lp/archivepublisher/tests/test_config.py (+5/-0)
lib/lp/archivepublisher/tests/test_uefi.py (+141/-27)
lib/lp/archivepublisher/uefi.py (+72/-23)
To merge this branch: bzr merge lp:~apw/launchpad/uefi-auto-key
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Andy Whitcroft (community) Abstain
Review via email: mp+293087@code.launchpad.net

Commit message

Move the existing raw-uefi custom upload to validating the uefi signing keys on first use. For PPAs generate the keys on first use if they are missing.

Description of the change

Update the uefi custom upload to optionally manage key generation. Also in preparation for signing generification move key validation/generation to first use.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Needs Fixing
Revision history for this message
Andy Whitcroft (apw) wrote :

I have pushed up a number of updates on top of the previous code. I believe this should reflect your review comments. In particular I have revamps the testing to split command generation checks from flow testing. Oh and the lint roller has been applied.

review: Needs Resubmitting
Revision history for this message
Andy Whitcroft (apw) :
review: Abstain
Revision history for this message
Colin Watson (cjwatson) wrote :

Definitely getting there, but still some things to fix, mostly in the tests.

review: Needs Fixing
Revision history for this message
Andy Whitcroft (apw) wrote :

Ok. I pushed up another attempt. I hope this one covers your remaining review comments. As expected the majority of the diff is in the tests. Looking at the diff against mainline there is actually very little delta for the operational code (also as expected).

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

Looks good now!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/config.py'
2--- lib/lp/archivepublisher/config.py 2016-02-05 16:51:12 +0000
3+++ lib/lp/archivepublisher/config.py 2016-05-03 11:13:13 +0000
4@@ -79,12 +79,15 @@
5
6 if archive.is_main:
7 pubconf.uefiroot = pubconf.archiveroot + '-uefi'
8+ pubconf.uefiautokey = False
9 elif archive.is_ppa:
10 pubconf.uefiroot = os.path.join(
11 ppa_config.signing_keys_root, "uefi",
12 archive.owner.name, archive.name)
13+ pubconf.uefiautokey = True
14 else:
15 pubconf.uefiroot = None
16+ pubconf.uefiautokey = False
17
18 pubconf.poolroot = os.path.join(pubconf.archiveroot, 'pool')
19 pubconf.distsroot = os.path.join(pubconf.archiveroot, 'dists')
20
21=== modified file 'lib/lp/archivepublisher/tests/test_config.py'
22--- lib/lp/archivepublisher/tests/test_config.py 2016-01-13 17:09:34 +0000
23+++ lib/lp/archivepublisher/tests/test_config.py 2016-05-03 11:13:13 +0000
24@@ -47,6 +47,7 @@
25 self.assertEqual(
26 self.root + "/ubuntutest-temp", primary_config.temproot)
27 self.assertEqual(archiveroot + "-uefi", primary_config.uefiroot)
28+ self.assertFalse(primary_config.uefiautokey)
29 self.assertIs(None, primary_config.metaroot)
30 self.assertEqual(archiveroot + "-staging", primary_config.stagingroot)
31
32@@ -70,6 +71,7 @@
33 self.assertEqual(
34 self.root + "/ubuntutest-temp", partner_config.temproot)
35 self.assertEqual(archiveroot + "-uefi", partner_config.uefiroot)
36+ self.assertFalse(partner_config.uefiautokey)
37 self.assertIs(None, partner_config.metaroot)
38 self.assertEqual(archiveroot + "-staging", partner_config.stagingroot)
39
40@@ -92,6 +94,7 @@
41 self.assertEqual(archiveroot + "-misc", copy_config.miscroot)
42 self.assertEqual(archiveroot + "-temp", copy_config.temproot)
43 self.assertIsNone(copy_config.uefiroot)
44+ self.assertFalse(copy_config.uefiautokey)
45 self.assertIs(None, copy_config.metaroot)
46 self.assertIs(None, copy_config.stagingroot)
47
48@@ -131,6 +134,7 @@
49 uefiroot = "/var/tmp/ppa-signing-keys.test/uefi/%s/%s" % (
50 self.ppa.owner.name, self.ppa.name)
51 self.assertEqual(uefiroot, self.ppa_config.uefiroot)
52+ self.assertTrue(self.ppa_config.uefiautokey)
53 self.assertIs(None, self.ppa_config.metaroot)
54 self.assertIs(None, self.ppa_config.stagingroot)
55
56@@ -165,6 +169,7 @@
57 uefiroot = "/var/tmp/ppa-signing-keys.test/uefi/%s/%s" % (
58 p3a.owner.name, p3a.name)
59 self.assertEqual(uefiroot, p3a_config.uefiroot)
60+ self.assertTrue(self.ppa_config.uefiautokey)
61 self.assertIs(None, p3a_config.metaroot)
62 self.assertIs(None, p3a_config.stagingroot)
63
64
65=== modified file 'lib/lp/archivepublisher/tests/test_uefi.py'
66--- lib/lp/archivepublisher/tests/test_uefi.py 2016-02-05 16:51:12 +0000
67+++ lib/lp/archivepublisher/tests/test_uefi.py 2016-05-03 11:13:13 +0000
68@@ -7,6 +7,8 @@
69
70 import os
71
72+from fixtures import MonkeyPatch
73+
74 from lp.archivepublisher.customupload import (
75 CustomUploadAlreadyExists,
76 CustomUploadBadUmask,
77@@ -18,11 +20,35 @@
78 from lp.testing.fakemethod import FakeMethod
79
80
81+class FakeMethodGenUefiKeys(FakeMethod):
82+ """Fake execution of generation of Uefi keys pairs."""
83+ def __init__(self, upload=None, *args, **kwargs):
84+ super(FakeMethodGenUefiKeys, self).__init__(*args, **kwargs)
85+ self.upload = upload
86+
87+ def __call__(self, *args, **kwargs):
88+ super(FakeMethodGenUefiKeys, self).__call__(*args, **kwargs)
89+
90+ write_file(self.upload.key, "")
91+ write_file(self.upload.cert, "")
92+
93+
94 class FakeConfig:
95- """A fake publisher configuration."""
96- def __init__(self, archiveroot, uefiroot):
97- self.archiveroot = archiveroot
98- self.uefiroot = uefiroot
99+ """A fake publisher configuration for the main archive."""
100+ def __init__(self, distroroot, uefiroot):
101+ self.distroroot = distroroot
102+ self.uefiroot = uefiroot
103+ self.archiveroot = os.path.join(self.distroroot, 'ubuntu')
104+ self.uefiautokey = False
105+
106+
107+class FakeConfigPPA:
108+ """A fake publisher configuration for a PPA."""
109+ def __init__(self, distroroot, uefiroot, owner, ppa):
110+ self.distroroot = distroroot
111+ self.uefiroot = uefiroot
112+ self.archiveroot = os.path.join(self.distroroot, owner, ppa, 'ubuntu')
113+ self.uefiautokey = True
114
115
116 class TestUefi(TestCase):
117@@ -37,11 +63,17 @@
118 old_umask = os.umask(0o022)
119 self.addCleanup(os.umask, old_umask)
120
121- def setUpKeyAndCert(self):
122+ def setUpPPA(self):
123+ self.pubconf = FakeConfigPPA(self.temp_dir, self.uefi_dir,
124+ 'ubuntu-archive', 'testing')
125+ self.testcase_cn = '/CN=PPA ubuntu-archive testing/'
126+
127+ def setUpKeyAndCert(self, create=True):
128 self.key = os.path.join(self.uefi_dir, "uefi.key")
129 self.cert = os.path.join(self.uefi_dir, "uefi.crt")
130- write_file(self.key, "")
131- write_file(self.cert, "")
132+ if create:
133+ write_file(self.key, "")
134+ write_file(self.cert, "")
135
136 def openArchive(self, loader_type, version, arch):
137 self.path = os.path.join(
138@@ -52,41 +84,48 @@
139 def process(self):
140 self.archive.close()
141 self.buffer.close()
142+ fake_call = FakeMethod()
143 upload = UefiUpload()
144- upload.sign = FakeMethod()
145- upload.process(self.pubconf, self.path, self.suite)
146+ upload.signUefi = FakeMethod()
147+ with self.useFixture(MonkeyPatch("subprocess.call", fake_call)):
148+ upload.process(self.pubconf, self.path, self.suite)
149+ # Under no circumstances is it safe to execute actual commands.
150+ self.assertEqual(0, fake_call.call_count)
151+
152 return upload
153
154 def getUefiPath(self, loader_type, arch):
155 return os.path.join(
156- self.temp_dir, "dists", self.suite, "main", "uefi",
157+ self.pubconf.archiveroot, "dists", self.suite, "main", "uefi",
158 "%s-%s" % (loader_type, arch))
159
160 def test_unconfigured(self):
161 # If there is no key/cert configuration, processing succeeds but
162- # nothing is signed.
163+ # nothing is signed. Signing is attempted.
164 self.pubconf = FakeConfig(self.temp_dir, None)
165 self.openArchive("test", "1.0", "amd64")
166 self.archive.add_file("1.0/empty.efi", "")
167 upload = self.process()
168- self.assertEqual(0, upload.sign.call_count)
169+ self.assertEqual(1, upload.signUefi.call_count)
170
171 def test_missing_key_and_cert(self):
172 # If the configured key/cert are missing, processing succeeds but
173- # nothing is signed.
174+ # nothing is signed. Signing is attempted.
175 self.openArchive("test", "1.0", "amd64")
176 self.archive.add_file("1.0/empty.efi", "")
177 upload = self.process()
178- self.assertEqual(0, upload.sign.call_count)
179+ self.assertEqual(1, upload.signUefi.call_count)
180
181 def test_no_efi_files(self):
182 # Tarballs containing no *.efi files are extracted without complaint.
183+ # Nothing is signed.
184 self.setUpKeyAndCert()
185 self.openArchive("empty", "1.0", "amd64")
186 self.archive.add_file("1.0/hello", "world")
187- self.process()
188+ upload = self.process()
189 self.assertTrue(os.path.exists(os.path.join(
190 self.getUefiPath("empty", "amd64"), "1.0", "hello")))
191+ self.assertEqual(0, upload.signUefi.call_count)
192
193 def test_already_exists(self):
194 # If the target directory already exists, processing fails.
195@@ -104,15 +143,60 @@
196 os.umask(0o002) # cleanup already handled by setUp
197 self.assertRaises(CustomUploadBadUmask, self.process)
198
199- def test_correct_signing_command(self):
200- # getSigningCommand returns the correct command.
201+ def test_correct_uefi_signing_command_executed(self):
202+ # Check that calling signUefi() will generate the expected command
203+ # when appropriate keys are present.
204 self.setUpKeyAndCert()
205- upload = UefiUpload()
206- upload.setTargetDirectory(
207- self.pubconf, "test_1.0_amd64.tar.gz", "distroseries")
208- expected_command = [
209- "sbsign", "--key", self.key, "--cert", self.cert, "t.efi"]
210- self.assertEqual(expected_command, upload.getSigningCommand("t.efi"))
211+ fake_call = FakeMethod()
212+ with self.useFixture(MonkeyPatch("subprocess.call", fake_call)):
213+ upload = UefiUpload()
214+ upload.generateUefiKeys = FakeMethod()
215+ upload.setTargetDirectory(
216+ self.pubconf, "test_1.0_amd64.tar.gz", "distroseries")
217+ upload.signUefi('t.efi')
218+ self.assertEqual(1, fake_call.call_count)
219+ # Assert command form.
220+ args = fake_call.calls[0][0][0]
221+ expected_cmd = [
222+ 'sbsign', '--key', self.key, '--cert', self.cert, 't.efi',
223+ ]
224+ self.assertEqual(expected_cmd, args)
225+ self.assertEqual(0, upload.generateUefiKeys.call_count)
226+
227+ def test_correct_uefi_signing_command_executed_no_keys(self):
228+ # Check that calling signUefi() will generate no commands when
229+ # no keys are present.
230+ self.setUpKeyAndCert(create=False)
231+ fake_call = FakeMethod()
232+ with self.useFixture(MonkeyPatch("subprocess.call", fake_call)):
233+ upload = UefiUpload()
234+ upload.generateUefiKeys = FakeMethod()
235+ upload.setTargetDirectory(
236+ self.pubconf, "test_1.0_amd64.tar.gz", "distroseries")
237+ upload.signUefi('t.efi')
238+ self.assertEqual(0, fake_call.call_count)
239+ self.assertEqual(0, upload.generateUefiKeys.call_count)
240+
241+ def test_correct_uefi_keygen_command_executed(self):
242+ # Check that calling generateUefiKeys() will generate the
243+ # expected command.
244+ self.setUpPPA()
245+ self.setUpKeyAndCert(create=False)
246+ fake_call = FakeMethod()
247+ with self.useFixture(MonkeyPatch("subprocess.call", fake_call)):
248+ upload = UefiUpload()
249+ upload.setTargetDirectory(
250+ self.pubconf, "test_1.0_amd64.tar.gz", "distroseries")
251+ upload.generateUefiKeys()
252+ self.assertEqual(1, fake_call.call_count)
253+ # Assert the actual command matches.
254+ args = fake_call.calls[0][0][0]
255+ expected_cmd = [
256+ 'openssl', 'req', '-new', '-x509', '-newkey', 'rsa:2048',
257+ '-subj', self.testcase_cn, '-keyout', self.key, '-out', self.cert,
258+ '-days', '3650', '-nodes', '-sha256',
259+ ]
260+ self.assertEqual(expected_cmd, args)
261
262 def test_signs_image(self):
263 # Each image in the tarball is signed.
264@@ -120,10 +204,7 @@
265 self.openArchive("test", "1.0", "amd64")
266 self.archive.add_file("1.0/empty.efi", "")
267 upload = self.process()
268- self.assertEqual(1, upload.sign.call_count)
269- self.assertEqual(1, len(upload.sign.calls[0][0]))
270- self.assertEqual(
271- "empty.efi", os.path.basename(upload.sign.calls[0][0][0]))
272+ self.assertEqual(1, upload.signUefi.call_count)
273
274 def test_installed(self):
275 # Files in the tarball are installed correctly.
276@@ -133,3 +214,36 @@
277 self.process()
278 self.assertTrue(os.path.exists(os.path.join(
279 self.getUefiPath("test", "amd64"), "1.0", "empty.efi")))
280+
281+ def test_create_uefi_keys_autokey_off(self):
282+ # Keys are not created.
283+ self.setUpKeyAndCert(create=False)
284+ self.assertFalse(os.path.exists(self.key))
285+ self.assertFalse(os.path.exists(self.cert))
286+ fake_call = FakeMethod()
287+ with self.useFixture(MonkeyPatch("subprocess.call", fake_call)):
288+ upload = UefiUpload()
289+ upload.generateUefiKeys = FakeMethodGenUefiKeys(upload=upload)
290+ upload.setTargetDirectory(
291+ self.pubconf, "test_1.0_amd64.tar.gz", "distroseries")
292+ upload.signUefi('t.efi')
293+ self.assertEqual(0, upload.generateUefiKeys.call_count)
294+ self.assertFalse(os.path.exists(self.key))
295+ self.assertFalse(os.path.exists(self.cert))
296+
297+ def test_create_uefi_keys_autokey_on(self):
298+ # Keys are created on demand.
299+ self.setUpPPA()
300+ self.setUpKeyAndCert(create=False)
301+ self.assertFalse(os.path.exists(self.key))
302+ self.assertFalse(os.path.exists(self.cert))
303+ fake_call = FakeMethod()
304+ with self.useFixture(MonkeyPatch("subprocess.call", fake_call)):
305+ upload = UefiUpload()
306+ upload.generateUefiKeys = FakeMethodGenUefiKeys(upload=upload)
307+ upload.setTargetDirectory(
308+ self.pubconf, "test_1.0_amd64.tar.gz", "distroseries")
309+ upload.signUefi('t.efi')
310+ self.assertEqual(1, upload.generateUefiKeys.call_count)
311+ self.assertTrue(os.path.exists(self.key))
312+ self.assertTrue(os.path.exists(self.cert))
313
314=== modified file 'lib/lp/archivepublisher/uefi.py'
315--- lib/lp/archivepublisher/uefi.py 2013-07-25 16:45:20 +0000
316+++ lib/lp/archivepublisher/uefi.py 2016-05-03 11:13:13 +0000
317@@ -68,24 +68,17 @@
318 self.logger.warning("No UEFI root configured for this archive")
319 self.key = None
320 self.cert = None
321+ self.autokey = False
322 else:
323 self.key = os.path.join(pubconf.uefiroot, "uefi.key")
324 self.cert = os.path.join(pubconf.uefiroot, "uefi.crt")
325- if not os.access(self.key, os.R_OK):
326- if self.logger is not None:
327- self.logger.warning(
328- "UEFI private key %s not readable" % self.key)
329- self.key = None
330- if not os.access(self.cert, os.R_OK):
331- if self.logger is not None:
332- self.logger.warning(
333- "UEFI certificate %s not readable" % self.cert)
334- self.cert = None
335+ self.autokey = pubconf.uefiautokey
336
337 self.setComponents(tarfile_path)
338 self.targetdir = os.path.join(
339 pubconf.archiveroot, "dists", distroseries, "main", "uefi",
340 "%s-%s" % (self.loader_type, self.arch))
341+ self.archiveroot = pubconf.archiveroot
342
343 @classmethod
344 def getSeriesKey(cls, tarfile_path):
345@@ -102,17 +95,74 @@
346 if filename.endswith(".efi"):
347 yield os.path.join(dirpath, filename)
348
349- def getSigningCommand(self, image):
350- """Return the command used to sign an image."""
351- return ["sbsign", "--key", self.key, "--cert", self.cert, image]
352-
353- def sign(self, image):
354- """Sign an image."""
355- if subprocess.call(self.getSigningCommand(image)) != 0:
356+ def generateUefiKeys(self):
357+ """Generate new UEFI Keys for this archive."""
358+ directory = os.path.dirname(self.key)
359+ if not os.path.exists(directory):
360+ os.makedirs(directory)
361+
362+ # XXX: pull out the PPA owner and name to seed key CN
363+ archive_name = os.path.dirname(self.archiveroot)
364+ owner_name = os.path.basename(os.path.dirname(archive_name))
365+ archive_name = os.path.basename(archive_name)
366+ common_name = '/CN=PPA ' + owner_name + ' ' + archive_name + '/'
367+
368+ old_mask = os.umask(0o077)
369+ try:
370+ new_key_cmd = [
371+ 'openssl', 'req', '-new', '-x509', '-newkey', 'rsa:2048',
372+ '-subj', common_name, '-keyout', self.key, '-out', self.cert,
373+ '-days', '3650', '-nodes', '-sha256',
374+ ]
375+ if subprocess.call(new_key_cmd) != 0:
376+ # Just log this rather than failing, since custom upload errors
377+ # tend to make the publisher rather upset.
378+ if self.logger is not None:
379+ self.logger.warning(
380+ "Failed to generate UEFI signing keys for %s" %
381+ common_name)
382+ finally:
383+ os.umask(old_mask)
384+
385+ if os.path.exists(self.cert):
386+ os.chmod(self.cert, 0o644)
387+
388+ def getUefiKeys(self):
389+ """Validate and return the uefi key and cert for encryption."""
390+
391+ if self.key and self.cert:
392+ # If neither of the key files exists then attempt to
393+ # generate them.
394+ if (self.autokey and not os.path.exists(self.key)
395+ and not os.path.exists(self.cert)):
396+ self.generateUefiKeys()
397+
398+ # If we have keys, but cannot read them they are dead to us.
399+ if not os.access(self.key, os.R_OK):
400+ if self.logger is not None:
401+ self.logger.warning(
402+ "UEFI private key %s not readable" % self.key)
403+ self.key = None
404+ if not os.access(self.cert, os.R_OK):
405+ if self.logger is not None:
406+ self.logger.warning(
407+ "UEFI certificate %s not readable" % self.cert)
408+ self.cert = None
409+
410+ return (self.key, self.cert)
411+
412+ def signUefi(self, image):
413+ """Attempt to sign an image."""
414+ (key, cert) = self.getUefiKeys()
415+ if not key or not cert:
416+ return
417+ cmdl = ["sbsign", "--key", key, "--cert", cert, image]
418+ if subprocess.call(cmdl) != 0:
419 # Just log this rather than failing, since custom upload errors
420 # tend to make the publisher rather upset.
421 if self.logger is not None:
422- self.logger.warning("Failed to sign %s" % image)
423+ self.logger.warning("UEFI Signing Failed '%s'" %
424+ " ".join(cmdl))
425
426 def extract(self):
427 """Copy the custom upload to a temporary directory, and sign it.
428@@ -120,11 +170,10 @@
429 No actual extraction is required.
430 """
431 super(UefiUpload, self).extract()
432- if self.key is not None and self.cert is not None:
433- efi_filenames = list(self.findEfiFilenames())
434- for efi_filename in efi_filenames:
435- remove_if_exists("%s.signed" % efi_filename)
436- self.sign(efi_filename)
437+ efi_filenames = list(self.findEfiFilenames())
438+ for efi_filename in efi_filenames:
439+ remove_if_exists("%s.signed" % efi_filename)
440+ self.signUefi(efi_filename)
441
442 def shouldInstall(self, filename):
443 return filename.startswith("%s/" % self.version)