Merge branch 'sdkmanager-bookworm-backports' into 'master'

Draft: buildserver: use sdkmanager from backports

See merge request fdroid/fdroidserver!1475

.gitlab-ci.yml

@@ -127,7 +127,7 @@ ubuntu_lts_ppa:
     - export ANDROID_HOME=/usr/lib/android-sdk
     - apt-get install gnupg
     - while ! apt-key adv --keyserver keyserver.ubuntu.com --recv-key 9AAC253193B65D4DF1D0A13EEC4632C79C5E0151; do sleep 15; done
-    - export RELEASE=`sed -n 's,^deb [^ ][^ ]* \([a-z]*\).*,\1,p' /etc/apt/sources.list | head -1`
+    - export RELEASE=$(sed -n 's,^Suites\x3a \([a-z]*\).*,\1,p' /etc/apt/sources.list.d/*.sources | head -1)
     - echo "deb http://ppa.launchpad.net/fdroid/fdroidserver/ubuntu $RELEASE main" >> /etc/apt/sources.list
     - apt-get update
     - apt-get dist-upgrade
@@ -285,9 +285,7 @@ black:
     - black --check --diff --color $CI_PROJECT_DIR
 
 fedora_latest:
-  image: fedora:latest
+  image: fedora:39  # support ends on 2024-11-12
-  only:
-    - master@fdroid/fdroidserver
   script:
     # tricks to hopefully make runs more reliable
     - echo "timeout=600" >> /etc/dnf/dnf.conf

MANIFEST.in

@@ -591,6 +591,11 @@ include tests/index.TestCase
 include tests/init.TestCase
 include tests/install.TestCase
 include tests/IsMD5Disabled.java
+include tests/issue-1128-min-sdk-30-poc.apk
+include tests/issue-1128-poc1.apk
+include tests/issue-1128-poc2.apk
+include tests/issue-1128-poc3a.apk
+include tests/issue-1128-poc3b.apk
 include tests/janus.apk
 include tests/keystore.jks
 include tests/key-tricks.py
@@ -723,6 +728,8 @@ include tests/repo/urzip-*.apk
 include tests/repo/v1.v2.sig_1020.apk
 include tests/rewritemeta.TestCase
 include tests/run-tests
+include tests/SANAPPSI.RSA
+include tests/SANAPPSI.SF
 include tests/scanner.TestCase
 include tests/signatures.TestCase
 include tests/signindex.TestCase

buildserver/provision-apt-get-install

@@ -116,7 +116,7 @@ packages="
  patch
  python3-packaging
  rsync
- sdkmanager
+ sdkmanager/bookworm-backports
  sudo
  unzip
 "

fdroidserver/common.py

@@ -54,16 +54,13 @@ from pathlib import Path
 
 import defusedxml.ElementTree as XMLElementTree
 
+from asn1crypto import cms
 from base64 import urlsafe_b64encode
 from binascii import hexlify
 from datetime import datetime, timedelta, timezone
 from queue import Queue
 from zipfile import ZipFile
 
-from pyasn1.codec.der import decoder, encoder
-from pyasn1_modules import rfc2315
-from pyasn1.error import PyAsn1Error
-
 import fdroidserver.metadata
 import fdroidserver.lint
 from fdroidserver import _
@@ -2354,6 +2351,8 @@ def prepare_source(vcs, app, build, build_dir, srclib_dir, extlib_dir, onserver=
                 gradlefile = build_gradle
             elif os.path.exists(build_gradle_kts):
                 gradlefile = build_gradle_kts
+            else:
+                raise BuildException("No gradle file found")
             regsub_file(r'compileSdkVersion[ =]+[0-9]+',
                         r'compileSdkVersion %s' % n,
                         gradlefile)
@@ -2662,7 +2661,7 @@ def _androguard_logging_level(level=logging.ERROR):
         pass
 
 
-def get_androguard_APK(apkfile):
+def get_androguard_APK(apkfile, skip_analysis=False):
     try:
         # these were moved in androguard 4.0
         from androguard.core.apk import APK
@@ -2670,7 +2669,7 @@ def get_androguard_APK(apkfile):
         from androguard.core.bytecodes.apk import APK
     _androguard_logging_level()
 
-    return APK(apkfile)
+    return APK(apkfile, skip_analysis=skip_analysis)
 
 
 def ensure_final_value(packageName, arsc, value):
@@ -3162,10 +3161,7 @@ def signer_fingerprint_short(cert_encoded):
 
 
 def signer_fingerprint(cert_encoded):
-    """Obtain sha256 signing-key fingerprint for pkcs7 DER certificate.
+    """Return SHA-256 signer fingerprint for PKCS#7 DER-encoded signature.
-
-    Extracts hexadecimal sha256 signing-key fingerprint string
-    for a given pkcs7 signature.
 
     Parameters
     ----------
@@ -3173,46 +3169,113 @@ def signer_fingerprint(cert_encoded):
 
     Returns
     -------
-    shortened signature fingerprint.
+    Standard SHA-256 signer fingerprint.
+
     """
     return hashlib.sha256(cert_encoded).hexdigest()
 
 
 def get_first_signer_certificate(apkpath):
-    """Get the first signing certificate from the APK, DER-encoded."""
+    """Get the first signing certificate from the APK, DER-encoded.
-    certs = None
-    cert_encoded = None
-    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]))
 
-    if not cert_encoded:
+    JAR and APK Signatures allow for multiple signers, though it is
-        apkobject = get_androguard_APK(apkpath)
+    rarely used, and this is poorly documented.  So this method only
-        certs = apkobject.get_certificates_der_v2()
+    fetches the first certificate, and errors out if there are more.
-        if len(certs) > 0:
+
-            logging.debug(_('Using APK Signature v2'))
+    Starting with targetSdkVersion 30, APK v2 Signatures are required.
-            cert_encoded = certs[0]
+    https://developer.android.com/about/versions/11/behavior-changes-11#minimum-signature-scheme
+
+    When a APK v2+ signature is present, the JAR signature is not
+    verified.  The verifier parses the signers from the v2+ signature
+    and does not seem to look at the JAR signature.
+    https://source.android.com/docs/security/features/apksigning/v2#apk-signature-scheme-v2-block
+    https://android.googlesource.com/platform/tools/apksig/+/refs/tags/android-13.0.0_r3/src/main/java/com/android/apksig/ApkVerifier.java#270
+
+    apksigner checks that the signers from all the APK signatures match:
+    https://android.googlesource.com/platform/tools/apksig/+/refs/tags/android-13.0.0_r3/src/main/java/com/android/apksig/ApkVerifier.java#383
+
+    apksigner verifies each signer's signature block file
+    .(RSA|DSA|EC) against the corresponding signature file .SF
+    https://android.googlesource.com/platform/tools/apksig/+/refs/tags/android-13.0.0_r3/src/main/java/com/android/apksig/internal/apk/v1/V1SchemeVerifier.java#280
+
+    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):
+        def __setitem__(self, k, v):
+            if k not in self:
+                super().__setitem__(k, v)
+
+    cert_encoded = None
+    found_certs = []
+    apkobject = get_androguard_APK(apkpath)
+    apkobject._v2_blocks = NoOverwriteDict()
+    certs_v3 = apkobject.get_certificates_der_v3()
+    if certs_v3:
+        cert_v3 = certs_v3[0]
+        found_certs.append(cert_v3)
         if not cert_encoded:
-            certs = apkobject.get_certificates_der_v3()
+            logging.debug(_('Using APK Signature v3'))
-            if len(certs) > 0:
+            cert_encoded = cert_v3
-                logging.debug(_('Using APK Signature v3'))
+
-                cert_encoded = certs[0]
+    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) < 24 or (
+        not (certs_v3 or certs_v2) and get_effective_target_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
+            elif len(cert_files) == 1:
+                signature_block_file = cert_files[0]
+                signature_file = (
+                    cert_files[0][: signature_block_file.rindex('.')] + '.SF'
+                )
+                cert_v1 = get_certificate(
+                    apk.read(signature_block_file),
+                    apk.read(signature_file),
+                )
+                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
+        return
+
+    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
+
     return cert_encoded
 
 
 def apk_signer_fingerprint(apk_path):
-    """Obtain sha256 signing-key fingerprint for APK.
+    """Get SHA-256 fingerprint string for the first signer from given APK.
-
-    Extracts hexadecimal sha256 signing-key fingerprint string
-    for a given APK.
 
     Parameters
     ----------
@@ -3221,7 +3284,8 @@ def apk_signer_fingerprint(apk_path):
 
     Returns
     -------
-    signature fingerprint
+    Standard SHA-256 signer fingerprint
+
     """
     cert_encoded = get_first_signer_certificate(apk_path)
     if not cert_encoded:
@@ -3230,10 +3294,7 @@ def apk_signer_fingerprint(apk_path):
 
 
 def apk_signer_fingerprint_short(apk_path):
-    """Obtain shortened sha256 signing-key fingerprint for APK.
+    """Get 7 hex digit SHA-256 fingerprint string for the first signer from given APK.
-
-    Extracts the first 7 hexadecimal digits of sha256 signing-key fingerprint
-    for a given pkcs7 APK.
 
     Parameters
     ----------
@@ -3242,7 +3303,8 @@ def apk_signer_fingerprint_short(apk_path):
 
     Returns
     -------
-    shortened signing-key fingerprint
+    first 7 chars of the standard SHA-256 signer fingerprint
+
     """
     return apk_signer_fingerprint(apk_path)[:7]
 
@@ -3461,7 +3523,7 @@ def apk_extract_signatures(apkpath, outdir):
 
 
 def get_min_sdk_version(apk):
-    """Wrap the androguard function to always return and int.
+    """Wrap the androguard function to always return an integer.
 
     Fall back to 1 if we can't get a valid minsdk version.
 
@@ -3472,7 +3534,7 @@ def get_min_sdk_version(apk):
 
     Returns
     -------
-    minsdk: int
+    minSdkVersion: int
     """
     try:
         return int(apk.get_min_sdk_version())
@@ -3480,6 +3542,24 @@ def get_min_sdk_version(apk):
         return 1
 
 
+def get_effective_target_sdk_version(apk):
+    """Wrap the androguard function to always return an integer.
+
+    Parameters
+    ----------
+    apk
+        androguard APK object
+
+    Returns
+    -------
+    targetSdkVersion: int
+    """
+    try:
+        return int(apk.get_effective_target_sdk_version())
+    except TypeError:
+        return get_min_sdk_version(apk)
+
+
 def get_apksigner_smartcardoptions(smartcardoptions):
     if '-providerName' in smartcardoptions.copy():
         pos = smartcardoptions.index('-providerName')
@@ -3891,14 +3971,33 @@ def get_cert_fingerprint(pubkey):
     return " ".join(ret)
 
 
-def get_certificate(signature_block_file):
+def get_certificate(signature_block_file, signature_file=None):
-    """Extract a DER certificate from JAR Signature's "Signature Block File".
+    """Extract a single DER certificate from JAR Signature's "Signature Block File".
+
+    If there is more than one signer certificate, this exits with an
+    error, unless the signature_file is provided.  If that is set, it
+    will return the certificate that matches the Signature File, for
+    example, if there is a certificate chain, like TLS does.  In the
+    fdroidserver use cases, there should always be a single signer.
+    But rarely, some APKs include certificate chains.
+
+    This could be replaced by androguard's APK.get_certificate_der()
+    provided the cert chain fix was merged there.  Maybe in 4.1.2?
+    https://github.com/androguard/androguard/pull/1038
+
+    https://docs.oracle.com/en/java/javase/21/docs/specs/man/jarsigner.html#the-signed-jar-file
 
     Parameters
     ----------
     signature_block_file
-        file bytes (as string) representing the
+        Bytes representing the PKCS#7 signer certificate and
-        certificate, as read directly out of the APK/ZIP
+        signature, as read directly out of the JAR/APK, e.g. CERT.RSA.
+
+    signature_file
+        Bytes representing the manifest signed by the Signature Block
+        File, e.g. CERT.SF.  If this is not given, the assumption is
+        there will be only a single certificate in
+        signature_block_file, otherwise it is an error.
 
     Returns
     -------
@@ -3906,18 +4005,107 @@ def get_certificate(signature_block_file):
     or None in case of error
 
     """
-    content = decoder.decode(signature_block_file, asn1Spec=rfc2315.ContentInfo())[0]
+    pkcs7obj = cms.ContentInfo.load(signature_block_file)
-    if content.getComponentByName('contentType') != rfc2315.signedData:
+    certificates = pkcs7obj['content']['certificates']
-        return None
+    if len(certificates) == 1:
-    content = decoder.decode(content.getComponentByName('content'),
+        return certificates[0].chosen.dump()
-                             asn1Spec=rfc2315.SignedData())[0]
+    elif not signature_file:
-    try:
+        logging.error(_('Found multiple Signer Certificates!'))
-        certificates = content.getComponentByName('certificates')
+        return
-        cert = certificates[0].getComponentByName('certificate')
+    certificate = get_jar_signer_certificate(pkcs7obj, signature_file)
-    except PyAsn1Error:
+    if certificate:
-        logging.error("Certificates not found.")
+        return certificate.chosen.dump()
-        return None
+
-    return encoder.encode(cert)
+
+def _find_matching_certificate(signer_info, certificate):
+    """Find the certificates that matches signer_info using issuer and serial number.
+
+    https://android.googlesource.com/platform/tools/apksig/+/refs/tags/android-13.0.0_r3/src/main/java/com/android/apksig/internal/apk/v1/V1SchemeVerifier.java#590
+    https://android.googlesource.com/platform/tools/apksig/+/refs/tags/android-13.0.0_r3/src/main/java/com/android/apksig/internal/x509/Certificate.java#55
+
+    """
+    certificate_serial = certificate.chosen['tbs_certificate']['serial_number']
+    expected_issuer_serial = signer_info['sid'].chosen
+    return (
+        expected_issuer_serial['issuer'] == certificate.chosen.issuer
+        and expected_issuer_serial['serial_number'] == certificate_serial
+    )
+
+
+def get_jar_signer_certificate(pkcs7obj: cms.ContentInfo, signature_file: bytes):
+    """Return the one certificate in a chain that actually signed the manifest.
+
+    PKCS#7-signed data can include certificate chains for use cases
+    where an Certificate Authority (CA) is used.  Android does not
+    validate the certificate chain on APK signatures, so neither does
+    this.
+    https://android.googlesource.com/platform/tools/apksig/+/refs/tags/android-13.0.0_r3/src/main/java/com/android/apksig/internal/apk/v1/V1SchemeVerifier.java#512
+
+    Some useful fodder for understanding all this:
+    https://docs.oracle.com/javase/tutorial/deployment/jar/intro.html
+    https://technotes.shemyak.com/posts/jar-signature-block-file-format/
+    https://docs.oracle.com/en/java/javase/21/docs/specs/man/jarsigner.html#the-signed-jar-file
+    https://qistoph.blogspot.com/2012/01/manual-verify-pkcs7-signed-data-with.html
+
+    """
+    import oscrypto.asymmetric
+    import oscrypto.errors
+
+    # Android attempts to verify all SignerInfos and then picks the first verified SignerInfo.
+    first_verified_signer_info = None
+    first_verified_signer_info_signing_certificate = None
+    for signer_info in pkcs7obj['content']['signer_infos']:
+        signature = signer_info['signature'].contents
+        digest_algorithm = signer_info["digest_algorithm"]["algorithm"].native
+        public_key = None
+        for certificate in pkcs7obj['content']['certificates']:
+            if _find_matching_certificate(signer_info, certificate):
+                public_key = oscrypto.asymmetric.load_public_key(certificate.chosen.public_key)
+                break
+        if public_key is None:
+            logging.info('No certificate found that matches signer info!')
+            continue
+
+        signature_algo = signer_info['signature_algorithm'].signature_algo
+        if signature_algo == 'rsassa_pkcs1v15':
+            # ASN.1 - 1.2.840.113549.1.1.1
+            verify_func = oscrypto.asymmetric.rsa_pkcs1v15_verify
+        elif signature_algo == 'rsassa_pss':
+            # ASN.1 - 1.2.840.113549.1.1.10
+            verify_func = oscrypto.asymmetric.rsa_pss_verify
+        elif signature_algo == 'dsa':
+            # ASN.1 - 1.2.840.10040.4.1
+            verify_func = oscrypto.asymmetric.dsa_verify
+        elif signature_algo == 'ecdsa':
+            # ASN.1 - 1.2.840.10045.4
+            verify_func = oscrypto.asymmetric.ecdsa_verify
+        else:
+            logging.error(
+                'Unknown signature algorithm %s:\n  %s\n  %s'
+                % (
+                    signature_algo,
+                    hexlify(certificate.chosen.sha256).decode(),
+                    certificate.chosen.subject.human_friendly,
+                ),
+            )
+            return
+
+        try:
+            verify_func(public_key, signature, signature_file, digest_algorithm)
+            if not first_verified_signer_info:
+                first_verified_signer_info = signer_info
+                first_verified_signer_info_signing_certificate = certificate
+
+        except oscrypto.errors.SignatureError as e:
+            logging.error(
+                '"%s", skipping:\n  %s\n  %s' % (
+                    e,
+                    hexlify(certificate.chosen.sha256).decode(),
+                    certificate.chosen.subject.human_friendly),
+            )
+
+    if first_verified_signer_info_signing_certificate:
+        return first_verified_signer_info_signing_certificate
 
 
 def load_stats_fdroid_signing_key_fingerprints():

fdroidserver/lint.py

@@ -722,7 +722,13 @@ def check_updates_ucm_http_aum_pattern(app):  # noqa: D403
 
 
 def check_certificate_pinned_binaries(app):
-    if len(app.get('AllowedAPKSigningKeys')) > 0:
+    keys = app.get('AllowedAPKSigningKeys')
+    known_keys = common.config.get('apk_signing_key_block_list', [])
+    if keys:
+        if known_keys:
+            for key in keys:
+                if key in known_keys:
+                    yield _('Known debug key is used in AllowedAPKSigningKeys: ') + key
         return
     if app.get('Binaries') is not None:
         yield _(

setup.py

@@ -93,14 +93,14 @@ setup(
     install_requires=[
         'appdirs',
         'androguard >= 3.3.5',
+        'asn1crypto',
         'clint',
         'defusedxml',
         'GitPython',
+        'oscrypto',
         'paramiko',
         'Pillow',
         'apache-libcloud >= 0.14.1',
-        'pyasn1 >=0.4.1',
-        'pyasn1-modules >= 0.2.1',
         'python-vagrant',
         'PyYAML',
         'qrcode',

tests/common.TestCase

@@ -615,6 +615,27 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(fdroidserver.common.verify_apk_signature(twosigapk))
         self.assertIsNone(fdroidserver.common.verify_apks(sourceapk, twosigapk, self.tmpdir))
 
+    def test_get_certificate_with_chain_sandisk(self):
+        """Test that APK signatures with a cert chain are parsed like apksigner.
+
+        SanDisk signs their APKs with a X.509 certificate chain of
+        trust, so there are actually three certificates
+        included. apksigner only cares about the certificate in the
+        chain that actually signs the manifest.
+
+        The correct value comes from:
+        apksigner verify --print-certs 883cbdae7aeb2e4b122e8ee8d89966c7062d0d49107a130235fa220a5b994a79.apk
+
+        """
+        cert = fdroidserver.common.get_certificate(
+            signature_block_file=Path('SANAPPSI.RSA').read_bytes(),
+            signature_file=Path('SANAPPSI.SF').read_bytes(),
+        )
+        self.assertEqual(
+            'ea0abbf2a142e4b167405d516b2cc408c4af4b29cd50ba281aa4470d4aab3e53',
+            fdroidserver.common.signer_fingerprint(cert),
+        )
+
     def test_write_to_config(self):
         with tempfile.TemporaryDirectory() as tmpPath:
             cfgPath = os.path.join(tmpPath, 'config.py')
@@ -1039,6 +1060,11 @@ class CommonTest(unittest.TestCase):
             ('org.bitbucket.tickytacky.mirrormirror_3.apk', 'org.bitbucket.tickytacky.mirrormirror', 3, '1.0.2'),
             ('org.bitbucket.tickytacky.mirrormirror_4.apk', 'org.bitbucket.tickytacky.mirrormirror', 4, '1.0.3'),
             ('org.dyndns.fules.ck_20.apk', 'org.dyndns.fules.ck', 20, 'v1.6pre2'),
+            ('issue-1128-min-sdk-30-poc.apk', 'org.fdroid.ci', 1, '1.0'),
+            ('issue-1128-poc1.apk', 'android.appsecurity.cts.tinyapp', 10, '1.0'),
+            ('issue-1128-poc2.apk', 'android.appsecurity.cts.tinyapp', 10, '1.0'),
+            ('issue-1128-poc3a.apk', 'android.appsecurity.cts.tinyapp', 10, '1.0'),
+            ('issue-1128-poc3b.apk', 'android.appsecurity.cts.tinyapp', 10, '1.0'),
             ('urzip.apk', 'info.guardianproject.urzip', 100, '0.1'),
             ('urzip-badcert.apk', 'info.guardianproject.urzip', 100, '0.1'),
             ('urzip-badsig.apk', 'info.guardianproject.urzip', 100, '0.1'),
@@ -1154,6 +1180,11 @@ class CommonTest(unittest.TestCase):
             return apk.get_effective_target_sdk_version()
 
         self.assertEqual(4, get_minSdkVersion('bad-unicode-πÇÇ现代通用字-български-عربي1.apk'))
+        self.assertEqual(30, get_minSdkVersion('issue-1128-min-sdk-30-poc.apk'))
+        self.assertEqual(29, get_minSdkVersion('issue-1128-poc1.apk'))
+        self.assertEqual(29, get_minSdkVersion('issue-1128-poc2.apk'))
+        self.assertEqual(23, get_minSdkVersion('issue-1128-poc3a.apk'))
+        self.assertEqual(23, get_minSdkVersion('issue-1128-poc3b.apk'))
         self.assertEqual(14, get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_1.apk'))
         self.assertEqual(14, get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_2.apk'))
         self.assertEqual(14, get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_3.apk'))
@@ -1164,6 +1195,7 @@ class CommonTest(unittest.TestCase):
         self.assertEqual(4, get_minSdkVersion('urzip-badsig.apk'))
         self.assertEqual(4, get_minSdkVersion('urzip-release.apk'))
         self.assertEqual(4, get_minSdkVersion('urzip-release-unsigned.apk'))
+        self.assertEqual(27, get_minSdkVersion('v2.only.sig_2.apk'))
         self.assertEqual(3, get_minSdkVersion('repo/com.politedroid_3.apk'))
         self.assertEqual(3, get_minSdkVersion('repo/com.politedroid_4.apk'))
         self.assertEqual(3, get_minSdkVersion('repo/com.politedroid_5.apk'))
@@ -2915,6 +2947,274 @@ class CommonTest(unittest.TestCase):
         )
 
 
+APKS_WITH_JAR_SIGNATURES = (
+    (
+        'SpeedoMeterApp.main_1.apk',
+        '2e6b3126fb7e0db6a9d4c2a06df690620655454d6e152cf244cc9efe9787a77d',
+    ),
+    (
+        'apk.embedded_1.apk',
+        '764f0eaac0cdcde35023658eea865c4383ab580f9827c62fdd3daf9e654199ee',
+    ),
+    (
+        'bad-unicode-πÇÇ现代通用字-български-عربي1.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+    (
+        'issue-1128-poc3a.apk',
+        '1dbb8be012293e988a0820f7d455b07abd267d2c0b500fc793fcfd80141cb5ce',
+    ),
+    (
+        'issue-1128-poc3b.apk',
+        '1dbb8be012293e988a0820f7d455b07abd267d2c0b500fc793fcfd80141cb5ce',
+    ),
+    (
+        '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',
+    ),
+)
+APKS_WITHOUT_JAR_SIGNATURES = (
+    (
+        'issue-1128-poc1.apk',  # APK v3 Signature only
+        '1dbb8be012293e988a0820f7d455b07abd267d2c0b500fc793fcfd80141cb5ce',
+    ),
+    (
+        'issue-1128-poc2.apk',  # APK v3 Signature only
+        '1dbb8be012293e988a0820f7d455b07abd267d2c0b500fc793fcfd80141cb5ce',
+    ),
+    (
+        'issue-1128-min-sdk-30-poc.apk',  # APK v3 Signature only
+        '09350d5f3460a8a0ea5cf6b68ccd296a58754f7e683ba6aa08c19be8353504f3',
+    ),
+    (
+        'v2.only.sig_2.apk',
+        '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6',
+    ),
+)
+
+
+class SignerExtractionTest(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 + APKS_WITHOUT_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:
+            self.assertEqual(
+                fingerprint,
+                fdroidserver.common.apk_signer_fingerprint(apk),
+                f'apk_signer_fingerprint should match stored fingerprint for {apk}',
+            )
+
+    def test_apk_signer_fingerprint_without_v1_apks(self):
+        for apk, fingerprint in APKS_WITHOUT_JAR_SIGNATURES:
+            self.assertEqual(
+                fingerprint,
+                fdroidserver.common.apk_signer_fingerprint(apk),
+                f'apk_signer_fingerprint should match stored fingerprint for {apk}',
+            )
+
+    def test_get_first_signer_certificate_with_unsigned_jar(self):
+        self.assertIsNone(
+            fdroidserver.common.get_first_signer_certificate('signindex/unsigned.jar')
+        )
+
+    def test_apk_extract_fingerprint(self):
+        """Test extraction of JAR signatures (does not cover APK v2+ extraction)."""
+        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:
+                # nothing to test here when this error is thrown
+                continue
+            v1_certs = [str(cert) for cert in Path(outdir).glob('*.[DR]SA')]
+            cert = fdroidserver.common.get_certificate(
+                signature_block_file=Path(v1_certs[0]).read_bytes(),
+                signature_file=Path(v1_certs[0][:-4] + '.SF').read_bytes(),
+            )
+            self.assertEqual(
+                fingerprint,
+                fdroidserver.common.signer_fingerprint(cert),
+            )
+            apkobject = fdroidserver.common.get_androguard_APK(apk, skip_analysis=True)
+            v2_certs = apkobject.get_certificates_der_v2()
+            if v2_certs:
+                if v1_certs:
+                    self.assertEqual(len(v1_certs), len(v2_certs))
+                self.assertEqual(
+                    fingerprint,
+                    fdroidserver.common.signer_fingerprint(v2_certs[0]),
+                )
+            v3_certs = apkobject.get_certificates_der_v3()
+            if v3_certs:
+                if v2_certs:
+                    self.assertEqual(len(v2_certs), len(v3_certs))
+                self.assertEqual(
+                    fingerprint,
+                    fdroidserver.common.signer_fingerprint(v3_certs[0]),
+                )
+
+
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 

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')
@@ -701,8 +713,14 @@ class IndexTest(unittest.TestCase):
         app = apps[appid]
         metadata = index.package_metadata(app, 'repo')
         # files
-        self.assertEqual(36027, metadata['featureGraphic']['en-US']['size'])
+        self.assertEqual(
-        self.assertEqual(1413, metadata['icon']['en-US']['size'])
+            os.path.getsize(f'repo/{appid}/en-US/featureGraphic.png'),
+            metadata['featureGraphic']['en-US']['size'],
+        )
+        self.assertEqual(
+            os.path.getsize(f'repo/{appid}/en-US/icon.png'),
+            metadata['icon']['en-US']['size'],
+        )
         # localized strings
         self.assertEqual({'en-US': 'title'}, metadata['name'])
         self.assertEqual({'en-US': 'video'}, metadata['video'])

tests/lint.TestCase

@@ -438,6 +438,45 @@ class LintTest(unittest.TestCase):
         with self.assertRaises(TypeError):
             fdroidserver.lint.lint_config('mirrors.yml')
 
+    def test_check_certificate_pinned_binaries_empty(self):
+        fdroidserver.common.config = {}
+        app = fdroidserver.metadata.App()
+        app.AllowedAPKSigningKeys = [
+            'a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc'
+        ]
+        self.assertEqual(
+            [],
+            list(fdroidserver.lint.check_certificate_pinned_binaries(app)),
+            "when the config is empty, any signing key should be allowed",
+        )
+
+    def test_lint_known_debug_keys_no_match(self):
+        fdroidserver.common.config = {
+            "apk_signing_key_block_list": "a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc"
+        }
+        app = fdroidserver.metadata.App()
+        app.AllowedAPKSigningKeys = [
+            '2fd4fd5f54babba4bcb21237809bb653361d0d2583c80964ec89b28a26e9539e'
+        ]
+        self.assertEqual(
+            [],
+            list(fdroidserver.lint.check_certificate_pinned_binaries(app)),
+            "A signing key that does not match one in the config should be allowed",
+        )
+
+    def test_lint_known_debug_keys(self):
+        fdroidserver.common.config = {
+            'apk_signing_key_block_list': 'a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc'
+        }
+        app = fdroidserver.metadata.App()
+        app.AllowedAPKSigningKeys = [
+            'a40da80a59d170caa950cf15c18c454d47a39b26989d8b640ecd745ba71bf5dc'
+        ]
+        for warn in fdroidserver.lint.check_certificate_pinned_binaries(app):
+            anywarns = True
+            logging.debug(warn)
+        self.assertTrue(anywarns)
+
 
 class LintAntiFeaturesTest(unittest.TestCase):
     def setUp(self):

tests/nightly.TestCase

@@ -70,7 +70,10 @@ class NightlyTest(unittest.TestCase):
 
     def tearDown(self):
         self.tempdir.cleanup()
-        os.rmdir(self.testroot)
+        try:
+            os.rmdir(self.testroot)
+        except OSError:  # other test modules might have left stuff around
+            pass
 
     def _copy_test_debug_keystore(self):
         self.dot_android.mkdir()

tests/testcommon.py

@@ -18,6 +18,9 @@
 import os
 import sys
 import tempfile
+import unittest
+
+from pathlib import Path
 
 
 class TmpCwd:
@@ -60,3 +63,12 @@ def mkdtemp():
         return tempfile.TemporaryDirectory()
     else:
         return tempfile.TemporaryDirectory(ignore_cleanup_errors=True)
+
+
+def mkdir_testfiles(localmodule, test):
+    """Keep the test files in a labeled test dir for easy reference"""
+    testroot = Path(localmodule) / '.testfiles'
+    testroot.mkdir(exist_ok=True)
+    testdir = testroot / unittest.TestCase.id(test)
+    testdir.mkdir(exist_ok=True)
+    return tempfile.mkdtemp(dir=testdir)