launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/lp-signing:add-android-kernel into lp-signing:master with ~ilasc/lp-signing:add-android-kernel as a prerequisite.

Commit message:
Sign Android kernel boot images

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This is just a couple of small fixes on top of https://code.launchpad.net/~ilasc/lp-signing/+git/lp-signing/+merge/404686, since Ioana's out for the week and we want to keep this moving.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lp-signing:add-android-kernel into lp-signing:master.

diff --git a/lp_signing/model/tests/test_key.py b/lp_signing/model/tests/test_key.py
index 7ce5ceb..7f7f743 100644
--- a/lp_signing/model/tests/test_key.py
+++ b/lp_signing/model/tests/test_key.py
@@ -416,9 +416,12 @@ class TestKey(TestCase):
 
     def test_generate_android_kernel(self):
         private_key = factory.generate_random_bytes(size=64)
+        private_key_pkcs1 = factory.generate_random_bytes(size=64)
         public_key = factory.generate_random_bytes(size=64)
         fingerprint = hashlib.sha1(public_key).hexdigest().upper()
-        fake_openssl = FakeOpenSSL(private_key, public_key, fingerprint)
+        fake_openssl = FakeOpenSSL(
+            private_key, public_key, fingerprint,
+            private_key_pkcs1=private_key_pkcs1)
         self.processes_fixture.add(fake_openssl)
         key = Key.generate(
             KeyType.ANDROID_KERNEL,
@@ -430,33 +433,33 @@ class TestKey(TestCase):
             public_key=public_key,
             created_at=now,
             updated_at=now))
-        self.assertEqual(private_key, key.getPrivateKey())
+        self.assertEqual(private_key_pkcs1, key.getPrivateKey())
         self.assertEqual(
             key, Key.getByTypeAndFingerprint(
                 KeyType.ANDROID_KERNEL, fingerprint))
 
-        genpkey_args = [
+        req_args = [
             "openssl", "req", "-new", "-x509", "-newkey", "rsa:2048",
             "-subj", r"/CN=~signing-owner\/ubuntu\/testing Android Kernel/",
             "-keyout", EndsWith("android_kernel.key"),
             "-out", EndsWith("android_kernel.crt"), "-days", "10956",
             "-nodes", "-sha256",
             ]
-
-        pkey_args = [
+        rsa_args = [
             "openssl", "rsa", "-in", EndsWith("android_kernel.key"),
             "-out", EndsWith("android_kernel.key"),
             ]
-        pkey_der_args = ["openssl", "x509", "-inform", "PEM",
-                         "-noout", "-fingerprint"]
+        x509_args = [
+            "openssl", "x509", "-inform", "PEM", "-noout", "-fingerprint",
+            ]
 
         self.assertThat(
             self.processes_fixture.procs,
             MatchesListwise([
-                RanCommand(genpkey_args, stdin=Is(None)),
-                RanCommand(pkey_args, stdin=Is(None)),
+                RanCommand(req_args, stdin=Is(None)),
+                RanCommand(rsa_args, stdin=Is(None)),
                 RanCommand(
-                    pkey_der_args,
+                    x509_args,
                     stdin=AfterPreprocessing(
                         lambda f: f.getvalue(),
                         Equals(public_key))),
diff --git a/lp_signing/tests/test_webapi.py b/lp_signing/tests/test_webapi.py
index c53decd..468d91f 100644
--- a/lp_signing/tests/test_webapi.py
+++ b/lp_signing/tests/test_webapi.py
@@ -767,7 +767,7 @@ class TestGenerateView(TestCase):
         self.assertThat(resp, HasAPIError(MatchesRegex(error_re), 500))
         self.assertNonceConsumed()
 
-    def test_generate_android_kernel_fingerprint_error(self):
+    def test_generate_android_kernel_rsa_error(self):
         processes_fixture = self.useFixture(FakeProcesses())
         private_key = factory.generate_random_bytes(size=64)
         public_key = factory.generate_random_bytes(size=64)
@@ -779,22 +779,27 @@ class TestGenerateView(TestCase):
                 "description": "PPA test-owner test-archive",
                 })
         error_re = (
-            r"Failed to get fingerprint of new key: "
-            r"Command .*'-fingerprint'.* returned non-zero exit status 1")
+            r"Failed to generate key: "
+            r"Command .*'openssl', 'rsa'.* returned non-zero exit status 1")
         self.assertThat(resp, HasAPIError(MatchesRegex(error_re), 500))
         self.assertNonceConsumed()
 
-    def test_generate_android_kernel_genpkey_error(self):
+    def test_generate_android_kernel_fingerprint_error(self):
         processes_fixture = self.useFixture(FakeProcesses())
-        processes_fixture.add(lambda _: {"returncode": 1}, name="openssl")
-        resp = self.post_generate({
-            "key-type": "ANDROID_KERNEL",
-            "description": "PPA test-owner test-archive",
-            })
+        private_key = factory.generate_random_bytes(size=64)
+        private_key_pkcs1 = factory.generate_random_bytes(size=64)
+        public_key = factory.generate_random_bytes(size=64)
+        fake_openssl = FakeOpenSSL(
+            private_key, public_key, None, private_key_pkcs1=private_key_pkcs1)
+        processes_fixture.add(fake_openssl)
+        resp = self.post_generate(
+            {
+                "key-type": "ANDROID_KERNEL",
+                "description": "PPA test-owner test-archive",
+                })
         error_re = (
-            r"Failed to generate key: "
-            r"Command .*'req', '-new'.* returned non-zero exit status "
-            r"1")
+            r"Failed to get fingerprint of new key: "
+            r"Command .*'-fingerprint'.* returned non-zero exit status 1")
         self.assertThat(resp, HasAPIError(MatchesRegex(error_re), 500))
         self.assertNonceConsumed()
 
diff --git a/lp_signing/tests/testfixtures.py b/lp_signing/tests/testfixtures.py
index c012c42..766772a 100644
--- a/lp_signing/tests/testfixtures.py
+++ b/lp_signing/tests/testfixtures.py
@@ -173,10 +173,12 @@ class FakeOpenSSL:
 
     name = "openssl"
 
-    def __init__(self, private_key, public_key, fingerprint):
+    def __init__(self, private_key, public_key, fingerprint,
+                 private_key_pkcs1=None):
         self.private_key = private_key
         self.public_key = public_key
         self.fingerprint = fingerprint
+        self.private_key_pkcs1 = private_key_pkcs1
         self.keygen_text = None
 
     def __call__(self, proc_args):
@@ -196,6 +198,13 @@ class FakeOpenSSL:
                         args[args.index("-outform") + 1] != "PEM"):
                     cert_path = args[args.index("-out") + 1]
                     Path(cert_path).write_bytes(self.public_key)
+        elif args[1] == "rsa":
+            if "-out" in args:
+                if self.private_key_pkcs1 is not None:
+                    key_pkcs1_path = args[args.index("-out") + 1]
+                    Path(key_pkcs1_path).write_bytes(self.private_key_pkcs1)
+                else:
+                    info["returncode"] = 1
         elif args[1] == "x509":
             if "-out" in args:
                 cert_path = args[args.index("-out") + 1]