Merge lp:~apw/launchpad/signing-key-generate-subject-limit into lp:launchpad

Proposed by Andy Whitcroft
Status: Merged
Merged at revision: 18427
Proposed branch: lp:~apw/launchpad/signing-key-generate-subject-limit
Merge into: lp:launchpad
Diff against target: 115 lines (+58/-4)
2 files modified
lib/lp/archivepublisher/signing.py (+18/-4)
lib/lp/archivepublisher/tests/test_signing.py (+40/-0)
To merge this branch: bzr merge lp:~apw/launchpad/signing-key-generate-subject-limit
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+301642@code.launchpad.net

Commit message

signing: truncate signing key common-names to 64 characters

Description of the change

When owner+ppa name is very long the subjects generated for the EFI and KMOD keys are too long to represent in the keys; 64 characters maximum. As these are purely visual truncate the generated cname strings to 64 characters.

Also drops extraneous framing from kmod keys.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

Looks good, but any chance of a test for the truncation case?

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

> Looks good, but any chance of a test for the truncation case?

Sounds reasonable indeed. For this purpose I have pulled out the common name generation to a shiny new helper. This I have added a range of tests for.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/signing.py'
2--- lib/lp/archivepublisher/signing.py 2016-06-22 08:54:11 +0000
3+++ lib/lp/archivepublisher/signing.py 2017-07-24 10:01:21 +0000
4@@ -193,20 +193,30 @@
5 return [None for k in keynames]
6 return keynames
7
8+ def generateKeyCommonName(self, owner, archive, suffix=''):
9+ # PPA <owner> <archive> <suffix>
10+ # truncate <owner> <archive> to ensure the overall form is shorter
11+ # than 64 characters but the suffix is maintained
12+ if suffix:
13+ suffix = " " + suffix
14+ common_name = "PPA %s %s" % (owner, archive)
15+ return common_name[0:64 - len(suffix)] + suffix
16+
17 def generateUefiKeys(self):
18 """Generate new UEFI Keys for this archive."""
19 directory = os.path.dirname(self.uefi_key)
20 if not os.path.exists(directory):
21 os.makedirs(directory)
22
23- common_name = '/CN=PPA %s %s/' % (
24+ common_name = self.generateKeyCommonName(
25 self.archive.owner.name, self.archive.name)
26+ subject = '/CN=' + common_name + '/'
27
28 old_mask = os.umask(0o077)
29 try:
30 new_key_cmd = [
31 'openssl', 'req', '-new', '-x509', '-newkey', 'rsa:2048',
32- '-subj', common_name, '-keyout', self.uefi_key,
33+ '-subj', subject, '-keyout', self.uefi_key,
34 '-out', self.uefi_cert, '-days', '3650', '-nodes', '-sha256',
35 ]
36 self.callLog("UEFI keygen", new_key_cmd)
37@@ -233,6 +243,10 @@
38 if not os.path.exists(directory):
39 os.makedirs(directory)
40
41+ # Truncate name to 64 character maximum.
42+ common_name = self.generateKeyCommonName(
43+ self.archive.owner.name, self.archive.name, "kmod")
44+
45 old_mask = os.umask(0o077)
46 try:
47 with tempfile.NamedTemporaryFile(suffix='.keygen') as tf:
48@@ -245,14 +259,14 @@
49 x509_extensions = myexts
50
51 [ req_distinguished_name ]
52- CN = /CN=PPA %s %s kmod/
53+ CN = %s
54
55 [ myexts ]
56 basicConstraints=critical,CA:FALSE
57 keyUsage=digitalSignature
58 subjectKeyIdentifier=hash
59 authorityKeyIdentifier=keyid
60- """ % (self.archive.owner.name, self.archive.name))
61+ """ % common_name)
62
63 print(genkey_text, file=tf)
64
65
66=== modified file 'lib/lp/archivepublisher/tests/test_signing.py'
67--- lib/lp/archivepublisher/tests/test_signing.py 2017-04-29 15:24:32 +0000
68+++ lib/lp/archivepublisher/tests/test_signing.py 2017-07-24 10:01:21 +0000
69@@ -250,6 +250,46 @@
70 self.assertEqual(1, upload.callLog.caller_count('UEFI signing'))
71 self.assertEqual(1, upload.callLog.caller_count('Kmod signing'))
72
73+ def test_common_name_plain(self):
74+ upload = SigningUpload()
75+ common_name = upload.generateKeyCommonName('testing-team', 'ppa')
76+ self.assertEqual('PPA testing-team ppa', common_name)
77+
78+ def test_common_name_suffix(self):
79+ upload = SigningUpload()
80+ common_name = upload.generateKeyCommonName(
81+ 'testing-team', 'ppa', 'kmod')
82+ self.assertEqual('PPA testing-team ppa kmod', common_name)
83+
84+ def test_common_name_plain_just_short(self):
85+ upload = SigningUpload()
86+ common_name = upload.generateKeyCommonName('t' * 30, 'p' * 29)
87+ expected_name = 'PPA ' + 't' * 30 + ' ' + 'p' * 29
88+ self.assertEqual(expected_name, common_name)
89+ self.assertEqual(64, len(common_name))
90+
91+ def test_common_name_suffix_just_short(self):
92+ upload = SigningUpload()
93+ common_name = upload.generateKeyCommonName('t' * 30, 'p' * 24, 'kmod')
94+ expected_name = 'PPA ' + 't' * 30 + ' ' + 'p' * 24 + ' kmod'
95+ self.assertEqual(expected_name, common_name)
96+ self.assertEqual(64, len(common_name))
97+
98+ def test_common_name_plain_long(self):
99+ upload = SigningUpload()
100+ common_name = upload.generateKeyCommonName('t' * 40, 'p' * 40)
101+ expected_name = 'PPA ' + 't' * 40 + ' ' + 'p' * 19
102+ self.assertEqual(expected_name, common_name)
103+ self.assertEqual(64, len(common_name))
104+
105+ def test_common_name_suffix_long(self):
106+ upload = SigningUpload()
107+ common_name = upload.generateKeyCommonName(
108+ 't' * 40, 'p' * 40, 'kmod-plus')
109+ expected_name = 'PPA ' + 't' * 40 + ' ' + 'p' * 9 + ' kmod-plus'
110+ self.assertEqual(expected_name, common_name)
111+ self.assertEqual(64, len(common_name))
112+
113 def test_options_handling_none(self):
114 # If the configured key/cert are missing, processing succeeds but
115 # nothing is signed.