From 66472637ada0f7013c3a6e2ff767a2af5aa3a3b3 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 4 Dec 2017 17:16:36 -0800 Subject: [PATCH] releasetools: Check for duplicate entries in ReplaceCerts(). testdata/{media,platform}.x509.pem files are copied from build/target/product/security/. Fixes: 69479366 Test: python -m unittest test_sign_target_files_apks Change-Id: I8ba42b6f5b5432ee4b8786b241daff11db792c14 --- tools/releasetools/sign_target_files_apks.py | 74 +++++++++++------ .../test_sign_target_files_apks.py | 80 ++++++++++++++++++- tools/releasetools/testdata/media.x509.pem | 27 +++++++ tools/releasetools/testdata/platform.x509.pem | 27 +++++++ 4 files changed, 183 insertions(+), 25 deletions(-) create mode 100644 tools/releasetools/testdata/media.x509.pem create mode 100644 tools/releasetools/testdata/platform.x509.pem diff --git a/tools/releasetools/sign_target_files_apks.py b/tools/releasetools/sign_target_files_apks.py index c96e61658..fa62c8f63 100755 --- a/tools/releasetools/sign_target_files_apks.py +++ b/tools/releasetools/sign_target_files_apks.py @@ -104,6 +104,7 @@ import subprocess import sys import tempfile import zipfile +from xml.etree import ElementTree import add_img_to_target_files import common @@ -290,6 +291,8 @@ def ProcessTargetFiles(input_tf_zip, output_tf_zip, misc_info, new_data = RewriteProps(data) common.ZipWriteStr(output_tf_zip, out_info, new_data) + # Replace the certs in *mac_permissions.xml (there could be multiple, such + # as {system,vendor}/etc/selinux/{plat,nonplat}_mac_permissions.xml). elif info.filename.endswith("mac_permissions.xml"): print("Rewriting %s with new keys." % (info.filename,)) new_data = ReplaceCerts(data) @@ -361,31 +364,54 @@ def ProcessTargetFiles(input_tf_zip, output_tf_zip, misc_info, def ReplaceCerts(data): - """Given a string of data, replace all occurences of a set - of X509 certs with a newer set of X509 certs and return - the updated data string.""" - for old, new in OPTIONS.key_map.iteritems(): - try: - if OPTIONS.verbose: - print(" Replacing %s.x509.pem with %s.x509.pem" % (old, new)) - f = open(old + ".x509.pem") - old_cert16 = base64.b16encode(common.ParseCertificate(f.read())).lower() - f.close() - f = open(new + ".x509.pem") - new_cert16 = base64.b16encode(common.ParseCertificate(f.read())).lower() - f.close() - # Only match entire certs. - pattern = "\\b" + old_cert16 + "\\b" - (data, num) = re.subn(pattern, new_cert16, data, flags=re.IGNORECASE) - if OPTIONS.verbose: - print(" Replaced %d occurence(s) of %s.x509.pem with " - "%s.x509.pem" % (num, old, new)) - except IOError as e: - if e.errno == errno.ENOENT and not OPTIONS.verbose: - continue + """Replaces all the occurences of X.509 certs with the new ones. - print(" Error accessing %s. %s. Skip replacing %s.x509.pem with " - "%s.x509.pem." % (e.filename, e.strerror, old, new)) + The mapping info is read from OPTIONS.key_map. Non-existent certificate will + be skipped. After the replacement, it additionally checks for duplicate + entries, which would otherwise fail the policy loading code in + frameworks/base/services/core/java/com/android/server/pm/SELinuxMMAC.java. + + Args: + data: Input string that contains a set of X.509 certs. + + Returns: + A string after the replacement. + + Raises: + AssertionError: On finding duplicate entries. + """ + for old, new in OPTIONS.key_map.iteritems(): + if OPTIONS.verbose: + print(" Replacing %s.x509.pem with %s.x509.pem" % (old, new)) + + try: + with open(old + ".x509.pem") as old_fp: + old_cert16 = base64.b16encode( + common.ParseCertificate(old_fp.read())).lower() + with open(new + ".x509.pem") as new_fp: + new_cert16 = base64.b16encode( + common.ParseCertificate(new_fp.read())).lower() + except IOError as e: + if OPTIONS.verbose or e.errno != errno.ENOENT: + print(" Error accessing %s: %s.\nSkip replacing %s.x509.pem with " + "%s.x509.pem." % (e.filename, e.strerror, old, new)) + continue + + # Only match entire certs. + pattern = "\\b" + old_cert16 + "\\b" + (data, num) = re.subn(pattern, new_cert16, data, flags=re.IGNORECASE) + + if OPTIONS.verbose: + print(" Replaced %d occurence(s) of %s.x509.pem with %s.x509.pem" % ( + num, old, new)) + + # Verify that there're no duplicate entries after the replacement. Note that + # it's only checking entries with global seinfo at the moment (i.e. ignoring + # the ones with inner packages). (Bug: 69479366) + root = ElementTree.fromstring(data) + signatures = [signer.attrib['signature'] for signer in root.findall('signer')] + assert len(signatures) == len(set(signatures)), \ + "Found duplicate entries after cert replacement: {}".format(data) return data diff --git a/tools/releasetools/test_sign_target_files_apks.py b/tools/releasetools/test_sign_target_files_apks.py index 0cab80195..26f9e10e8 100644 --- a/tools/releasetools/test_sign_target_files_apks.py +++ b/tools/releasetools/test_sign_target_files_apks.py @@ -16,17 +16,25 @@ from __future__ import print_function +import base64 import os.path import unittest import zipfile import common import test_utils -from sign_target_files_apks import EditTags, ReplaceVerityKeyId, RewriteProps +from sign_target_files_apks import ( + EditTags, ReplaceCerts, ReplaceVerityKeyId, RewriteProps) class SignTargetFilesApksTest(unittest.TestCase): + MAC_PERMISSIONS_XML = """ + + + +""" + def setUp(self): self.testdata_dir = test_utils.get_testdata_dir() @@ -133,3 +141,73 @@ class SignTargetFilesApksTest(unittest.TestCase): with zipfile.ZipFile(output_file) as output_zip: self.assertEqual(BOOT_CMDLINE, output_zip.read('BOOT/cmdline')) + + def test_ReplaceCerts(self): + cert1_path = os.path.join(self.testdata_dir, 'platform.x509.pem') + with open(cert1_path) as cert1_fp: + cert1 = cert1_fp.read() + cert2_path = os.path.join(self.testdata_dir, 'media.x509.pem') + with open(cert2_path) as cert2_fp: + cert2 = cert2_fp.read() + cert3_path = os.path.join(self.testdata_dir, 'testkey.x509.pem') + with open(cert3_path) as cert3_fp: + cert3 = cert3_fp.read() + + # Replace cert1 with cert3. + input_xml = self.MAC_PERMISSIONS_XML.format( + base64.b16encode(common.ParseCertificate(cert1)).lower(), + base64.b16encode(common.ParseCertificate(cert2)).lower()) + + output_xml = self.MAC_PERMISSIONS_XML.format( + base64.b16encode(common.ParseCertificate(cert3)).lower(), + base64.b16encode(common.ParseCertificate(cert2)).lower()) + + common.OPTIONS.key_map = { + cert1_path[:-9] : cert3_path[:-9], + } + + self.assertEqual(output_xml, ReplaceCerts(input_xml)) + + def test_ReplaceCerts_duplicateEntries(self): + cert1_path = os.path.join(self.testdata_dir, 'platform.x509.pem') + with open(cert1_path) as cert1_fp: + cert1 = cert1_fp.read() + cert2_path = os.path.join(self.testdata_dir, 'media.x509.pem') + with open(cert2_path) as cert2_fp: + cert2 = cert2_fp.read() + + # Replace cert1 with cert2, which leads to duplicate entries. + input_xml = self.MAC_PERMISSIONS_XML.format( + base64.b16encode(common.ParseCertificate(cert1)).lower(), + base64.b16encode(common.ParseCertificate(cert2)).lower()) + + common.OPTIONS.key_map = { + cert1_path[:-9] : cert2_path[:-9], + } + self.assertRaises(AssertionError, ReplaceCerts, input_xml) + + def test_ReplaceCerts_skipNonExistentCerts(self): + cert1_path = os.path.join(self.testdata_dir, 'platform.x509.pem') + with open(cert1_path) as cert1_fp: + cert1 = cert1_fp.read() + cert2_path = os.path.join(self.testdata_dir, 'media.x509.pem') + with open(cert2_path) as cert2_fp: + cert2 = cert2_fp.read() + cert3_path = os.path.join(self.testdata_dir, 'testkey.x509.pem') + with open(cert3_path) as cert3_fp: + cert3 = cert3_fp.read() + + input_xml = self.MAC_PERMISSIONS_XML.format( + base64.b16encode(common.ParseCertificate(cert1)).lower(), + base64.b16encode(common.ParseCertificate(cert2)).lower()) + + output_xml = self.MAC_PERMISSIONS_XML.format( + base64.b16encode(common.ParseCertificate(cert3)).lower(), + base64.b16encode(common.ParseCertificate(cert2)).lower()) + + common.OPTIONS.key_map = { + cert1_path[:-9] : cert3_path[:-9], + 'non-existent' : cert3_path[:-9], + cert2_path[:-9] : 'non-existent', + } + self.assertEqual(output_xml, ReplaceCerts(input_xml)) diff --git a/tools/releasetools/testdata/media.x509.pem b/tools/releasetools/testdata/media.x509.pem new file mode 100644 index 000000000..98cd443db --- /dev/null +++ b/tools/releasetools/testdata/media.x509.pem @@ -0,0 +1,27 @@ +-----BEGIN CERTIFICATE----- +MIIEqDCCA5CgAwIBAgIJAPK5jmEjVyxOMA0GCSqGSIb3DQEBBAUAMIGUMQswCQYD +VQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4g +VmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UE +AxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTAe +Fw0wODA0MTUyMzQwNTdaFw0zNTA5MDEyMzQwNTdaMIGUMQswCQYDVQQGEwJVUzET +MBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4G +A1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9p +ZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTCCASAwDQYJKoZI +hvcNAQEBBQADggENADCCAQgCggEBAK4lDFoW75f8KGmsZRsyF8w2ug6GlkFo1YoE +n0DOhYZxI6P/tPbZScM88to6BcI+rKpX2AOImxdZvPWefG8hiQriUIW37VaqYmwJ +ie+czTY2LKDo0blgP9TYModnkmzMCQxot3Wuf/MJNMw2nvKFWiZn3wxmf9DHz12O +umVYBnNzA7tiRybquu37cvB+16dqs8uaOBxLfc2AmxQNiR8AITvkAfWNagamHq3D +qcLxxlZyhbCa4JNCpm+kIer5Ot91c6AowzHXBgGrOvfMhAM+znx3KjpbhrDb6dd3 +w6SKqYAe3O4ngVifRNnkETl5YAV2qZQQuoEJElna2YxsaP94S48CAQOjgfwwgfkw +HQYDVR0OBBYEFMopPKqLwO0+VC7vQgWiv/K1fk11MIHJBgNVHSMEgcEwgb6AFMop +PKqLwO0+VC7vQgWiv/K1fk11oYGapIGXMIGUMQswCQYDVQQGEwJVUzETMBEGA1UE +CBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMH +QW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAG +CSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbYIJAPK5jmEjVyxOMAwGA1Ud +EwQFMAMBAf8wDQYJKoZIhvcNAQEEBQADggEBAITelRbV5KhyF6c9qEhwSPUzc6X3 +M/OQ1hvfPMnlJRYlv8qnwxWcriddFyqa4eh21UWBJ6xUL2gpDdUQwAKdj1Hg7hVr +e3tazbOUJBuOx4t05cQsXK+uFWyvW9GZojonUk2gct6743hGSlM2MLDk0P+34I7L +cB+ttjecdEZ/bgDG7YiFlTgHkgOHVgB4csjjAHr0I6V6LKs6KChptkxLe9X8GH0K +fiQVll1ark4Hpt91G0p16Xk8kYphK4HNC2KK7gFo3ETkexDTWTJghJ1q321yfcJE +RMIh0/nsw2jK0HmZ8rgQW8HyDTjUEGbMFBHCV6lupDSfV0ZWVQfk6AIKGoE= +-----END CERTIFICATE----- diff --git a/tools/releasetools/testdata/platform.x509.pem b/tools/releasetools/testdata/platform.x509.pem new file mode 100644 index 000000000..087f02e6a --- /dev/null +++ b/tools/releasetools/testdata/platform.x509.pem @@ -0,0 +1,27 @@ +-----BEGIN CERTIFICATE----- +MIIEqDCCA5CgAwIBAgIJALOZgIbQVs/6MA0GCSqGSIb3DQEBBAUAMIGUMQswCQYD +VQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4g +VmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UE +AxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTAe +Fw0wODA0MTUyMjQwNTBaFw0zNTA5MDEyMjQwNTBaMIGUMQswCQYDVQQGEwJVUzET +MBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4G +A1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9p +ZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTCCASAwDQYJKoZI +hvcNAQEBBQADggENADCCAQgCggEBAJx4BZKsDV04HN6qZezIpgBuNkgMbXIHsSAR +vlCGOqvitV0Amt9xRtbyICKAx81Ne9smJDuKgGwms0sTdSOkkmgiSQTcAUk+fArP +GgXIdPabA3tgMJ2QdNJCgOFrrSqHNDYZUer3KkgtCbIEsYdeEqyYwap3PWgAuer9 +5W1Yvtjo2hb5o2AJnDeoNKbf7be2tEoEngeiafzPLFSW8s821k35CjuNjzSjuqtM +9TNxqydxmzulh1StDFP8FOHbRdUeI0+76TybpO35zlQmE1DsU1YHv2mi/0qgfbX3 +6iANCabBtJ4hQC+J7RGQiTqrWpGA8VLoL4WkV1PPX8GQccXuyCcCAQOjgfwwgfkw +HQYDVR0OBBYEFE/koLPdnLop9x1yh8Tnw48ghsKZMIHJBgNVHSMEgcEwgb6AFE/k +oLPdnLop9x1yh8Tnw48ghsKZoYGapIGXMIGUMQswCQYDVQQGEwJVUzETMBEGA1UE +CBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMH +QW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAG +CSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbYIJALOZgIbQVs/6MAwGA1Ud +EwQFMAMBAf8wDQYJKoZIhvcNAQEEBQADggEBAFclUbjZOh9z3g9tRp+G2tZwFAAp +PIigzXzXeLc9r8wZf6t25iEuVsHHYc/EL9cz3lLFCuCIFM78CjtaGkNGBU2Cnx2C +tCsgSL+ItdFJKe+F9g7dEtctVWV+IuPoXQTIMdYT0Zk4u4mCJH+jISVroS0dao+S +6h2xw3Mxe6DAN/DRr/ZFrvIkl5+6bnoUvAJccbmBOM7z3fwFlhfPJIRc97QNY4L3 +J17XOElatuWTG5QhdlxJG3L7aOCA29tYwgKdNHyLMozkPvaosVUz7fvpib1qSN1L +IC7alMarjdW4OZID2q4u1EYjLk/pvZYTlMYwDlE448/Shebk5INTjLixs1c= +-----END CERTIFICATE-----