Merge lp:~apw/launchpad/signing-gpg-sign-checksum-files into lp:launchpad

Proposed by Andy Whitcroft on 2016-06-13
Status: Merged
Merged at revision: 18108
Proposed branch: lp:~apw/launchpad/signing-gpg-sign-checksum-files
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/refactor-custom-archive-config
Diff against target: 528 lines (+285/-52)
7 files modified
lib/lp/archivepublisher/archivesigningkey.py (+46/-16)
lib/lp/archivepublisher/interfaces/archivesigningkey.py (+9/-0)
lib/lp/archivepublisher/publishing.py (+9/-7)
lib/lp/archivepublisher/signing.py (+15/-5)
lib/lp/archivepublisher/tests/test_archivesigningkey.py (+80/-0)
lib/lp/archivepublisher/tests/test_publisher.py (+100/-24)
lib/lp/archivepublisher/tests/test_signing.py (+26/-0)
To merge this branch: bzr merge lp:~apw/launchpad/signing-gpg-sign-checksum-files
Reviewer Review Type Date Requested Status
Colin Watson 2016-06-13 Approve on 2016-06-20
Review via email: mp+297177@code.launchpad.net

Commit Message

Sign published checksum files for raw-signing/raw-uefi uploads.

To post a comment you must log in.
Colin Watson (cjwatson) wrote :

Looks mostly good, but one bit of interface I'd prefer to see adjusted for safety.

review: Needs Fixing
Andy Whitcroft (apw) wrote :

Ok. Following some discussion on IRC we decided that the signing object should only sign things in the archive root. To do this we now delay hashing and signing of the custom upload till after it has been extracted and moved into place. This lets us enforce the signing location.

Pushed up a new version which includes the above changes and should also should address the other formatting issues identified.

