Merge ~pappacena/launchpad:inject-lp-signing-generated-keys into launchpad:master

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: 00e44a212547fc747080ec3266508faee1a25fcc
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:inject-lp-signing-generated-keys
Merge into: launchpad:master
Diff against target: 433 lines (+255/-6)
6 files modified
lib/lp/archivepublisher/signing.py (+77/-1)
lib/lp/archivepublisher/tests/test_signing.py (+111/-3)
lib/lp/services/signing/interfaces/signingkey.py (+4/-1)
lib/lp/services/signing/model/signingkey.py (+6/-1)
lib/lp/services/signing/tests/helpers.py (+10/-0)
lib/lp/services/signing/tests/test_signingkey.py (+47/-0)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+382779@code.launchpad.net

Commit message

Adding the possibility to inject into lp-signing the locally generated signing keys.

It is possible to control which key types to inject when auto-generating them by setting the feature flag `archivepublisher.signing_service.injection.enabled` with a list of key types (separated by spaces). Eg.: "KMOD UEFI".

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Needs Information
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

Pushed the requested changes. Let me know if you need another round of review, cjwatson.

Revision history for this message
Colin Watson (cjwatson) :
review: Needs Fixing
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

cjwatson, I'll push some of the requested changes, but I would like further clarification on some of your previous comments.

Revision history for this message
Colin Watson (cjwatson) :
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

Pushed the requested changes. It might be good to have another round of review on the duplicated ArchiveSigningKey check.

Revision history for this message
Colin Watson (cjwatson) :
review: Needs Fixing
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

Pushed the requested changes (with some comments, cjwatson).

