launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/lp-signing:escape-common-name into lp-signing:master.

Commit message:
Escape common names in openssl req -subj

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lp-signing/+git/lp-signing/+merge/382598

req(1ssl) says, for -subj:

  The arg must be formatted as "/type0=value0/type1=value1/type2=...".  Keyword characters may be escaped by \ (backslash), and whitespace is retained.

Accordingly, escape any "/" and "=" characters in the common name when building the -subj argument.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lp-signing:escape-common-name into lp-signing:master.

diff --git a/lp_signing/model/key.py b/lp_signing/model/key.py
index 84ddc46..451b3b2 100644
--- a/lp_signing/model/key.py
+++ b/lp_signing/model/key.py
@@ -12,6 +12,7 @@ from contextlib import contextmanager
 import json
 import logging
 from pathlib import Path
+import re
 import shutil
 import subprocess
 from subprocess import CalledProcessError
@@ -251,9 +252,10 @@ class Key(Storm):
         """
         key = tmp / f"{key_type.name.lower()}.key"
         cert = tmp / f"{key_type.name.lower()}.crt"
+        common_name_esc = re.sub(r'([/=])', r'\\\1', common_name)
         _log_subprocess_run([
             "openssl", "req", "-new", "-x509", "-newkey", "rsa:2048",
-            "-subj", f"/CN={common_name}/", "-keyout", str(key),
+            "-subj", f"/CN={common_name_esc}/", "-keyout", str(key),
             "-out", str(cert), "-days", "3650", "-nodes", "-sha256",
             ], check=True)
         return key.read_bytes(), cert.read_bytes()
diff --git a/lp_signing/model/tests/test_key.py b/lp_signing/model/tests/test_key.py
index 48bc737..8b7d1f1 100644
--- a/lp_signing/model/tests/test_key.py
+++ b/lp_signing/model/tests/test_key.py
@@ -104,7 +104,7 @@ class TestKey(TestCase):
         fingerprint = factory.generate_fingerprint()
         fake_openssl = FakeOpenSSL(private_key, public_key, fingerprint)
         self.processes_fixture.add(fake_openssl)
-        key = Key.generate(KeyType.UEFI, "PPA signing-owner testing")
+        key = Key.generate(KeyType.UEFI, "~signing-owner/ubuntu/testing")
         now = get_transaction_timestamp(store)
         self.assertThat(key, MatchesStructure.byEquality(
             key_type=KeyType.UEFI,
@@ -117,7 +117,7 @@ class TestKey(TestCase):
             key, Key.getByTypeAndFingerprint(KeyType.UEFI, fingerprint))
         req_args = [
             "openssl", "req", "-new", "-x509", "-newkey", "rsa:2048",
-            "-subj", "/CN=PPA signing-owner testing UEFI/",
+            "-subj", r"/CN=~signing-owner\/ubuntu\/testing UEFI/",
             "-keyout", EndsWith("uefi.key"), "-out", EndsWith("uefi.crt"),
             "-days", "3650", "-nodes", "-sha256",
             ]
@@ -141,7 +141,7 @@ class TestKey(TestCase):
         fingerprint = factory.generate_fingerprint()
         fake_openssl = FakeOpenSSL(private_key, public_key, fingerprint)
         self.processes_fixture.add(fake_openssl)
-        key = Key.generate(KeyType.KMOD, "PPA signing-owner testing")
+        key = Key.generate(KeyType.KMOD, "~signing-owner/ubuntu/testing")
         now = get_transaction_timestamp(store)
         self.assertThat(key, MatchesStructure.byEquality(
             key_type=KeyType.KMOD,
@@ -154,7 +154,7 @@ class TestKey(TestCase):
             key, Key.getByTypeAndFingerprint(KeyType.KMOD, fingerprint))
         self.assertIn("[ req ]", fake_openssl.keygen_text)
         self.assertThat(fake_openssl.keygen_text, MatchesRegex(
-            r".*\bCN\s*=\s*PPA signing-owner testing\b", flags=re.S))
+            r".*\bCN\s*=\s*~signing-owner/ubuntu/testing\b", flags=re.S))
         self.assertThat(fake_openssl.keygen_text, MatchesRegex(
             r".*\bextendedKeyUsage\s*=\s*"
             r"codeSigning,1.3.6.1.4.1.2312.16.1.2\s*\b", flags=re.S))
@@ -190,7 +190,7 @@ class TestKey(TestCase):
         fingerprint = factory.generate_fingerprint()
         fake_openssl = FakeOpenSSL(private_key, public_key, fingerprint)
         self.processes_fixture.add(fake_openssl)
-        key = Key.generate(KeyType.OPAL, "PPA signing-owner testing")
+        key = Key.generate(KeyType.OPAL, "~signing-owner/ubuntu/testing")
         now = get_transaction_timestamp(store)
         self.assertThat(key, MatchesStructure.byEquality(
             key_type=KeyType.OPAL,
@@ -203,7 +203,7 @@ class TestKey(TestCase):
             key, Key.getByTypeAndFingerprint(KeyType.OPAL, fingerprint))
         self.assertIn("[ req ]", fake_openssl.keygen_text)
         self.assertThat(fake_openssl.keygen_text, MatchesRegex(
-            r".*\bCN\s*=\s*PPA signing-owner testing\b", flags=re.S))
+            r".*\bCN\s*=\s*~signing-owner/ubuntu/testing\b", flags=re.S))
         self.assertNotIn("extendedKeyUsage", fake_openssl.keygen_text)
         req_args = [
             "openssl", "req", "-new", "-nodes", "-utf8", "-sha512",
@@ -237,7 +237,7 @@ class TestKey(TestCase):
         fingerprint = factory.generate_fingerprint()
         fake_openssl = FakeOpenSSL(private_key, public_key, fingerprint)
         self.processes_fixture.add(fake_openssl)
-        key = Key.generate(KeyType.SIPL, "PPA signing-owner testing")
+        key = Key.generate(KeyType.SIPL, "~signing-owner/ubuntu/testing")
         now = get_transaction_timestamp(store)
         self.assertThat(key, MatchesStructure.byEquality(
             key_type=KeyType.SIPL,
@@ -250,7 +250,7 @@ class TestKey(TestCase):
             key, Key.getByTypeAndFingerprint(KeyType.SIPL, fingerprint))
         self.assertIn("[ req ]", fake_openssl.keygen_text)
         self.assertThat(fake_openssl.keygen_text, MatchesRegex(
-            r".*\bCN\s*=\s*PPA signing-owner testing\b", flags=re.S))
+            r".*\bCN\s*=\s*~signing-owner/ubuntu/testing\b", flags=re.S))
         self.assertNotIn("extendedKeyUsage", fake_openssl.keygen_text)
         req_args = [
             "openssl", "req", "-new", "-nodes", "-utf8", "-sha512",
@@ -284,7 +284,7 @@ class TestKey(TestCase):
         fingerprint = factory.generate_fingerprint()
         fake_openssl = FakeOpenSSL(private_key, public_key, fingerprint)
         self.processes_fixture.add(fake_openssl)
-        key = Key.generate(KeyType.FIT, "PPA signing-owner testing")
+        key = Key.generate(KeyType.FIT, "~signing-owner/ubuntu/testing")
         now = get_transaction_timestamp(store)
         self.assertThat(key, MatchesStructure.byEquality(
             key_type=KeyType.FIT,
@@ -297,7 +297,7 @@ class TestKey(TestCase):
             key, Key.getByTypeAndFingerprint(KeyType.FIT, fingerprint))
         req_args = [
             "openssl", "req", "-new", "-x509", "-newkey", "rsa:2048",
-            "-subj", "/CN=PPA signing-owner testing FIT/",
+            "-subj", r"/CN=~signing-owner\/ubuntu\/testing FIT/",
             "-keyout", EndsWith("fit.key"), "-out", EndsWith("fit.crt"),
             "-days", "3650", "-nodes", "-sha256",
             ]