releasetools: Fix an issue with pubkey extraction.

When calling 'openssl x509 -pubkey' to extract the public key from a
certificate, openssl 1.0 and 1.1 handle the '-out' parameter
differently. openssl 1.0 doesn't write the output into the specified
filename, which leads to the payload verification failure in
check_ota_package_signature.VerifyAbOtaPayload(). This CL addresses
the issue by always collecting the output from stdout instead.

It also refactors the two copies into common.ExtractPublicKey(), and
adds unittest. get_testdata_dir() is moved into test_utils.py that holds
common utils for running the unittests.

Bug: 72884343
Test: python -m unittest test_common
Test: python -m unittest test_ota_from_target_files
Test: Run sign_target_files_apks with '--replace_ota_keys' on marlin
      target_files zip. Check the payload pubkey replacement.
Test: Trigger the tests with forrest, and tests no longer fail on
      machines with openssl 1.0.1.
Change-Id: Ib0389b360f064053e9aa7cc0546d718e7b23003b

tools/releasetools/check_ota_package_signature.py

@@ -154,15 +154,11 @@ def VerifyAbOtaPayload(cert, package):
   print('Verifying A/B OTA payload signatures...')
 
   # Dump pubkey from the certificate.
-  pubkey = common.MakeTempFile(prefix="key-", suffix=".key")
+  pubkey = common.MakeTempFile(prefix="key-", suffix=".pem")
-  cmd = ['openssl', 'x509', '-pubkey', '-noout', '-in', cert, '-out', pubkey]
+  with open(pubkey, 'wb') as pubkey_fp:
-  proc = common.Run(cmd, stdout=subprocess.PIPE)
+    pubkey_fp.write(common.ExtractPublicKey(cert))
-  stdoutdata, _ = proc.communicate()
-  assert proc.returncode == 0, \
-      'Failed to dump public key from certificate: %s\n%s' % (cert, stdoutdata)
 
-  package_dir = tempfile.mkdtemp(prefix='package-')
+  package_dir = common.MakeTempDir(prefix='package-')
-  common.OPTIONS.tempfiles.append(package_dir)
 
   # Signature verification with delta_generator.
   payload_file = package_zip.extract('payload.bin', package_dir)

tools/releasetools/common.py

@@ -1804,6 +1804,31 @@ def ParseCertificate(data):
   cert = "".join(cert).decode('base64')
   return cert
 
+
+def ExtractPublicKey(cert):
+  """Extracts the public key (PEM-encoded) from the given certificate file.
+
+  Args:
+    cert: The certificate filename.
+
+  Returns:
+    The public key string.
+
+  Raises:
+    AssertionError: On non-zero return from 'openssl'.
+  """
+  # The behavior with '-out' is different between openssl 1.1 and openssl 1.0.
+  # While openssl 1.1 writes the key into the given filename followed by '-out',
+  # openssl 1.0 (both of 1.0.1 and 1.0.2) doesn't. So we collect the output from
+  # stdout instead.
+  cmd = ['openssl', 'x509', '-pubkey', '-noout', '-in', cert]
+  proc = Run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+  pubkey, stderrdata = proc.communicate()
+  assert proc.returncode == 0, \
+      'Failed to dump public key from certificate: %s\n%s' % (cert, stderrdata)
+  return pubkey
+
+
 def MakeRecoveryPatch(input_dir, output_sink, recovery_img, boot_img,
                       info_dict=None):
   """Generate a binary patch that creates the recovery image starting

tools/releasetools/sign_target_files_apks.py

@@ -538,10 +538,7 @@ def ReplaceOtaKeys(input_tf_zip, output_tf_zip, misc_info):
             " as payload verification key.\n\n")
 
     print("Using %s for payload verification." % (mapped_keys[0],))
-    cmd = common.Run(
+    pubkey = common.ExtractPublicKey(mapped_keys[0])
-        ["openssl", "x509", "-pubkey", "-noout", "-in", mapped_keys[0]],
-        stdout=subprocess.PIPE)
-    pubkey, _ = cmd.communicate()
     common.ZipWriteStr(
         output_tf_zip,
         "SYSTEM/etc/update_engine/update-payload-key.pub.pem",

tools/releasetools/test_common.py

@@ -21,8 +21,10 @@ import zipfile
 from hashlib import sha1
 
 import common
+import test_utils
 import validate_target_files
 
+
 KiB = 1024
 MiB = 1024 * KiB
 GiB = 1024 * MiB
@@ -474,6 +476,18 @@ class CommonApkUtilsTest(unittest.TestCase):
     with zipfile.ZipFile(target_files, 'r') as input_zip:
       self.assertRaises(ValueError, common.ReadApkCerts, input_zip)
 
+  def test_ExtractPublicKey(self):
+    testdata_dir = test_utils.get_testdata_dir()
+    cert = os.path.join(testdata_dir, 'testkey.x509.pem')
+    pubkey = os.path.join(testdata_dir, 'testkey.pubkey.pem')
+    with open(pubkey, 'rb') as pubkey_fp:
+      self.assertEqual(pubkey_fp.read(), common.ExtractPublicKey(cert))
+
+  def test_ExtractPublicKey_invalidInput(self):
+    testdata_dir = test_utils.get_testdata_dir()
+    wrong_input = os.path.join(testdata_dir, 'testkey.pk8')
+    self.assertRaises(AssertionError, common.ExtractPublicKey, wrong_input)
+
 
 class InstallRecoveryScriptFormatTest(unittest.TestCase):
   """Checks the format of install-recovery.sh.