Colin Watson (cjwatson) :
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/archivesigningkey.py'
2--- lib/lp/archivepublisher/archivesigningkey.py 2016-03-23 17:55:39 +0000
3+++ lib/lp/archivepublisher/archivesigningkey.py 2016-06-18 10:11:19 +0000
4@@ -52,9 +52,8 @@
5 if not os.path.exists(os.path.dirname(export_path)):
6 os.makedirs(os.path.dirname(export_path))
7
8- export_file = open(export_path, 'w')
9- export_file.write(key.export())
10- export_file.close()
11+ with open(export_path, 'w') as export_file:
12+ export_file.write(key.export())
13
14 def generateSigningKey(self):
15 """See `IArchiveSigningKey`."""
16@@ -86,8 +85,9 @@
17 assert os.path.exists(key_path), (
18 "%s does not exist" % key_path)
19
20- secret_key = getUtility(IGPGHandler).importSecretKey(
21- open(key_path).read())
22+ with open(key_path) as key_file:
23+ secret_key_export = key_file.read()
24+ secret_key = getUtility(IGPGHandler).importSecretKey(secret_key_export)
25 self._setupSigningKey(secret_key)
26
27 def _setupSigningKey(self, secret_key):
28@@ -122,25 +122,55 @@
29 "Release file doesn't exist in the repository: %s"
30 % release_file_path)
31
32- secret_key_export = open(
33- self.getPathForSecretKey(self.archive.signing_key)).read()
34+ secret_key_path = self.getPathForSecretKey(self.archive.signing_key)
35+ with open(secret_key_path) as secret_key_file:
36+ secret_key_export = secret_key_file.read()
37
38 gpghandler = getUtility(IGPGHandler)
39 secret_key = gpghandler.importSecretKey(secret_key_export)
40
41- release_file_content = open(release_file_path).read()
42+ with open(release_file_path) as release_file:
43+ release_file_content = release_file.read()
44 signature = gpghandler.signContent(
45 release_file_content, secret_key, mode=gpgme.SIG_MODE_DETACH)
46
47- release_signature_file = open(
48- os.path.join(suite_path, 'Release.gpg'), 'w')
49- release_signature_file.write(signature)
50- release_signature_file.close()
51+ release_signature_path = os.path.join(suite_path, 'Release.gpg')
52+ with open(release_signature_path, 'w') as release_signature_file:
53+ release_signature_file.write(signature)
54
55 inline_release = gpghandler.signContent(
56 release_file_content, secret_key, mode=gpgme.SIG_MODE_CLEAR)
57
58- inline_release_file = open(
59- os.path.join(suite_path, 'InRelease'), 'w')
60- inline_release_file.write(inline_release)
61- inline_release_file.close()
62+ inline_release_path = os.path.join(suite_path, 'InRelease')
63+ with open(inline_release_path, 'w') as inline_release_file:
64+ inline_release_file.write(inline_release)
65+
66+ def signFile(self, path):
67+ """See `IArchiveSigningKey`."""
68+ assert self.archive.signing_key is not None, (
69+ "No signing key available for %s" % self.archive.displayname)
70+
71+ # Allow the passed path to be relative to the archive root.
72+ path = os.path.realpath(os.path.join(self._archive_root_path, path))
73+
74+ # Ensure the resulting path is within the archive root after
75+ # normalisation.
76+ # NOTE: uses os.sep to prevent /var/tmp/../tmpFOO attacks.
77+ archive_root = self._archive_root_path + os.sep
78+ assert path.startswith(archive_root), (
79+ "Attempting to sign file (%s) outside archive_root for %s" % (
80+ path, self.archive.displayname))
81+
82+ secret_key_path = self.getPathForSecretKey(self.archive.signing_key)
83+ with open(secret_key_path) as secret_key_file:
84+ secret_key_export = secret_key_file.read()
85+ gpghandler = getUtility(IGPGHandler)
86+ secret_key = gpghandler.importSecretKey(secret_key_export)
87+
88+ with open(path) as path_file:
89+ file_content = path_file.read()
90+ signature = gpghandler.signContent(
91+ file_content, secret_key, mode=gpgme.SIG_MODE_DETACH)
92+
93+ with open(os.path.join(path + '.gpg'), 'w') as signature_file:
94+ signature_file.write(signature)
95
96=== modified file 'lib/lp/archivepublisher/interfaces/archivesigningkey.py'
97--- lib/lp/archivepublisher/interfaces/archivesigningkey.py 2015-10-21 09:37:08 +0000
98+++ lib/lp/archivepublisher/interfaces/archivesigningkey.py 2016-06-18 10:11:19 +0000
99@@ -85,3 +85,12 @@
100 :raises AssertionError: if the context archive has no `signing_key`
101 or there is no Release file in the given suite.
102 """
103+
104+ def signFile(path):
105+ """Sign the corresponding file.
106+
107+ :param path: path within dists to sign with the archive key.
108+ :raises AssertionError: if the context archive has no `signing_key`.
109+ :raises AssertionError: if the given 'path' is outside of the
110+ archive root.
111+ """
112
113=== modified file 'lib/lp/archivepublisher/publishing.py'
114--- lib/lp/archivepublisher/publishing.py 2016-06-07 17:05:57 +0000
115+++ lib/lp/archivepublisher/publishing.py 2016-06-18 10:11:19 +0000
116@@ -1478,16 +1478,16 @@
117 class DirectoryHash:
118 """Represents a directory hierarchy for hashing."""
119
120- def __init__(self, root, tmpdir, log):
121+ def __init__(self, root, tmpdir, signer=None):
122 self.root = root
123 self.tmpdir = tmpdir
124- self.log = log
125+ self.signer = signer
126 self.checksum_hash = []
127
128 for usable in self._usable_archive_hashes:
129- checksum_file = os.path.join(self.root, usable.dh_name)
130- self.checksum_hash.append(
131- (RepositoryIndexFile(checksum_file, self.tmpdir), usable))
132+ csum_path = os.path.join(self.root, usable.dh_name)
133+ self.checksum_hash.append((csum_path,
134+ RepositoryIndexFile(csum_path, self.tmpdir), usable))
135
136 def __enter__(self):
137 return self
138@@ -1505,7 +1505,7 @@
139 """Add a path to be checksummed."""
140 hashes = [
141 (checksum_file, archive_hash.hash_factory())
142- for (checksum_file, archive_hash) in self.checksum_hash]
143+ for (_, checksum_file, archive_hash) in self.checksum_hash]
144 with open(path, 'rb') as in_file:
145 for chunk in iter(lambda: in_file.read(256 * 1024), ""):
146 for (checksum_file, hashobj) in hashes:
147@@ -1522,5 +1522,7 @@
148 self.add(os.path.join(dirpath, filename))
149
150 def close(self):
151- for (checksum_file, archive_hash) in self.checksum_hash:
152+ for (checksum_path, checksum_file, archive_hash) in self.checksum_hash:
153 checksum_file.close()
154+ if self.signer:
155+ self.signer.signFile(checksum_path)
156
157=== modified file 'lib/lp/archivepublisher/signing.py'
158--- lib/lp/archivepublisher/signing.py 2016-06-14 13:29:07 +0000
159+++ lib/lp/archivepublisher/signing.py 2016-06-18 10:11:19 +0000
160@@ -28,6 +28,9 @@
161
162 from lp.archivepublisher.config import getPubConfig
163 from lp.archivepublisher.customupload import CustomUpload
164+from lp.archivepublisher.interfaces.archivesigningkey import (
165+ IArchiveSigningKey,
166+ )
167 from lp.services.osutils import remove_if_exists
168 from lp.soyuz.interfaces.queue import CustomUploadError
169
170@@ -309,9 +312,6 @@
171
172 No actual extraction is required.
173 """
174- # Avoid circular import.
175- from lp.archivepublisher.publishing import DirectoryHash
176-
177 super(SigningUpload, self).extract()
178 self.setSigningOptions()
179 filehandlers = list(self.findSigningHandlers())
180@@ -327,8 +327,18 @@
181 if 'tarball' in self.signing_options:
182 self.convertToTarball()
183
184- versiondir = os.path.join(self.tmpdir, self.version)
185- with DirectoryHash(versiondir, self.tmpdir, self.logger) as hasher:
186+ def installFiles(self):
187+ """After installation hash and sign the installed result."""
188+ # Avoid circular import.
189+ from lp.archivepublisher.publishing import DirectoryHash
190+
191+ super(SigningUpload, self).installFiles()
192+
193+ versiondir = os.path.join(self.targetdir, self.version)
194+ signer = None
195+ if self.archive.signing_key:
196+ signer = IArchiveSigningKey(self.archive)
197+ with DirectoryHash(versiondir, self.tmpdir, signer) as hasher:
198 hasher.add_dir(versiondir)
199
200 def shouldInstall(self, filename):
201
202=== added file 'lib/lp/archivepublisher/tests/test_archivesigningkey.py'
203--- lib/lp/archivepublisher/tests/test_archivesigningkey.py 1970-01-01 00:00:00 +0000
204+++ lib/lp/archivepublisher/tests/test_archivesigningkey.py 2016-06-18 10:11:19 +0000
205@@ -0,0 +1,80 @@
206+# Copyright 2016 Canonical Ltd. This software is licensed under the
207+# GNU Affero General Public License version 3 (see the file LICENSE).
208+
209+"""Test ArchiveSigningKey."""
210+
211+__metaclass__ = type
212+
213+import os
214+
215+from zope.component import getUtility
216+
217+from lp.archivepublisher.config import getPubConfig
218+from lp.archivepublisher.interfaces.archivesigningkey import (
219+ IArchiveSigningKey,
220+ )
221+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
222+from lp.services.osutils import write_file
223+from lp.soyuz.enums import ArchivePurpose
224+from lp.testing import TestCaseWithFactory
225+from lp.testing.gpgkeys import gpgkeysdir
226+from lp.testing.keyserver import KeyServerTac
227+from lp.testing.layers import ZopelessDatabaseLayer
228+
229+
230+class TestArchiveSigningKey(TestCaseWithFactory):
231+
232+ layer = ZopelessDatabaseLayer
233+
234+ def setUp(self):
235+ super(TestArchiveSigningKey, self).setUp()
236+ self.temp_dir = self.makeTemporaryDirectory()
237+ self.distro = self.factory.makeDistribution()
238+ db_pubconf = getUtility(IPublisherConfigSet).getByDistribution(
239+ self.distro)
240+ db_pubconf.root_dir = unicode(self.temp_dir)
241+ self.archive = self.factory.makeArchive(
242+ distribution=self.distro, purpose=ArchivePurpose.PRIMARY)
243+ self.archive_root = getPubConfig(self.archive).archiveroot
244+ self.suite = "distroseries"
245+
246+ with KeyServerTac():
247+ key_path = os.path.join(gpgkeysdir, 'ppa-sample@canonical.com.sec')
248+ IArchiveSigningKey(self.archive).setSigningKey(key_path)
249+
250+ def test_signfile_absolute_within_archive(self):
251+ filename = os.path.join(self.archive_root, "signme")
252+ write_file(filename, "sign this")
253+
254+ signer = IArchiveSigningKey(self.archive)
255+ signer.signFile(filename)
256+
257+ signature = filename + '.gpg'
258+ self.assertTrue(os.path.exists(signature))
259+
260+ def test_signfile_absolute_outside_archive(self):
261+ filename = os.path.join(self.temp_dir, "signme")
262+ write_file(filename, "sign this")
263+
264+ signer = IArchiveSigningKey(self.archive)
265+ self.assertRaises(AssertionError, lambda: signer.signFile(filename))
266+
267+ def test_signfile_relative_within_archive(self):
268+ filename_relative = "signme"
269+ filename = os.path.join(self.archive_root, filename_relative)
270+ write_file(filename, "sign this")
271+
272+ signer = IArchiveSigningKey(self.archive)
273+ signer.signFile(filename_relative)
274+
275+ signature = filename + '.gpg'
276+ self.assertTrue(os.path.exists(signature))
277+
278+ def test_signfile_relative_outside_archive(self):
279+ filename_relative = "../signme"
280+ filename = os.path.join(self.temp_dir, filename_relative)
281+ write_file(filename, "sign this")
282+
283+ signer = IArchiveSigningKey(self.archive)
284+ self.assertRaises(
285+ AssertionError, lambda: signer.signFile(filename_relative))
286
287=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
288--- lib/lp/archivepublisher/tests/test_publisher.py 2016-06-06 17:13:42 +0000
289+++ lib/lp/archivepublisher/tests/test_publisher.py 2016-06-18 10:11:19 +0000
290@@ -6,7 +6,10 @@
291 __metaclass__ = type
292
293 import bz2
294-from collections import OrderedDict
295+from collections import (
296+ defaultdict,
297+ OrderedDict,
298+ )
299 import crypt
300 from datetime import (
301 datetime,
302@@ -63,6 +66,7 @@
303 Publisher,
304 )
305 from lp.archivepublisher.utils import RepositoryIndexFile
306+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
307 from lp.registry.interfaces.distribution import IDistributionSet
308 from lp.registry.interfaces.distroseries import IDistroSeries
309 from lp.registry.interfaces.person import IPersonSet
310@@ -94,10 +98,7 @@
311 from lp.soyuz.interfaces.archive import IArchiveSet
312 from lp.soyuz.interfaces.archivefile import IArchiveFileSet
313 from lp.soyuz.tests.test_publishing import TestNativePublishingBase
314-from lp.testing import (
315- TestCase,
316- TestCaseWithFactory,
317- )
318+from lp.testing import TestCaseWithFactory
319 from lp.testing.fakemethod import FakeMethod
320 from lp.testing.gpgkeys import gpgkeysdir
321 from lp.testing.keyserver import KeyServerTac
322@@ -3167,8 +3168,8 @@
323 self.assertEqual([], self.makePublisher(partner).subcomponents)
324
325
326-class TestDirectoryHash(TestCase):
327- """Unit tests for DirectoryHash object."""
328+class TestDirectoryHashHelpers(TestCaseWithFactory):
329+ """Helper functions for DirectoryHash testing."""
330
331 def createTestFile(self, path, content):
332 with open(path, "w") as tfd:
333@@ -3184,15 +3185,30 @@
334 return ['SHA256SUMS']
335
336 def fetchSums(self, rootdir):
337- result = {}
338+ result = defaultdict(list)
339 for dh_file in self.all_hash_files:
340 checksum_file = os.path.join(rootdir, dh_file)
341 if os.path.exists(checksum_file):
342 with open(checksum_file, "r") as sfd:
343 for line in sfd:
344- file_list = result.setdefault(dh_file, [])
345- file_list.append(line.strip().split(' '))
346- return result
347+ result[dh_file].append(line.strip().split(' '))
348+ return result
349+
350+ def fetchSigs(self, rootdir):
351+ result = defaultdict(list)
352+ for dh_file in self.all_hash_files:
353+ checksum_sig = os.path.join(rootdir, dh_file) + '.gpg'
354+ if os.path.exists(checksum_sig):
355+ with open(checksum_sig, "r") as sfd:
356+ for line in sfd:
357+ result[dh_file].append(line)
358+ return result
359+
360+
361+class TestDirectoryHash(TestDirectoryHashHelpers):
362+ """Unit tests for DirectoryHash object."""
363+
364+ layer = ZopelessDatabaseLayer
365
366 def test_checksum_files_created(self):
367 tmpdir = unicode(self.makeTemporaryDirectory())
368@@ -3202,7 +3218,7 @@
369 checksum_file = os.path.join(rootdir, dh_file)
370 self.assertFalse(os.path.exists(checksum_file))
371
372- with DirectoryHash(rootdir, tmpdir, None) as dh:
373+ with DirectoryHash(rootdir, tmpdir, None):
374 pass
375
376 for dh_file in self.all_hash_files:
377@@ -3226,7 +3242,7 @@
378 test3_file = os.path.join(rootdir, "subdir1", "test3")
379 test3_hash = self.createTestFile(test3_file, "test3")
380
381- with DirectoryHash(rootdir, tmpdir, None) as dh:
382+ with DirectoryHash(rootdir, tmpdir) as dh:
383 dh.add(test1_file)
384 dh.add(test2_file)
385 dh.add(test3_file)
386@@ -3254,14 +3270,74 @@
387 test3_file = os.path.join(rootdir, "subdir1", "test3")
388 test3_hash = self.createTestFile(test3_file, "test3 dir")
389
390- with DirectoryHash(rootdir, tmpdir, None) as dh:
391- dh.add_dir(rootdir)
392-
393- expected = {
394- 'SHA256SUMS': MatchesSetwise(
395- Equals([test1_hash, "*test1"]),
396- Equals([test2_hash, "*test2"]),
397- Equals([test3_hash, "*subdir1/test3"]),
398- ),
399- }
400- self.assertThat(self.fetchSums(rootdir), MatchesDict(expected))
401+ with DirectoryHash(rootdir, tmpdir) as dh:
402+ dh.add_dir(rootdir)
403+
404+ expected = {
405+ 'SHA256SUMS': MatchesSetwise(
406+ Equals([test1_hash, "*test1"]),
407+ Equals([test2_hash, "*test2"]),
408+ Equals([test3_hash, "*subdir1/test3"]),
409+ ),
410+ }
411+ self.assertThat(self.fetchSums(rootdir), MatchesDict(expected))
412+
413+
414+class TestDirectoryHashSigning(TestDirectoryHashHelpers):
415+ """Unit tests for DirectoryHash object, signing functionality."""
416+
417+ layer = ZopelessDatabaseLayer
418+
419+ def setUp(self):
420+ super(TestDirectoryHashSigning, self).setUp()
421+ self.temp_dir = self.makeTemporaryDirectory()
422+ self.distro = self.factory.makeDistribution()
423+ db_pubconf = getUtility(IPublisherConfigSet).getByDistribution(
424+ self.distro)
425+ db_pubconf.root_dir = unicode(self.temp_dir)
426+ self.archive = self.factory.makeArchive(
427+ distribution=self.distro, purpose=ArchivePurpose.PRIMARY)
428+ self.archive_root = getPubConfig(self.archive).archiveroot
429+ self.suite = "distroseries"
430+
431+ # Setup a keyserver so we can install the archive key.
432+ tac = KeyServerTac()
433+ tac.setUp()
434+
435+ key_path = os.path.join(gpgkeysdir, 'ppa-sample@canonical.com.sec')
436+ IArchiveSigningKey(self.archive).setSigningKey(key_path)
437+
438+ tac.tearDown()
439+
440+ def test_basic_directory_add_signed(self):
441+ tmpdir = unicode(self.makeTemporaryDirectory())
442+ rootdir = self.archive_root
443+ os.makedirs(rootdir)
444+
445+ test1_file = os.path.join(rootdir, "test1")
446+ test1_hash = self.createTestFile(test1_file, "test1 dir")
447+
448+ test2_file = os.path.join(rootdir, "test2")
449+ test2_hash = self.createTestFile(test2_file, "test2 dir")
450+
451+ os.mkdir(os.path.join(rootdir, "subdir1"))
452+
453+ test3_file = os.path.join(rootdir, "subdir1", "test3")
454+ test3_hash = self.createTestFile(test3_file, "test3 dir")
455+
456+ signer = IArchiveSigningKey(self.archive)
457+ with DirectoryHash(rootdir, tmpdir, signer=signer) as dh:
458+ dh.add_dir(rootdir)
459+
460+ expected = {
461+ 'SHA256SUMS': MatchesSetwise(
462+ Equals([test1_hash, "*test1"]),
463+ Equals([test2_hash, "*test2"]),
464+ Equals([test3_hash, "*subdir1/test3"]),
465+ ),
466+ }
467+ self.assertThat(self.fetchSums(rootdir), MatchesDict(expected))
468+ sig_content = self.fetchSigs(rootdir)
469+ for dh_file in sig_content:
470+ self.assertEqual(
471+ sig_content[dh_file][0], '-----BEGIN PGP SIGNATURE-----\n')
472
473=== modified file 'lib/lp/archivepublisher/tests/test_signing.py'
474--- lib/lp/archivepublisher/tests/test_signing.py 2016-06-14 13:29:07 +0000
475+++ lib/lp/archivepublisher/tests/test_signing.py 2016-06-18 10:11:19 +0000
476@@ -17,6 +17,9 @@
477 CustomUploadAlreadyExists,
478 CustomUploadBadUmask,
479 )
480+from lp.archivepublisher.interfaces.archivesigningkey import (
481+ IArchiveSigningKey,
482+ )
483 from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
484 from lp.archivepublisher.signing import (
485 SigningUpload,
486@@ -27,6 +30,8 @@
487 from lp.soyuz.enums import ArchivePurpose
488 from lp.testing import TestCaseWithFactory
489 from lp.testing.fakemethod import FakeMethod
490+from lp.testing.gpgkeys import gpgkeysdir
491+from lp.testing.keyserver import KeyServerTac
492 from lp.testing.layers import ZopelessDatabaseLayer
493
494
495@@ -111,6 +116,11 @@
496 self.temp_dir, "signing", "signing-owner", "testing")
497 self.testcase_cn = '/CN=PPA signing-owner testing/'
498
499+ def setUpArchiveKey(self):
500+ with KeyServerTac():
501+ key_path = os.path.join(gpgkeysdir, 'ppa-sample@canonical.com.sec')
502+ IArchiveSigningKey(self.archive).setSigningKey(key_path)
503+
504 def setUpUefiKeys(self, create=True):
505 self.key = os.path.join(self.signing_dir, "uefi.key")
506 self.cert = os.path.join(self.signing_dir, "uefi.crt")
507@@ -629,6 +639,22 @@
508 "1.0", "SHA256SUMS")
509 self.assertTrue(os.path.exists(sha256file))
510
511+ def test_checksumming_tree_signed(self):
512+ # Specifying no options should leave us with an open tree,
513+ # confirm it is checksummed. Supply an archive signing key
514+ # which should trigger signing of the checksum file.
515+ self.setUpArchiveKey()
516+ self.setUpUefiKeys()
517+ self.setUpKmodKeys()
518+ self.openArchive("test", "1.0", "amd64")
519+ self.tarfile.add_file("1.0/empty.efi", "")
520+ self.tarfile.add_file("1.0/empty.ko", "")
521+ self.process_emulate()
522+ sha256file = os.path.join(self.getSignedPath("test", "amd64"),
523+ "1.0", "SHA256SUMS")
524+ self.assertTrue(os.path.exists(sha256file))
525+ self.assertTrue(os.path.exists(sha256file + '.gpg'))
526+
527
528 class TestUefi(TestSigningHelpers):
529