Revision history for this message
Thiago F. Pappacena (pappacena) :
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/archivepublisher/signing.py b/lib/lp/archivepublisher/signing.py
2index 9a175db..1eca0a4 100644
3--- a/lib/lp/archivepublisher/signing.py
4+++ b/lib/lp/archivepublisher/signing.py
5@@ -18,6 +18,7 @@ __all__ = [
6 "UefiUpload",
7 ]
8
9+from datetime import datetime
10 from functools import partial
11 import os
12 import shutil
13@@ -27,6 +28,7 @@ import tarfile
14 import tempfile
15 import textwrap
16
17+from pytz import utc
18 import scandir
19 from zope.component import getUtility
20
21@@ -42,6 +44,8 @@ from lp.soyuz.interfaces.queue import CustomUploadError
22
23 PUBLISHER_USES_SIGNING_SERVICE = (
24 'archivepublisher.signing_service.enabled')
25+PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS = (
26+ 'archivepublisher.signing_service.injection.enabled')
27
28
29 class SigningUploadPackError(CustomUploadError):
30@@ -59,6 +63,10 @@ class SigningServiceError(Exception):
31 pass
32
33
34+class SigningKeyConflict(Exception):
35+ pass
36+
37+
38 class SigningUpload(CustomUpload):
39 """Signing custom upload.
40
41@@ -260,7 +268,7 @@ class SigningUpload(CustomUpload):
42 # tend to make the publisher rather upset.
43 if self.logger is not None:
44 self.logger.warning("%s Failed (cmd='%s')" %
45- (description, " ".join(cmdl)))
46+ (description, " ".join(cmdl)))
47 return status
48
49 def findSigningHandlers(self):
50@@ -421,6 +429,56 @@ class SigningUpload(CustomUpload):
51 return [None for k in keynames]
52 return keynames
53
54+ def injectIntoSigningService(
55+ self, key_type, private_key_file, public_key_file):
56+ """Injects the given key pair into signing service for current
57+ archive.
58+
59+ Note that this injection should only be used for freshly
60+ autogenerated keys, always injecting the key for the archive in
61+ general (not setting earliest_distro_series).
62+ """
63+ if key_type not in SigningKeyType:
64+ raise ValueError("%s is not a valid key type to inject" % key_type)
65+
66+ feature_flag = (
67+ getFeatureFlag(PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS) or '')
68+ key_types_to_inject = [i.strip() for i in feature_flag.split()]
69+
70+ if key_type.name not in key_types_to_inject:
71+ if self.logger:
72+ self.logger.info(
73+ "Skipping injection for key type %s: not in %s",
74+ key_type, key_types_to_inject)
75+ return
76+
77+ key_set = getUtility(IArchiveSigningKeySet)
78+ current_key = key_set.getSigningKey(
79+ key_type, self.archive, None, exact_match=True)
80+ if current_key is not None:
81+ self.logger.info("Skipping injection for key type %s: archive "
82+ "already has a key on lp-signing.", key_type)
83+ raise SigningKeyConflict(
84+ "Archive %s already has a signing key type %s on lp-signing."
85+ % (self.archive.reference, key_type))
86+
87+ if self.logger:
88+ self.logger.info(
89+ "Injecting key_type %s for archive %s into signing service",
90+ key_type, self.archive.name)
91+
92+ with open(private_key_file, 'rb') as fd:
93+ private_key = fd.read()
94+ with open(public_key_file, 'rb') as fd:
95+ public_key = fd.read()
96+
97+ now = datetime.now().replace(tzinfo=utc)
98+ description = (
99+ u"%s key for %s" % (key_type.name, self.archive.reference))
100+ key_set.inject(
101+ key_type, private_key, public_key,
102+ description, now, self.archive, earliest_distro_series=None)
103+
104 def generateKeyCommonName(self, owner, archive, suffix=''):
105 # PPA <owner> <archive> <suffix>
106 # truncate <owner> <archive> to ensure the overall form is shorter
107@@ -454,6 +512,15 @@ class SigningUpload(CustomUpload):
108 if os.path.exists(cert_filename):
109 os.chmod(cert_filename, 0o644)
110
111+ signing_key_type = getattr(SigningKeyType, key_type.upper())
112+ try:
113+ self.injectIntoSigningService(
114+ signing_key_type, key_filename, cert_filename)
115+ except SigningKeyConflict:
116+ os.unlink(key_filename)
117+ os.unlink(cert_filename)
118+ raise
119+
120 def generateUefiKeys(self):
121 """Generate new UEFI Keys for this archive."""
122 self.generateKeyCrtPair("UEFI", self.uefi_key, self.uefi_cert)
123@@ -541,6 +608,15 @@ class SigningUpload(CustomUpload):
124 if os.path.exists(x509_filename):
125 os.chmod(x509_filename, 0o644)
126
127+ signing_key_type = getattr(SigningKeyType, key_type.upper())
128+ try:
129+ self.injectIntoSigningService(
130+ signing_key_type, pem_filename, x509_filename)
131+ except SigningKeyConflict:
132+ os.unlink(pem_filename)
133+ os.unlink(x509_filename)
134+ raise
135+
136 def generateKmodKeys(self):
137 """Generate new Kernel Signing Keys for this archive."""
138 config = self.generateOpensslConfig("Kmod", self.openssl_config_kmod)
139diff --git a/lib/lp/archivepublisher/tests/test_signing.py b/lib/lp/archivepublisher/tests/test_signing.py
140index 87cf391..e51bb25 100644
141--- a/lib/lp/archivepublisher/tests/test_signing.py
142+++ b/lib/lp/archivepublisher/tests/test_signing.py
143@@ -7,14 +7,19 @@ from __future__ import absolute_import, print_function, unicode_literals
144
145 __metaclass__ = type
146
147+from datetime import datetime
148 import os
149 import re
150 import shutil
151 import stat
152 import tarfile
153
154-from fixtures import MonkeyPatch
155+from fixtures import (
156+ MockPatch,
157+ MonkeyPatch,
158+ )
159 from mock import call
160+from pytz import utc
161 import scandir
162 from testtools.matchers import (
163 Contains,
164@@ -42,12 +47,15 @@ from lp.archivepublisher.interfaces.archivegpgsigningkey import (
165 )
166 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
167 from lp.archivepublisher.signing import (
168+ PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS,
169 PUBLISHER_USES_SIGNING_SERVICE,
170+ SigningKeyConflict,
171 SigningUpload,
172 UefiUpload,
173 )
174 from lp.archivepublisher.tests.test_run_parts import RunPartsMixin
175 from lp.services.features.testing import FeatureFixture
176+from lp.services.log.logger import BufferLogger
177 from lp.services.osutils import write_file
178 from lp.services.signing.enums import SigningMode
179 from lp.services.signing.proxy import SigningKeyType
180@@ -1851,7 +1859,8 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
181
182 self.openArchive("test", "1.0", "amd64")
183 for filename in filenames:
184- self.tarfile.add_file(filename, b"data - %s" % filename)
185+ self.tarfile.add_file(
186+ filename, b"data - %s" % filename.encode("UTF-8"))
187
188 self.tarfile.close()
189 self.buffer.close()
190@@ -1965,7 +1974,8 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
191
192 self.openArchive("test", "1.0", "amd64")
193 for filename in filenames:
194- self.tarfile.add_file(filename, b"data - %s" % filename)
195+ self.tarfile.add_file(
196+ filename, b"data - %s" % filename.encode("UTF-8"))
197
198 self.tarfile.close()
199 self.buffer.close()
200@@ -2009,3 +2019,101 @@ class TestSigningUploadWithSigningService(TestSigningHelpers):
201 self.assertEqual(
202 [(os.path.join(upload.tmpdir_used, "1.0/empty.efi"),)],
203 upload.signUefi.extract_args())
204+
205+ def test_fallback_injects_key(self):
206+ self.useFixture(FeatureFixture({PUBLISHER_USES_SIGNING_SERVICE: ''}))
207+ self.useFixture(FeatureFixture({
208+ PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS: 'SIPL OPAL'}))
209+
210+ now = datetime.now()
211+ mock_datetime = self.useFixture(MockPatch(
212+ 'lp.archivepublisher.signing.datetime')).mock
213+ mock_datetime.now = lambda: now
214+
215+ logger = BufferLogger()
216+ upload = SigningUpload(logger=logger)
217+
218+ # Setup PPA to ensure it auto-generates keys.
219+ self.setUpPPA()
220+
221+ filenames = ["1.0/empty.efi", "1.0/empty.opal"]
222+
223+ self.openArchive("test", "1.0", "amd64")
224+ for filename in filenames:
225+ self.tarfile.add_file(
226+ filename, b"data - %s" % filename.encode("UTF-8"))
227+ self.tarfile.close()
228+ self.buffer.close()
229+
230+ upload.process(self.archive, self.path, self.suite)
231+ self.assertTrue(upload.autokey)
232+
233+ # Read the key file content
234+ with open(upload.opal_pem, 'rb') as fd:
235+ private_key = fd.read()
236+ with open(upload.opal_x509, 'rb') as fd:
237+ public_key = fd.read()
238+
239+ # Check if we called lp-signing's /inject endpoint correctly
240+ self.assertEqual(1, self.signing_service_client.inject.call_count)
241+ self.assertEqual(
242+ (SigningKeyType.OPAL, private_key, public_key,
243+ u"OPAL key for %s" % self.archive.reference,
244+ now.replace(tzinfo=utc)),
245+ self.signing_service_client.inject.call_args[0])
246+
247+ log_content = logger.content.as_text()
248+ self.assertIn(
249+ "INFO Injecting key_type OPAL for archive %s into signing "
250+ "service" % (self.archive.name),
251+ log_content)
252+
253+ self.assertIn(
254+ "INFO Skipping injection for key type UEFI: "
255+ "not in [u'SIPL', u'OPAL']",
256+ log_content)
257+
258+ def test_fallback_skips_key_injection_for_existing_keys(self):
259+ self.useFixture(FeatureFixture({PUBLISHER_USES_SIGNING_SERVICE: ''}))
260+ self.useFixture(FeatureFixture({
261+ PUBLISHER_SIGNING_SERVICE_INJECTS_KEYS: 'UEFI'}))
262+
263+ now = datetime.now()
264+ mock_datetime = self.useFixture(MockPatch(
265+ 'lp.archivepublisher.signing.datetime')).mock
266+ mock_datetime.now = lambda: now
267+
268+ # Setup PPA to ensure it auto-generates keys.
269+ self.setUpPPA()
270+
271+ signing_key = self.factory.makeSigningKey(key_type=SigningKeyType.UEFI)
272+ self.factory.makeArchiveSigningKey(
273+ archive=self.archive, signing_key=signing_key)
274+
275+ logger = BufferLogger()
276+ upload = SigningUpload(logger=logger)
277+
278+ filenames = ["1.0/empty.efi"]
279+
280+ self.openArchive("test", "1.0", "amd64")
281+ for filename in filenames:
282+ self.tarfile.add_file(
283+ filename, b"data - %s" % filename.encode("UTF-8"))
284+ self.tarfile.close()
285+ self.buffer.close()
286+
287+ self.assertRaises(SigningKeyConflict,
288+ upload.process, self.archive, self.path, self.suite)
289+ self.assertTrue(upload.autokey)
290+
291+ # Make sure we deleted the locally generated keys.
292+ self.assertFalse(os.path.exists(upload.uefi_cert))
293+ self.assertFalse(os.path.exists(upload.uefi_key))
294+
295+ # Make sure we didn't call lp-signing's /inject endpoint
296+ self.assertEqual(0, self.signing_service_client.inject.call_count)
297+ log_content = logger.content.as_text()
298+ self.assertIn(
299+ "INFO Skipping injection for key type %s: archive "
300+ "already has a key on lp-signing." % SigningKeyType.UEFI,
301+ log_content)
302diff --git a/lib/lp/services/signing/interfaces/signingkey.py b/lib/lp/services/signing/interfaces/signingkey.py
303index 587bbb0..eccf2a0 100644
304--- a/lib/lp/services/signing/interfaces/signingkey.py
305+++ b/lib/lp/services/signing/interfaces/signingkey.py
306@@ -117,10 +117,13 @@ class IArchiveSigningKeySet(Interface):
307 False if it was updated).
308 """
309
310- def getSigningKey(key_type, archive, distro_series):
311+ def getSigningKey(key_type, archive, distro_series, exact_match=False):
312 """Get the most suitable key for a given archive / distro series
313 pair.
314
315+ :param exact_match: If True, returns the ArchiveSigningKey matching
316+ exactly the given key_type, archive and
317+ distro_series. If False, gets the best match.
318 :return: The most suitable key
319 """
320
321diff --git a/lib/lp/services/signing/model/signingkey.py b/lib/lp/services/signing/model/signingkey.py
322index 0d5e137..0479b61 100644
323--- a/lib/lp/services/signing/model/signingkey.py
324+++ b/lib/lp/services/signing/model/signingkey.py
325@@ -167,7 +167,8 @@ class ArchiveSigningKeySet:
326 return obj
327
328 @classmethod
329- def getSigningKey(cls, key_type, archive, distro_series):
330+ def getSigningKey(cls, key_type, archive, distro_series,
331+ exact_match=False):
332 store = IStore(ArchiveSigningKey)
333 # Gets all the keys of the given key_type available for the archive
334 rs = store.find(ArchiveSigningKey,
335@@ -176,6 +177,10 @@ class ArchiveSigningKeySet:
336 ArchiveSigningKey.key_type == key_type,
337 ArchiveSigningKey.archive == archive)
338
339+ if exact_match:
340+ rs = rs.find(
341+ ArchiveSigningKey.earliest_distro_series == distro_series)
342+
343 # prefetch related signing keys to avoid extra queries.
344 signing_keys = store.find(SigningKey, [
345 SigningKey.id.is_in([i.signing_key_id for i in rs])])
346diff --git a/lib/lp/services/signing/tests/helpers.py b/lib/lp/services/signing/tests/helpers.py
347index e01730f..5675bc6 100644
348--- a/lib/lp/services/signing/tests/helpers.py
349+++ b/lib/lp/services/signing/tests/helpers.py
350@@ -39,8 +39,12 @@ class SigningServiceClientFixture(fixtures.Fixture):
351 self.sign = mock.Mock()
352 self.sign.side_effect = self._sign
353
354+ self.inject = mock.Mock()
355+ self.inject.side_effect = self._inject
356+
357 self.generate_returns = []
358 self.sign_returns = []
359+ self.inject_returns = []
360
361 def _generate(self, key_type, description):
362 key = bytes(PrivateKey.generate().public_key)
363@@ -59,6 +63,12 @@ class SigningServiceClientFixture(fixtures.Fixture):
364 self.sign_returns.append((key_type, data))
365 return data
366
367+ def _inject(self, key_type, private_key, public_key, description,
368+ created_at):
369+ data = {'fingerprint': text_type(self.factory.getUniqueHexString(40))}
370+ self.inject_returns.append(data)
371+ return data
372+
373 def _setUp(self):
374 self.useFixture(ZopeUtilityFixture(self, ISigningServiceClient))
375
376diff --git a/lib/lp/services/signing/tests/test_signingkey.py b/lib/lp/services/signing/tests/test_signingkey.py
377index 3610303..8613dd7 100644
378--- a/lib/lp/services/signing/tests/test_signingkey.py
379+++ b/lib/lp/services/signing/tests/test_signingkey.py
380@@ -244,6 +244,53 @@ class TestArchiveSigningKey(TestCaseWithFactory):
381 arch_kmod_key.signing_key,
382 arch_signing_key_set.getSigningKey(KMOD, archive, distro_series))
383
384+ def test_get_signing_key_exact_match(self):
385+ UEFI = SigningKeyType.UEFI
386+ KMOD = SigningKeyType.KMOD
387+
388+ archive = self.factory.makeArchive()
389+ distro_series1 = archive.distribution.series[0]
390+ distro_series2 = archive.distribution.series[1]
391+ uefi_key = self.factory.makeSigningKey(
392+ key_type=SigningKeyType.UEFI)
393+ kmod_key = self.factory.makeSigningKey(
394+ key_type=SigningKeyType.KMOD)
395+
396+ arch_signing_key_set = getUtility(IArchiveSigningKeySet)
397+
398+ # Create a key for the first distro series
399+ series1_uefi_key = arch_signing_key_set.create(
400+ archive, distro_series1, uefi_key)
401+
402+ # Create a key for the archive
403+ arch_kmod_key = arch_signing_key_set.create(
404+ archive, None, kmod_key)
405+
406+ # Should get the UEFI key for distro_series1
407+ self.assertEqual(
408+ series1_uefi_key.signing_key,
409+ arch_signing_key_set.getSigningKey(
410+ UEFI, archive, distro_series1, exact_match=True)
411+ )
412+ # Should get the archive's KMOD key.
413+ self.assertEqual(
414+ arch_kmod_key.signing_key,
415+ arch_signing_key_set.getSigningKey(
416+ KMOD, archive, None, exact_match=True)
417+ )
418+ # distro_series1 has no KMOD key.
419+ self.assertEqual(
420+ None,
421+ arch_signing_key_set.getSigningKey(
422+ KMOD, archive, distro_series1, exact_match=True)
423+ )
424+ # distro_series2 has no key at all.
425+ self.assertEqual(
426+ None,
427+ arch_signing_key_set.getSigningKey(
428+ KMOD, archive, distro_series2, exact_match=True)
429+ )
430+
431 def test_get_signing_keys_with_distro_series_configured(self):
432 UEFI = SigningKeyType.UEFI
433 KMOD = SigningKeyType.KMOD