tools/releasetools/test_ota_from_target_files.py

@@ -21,18 +21,12 @@ import unittest
 import zipfile
 
 import common
+import test_utils
 from ota_from_target_files import (
     _LoadOemDicts, BuildInfo, GetPackageMetadata, Payload, PayloadSigner,
     WriteFingerprintAssertion)
 
 
-def get_testdata_dir():
-  """Returns the testdata dir, in relative to the script dir."""
-  # The script dir is the one we want, which could be different from pwd.
-  current_dir = os.path.dirname(os.path.realpath(__file__))
-  return os.path.join(current_dir, 'testdata')
-
-
 class MockScriptWriter(object):
   """A class that mocks edify_generator.EdifyGenerator.
 
@@ -495,7 +489,7 @@ class PayloadSignerTest(unittest.TestCase):
   SIGNED_SIGFILE = 'signed-sigfile.bin'
 
   def setUp(self):
-    self.testdata_dir = get_testdata_dir()
+    self.testdata_dir = test_utils.get_testdata_dir()
     self.assertTrue(os.path.exists(self.testdata_dir))
 
     common.OPTIONS.payload_signer = None
@@ -571,7 +565,7 @@ class PayloadSignerTest(unittest.TestCase):
 class PayloadTest(unittest.TestCase):
 
   def setUp(self):
-    self.testdata_dir = get_testdata_dir()
+    self.testdata_dir = test_utils.get_testdata_dir()
     self.assertTrue(os.path.exists(self.testdata_dir))
 
     common.OPTIONS.wipe_user_data = False

tools/releasetools/test_utils.py

@@ -0,0 +1,28 @@
+#
+# Copyright (C) 2018 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""
+Utils for running unittests.
+"""
+
+import os.path
+
+
+def get_testdata_dir():
+  """Returns the testdata dir, in relative to the script dir."""
+  # The script dir is the one we want, which could be different from pwd.
+  current_dir = os.path.dirname(os.path.realpath(__file__))
+  return os.path.join(current_dir, 'testdata')

tools/releasetools/testdata/testkey.pubkey.pem

@@ -0,0 +1,9 @@
+-----BEGIN PUBLIC KEY-----
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvjvyO2LwWgmQNyq7z+xK
+04eg0t3AL4y2NhpAAOzVnFyCArFcFjLTGQDDvkbZP6N12O6+dwJoPLntnm9A+VnP
+IFFRHg0HUWSbHM+Qk8Jgv2/2AVkAUj5J1r9t4X+2WI0eRzJP15Zjn68pQKGmcyci
+ry0gbvmYvXL2ZUmTm56DmEfCUCRIY2IGJ/CcMnFeItVU0LxKsV5Mlt5BO0Vv/CV4
+EaiOLwyCnoZuUhYto7dHlO/47v/H9zhkJC54OA1dkD38EPgO5GnfhGFSNXQRmJDT
+XrFgd6O+QO4yUNX8lYP10MzimUpItZa05t68NADqwYl3T7nWzvuC9r4IqZDyPf21
+TQIDAQAB
+-----END PUBLIC KEY-----