reliable implementation of get_first_signer_certificate()

This adds back some pieces of @linsui's algorithm, specifically the check that all certificates are the same. apksigner also does this check. closes #1128
2024-04-19 19:26:18 +02:00 · 2024-04-19 19:26:18 +02:00 · 9ff7b2402c
parent fc0201525e
commit 9ff7b2402c
4 changed files with 259 additions and 11 deletions
--- a/fdroidserver/common.py
+++ b/fdroidserver/common.py
@ -3147,6 +3147,10 @@ def signer_fingerprint(cert_encoded):
 def get_first_signer_certificate(apkpath):
    """Get the first signing certificate from the APK, DER-encoded.

+    JAR and APK Signatures allow for multiple signers, though it is
+    rarely used, and this is poorly documented.  So this method only
+    fetches the first certificate, and errors out if there are more.
+
    Starting with SDK 30, APK v2 Signatures are required.
    https://developer.android.com/about/versions/11/behavior-changes-11#minimum-signature-scheme

@ -3162,6 +3166,9 @@ def get_first_signer_certificate(apkpath):
    NoOverwriteDict is a workaround for:
    https://github.com/androguard/androguard/issues/1030

+    Lots more discusion here:
+    https://gitlab.com/fdroid/fdroidserver/-/issues/1128
+
    """

    class NoOverwriteDict(dict):
@ -3170,30 +3177,47 @@ def get_first_signer_certificate(apkpath):
                super().__setitem__(k, v)

    cert_encoded = None
+    found_certs = []
    apkobject = _get_androguard_APK(apkpath)
    apkobject._v2_blocks = NoOverwriteDict()
-    certs = apkobject.get_certificates_der_v3()
-    if len(certs) > 0:
-        logging.debug(_('Using APK Signature v3'))
-        cert_encoded = certs[0]
-    if not cert_encoded:
-        certs = apkobject.get_certificates_der_v2()
-        if len(certs) > 0:
-            logging.debug(_('Using APK Signature v2'))
-            cert_encoded = certs[0]
+    certs_v3 = apkobject.get_certificates_der_v3()
+    if certs_v3:
+        cert_v3 = certs_v3[0]
+        found_certs.append(cert_v3)
+        if not cert_encoded:
+            logging.debug(_('Using APK Signature v3'))
+            cert_encoded = cert_v3

-    if not cert_encoded and get_min_sdk_version(apkobject) < 30:
+    certs_v2 = apkobject.get_certificates_der_v2()
+    if certs_v2:
+        cert_v2 = certs_v2[0]
+        found_certs.append(cert_v2)
+        if not cert_encoded:
+            logging.debug(_('Using APK Signature v2'))
+            cert_encoded = cert_v2
+
+    if get_min_sdk_version(apkobject) < 30:
        with zipfile.ZipFile(apkpath, 'r') as apk:
            cert_files = [n for n in apk.namelist() if SIGNATURE_BLOCK_FILE_REGEX.match(n)]
            if len(cert_files) > 1:
                logging.error(_("Found multiple JAR Signature Block Files in {path}").format(path=apkpath))
                return None
            elif len(cert_files) == 1:
-                cert_encoded = get_certificate(apk.read(cert_files[0]))
+                cert_v1 = get_certificate(apk.read(cert_files[0]))
+                found_certs.append(cert_v1)
+                if not cert_encoded:
+                    logging.debug(_('Using JAR Signature'))
+                    cert_encoded = cert_v1

    if not cert_encoded:
        logging.error(_("No signing certificates found in {path}").format(path=apkpath))
        return None
+    if not all(cert == found_certs[0] for cert in found_certs):
+        logging.error(
+            _("APK signatures have different certificates in {path}:").format(
+                path=apkpath
+            )
+        )
    return cert_encoded


--- a/tests/common.TestCase
+++ b/tests/common.TestCase
@ -2934,6 +2934,218 @@ class CommonTest(unittest.TestCase):
        )


