From 5c7b034a5ce521d5e3834c839292aa5352c811d0 Mon Sep 17 00:00:00 2001 From: Bill Peckham Date: Fri, 3 Apr 2020 15:36:23 -0700 Subject: [PATCH] Make the `partition=` tag optional. Since we might use ToT release tools to sign a package generated by an older build, we make the new `partition=` tag optional. This also means we need to be careful to use non-greedy regex matching. Bug: 153133823 Test: python3 -m unittest Test: input with and without the new `partition=` tag Test: new test_ReadApkCerts_WithWithoutOptionalFields Change-Id: Ic57efd34e745ad302ae17150c6f2318f0b4524cb --- tools/releasetools/common.py | 4 +-- tools/releasetools/merge_target_files.py | 15 ++++++----- tools/releasetools/sign_target_files_apks.py | 4 +-- tools/releasetools/test_common.py | 27 +++++++++++++++++++ .../test_sign_target_files_apks.py | 6 ++++- 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/tools/releasetools/common.py b/tools/releasetools/common.py index 9ea3ea8f1..dcf470a81 100644 --- a/tools/releasetools/common.py +++ b/tools/releasetools/common.py @@ -1786,8 +1786,8 @@ def ReadApkCerts(tf_zip): continue m = re.match( r'^name="(?P.*)"\s+certificate="(?P.*)"\s+' - r'private_key="(?P.*?)"(\s+compressed="(?P.*)")?' - r'(\s+partition="(?P.*)")?$', + r'private_key="(?P.*?)"(\s+compressed="(?P.*?)")?' + r'(\s+partition="(?P.*?)")?$', line) if not m: continue diff --git a/tools/releasetools/merge_target_files.py b/tools/releasetools/merge_target_files.py index 8e9750987..dd7806bde 100755 --- a/tools/releasetools/merge_target_files.py +++ b/tools/releasetools/merge_target_files.py @@ -117,14 +117,15 @@ OPTIONS.keep_tmp = False PARTITION_ITEM_PATTERN = re.compile(r'^([A-Z_]+)/\*$') -# In apexkeys.txt or apkcerts.txt, we may find partition tags on the various -# entries in the file. We use these partition tags to filter the entries in -# those files from the two different target files packages to produce a merged -# apexkeys.txt or apkcerts.txt file. A partition tag (e.g., for the product -# partition) looks like this: 'partition="_PRODUCT"' or 'partition="product". -# We use the group syntax grab the value of the tag. +# In apexkeys.txt or apkcerts.txt, we will find partition tags on each entry in +# the file. We use these partition tags to filter the entries in those files +# from the two different target files packages to produce a merged apexkeys.txt +# or apkcerts.txt file. A partition tag (e.g., for the product partition) looks +# like this: 'partition="product"'. We use the group syntax grab the value of +# the tag. We use non-greedy matching in case there are other fields on the +# same line. -PARTITION_TAG_PATTERN = re.compile(r'partition="(.*)"') +PARTITION_TAG_PATTERN = re.compile(r'partition="(.*?)"') # The sorting algorithm for apexkeys.txt and apkcerts.txt does not include the # ".apex" or ".apk" suffix, so we use the following pattern to extract a key. diff --git a/tools/releasetools/sign_target_files_apks.py b/tools/releasetools/sign_target_files_apks.py index 783d63cb9..52cd9a850 100755 --- a/tools/releasetools/sign_target_files_apks.py +++ b/tools/releasetools/sign_target_files_apks.py @@ -1082,8 +1082,8 @@ def ReadApexKeysInfo(tf_zip): r'public_key="(?P.*)"\s+' r'private_key="(?P.*)"\s+' r'container_certificate="(?P.*)"\s+' - r'container_private_key="(?P.*)"\s+' - r'partition="(?P.*)"$', + r'container_private_key="(?P.*?)"' + r'(\s+partition="(?P.*?)")?$', line) if not matches: continue diff --git a/tools/releasetools/test_common.py b/tools/releasetools/test_common.py index 9195e4943..7058da744 100644 --- a/tools/releasetools/test_common.py +++ b/tools/releasetools/test_common.py @@ -710,6 +710,25 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase): 'Compressed4.apk' : 'certs/compressed4', } + # Test parsing with no optional fields, both optional fields, and only the + # partition optional field. + APKCERTS_TXT4 = ( + 'name="RecoveryLocalizer.apk" certificate="certs/devkey.x509.pem"' + ' private_key="certs/devkey.pk8"\n' + 'name="Settings.apk"' + ' certificate="build/make/target/product/security/platform.x509.pem"' + ' private_key="build/make/target/product/security/platform.pk8"' + ' compressed="gz" partition="system"\n' + 'name="TV.apk" certificate="PRESIGNED" private_key=""' + ' partition="product"\n' + ) + + APKCERTS_CERTMAP4 = { + 'RecoveryLocalizer.apk' : 'certs/devkey', + 'Settings.apk' : 'build/make/target/product/security/platform', + 'TV.apk' : 'PRESIGNED', + } + def setUp(self): self.testdata_dir = test_utils.get_testdata_dir() @@ -786,6 +805,14 @@ class CommonApkUtilsTest(test_utils.ReleaseToolsTestCase): with zipfile.ZipFile(target_files, 'r') as input_zip: self.assertRaises(ValueError, common.ReadApkCerts, input_zip) + def test_ReadApkCerts_WithWithoutOptionalFields(self): + target_files = self._write_apkcerts_txt(self.APKCERTS_TXT4) + with zipfile.ZipFile(target_files, 'r') as input_zip: + certmap, ext = common.ReadApkCerts(input_zip) + + self.assertDictEqual(self.APKCERTS_CERTMAP4, certmap) + self.assertIsNone(ext) + def test_ExtractPublicKey(self): cert = os.path.join(self.testdata_dir, 'testkey.x509.pem') pubkey = os.path.join(self.testdata_dir, 'testkey.pubkey.pem') diff --git a/tools/releasetools/test_sign_target_files_apks.py b/tools/releasetools/test_sign_target_files_apks.py index 2dacd5037..308172f35 100644 --- a/tools/releasetools/test_sign_target_files_apks.py +++ b/tools/releasetools/test_sign_target_files_apks.py @@ -35,9 +35,13 @@ class SignTargetFilesApksTest(test_utils.ReleaseToolsTestCase): """ + # Note that we test one apex with the partition tag, and another without to + # make sure that new OTA tools can process an older target files package that + # does not include the partition tag. + # pylint: disable=line-too-long APEX_KEYS_TXT = """name="apex.apexd_test.apex" public_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package.avbpubkey" private_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package.pem" container_certificate="build/make/target/product/security/testkey.x509.pem" container_private_key="build/make/target/product/security/testkey.pk8" partition="system" -name="apex.apexd_test_different_app.apex" public_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.avbpubkey" private_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.pem" container_certificate="build/make/target/product/security/testkey.x509.pem" container_private_key="build/make/target/product/security/testkey.pk8" partition="system" +name="apex.apexd_test_different_app.apex" public_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.avbpubkey" private_key="system/apex/apexd/apexd_testdata/com.android.apex.test_package_2.pem" container_certificate="build/make/target/product/security/testkey.x509.pem" container_private_key="build/make/target/product/security/testkey.pk8" """ def setUp(self):