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

[Andy Whitcroft]

signing: truncate signing key common-names to 64 characters

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.

[Colin Watson (cjwatson)]

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

review: Approve

[Andy Whitcroft (apw)]

> 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.

=== modified file 'lib/lp/archivepublisher/signing.py'
--- lib/lp/archivepublisher/signing.py	2016-06-22 08:54:11 +0000
+++ lib/lp/archivepublisher/signing.py	2017-07-24 10:01:21 +0000
@@ -193,20 +193,30 @@
             return [None for k in keynames]
         return keynames
 
+    def generateKeyCommonName(self, owner, archive, suffix=''):
+        # PPA <owner> <archive> <suffix>
+        # truncate <owner> <archive> to ensure the overall form is shorter
+        # than 64 characters but the suffix is maintained
+        if suffix:
+            suffix = " " + suffix
+        common_name = "PPA %s %s" % (owner, archive)
+        return common_name[0:64 - len(suffix)] + suffix
+
     def generateUefiKeys(self):
         """Generate new UEFI Keys for this archive."""
         directory = os.path.dirname(self.uefi_key)
         if not os.path.exists(directory):
             os.makedirs(directory)
 
-        common_name = '/CN=PPA %s %s/' % (
+        common_name = self.generateKeyCommonName(
             self.archive.owner.name, self.archive.name)
+        subject = '/CN=' + common_name + '/'
 
         old_mask = os.umask(0o077)
         try:
             new_key_cmd = [
                 'openssl', 'req', '-new', '-x509', '-newkey', 'rsa:2048',
-                '-subj', common_name, '-keyout', self.uefi_key,
+                '-subj', subject, '-keyout', self.uefi_key,
                 '-out', self.uefi_cert, '-days', '3650', '-nodes', '-sha256',
                 ]
             self.callLog("UEFI keygen", new_key_cmd)
@@ -233,6 +243,10 @@
         if not os.path.exists(directory):
             os.makedirs(directory)
 
+        # Truncate name to 64 character maximum.
+        common_name = self.generateKeyCommonName(
+            self.archive.owner.name, self.archive.name, "kmod")
+
         old_mask = os.umask(0o077)
         try:
             with tempfile.NamedTemporaryFile(suffix='.keygen') as tf:
@@ -245,14 +259,14 @@
                     x509_extensions = myexts
 
                     [ req_distinguished_name ]
-                    CN = /CN=PPA %s %s kmod/
+                    CN = %s
 
                     [ myexts ]
                     basicConstraints=critical,CA:FALSE
                     keyUsage=digitalSignature
                     subjectKeyIdentifier=hash
                     authorityKeyIdentifier=keyid
-                    """ % (self.archive.owner.name, self.archive.name))
+                    """ % common_name)
 
                 print(genkey_text, file=tf)
 

=== modified file 'lib/lp/archivepublisher/tests/test_signing.py'
--- lib/lp/archivepublisher/tests/test_signing.py	2017-04-29 15:24:32 +0000
+++ lib/lp/archivepublisher/tests/test_signing.py	2017-07-24 10:01:21 +0000
@@ -250,6 +250,46 @@
         self.assertEqual(1, upload.callLog.caller_count('UEFI signing'))
         self.assertEqual(1, upload.callLog.caller_count('Kmod signing'))
 
+    def test_common_name_plain(self):
+        upload = SigningUpload()
+        common_name = upload.generateKeyCommonName('testing-team', 'ppa')
+        self.assertEqual('PPA testing-team ppa', common_name)
+
+    def test_common_name_suffix(self):
+        upload = SigningUpload()
+        common_name = upload.generateKeyCommonName(
+            'testing-team', 'ppa', 'kmod')
+        self.assertEqual('PPA testing-team ppa kmod', common_name)
+
+    def test_common_name_plain_just_short(self):
+        upload = SigningUpload()
+        common_name = upload.generateKeyCommonName('t' * 30, 'p' * 29)
+        expected_name = 'PPA ' + 't' * 30 + ' ' + 'p' * 29
+        self.assertEqual(expected_name, common_name)
+        self.assertEqual(64, len(common_name))
+
+    def test_common_name_suffix_just_short(self):
+        upload = SigningUpload()
+        common_name = upload.generateKeyCommonName('t' * 30, 'p' * 24, 'kmod')
+        expected_name = 'PPA ' + 't' * 30 + ' ' + 'p' * 24 + ' kmod'
+        self.assertEqual(expected_name, common_name)
+        self.assertEqual(64, len(common_name))
+
+    def test_common_name_plain_long(self):
+        upload = SigningUpload()
+        common_name = upload.generateKeyCommonName('t' * 40, 'p' * 40)
+        expected_name = 'PPA ' + 't' * 40 + ' ' + 'p' * 19
+        self.assertEqual(expected_name, common_name)
+        self.assertEqual(64, len(common_name))
+
+    def test_common_name_suffix_long(self):
+        upload = SigningUpload()
+        common_name = upload.generateKeyCommonName(
+            't' * 40, 'p' * 40, 'kmod-plus')
+        expected_name = 'PPA ' + 't' * 40 + ' ' + 'p' * 9 + ' kmod-plus'
+        self.assertEqual(expected_name, common_name)
+        self.assertEqual(64, len(common_name))
+
     def test_options_handling_none(self):
         # If the configured key/cert are missing, processing succeeds but
         # nothing is signed.