+APKS_WITH_JAR_SIGNATURES = (
+    (
+        'SpeedoMeterApp.main_1.apk',
+        '2e6b3126fb7e0db6a9d4c2a06df690620655454d6e152cf244cc9efe9787a77d',
+    ),
+    (
+        'SystemWebView-repack.apk',
+        'b5358886cf36cadab87bc992da9f9016ae9370bdd019e48ffb930674d5ed27c4',
+    ),
+    (
+        'apk.embedded_1.apk',
+        '764f0eaac0cdcde35023658eea865c4383ab580f9827c62fdd3daf9e654199ee',
+    ),
+    (
+        'bad-unicode-πÇÇ现代通用字-български-عربي1.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'issue-1128-poc.apk',
+        '09350d5f3460a8a0ea5cf6b68ccd296a58754f7e683ba6aa08c19be8353504f3',
+    ),
+    (
+        'janus.apk',
+        'ebb0fedf1942a099b287c3db00ff732162152481abb2b6c7cbcdb2ba5894a768',
+    ),
+    (
+        'org.bitbucket.tickytacky.mirrormirror_1.apk',
+        'feaa63df35b4635cf091513dfcd6d11209632555efdfc47e33b70d4e4eb5ba28',
+    ),
+    (
+        'org.bitbucket.tickytacky.mirrormirror_2.apk',
+        'feaa63df35b4635cf091513dfcd6d11209632555efdfc47e33b70d4e4eb5ba28',
+    ),
+    (
+        'org.bitbucket.tickytacky.mirrormirror_3.apk',
+        'feaa63df35b4635cf091513dfcd6d11209632555efdfc47e33b70d4e4eb5ba28',
+    ),
+    (
+        'org.bitbucket.tickytacky.mirrormirror_4.apk',
+        'feaa63df35b4635cf091513dfcd6d11209632555efdfc47e33b70d4e4eb5ba28',
+    ),
+    (
+        'org.dyndns.fules.ck_20.apk',
+        '9326a2cc1a2f148202bc7837a0af3b81200bd37fd359c9e13a2296a71d342056',
+    ),
+    (
+        'org.sajeg.fallingblocks_3.apk',
+        '033389681f4288fdb3e72a28058c8506233ca50de75452ab6c9c76ea1ca2d70f',
+    ),
+    (
+        'repo/com.example.test.helloworld_1.apk',
+        'c3a5ca5465a7585a1bda30218ae4017083605e3576867aa897d724208d99696c',
+    ),
+    (
+        'repo/com.politedroid_3.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/com.politedroid_4.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/com.politedroid_5.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/com.politedroid_6.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/duplicate.permisssions_9999999.apk',
+        '659e1fd284549f70d13fb02c620100e27eeea3420558cce62b0f5d4cf2b77d84',
+    ),
+    (
+        'repo/info.zwanenburg.caffeinetile_4.apk',
+        '51cfa5c8a743833ad89acf81cb755936876a5c8b8eca54d1ffdcec0cdca25d0e',
+    ),
+    (
+        'repo/no.min.target.sdk_987.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/obb.main.oldversion_1444412523.apk',
+        '818e469465f96b704e27be2fee4c63ab9f83ddf30e7a34c7371a4728d83b0bc1',
+    ),
+    (
+        'repo/obb.main.twoversions_1101613.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/obb.main.twoversions_1101615.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/obb.main.twoversions_1101617.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/obb.mainpatch.current_1619.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/obb.mainpatch.current_1619_another-release-key.apk',
+        'ce9e200667f02d96d49891a2e08a3c178870e91853d61bdd33ef5f0b54701aa5',
+    ),
+    (
+        'repo/souch.smsbypass_9.apk',
+        'd3aec784b1fd71549fc22c999789122e3639895db6bd585da5835fbe3db6985c',
+    ),
+    (
+        'repo/urzip-; Рахма́, [rɐxˈmanʲɪnəf] سيرجي_رخمانينوف 谢·.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'repo/v1.v2.sig_1020.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'urzip-release.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'urzip.apk',
+        '7eabd8c15de883d1e82b5df2fd4f7f769e498078e9ad6dc901f0e96db77ceac3',
+    ),
+)
+
+class SignerExtractionText(unittest.TestCase):
+    """Test extraction of the signer certificate from JARs and APKs
+
+    These fingerprints can be confirmed with:
+    apksigner verify --print-certs foo.apk | grep SHA-256
+    keytool -printcert -file ____.RSA
+    """
+
+    def setUp(self):
+        os.chdir(os.path.join(localmodule, 'tests'))
+        self._td = mkdtemp()
+        self.testdir = self._td.name
+
+        self.apksigner = shutil.which('apksigner')
+        self.keytool = shutil.which('keytool')
+
+    def tearDown(self):
+        self._td.cleanup()
+
+    def test_get_first_signer_certificate_with_jars(self):
+        for jar in (
+            'signindex/guardianproject-v1.jar',
+            'signindex/guardianproject.jar',
+            'signindex/testy.jar',
+        ):
+            outdir = os.path.join(self.testdir, jar[:-4].replace('/', '_'))
+            os.mkdir(outdir)
+            fdroidserver.common.apk_extract_signatures(jar, outdir)
+            certs = glob.glob(os.path.join(outdir, '*.RSA'))
+            with open(certs[0], 'rb') as fp:
+                self.assertEqual(
+                    fdroidserver.common.get_certificate(fp.read()),
+                    fdroidserver.common.get_first_signer_certificate(jar),
+                )
+
+    @unittest.skip("slow and only needed when adding to APKS_WITH_JAR_SIGNATURES")
+    def test_vs_keytool(self):
+        unittest.skipUnless(self.keytool, 'requires keytool to run')
+        pat = re.compile(r'[0-9A-F:]{95}')
+        cmd = [self.keytool, '-printcert', '-jarfile']
+        for apk, fingerprint in APKS_WITH_JAR_SIGNATURES:
+            o = subprocess.check_output(cmd + [apk], text=True)
+            try:
+                self.assertEqual(
+                    fingerprint,
+                    pat.search(o).group().replace(':', '').lower(),
+                )
+            except AttributeError as e:
+                print(e, o)
+
+    @unittest.skip("slow and only needed when adding to APKS_WITH_JAR_SIGNATURES")
+    def test_vs_apksigner(self):
+        unittest.skipUnless(self.apksigner, 'requires apksigner to run')
+        pat = re.compile(r'\s[0-9a-f]{64}\s')
+        cmd = [self.apksigner, 'verify', '--print-certs']
+        for apk, fingerprint in APKS_WITH_JAR_SIGNATURES:
+            output = subprocess.check_output(cmd + [apk], text=True)
+            self.assertEqual(
+                fingerprint,
+                pat.search(output).group().strip(),
+                apk + " should have matching signer fingerprints",
+            )
+
+    def test_apk_signer_fingerprint_with_v1_apks(self):
+        for apk, fingerprint in APKS_WITH_JAR_SIGNATURES:
+            outdir = os.path.join(self.testdir, apk[:-4].replace('/', '_'))
+            os.mkdir(outdir)
+            try:
+                fdroidserver.common.apk_extract_signatures(apk, outdir)
+            except fdroidserver.apksigcopier.APKSigCopierError as e:
+                # nothing to test when this error is thrown
+                continue
+            certs = glob.glob(os.path.join(outdir, '*.[DR]SA'))
+            with open(certs[0], 'rb') as fp:
+                self.assertEqual(
+                    fingerprint,
+                    fdroidserver.common.apk_signer_fingerprint(apk),
+                )
+
+    def test_get_first_signer_certificate_with_unsigned_jar(self):
+        self.assertIsNone(
+            fdroidserver.common.get_first_signer_certificate('signindex/unsigned.jar')
+        )
+
+
 if __name__ == "__main__":
    os.chdir(os.path.dirname(__file__))

--- a/tests/index.TestCase
+++ b/tests/index.TestCase
@ -2,6 +2,7 @@

 import copy
 import datetime
+import glob
 import inspect
 import logging
 import optparse
@ -418,6 +419,17 @@ class IndexTest(unittest.TestCase):
            self.maxDiff = None
            self.assertEqual(json.dumps(i, indent=2), json.dumps(o, indent=2))

+            # and test it still works with get_first_signer_certificate
+            outdir = os.path.join(self.testdir, 'publishsigkeys')
+            os.mkdir(outdir)
+            common.apk_extract_signatures(jarfile, outdir)
+            certs = glob.glob(os.path.join(outdir, '*.RSA'))
+            with open(certs[0], 'rb') as fp:
+                self.assertEqual(
+                    common.get_certificate(fp.read()),
+                    common.get_first_signer_certificate(jarfile),
+                )
+
    def test_make_v0_repo_only(self):
        os.chdir(self.testdir)
        os.mkdir('repo')
--- a/tests/issue-1128-poc.apk
+++ b/tests/issue-1128-poc.apk