Merge branch 'config-options-as-module-level-variables' into 'master'

Draft: clarify that 'config' and 'options' should only be module-level variables

Closes #1128

See merge request fdroid/fdroidserver!1477

.gitlab-ci.yml

@@ -285,9 +285,7 @@ black:
     - black --check --diff --color $CI_PROJECT_DIR
 
 fedora_latest:
-  image: fedora:latest
-  only:
-    - master@fdroid/fdroidserver
+  image: fedora:39  # support ends on 2024-11-12
   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

examples/fdroid_clean_repos.py

@@ -23,12 +23,11 @@ def main():
         help=_("applicationId with optional versionCode in the form APPID[:VERCODE]"),
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
-    common.options = options
+    options = common.parse_args(parser)
     pkgs = common.read_pkg_args(options.appid, True)
     allapps = metadata.read_metadata(pkgs)
     apps = common.read_app_args(options.appid, allapps, True)
-    common.read_config(options)
+    common.read_config()
 
     for appid, app in apps.items():
         if "Builds" in app and len(app["Builds"]) > 0:

examples/fdroid_export_keystore_to_nitrokey.py

@@ -25,8 +25,8 @@ def main():
     global config
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
-    config = common.read_config(options)
+    common.parse_args(parser)
+    config = common.read_config()
     destkeystore = config['keystore'].replace('.jks', '.p12').replace('/', '_')
     exportkeystore = config['keystore'].replace('.jks', '.pem').replace('/', '_')
     if os.path.exists(destkeystore) or os.path.exists(exportkeystore):

examples/fdroid_exportkeystore.py

@@ -14,8 +14,8 @@ fdroid_summary = 'export the keystore in standard PEM format'
 def main():
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
-    config = common.read_config(options)
+    common.parse_args(parser)
+    config = common.read_config()
     env_vars = {'LC_ALL': 'C.UTF-8',
                 'FDROID_KEY_STORE_PASS': config['keystorepass'],
                 'FDROID_KEY_PASS': config['keypass']}

examples/fdroid_extract_repo_pubkey.py

@@ -12,8 +12,8 @@ fdroid_summary = 'export the keystore in standard PEM format'
 def main():
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
-    common.config = common.read_config(options)
+    common.parse_args(parser)
+    common.read_config()
     pubkey, repo_pubkey_fingerprint = index.extract_pubkey()
     print('repo_pubkey = "%s"' % pubkey.decode())
 

examples/fdroid_fetchsrclibs.py

@@ -18,12 +18,11 @@ def main():
     common.setup_global_opts(parser)
     parser.add_argument("appid", nargs='*', help=_("applicationId with optional versionCode in the form APPID[:VERCODE]"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
-    common.options = options
+    options = common.parse_args(parser)
     pkgs = common.read_pkg_args(options.appid, True)
     allapps = metadata.read_metadata(pkgs)
     apps = common.read_app_args(options.appid, allapps, True)
-    common.read_config(options)
+    common.read_config()
     srclib_dir = os.path.join('build', 'srclib')
     os.makedirs(srclib_dir, exist_ok=True)
     srclibpaths = []

examples/fdroid_nitrokeyimport.py

@@ -11,8 +11,8 @@ fdroid_summary = 'import the local keystore into a SmartCard HSM'
 def main():
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
-    config = common.read_config(options)
+    common.parse_args(parser)
+    config = common.read_config()
     env_vars = {
         'LC_ALL': 'C.UTF-8',
         'FDROID_KEY_STORE_PASS': config['keystorepass'],

fdroidserver/__main__.py

@@ -182,7 +182,7 @@ def main():
                            "can not be specified at the same time."))
         sys.exit(1)
 
-    # Trick optparse into displaying the right usage when --help is used.
+    # Trick argparse into displaying the right usage when --help is used.
     sys.argv[0] += ' ' + command
 
     del sys.argv[1]

fdroidserver/btlog.py

@@ -46,9 +46,6 @@ from . import deploy
 from .exception import FDroidException
 
 
-options = None
-
-
 def make_binary_transparency_log(
     repodirs, btrepo='binary_transparency', url=None, commit_title='fdroid update'
 ):
@@ -149,8 +146,6 @@ For more info on this idea:
 
 
 def main():
-    global options
-
     parser = ArgumentParser()
     common.setup_global_opts(parser)
     parser.add_argument(
@@ -169,7 +164,7 @@ def main():
         default=None,
         help=_("Push the log to this git remote repository"),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     if options.verbose:
         logging.getLogger("requests").setLevel(logging.INFO)

fdroidserver/build.py

@@ -883,7 +883,7 @@ def parse_commandline():
     parser.add_argument("-w", "--wiki", default=False, action="store_true",
                         help=argparse.SUPPRESS)
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
     # Force --stop with --on-server to get correct exit code
@@ -929,7 +929,7 @@ def main():
         if not options.appid and not options.all:
             parser.error("option %s: If you really want to build all the apps, use --all" % "all")
 
-    config = common.read_config(opts=options)
+    config = common.read_config()
 
     if config['build_server_always']:
         options.server = True

fdroidserver/checkupdates.py

@@ -505,7 +505,7 @@ def operate_vercode(operation: str, vercode: int) -> int:
     return vercode
 
 
-def checkupdates_app(app: metadata.App) -> None:
+def checkupdates_app(app: metadata.App, auto: bool, commit: bool = False) -> None:
     """Check for new versions and updated name of a single app.
 
     Also write back changes to the metadata file and create a Git commit if
@@ -581,7 +581,7 @@ def checkupdates_app(app: metadata.App) -> None:
         logging.info('...updating to version %s' % ver)
         commitmsg = 'Update CurrentVersion of %s to %s' % (name, ver)
 
-    if options.auto:
+    if auto:
         mode = app.AutoUpdateMode
         if not app.CurrentVersionCode:
             raise MetaDataException(
@@ -664,7 +664,7 @@ def checkupdates_app(app: metadata.App) -> None:
 
     if commitmsg:
         metadata.write_metadata(app.metadatapath, app)
-        if options.commit:
+        if commit:
             logging.info("Commiting update for " + app.metadatapath)
             gitcmd = ["git", "commit", "-m", commitmsg]
             if 'auto_author' in config:
@@ -694,7 +694,6 @@ def status_update_json(processed: list, failed: dict) -> None:
 
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -704,7 +703,7 @@ def main():
     The behaviour of this function is influenced by the configuration file as
     well as command line parameters.
     """
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -719,10 +718,10 @@ def main():
     parser.add_argument("--allow-dirty", action="store_true", default=False,
                         help=_("Run on git repo that has uncommitted changes"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     if not options.allow_dirty:
         status = subprocess.check_output(['git', 'status', '--porcelain'])
@@ -748,7 +747,7 @@ def main():
         logging.info(msg)
 
         try:
-            checkupdates_app(app)
+            checkupdates_app(app, options.auto, options.commit)
             processed.append(appid)
         except Exception as e:
             msg = _("...checkupdate failed for {appid} : {error}").format(appid=appid, error=e)

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 _
@@ -194,6 +191,34 @@ default_config = {
 }
 
 
+def get_options():
+    """Return options as set up by parse_args().
+
+    This provides an easy way to get the global instance without
+    having to think about very confusing import and submodule
+    visibility.  The code should be probably refactored so it does not
+    need this.  If each individual option value was always passed to
+    functions as args, for example.
+
+    https://docs.python.org/3/reference/import.html#submodules
+
+    """
+    return fdroidserver.common.options
+
+
+def parse_args(parser):
+    """Call parser.parse_args(), store result in module-level variable and return it.
+
+    This is needed to set up the copy of the options instance in the
+    fdroidserver.common module.  A subcommand only needs to call this
+    if it uses functions from fdroidserver.common that expect the
+    "options" variable to be initialized.
+
+    """
+    fdroidserver.common.options = parser.parse_args()
+    return fdroidserver.common.options
+
+
 def setup_global_opts(parser):
     try:  # the buildserver VM might not have PIL installed
         from PIL import PngImagePlugin
@@ -376,20 +401,38 @@ def fill_config_defaults(thisconfig):
         thisconfig['gradle_version_dir'] = str(Path(thisconfig['cachedir']) / 'gradle')
 
 
-def get_config(opts=None):
-    """Get config instace. This function takes care of initializing config data before returning it."""
-    global config, options
+def get_config():
+    """Get the initalized, singleton config instance.
+
+    config and options are intertwined in read_config(), so they have
+    to be here too.  In the current ugly state of things, there are
+    multiple potential instances of config and options in use:
+
+    * global
+    * module-level in the subcommand module (e.g. fdroidserver/build.py)
+    * module-level in fdroidserver.common
+
+    There are some insane parts of the code that are probably
+    referring to multiple instances of these at different points.
+    This can be super confusing and maddening.
+
+    The current intermediate refactoring step is to move all
+    subcommands to always get/set config and options via this function
+    so that there is no longer a distinction between the global and
+    module-level instances.  Then there can be only one module-level
+    instance in fdroidserver.common.
+
+    """
+    global config
 
     if config is not None:
         return config
 
-    common.read_config(opts=opts)
+    read_config()
 
     # make sure these values are available in common.py even if they didn't
     # declare global in a scope
     common.config = config
-    if opts is not None:
-        common.options = opts
 
     return config
 
@@ -422,7 +465,7 @@ def config_type_check(path, data):
         )
 
 
-def read_config(opts=None):
+def read_config():
     """Read the repository config.
 
     The config is read from config_file, which is in the current
@@ -441,13 +484,11 @@ def read_config(opts=None):
     in git, it makes sense to use a globally standard encoding.
 
     """
-    global config, options
+    global config
 
     if config is not None:
         return config
 
-    options = opts
-
     config = {}
     config_file = 'config.yml'
     old_config_file = 'config.py'
@@ -480,8 +521,8 @@ def read_config(opts=None):
     # smartcardoptions must be a list since its command line args for Popen
     smartcardoptions = config.get('smartcardoptions')
     if isinstance(smartcardoptions, str):
-        options = re.sub(r'\s+', r' ', config['smartcardoptions']).split(' ')
-        config['smartcardoptions'] = [i.strip() for i in options if i]
+        sco_items = re.sub(r'\s+', r' ', config['smartcardoptions']).split(' ')
+        config['smartcardoptions'] = [i.strip() for i in sco_items if i]
     elif not smartcardoptions and 'keystore' in config and config['keystore'] == 'NONE':
         # keystore='NONE' means use smartcard, these are required defaults
         config['smartcardoptions'] = ['-storetype', 'PKCS11', '-providerName',
@@ -573,7 +614,7 @@ def parse_mirrors_config(mirrors):
 def file_entry(filename, hash_value=None):
     meta = {}
     meta["name"] = "/" + Path(filename).as_posix().split("/", 1)[1]
-    meta["sha256"] = hash_value or common.sha256sum(filename)
+    meta["sha256"] = hash_value or sha256sum(filename)
     meta["size"] = os.stat(filename).st_size
     return meta
 
@@ -2354,6 +2395,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 +2705,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 +2713,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 +3205,7 @@ def signer_fingerprint_short(cert_encoded):
 
 
 def signer_fingerprint(cert_encoded):
-    """Obtain sha256 signing-key fingerprint for pkcs7 DER certificate.
-
-    Extracts hexadecimal sha256 signing-key fingerprint string
-    for a given pkcs7 signature.
+    """Return SHA-256 signer fingerprint for PKCS#7 DER-encoded signature.
 
     Parameters
     ----------
@@ -3173,46 +3213,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."""
-    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]))
+    """Get the first signing certificate from the APK, DER-encoded.
 
-    if not cert_encoded:
-        apkobject = get_androguard_APK(apkpath)
-        certs = apkobject.get_certificates_der_v2()
-        if len(certs) > 0:
-            logging.debug(_('Using APK Signature v2'))
-            cert_encoded = certs[0]
+    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 targetSdkVersion 30, APK v2 Signatures are required.
+    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()
-            if len(certs) > 0:
-                logging.debug(_('Using APK Signature v3'))
-                cert_encoded = certs[0]
+            logging.debug(_('Using APK Signature v3'))
+            cert_encoded = cert_v3
+
+    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.
-
-    Extracts hexadecimal sha256 signing-key fingerprint string
-    for a given APK.
+    """Get SHA-256 fingerprint string for the first signer from given APK.
 
     Parameters
     ----------
@@ -3221,7 +3328,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 +3338,7 @@ def apk_signer_fingerprint(apk_path):
 
 
 def apk_signer_fingerprint_short(apk_path):
-    """Obtain shortened sha256 signing-key fingerprint for APK.
-
-    Extracts the first 7 hexadecimal digits of sha256 signing-key fingerprint
-    for a given pkcs7 APK.
+    """Get 7 hex digit SHA-256 fingerprint string for the first signer from given APK.
 
     Parameters
     ----------
@@ -3242,7 +3347,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 +3567,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 +3578,7 @@ def get_min_sdk_version(apk):
 
     Returns
     -------
-    minsdk: int
+    minSdkVersion: int
     """
     try:
         return int(apk.get_min_sdk_version())
@@ -3480,6 +3586,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 +4015,33 @@ def get_cert_fingerprint(pubkey):
     return " ".join(ret)
 
 
-def get_certificate(signature_block_file):
-    """Extract a DER certificate from JAR Signature's "Signature Block File".
+def get_certificate(signature_block_file, signature_file=None):
+    """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
-        certificate, as read directly out of the APK/ZIP
+        Bytes representing the PKCS#7 signer certificate and
+        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 +4049,107 @@ def get_certificate(signature_block_file):
     or None in case of error
 
     """
-    content = decoder.decode(signature_block_file, asn1Spec=rfc2315.ContentInfo())[0]
-    if content.getComponentByName('contentType') != rfc2315.signedData:
-        return None
-    content = decoder.decode(content.getComponentByName('content'),
-                             asn1Spec=rfc2315.SignedData())[0]
-    try:
-        certificates = content.getComponentByName('certificates')
-        cert = certificates[0].getComponentByName('certificate')
-    except PyAsn1Error:
-        logging.error("Certificates not found.")
-        return None
-    return encoder.encode(cert)
+    pkcs7obj = cms.ContentInfo.load(signature_block_file)
+    certificates = pkcs7obj['content']['certificates']
+    if len(certificates) == 1:
+        return certificates[0].chosen.dump()
+    elif not signature_file:
+        logging.error(_('Found multiple Signer Certificates!'))
+        return
+    certificate = get_jar_signer_certificate(pkcs7obj, signature_file)
+    if certificate:
+        return certificate.chosen.dump()
+
+
+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/deploy.py

@@ -36,7 +36,6 @@ from . import index
 from .exception import FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 GIT_BRANCH = 'master'
@@ -148,6 +147,7 @@ def update_awsbucket_s3cmd(repo_section):
             raise FDroidException()
 
     s3cmd_sync = s3cmd + ['sync', '--acl-public']
+    options = common.get_options()
     if options.verbose:
         s3cmd_sync += ['--verbose']
     if options.quiet:
@@ -312,13 +312,14 @@ def update_serverwebroot(serverwebroot, repo_section):
             _('rsync is missing or broken: {error}').format(error=e)
         ) from e
     rsyncargs = ['rsync', '--archive', '--delete-after', '--safe-links']
-    if not options or not options.no_checksum:
+    options = common.get_options()
+    if not options.no_checksum:
         rsyncargs.append('--checksum')
-    if options and options.verbose:
+    if options.verbose:
         rsyncargs += ['--verbose']
-    if options and options.quiet:
+    if options.quiet:
         rsyncargs += ['--quiet']
-    if options and options.identity_file:
+    if options.identity_file:
         rsyncargs += [
             '-e',
             'ssh -oBatchMode=yes -oIdentitiesOnly=yes -i ' + options.identity_file,
@@ -387,7 +388,7 @@ def sync_from_localcopy(repo_section, local_copy_dir):
     # trailing slashes have a meaning in rsync which is not needed here, so
     # make sure both paths have exactly one trailing slash
     common.local_rsync(
-        options,
+        common.get_options(),
         os.path.join(local_copy_dir, repo_section).rstrip('/') + '/',
         repo_section.rstrip('/') + '/',
     )
@@ -407,7 +408,7 @@ def update_localcopy(repo_section, local_copy_dir):
 
     """
     # local_copy_dir is guaranteed to have a trailing slash in main() below
-    common.local_rsync(options, repo_section, local_copy_dir)
+    common.local_rsync(common.get_options(), repo_section, local_copy_dir)
 
     offline_copy = os.path.join(os.getcwd(), BINARY_TRANSPARENCY_DIR)
     if os.path.isdir(os.path.join(offline_copy, '.git')):
@@ -446,6 +447,8 @@ def update_servergitmirrors(servergitmirrors, repo_section):
         )
         return
 
+    options = common.get_options()
+
     # right now we support only 'repo' git-mirroring
     if repo_section == 'repo':
         git_mirror_path = 'git-mirror'
@@ -595,7 +598,7 @@ def upload_to_android_observatory(repo_section):
 
     requests  # stop unused import warning
 
-    if options.verbose:
+    if common.get_options().verbose:
         logging.getLogger("requests").setLevel(logging.INFO)
         logging.getLogger("urllib3").setLevel(logging.INFO)
     else:
@@ -849,7 +852,7 @@ def push_binary_transparency(git_repo_path, git_remote):
 
 
 def main():
-    global config, options
+    global config
 
     parser = ArgumentParser()
     common.setup_global_opts(parser)
@@ -876,8 +879,8 @@ def main():
         default=False,
         help=_("If a git mirror gets to big, allow the archive to be deleted"),
     )
-    options = parser.parse_args()
-    config = common.read_config(options)
+    options = common.parse_args(parser)
+    config = common.read_config()
 
     if config.get('nonstandardwebroot') is True:
         standardwebroot = False

fdroidserver/gpgsign.py

@@ -28,7 +28,6 @@ from .common import FDroidPopen
 from .exception import FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -42,14 +41,14 @@ def status_update_json(signed):
 
 
 def main():
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     repodirs = ['repo']
     if config['archive_older'] != 0:

fdroidserver/import_subcommand.py

@@ -43,7 +43,6 @@ from .exception import FDroidException
 
 
 config = None
-options = None
 
 
 def handle_retree_error_on_windows(function, path, excinfo):
@@ -53,7 +52,7 @@ def handle_retree_error_on_windows(function, path, excinfo):
         function(path)
 
 
-def clone_to_tmp_dir(app):
+def clone_to_tmp_dir(app, rev=None):
     tmp_dir = Path('tmp')
     tmp_dir.mkdir(exist_ok=True)
 
@@ -62,7 +61,7 @@ def clone_to_tmp_dir(app):
     if tmp_dir.exists():
         shutil.rmtree(str(tmp_dir), onerror=handle_retree_error_on_windows)
     vcs = common.getvcs(app.RepoType, app.Repo, tmp_dir)
-    vcs.gotorevision(options.rev)
+    vcs.gotorevision(rev)
 
     return tmp_dir
 
@@ -183,7 +182,7 @@ def get_app_from_url(url):
 
 
 def main():
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -201,10 +200,10 @@ def main():
     parser.add_argument("--rev", default=None,
                         help=_("Allows a different revision (or git branch) to be specified for the initial import"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     apps = metadata.read_metadata()
     app = None
@@ -235,7 +234,7 @@ def main():
         write_local_file = True
     elif options.url:
         app = get_app_from_url(options.url)
-        tmp_importer_dir = clone_to_tmp_dir(app)
+        tmp_importer_dir = clone_to_tmp_dir(app, options.rev)
         git_repo = git.Repo(tmp_importer_dir)
 
         if not options.omit_disable:

fdroidserver/init.py

@@ -32,7 +32,6 @@ from . import common
 from .exception import FDroidException
 
 config = {}
-options = None
 
 
 def disable_in_config(key, value):
@@ -49,7 +48,7 @@ def disable_in_config(key, value):
 
 
 def main():
-    global options, config
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -81,7 +80,7 @@ def main():
         default=False,
         help=_("Do not prompt for Android SDK path, just fail"),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     common.set_console_logging(options.verbose)
 
@@ -171,7 +170,7 @@ def main():
         raise FDroidException('Repository already exists.')
 
     # now that we have a local config.yml, read configuration...
-    config = common.read_config(options)
+    config = common.read_config()
 
     # the NDK is optional and there may be multiple versions of it, so it's
     # left for the user to configure

fdroidserver/install.py

@@ -28,7 +28,6 @@ from . import common
 from .common import SdkToolsPopen
 from .exception import FDroidException
 
-options = None
 config = None
 
 
@@ -44,7 +43,7 @@ def devices():
 
 
 def main():
-    global options, config
+    global config
 
     # Parse command line...
     parser = ArgumentParser(
@@ -63,7 +62,7 @@ def main():
         default=False,
         help=_("Install all signed applications available"),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     common.set_console_logging(options.verbose)
 
@@ -73,7 +72,7 @@ def main():
             % "all"
         )
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     output_dir = 'repo'
     if not os.path.isdir(output_dir):

fdroidserver/lint.py

@@ -31,7 +31,6 @@ from . import metadata
 from . import rewritemeta
 
 config = None
-options = None
 
 
 def enforce_https(domain):
@@ -500,7 +499,7 @@ def check_files_dir(app):
 
 
 def check_format(app):
-    if options.format and not rewritemeta.proper_format(app):
+    if common.options.format and not rewritemeta.proper_format(app):
         yield _("Run rewritemeta to fix formatting")
 
 
@@ -722,7 +721,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 _(
@@ -778,7 +783,7 @@ def lint_config(arg):
 
 
 def main():
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser()
@@ -803,10 +808,10 @@ def main():
         "appid", nargs='*', help=_("application ID of file to operate on")
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
     load_antiFeatures_config()
     load_categories_config()
 

fdroidserver/mirror.py

@@ -15,11 +15,9 @@ from . import common
 from . import index
 from . import update
 
-options = None
 
-
-def _run_wget(path, urls):
-    if options.verbose:
+def _run_wget(path, urls, verbose=False):
+    if verbose:
         verbose = '--verbose'
     else:
         verbose = '--no-verbose'
@@ -48,8 +46,6 @@ def _run_wget(path, urls):
 
 
 def main():
-    global options
-
     parser = ArgumentParser()
     common.setup_global_opts(parser)
     parser.add_argument(
@@ -93,7 +89,7 @@ def main():
     parser.add_argument(
         "--output-dir", default=None, help=_("The directory to write the mirror to")
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
     common.set_console_logging(options.verbose)
 
@@ -119,7 +115,7 @@ def main():
         )
 
     if fingerprint:
-        config = common.read_config(options)
+        config = common.read_config()
         if not ('jarsigner' in config or 'apksigner' in config):
             logging.error(
                 _('Java JDK not found! Install in standard location or set java_paths!')
@@ -229,7 +225,7 @@ def main():
                                 _append_to_url_path(section, f[:-4] + '.log.gz')
                             )
 
-        _run_wget(sectiondir, urls)
+        _run_wget(sectiondir, urls, options.verbose)
 
         for app in data['apps']:
             localized = app.get('localized')
@@ -242,7 +238,7 @@ def main():
                         if f:
                             filepath_tuple = components + (f,)
                             urls.append(_append_to_url_path(*filepath_tuple))
-                    _run_wget(os.path.join(basedir, *components), urls)
+                    _run_wget(os.path.join(basedir, *components), urls, options.verbose)
                     for k in update.SCREENSHOT_DIRS:
                         urls = []
                         filelist = d.get(k)
@@ -251,7 +247,11 @@ def main():
                             for f in filelist:
                                 filepath_tuple = components + (f,)
                                 urls.append(_append_to_url_path(*filepath_tuple))
-                            _run_wget(os.path.join(basedir, *components), urls)
+                            _run_wget(
+                                os.path.join(basedir, *components),
+                                urls,
+                                options.verbose,
+                            )
 
         urls = dict()
         for app in data['apps']:
@@ -269,7 +269,11 @@ def main():
 
         for icondir in icondirs:
             if icondir in urls:
-                _run_wget(os.path.join(basedir, section, icondir), urls[icondir])
+                _run_wget(
+                    os.path.join(basedir, section, icondir),
+                    urls[icondir],
+                    options.verbose,
+                )
 
 
 if __name__ == "__main__":

fdroidserver/nightly.py

@@ -204,8 +204,7 @@ def main():
         help=_("Set maximum releases in repo before older ones are archived"),
     )
     # TODO add --with-btlog
-    options = parser.parse_args()
-    common.options = options
+    options = common.parse_args(parser)
 
     # force a tighter umask since this writes private key material
     umask = os.umask(0o077)
@@ -373,7 +372,7 @@ Last updated: {date}'''.format(repo_git_base=repo_git_base,
         with open('config.yml', 'w') as fp:
             yaml.dump(config, fp, default_flow_style=False)
         os.chmod('config.yml', 0o600)
-        config = common.read_config(options)
+        config = common.read_config()
         common.assert_config_keystore(config)
 
         for root, dirs, files in os.walk(cibase):

fdroidserver/publish.py

@@ -49,7 +49,6 @@ from .common import FDroidPopen
 from .exception import BuildException, FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -269,7 +268,7 @@ def create_key_if_not_existing(keyalias):
 
 
 def main():
-    global config, options
+    global config
 
     # Parse command line...
     parser = ArgumentParser(
@@ -289,10 +288,10 @@ def main():
         help=_("application ID with optional versionCode in the form APPID[:VERCODE]"),
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     if not ('jarsigner' in config and 'keytool' in config):
         logging.critical(

fdroidserver/readmeta.py

@@ -20,8 +20,6 @@ from argparse import ArgumentParser
 from . import common
 from . import metadata
 
-options = None
-
 
 def main():
     parser = ArgumentParser()
@@ -29,7 +27,7 @@ def main():
     metadata.add_metadata_arguments(parser)
     options = parser.parse_args()
     metadata.warnings_action = options.W
-    common.read_config(None)
+    common.read_config()
 
     metadata.read_metadata()
 

fdroidserver/rewritemeta.py

@@ -29,7 +29,6 @@ from . import common
 from . import metadata
 
 config = None
-options = None
 
 
 def proper_format(app):
@@ -62,7 +61,7 @@ def remove_blank_flags_from_builds(builds):
 
 
 def main():
-    global config, options
+    global config
 
     parser = ArgumentParser()
     common.setup_global_opts(parser)
@@ -77,10 +76,10 @@ def main():
         "appid", nargs='*', help=_("application ID of file to operate on")
     )
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     # Get all apps...
     allapps = metadata.read_metadata(options.appid)

fdroidserver/scanner.py

@@ -40,8 +40,6 @@ from . import metadata
 from .exception import BuildException, VCSException, ConfigurationException
 from . import scanner
 
-options = None
-
 
 @dataclass
 class MessageStore:
@@ -332,8 +330,9 @@ class ScannerTool:
 
         self.scanner_data_lookup()
 
-        config = common.get_config()
-        if (options and options.refresh_scanner) or config.get('refresh_scanner'):
+        options = common.get_options()
+        options_refresh_scanner = options and options.refresh_scanner
+        if options_refresh_scanner or common.get_config().get('refresh_scanner'):
             self.refresh()
 
         self.load()
@@ -589,6 +588,7 @@ def scan_source(build_dir, build=metadata.Build(), json_per_build=None):
         -------
         0 if the problem was ignored/deleted/is only a warning, 1 otherwise
         """
+        options = common.get_options()
         if toignore(path_in_build_dir):
             return ignoreproblem(what, path_in_build_dir, json_per_build)
         if todelete(path_in_build_dir):
@@ -776,9 +776,6 @@ def scan_source(build_dir, build=metadata.Build(), json_per_build=None):
 
 
 def main():
-    global options
-
-    # Parse command line...
     parser = ArgumentParser(
         usage="%(prog)s [options] [(APPID[:VERCODE] | path/to.apk) ...]"
     )
@@ -793,7 +790,7 @@ def main():
     parser.add_argument("-e", "--exit-code", action="store_true", default=False,
                         help=_("Exit with a non-zero code if problems were found"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
     json_output = dict()
@@ -804,7 +801,7 @@ def main():
             logging.getLogger().setLevel(logging.ERROR)
 
     # initialize/load configuration values
-    common.get_config(opts=options)
+    common.get_config()
 
     probcount = 0
 

fdroidserver/signatures.py

@@ -103,11 +103,8 @@ def main():
         "APK", nargs='*', help=_("signed APK, either a file-path or HTTPS URL.")
     )
     parser.add_argument("--no-check-https", action="store_true", default=False)
-    options = parser.parse_args()
-
+    options = common.parse_args(parser)
     common.set_console_logging(options.verbose)
-
-    # Read config.py...
-    common.read_config(options)
+    common.read_config()
 
     extract(options)

fdroidserver/signindex.py

@@ -29,7 +29,6 @@ from . import metadata
 from .exception import FDroidException
 
 config = None
-options = None
 start_timestamp = time.gmtime()
 
 
@@ -175,13 +174,13 @@ def status_update_json(signed):
 
 
 def main():
-    global config, options
+    global config
 
     parser = ArgumentParser()
     common.setup_global_opts(parser)
-    options = parser.parse_args()
+    common.parse_args(parser)
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     if 'jarsigner' not in config:
         raise FDroidException(

fdroidserver/update.py

@@ -2574,10 +2574,10 @@ def main():
     parser.add_argument("--allow-disabled-algorithms", action="store_true", default=False,
                         help=_("Include APKs that are signed with disabled algorithms like MD5"))
     metadata.add_metadata_arguments(parser)
-    options = parser.parse_args()
+    options = common.parse_args(parser)
     metadata.warnings_action = options.W
 
-    config = common.read_config(options)
+    config = common.read_config()
     common.setup_status_output(start_timestamp)
 
     if not (('jarsigner' in config or 'apksigner' in config)

fdroidserver/verify.py

@@ -30,7 +30,6 @@ from . import common
 from . import net
 from .exception import FDroidException
 
-options = None
 config = None
 
 
@@ -146,7 +145,7 @@ def write_json_report(url, remote_apk, unsigned_apk, compare_result):
 
 
 def main():
-    global options, config
+    global config
 
     # Parse command line...
     parser = ArgumentParser(
@@ -170,9 +169,9 @@ def main():
         default=False,
         help=_("Output JSON report to file named after APK."),
     )
-    options = parser.parse_args()
+    options = common.parse_args(parser)
 
-    config = common.read_config(options)
+    config = common.read_config()
 
     tmp_dir = 'tmp'
     if not os.path.isdir(tmp_dir):

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/build.TestCase

@@ -4,7 +4,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -29,7 +28,7 @@ import fdroidserver.common
 import fdroidserver.metadata
 import fdroidserver.scanner
 import fdroidserver.vmtools
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class FakeProcess:
@@ -562,7 +561,7 @@ class BuildTest(unittest.TestCase):
         os.chdir(self.testdir)
         os.mkdir("build")
 
-        config = fdroidserver.common.get_config()
+        config = fdroidserver.common.read_config()
         config['sdk_path'] = os.getenv('ANDROID_HOME')
         config['ndk_paths'] = {'r10d': os.getenv('ANDROID_NDK_HOME')}
         fdroidserver.common.config = config
@@ -1104,15 +1103,17 @@ class BuildTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(BuildTest))

tests/checkupdates.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -30,8 +29,6 @@ class CheckupdatesTest(unittest.TestCase):
         os.chdir(self.basedir)
 
     def test_autoupdatemode_no_suffix(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -52,7 +49,7 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9')
@@ -64,15 +61,13 @@ class CheckupdatesTest(unittest.TestCase):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
                     with self.assertRaises(FDroidException):
-                        fdroidserver.checkupdates.checkupdates_app(app)
+                        fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9')
         self.assertEqual(build.commit, '1.1.9')
 
     def test_autoupdatemode_suffix(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -93,15 +88,13 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9.10109-fdroid')
         self.assertEqual(build.commit, 'v1.1.9_10109')
 
     def test_autoupdate_multi_variants(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -134,7 +127,7 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-2]
         self.assertEqual(build.versionName, '1.1.9')
@@ -150,8 +143,6 @@ class CheckupdatesTest(unittest.TestCase):
         self.assertEqual(app.CurrentVersionCode, 101093)
 
     def test_checkupdates_app_http(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -165,7 +156,7 @@ class CheckupdatesTest(unittest.TestCase):
             'fdroidserver.checkupdates.check_http', lambda app: (None, 'bla')
         ):
             with self.assertRaises(FDroidException):
-                fdroidserver.checkupdates.checkupdates_app(app)
+                fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         with mock.patch(
             'fdroidserver.checkupdates.check_http', lambda app: ('1.1.9', 10109)
@@ -174,12 +165,10 @@ class CheckupdatesTest(unittest.TestCase):
                 'fdroidserver.metadata.write_metadata', mock.Mock()
             ) as wrmock:
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
                 wrmock.assert_called_with(app.metadatapath, app)
 
     def test_checkupdates_app_tags(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-        fdroidserver.checkupdates.options.auto = 'bleh'
         fdroidserver.checkupdates.config = {}
 
         app = fdroidserver.metadata.App()
@@ -200,7 +189,7 @@ class CheckupdatesTest(unittest.TestCase):
             lambda app, pattern: (None, 'bla', None),
         ):
             with self.assertRaises(FDroidException):
-                fdroidserver.checkupdates.checkupdates_app(app)
+                fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         with mock.patch(
             'fdroidserver.checkupdates.check_tags',
@@ -208,15 +197,13 @@ class CheckupdatesTest(unittest.TestCase):
         ):
             with mock.patch('fdroidserver.metadata.write_metadata', mock.Mock()):
                 with mock.patch('subprocess.call', lambda cmd: 0):
-                    fdroidserver.checkupdates.checkupdates_app(app)
+                    fdroidserver.checkupdates.checkupdates_app(app, auto=True)
 
         build = app['Builds'][-1]
         self.assertEqual(build.versionName, '1.1.9')
         self.assertEqual(build.commit, 'v1.1.9')
 
     def test_check_http(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-
         app = fdroidserver.metadata.App()
         app.id = 'loop.starts.shooting'
         app.metadatapath = 'metadata/' + app.id + '.yml'
@@ -243,8 +230,6 @@ class CheckupdatesTest(unittest.TestCase):
                 fdroidserver.checkupdates.check_http(app)
 
     def test_check_http_ignore(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-
         app = fdroidserver.metadata.App()
         app.id = 'loop.starts.shooting'
         app.metadatapath = 'metadata/' + app.id + '.yml'
@@ -260,8 +245,6 @@ class CheckupdatesTest(unittest.TestCase):
         self.assertEqual(vername, None)
 
     def test_check_tags_data(self):
-        fdroidserver.checkupdates.options = mock.Mock()
-
         app = fdroidserver.metadata.App()
         app.id = 'loop.starts.shooting'
         app.metadatapath = 'metadata/' + app.id + '.yml'
@@ -336,15 +319,18 @@ class CheckupdatesTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+    from testcommon import parse_args_for_test
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(CheckupdatesTest))

tests/common.TestCase

@@ -9,7 +9,6 @@ import importlib
 import inspect
 import json
 import logging
-import optparse
 import os
 import re
 import ruamel.yaml
@@ -22,6 +21,7 @@ import unittest
 import textwrap
 import yaml
 import gzip
+from argparse import ArgumentParser
 from zipfile import BadZipFile, ZipFile
 from unittest import mock
 from pathlib import Path
@@ -38,13 +38,19 @@ import fdroidserver.index
 import fdroidserver.signindex
 import fdroidserver.common
 import fdroidserver.metadata
-from testcommon import TmpCwd, mkdtemp
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 from fdroidserver.common import ANTIFEATURES_CONFIG_NAME, CATEGORIES_CONFIG_NAME
 from fdroidserver.exception import FDroidException, VCSException,\
     MetaDataException, VerificationException
 from fdroidserver.looseversion import LooseVersion
 
 
+def _mock_common_module_options_instance():
+    """Helper method to deal with difficult visibility of the module-level options."""
+    fdroidserver.common.options = mock.Mock()
+    fdroidserver.common.options.verbose = False
+
+
 class CommonTest(unittest.TestCase):
     '''fdroidserver/common.py'''
 
@@ -57,17 +63,22 @@ class CommonTest(unittest.TestCase):
         if not os.path.exists(self.tmpdir):
             os.makedirs(self.tmpdir)
         os.chdir(self.basedir)
+
+        # these are declared as None at the top of the module file
         fdroidserver.common.config = None
-        fdroidserver.common.options = mock.Mock()
-        fdroidserver.common.options.verbose = False
+        fdroidserver.common.options = None
         fdroidserver.metadata.srclibs = None
+
         self._td = mkdtemp()
         self.testdir = self._td.name
 
     def tearDown(self):
+        fdroidserver.common.config = None
+        fdroidserver.common.options = None
         os.chdir(self.basedir)
         self._td.cleanup()
-        shutil.rmtree(self.tmpdir)
+        if os.path.exists(self.tmpdir):
+            shutil.rmtree(self.tmpdir)
 
     def test_parse_human_readable_size(self):
         for k, v in (
@@ -356,6 +367,7 @@ class CommonTest(unittest.TestCase):
         config = dict()
         fdroidserver.common.fill_config_defaults(config)
         fdroidserver.common.config = config
+        _mock_common_module_options_instance()
 
         srclibname = 'FakeSrcLib'
         srclib_testdir = os.path.join(self.testdir, 'build', 'srclib')
@@ -397,6 +409,7 @@ class CommonTest(unittest.TestCase):
                                            onserver=True, refresh=False)  # do not clone in this test
 
     def test_prepare_sources_refresh(self):
+        _mock_common_module_options_instance()
         packageName = 'org.fdroid.ci.test.app'
         os.chdir(self.tmpdir)
         os.mkdir('build')
@@ -467,6 +480,7 @@ class CommonTest(unittest.TestCase):
         config = dict()
         fdroidserver.common.fill_config_defaults(config)
         fdroidserver.common.config = config
+        _mock_common_module_options_instance()
 
         commands = ['sh', '-c', 'echo stdout message && echo stderr message 1>&2']
 
@@ -477,7 +491,8 @@ class CommonTest(unittest.TestCase):
         self.assertEqual(p.output, 'stdout message\n')
 
     def test_signjar(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
         fdroidserver.signindex.config = config
@@ -497,7 +512,8 @@ class CommonTest(unittest.TestCase):
                 )
 
     def test_verify_apk_signature(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         fdroidserver.common.config = config
 
         self.assertTrue(fdroidserver.common.verify_apk_signature('bad-unicode-πÇÇ现代通用字-български-عربي1.apk'))
@@ -519,7 +535,8 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(fdroidserver.common.verify_apk_signature('urzip-release-unsigned.apk'))
 
     def test_verify_old_apk_signature(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
 
@@ -540,7 +557,7 @@ class CommonTest(unittest.TestCase):
 
     def test_verify_jar_signature(self):
         """Sign entry.jar and make sure it validates"""
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         config['keystore'] = os.path.join(self.basedir, 'keystore.jks')
         config['repo_keyalias'] = 'sova'
@@ -558,7 +575,7 @@ class CommonTest(unittest.TestCase):
 
     def test_verify_jar_signature_fails(self):
         """Test verify_jar_signature fails on unsigned and deprecated algorithms"""
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
         source_dir = os.path.join(self.basedir, 'signindex')
@@ -568,7 +585,7 @@ class CommonTest(unittest.TestCase):
                 fdroidserver.common.verify_jar_signature(testfile)
 
     def test_verify_deprecated_jar_signature(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
         source_dir = os.path.join(self.basedir, 'signindex')
@@ -581,8 +598,9 @@ class CommonTest(unittest.TestCase):
             fdroidserver.common.verify_deprecated_jar_signature(testfile)
 
     def test_verify_apks(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         fdroidserver.common.config = config
+        _mock_common_module_options_instance()
 
         sourceapk = os.path.join(self.basedir, 'urzip.apk')
 
@@ -615,6 +633,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')
@@ -868,7 +907,8 @@ class CommonTest(unittest.TestCase):
             )
 
     def test_sign_apk(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_apk, apksigner not installed!')
 
@@ -938,7 +978,8 @@ class CommonTest(unittest.TestCase):
 
     @unittest.skipIf(os.getuid() == 0, 'This is meaningless when run as root')
     def test_sign_apk_fail(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_apk_fail, apksigner not installed!')
 
@@ -961,7 +1002,8 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(os.path.isfile(signed))
 
     def test_sign_apk_corrupt(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_apk_corrupt, apksigner not installed!')
 
@@ -987,7 +1029,8 @@ class CommonTest(unittest.TestCase):
     )
     def test_resign_apk(self):
         """When using apksigner, it should resign signed APKs"""
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        _mock_common_module_options_instance()
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_resign_apk, apksigner not installed!')
 
@@ -1039,6 +1082,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 +1202,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 +1217,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'))
@@ -1882,7 +1936,7 @@ class CommonTest(unittest.TestCase):
         os.chdir(self.tmpdir)
         self.assertFalse(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse(config.get('update_stats'))
         self.assertIsNotNone(config.get('char_limits'))
 
@@ -1892,7 +1946,7 @@ class CommonTest(unittest.TestCase):
         open('config.yml', 'w').close()
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse(config.get('update_stats'))
         self.assertIsNotNone(config.get('char_limits'))
 
@@ -1903,7 +1957,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner: yml')
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('yml', config.get('apksigner'))
 
     def test_with_config_yml_utf8(self):
@@ -1914,7 +1968,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner: ' + teststr)
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual(teststr, config.get('apksigner'))
 
     def test_with_config_yml_utf8_as_ascii(self):
@@ -1925,7 +1979,7 @@ class CommonTest(unittest.TestCase):
             yaml.dump({'apksigner': teststr}, fp)
         self.assertTrue(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual(teststr, config.get('apksigner'))
 
     def test_with_config_yml_with_env_var(self):
@@ -1937,20 +1991,20 @@ class CommonTest(unittest.TestCase):
                 fp.write("""keypass: {'env': 'SECRET'}""")
             self.assertTrue(os.path.exists('config.yml'))
             self.assertFalse(os.path.exists('config.py'))
-            config = fdroidserver.common.read_config(fdroidserver.common.options)
+            config = fdroidserver.common.read_config()
             self.assertEqual(os.getenv('SECRET', 'fail'), config.get('keypass'))
 
     def test_with_config_yml_is_dict(self):
         os.chdir(self.tmpdir)
         Path('config.yml').write_text('apksigner = /placeholder/path')
         with self.assertRaises(TypeError):
-            fdroidserver.common.read_config(fdroidserver.common.options)
+            fdroidserver.common.read_config()
 
     def test_with_config_yml_is_not_mixed_type(self):
         os.chdir(self.tmpdir)
         Path('config.yml').write_text('k: v\napksigner = /placeholder/path')
         with self.assertRaises(yaml.scanner.ScannerError):
-            fdroidserver.common.read_config(fdroidserver.common.options)
+            fdroidserver.common.read_config()
 
     def test_with_config_py(self):
         """Make sure it is still possible to use config.py alone."""
@@ -1959,7 +2013,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner = "py"')
         self.assertFalse(os.path.exists('config.yml'))
         self.assertTrue(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual("py", config.get('apksigner'))
 
     def test_config_perm_warning(self):
@@ -1969,7 +2023,7 @@ class CommonTest(unittest.TestCase):
             fp.write('keystore: foo.jks')
         self.assertTrue(os.path.exists(fp.name))
         os.chmod(fp.name, 0o666)
-        fdroidserver.common.read_config(fdroidserver.common.options)
+        fdroidserver.common.read_config()
         os.remove(fp.name)
         fdroidserver.common.config = None
 
@@ -1977,7 +2031,7 @@ class CommonTest(unittest.TestCase):
             fp.write('keystore = "foo.jks"')
         self.assertTrue(os.path.exists(fp.name))
         os.chmod(fp.name, 0o666)
-        fdroidserver.common.read_config(fdroidserver.common.options)
+        fdroidserver.common.read_config()
 
     def test_with_both_config_yml_py(self):
         """If config.yml and config.py are present, config.py should be ignored."""
@@ -1988,7 +2042,7 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner = "py"')
         self.assertTrue(os.path.exists('config.yml'))
         self.assertTrue(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('yml', config.get('apksigner'))
 
     def test_config_repo_url(self):
@@ -2039,14 +2093,14 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner: yml')
         self.assertTrue(os.path.exists(fp.name))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse('keypass' in config)
         self.assertEqual('yml', config.get('apksigner'))
         fdroidserver.common.write_to_config(config, 'keypass', 'mysecretpassword')
         with open(fp.name) as fp:
             print(fp.read())
         fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('mysecretpassword', config['keypass'])
 
     def test_write_to_config_py(self):
@@ -2055,12 +2109,12 @@ class CommonTest(unittest.TestCase):
             fp.write('apksigner = "py"')
         self.assertTrue(os.path.exists(fp.name))
         self.assertFalse(os.path.exists('config.yml'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertFalse('keypass' in config)
         self.assertEqual('py', config.get('apksigner'))
         fdroidserver.common.write_to_config(config, 'keypass', 'mysecretpassword')
         fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('mysecretpassword', config['keypass'])
 
     def test_config_dict_with_int_keys(self):
@@ -2069,7 +2123,7 @@ class CommonTest(unittest.TestCase):
             fp.write('java_paths:\n  8: /usr/lib/jvm/java-8-openjdk\n')
         self.assertTrue(os.path.exists(fp.name))
         self.assertFalse(os.path.exists('config.py'))
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('/usr/lib/jvm/java-8-openjdk', config.get('java_paths', {}).get('8'))
 
     @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
@@ -2147,7 +2201,7 @@ class CommonTest(unittest.TestCase):
         shutil.copy(os.path.join(self.basedir, '..', 'buildserver', 'config.buildserver.yml'),
                     'config.yml')
         self.assertFalse(os.path.exists('config.py'))
-        fdroidserver.common.read_config(fdroidserver.common.options)
+        fdroidserver.common.read_config()
 
     def test_setup_status_output(self):
         os.chdir(self.tmpdir)
@@ -2500,6 +2554,7 @@ class CommonTest(unittest.TestCase):
 
     @mock.patch.dict(os.environ, clear=True)
     def test_FDroidPopen_envs_paths_can_be_pathlib(self):
+        _mock_common_module_options_instance()
         os.environ['PATH'] = '/usr/bin:/usr/sbin'
         envs = {'PATHLIB': Path('/pathlib/path'), 'STRING': '/string/path'}
         p = fdroidserver.common.FDroidPopen(['/bin/sh', '-c', 'export'], envs=envs)
@@ -2915,18 +2970,372 @@ 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]),
+                )
+
+
+class ConfigOptionsScopeTest(unittest.TestCase):
+    """Test assumptions about variable scope for "config" and "options".
+
+    The ancient architecture of config and options in fdroidserver has
+    weird issues around unexpected scope, like there are cases where
+    the global config is not the same as the module-level config, and
+    more.
+
+    This is about describing what is happening, it is not about
+    documenting behaviors that are good design. The config and options
+    handling should really be refactored into a well-known, workable
+    Pythonic pattern.
+
+    """
+
+    def setUp(self):
+        # these are declared as None at the top of the module file
+        fdroidserver.common.config = None
+        fdroidserver.common.options = None
+
+    def tearDown(self):
+        fdroidserver.common.config = None
+        fdroidserver.common.options = None
+        if 'config' in globals():
+            global config
+            del config
+        if 'options' in globals():
+            global options
+            del options
+
+    def test_parse_args(self):
+        """Test that options is properly set up at the module-level and not global."""
+        self.assertFalse('options' in globals())
+        self.assertIsNone(fdroidserver.common.options)
+        parser = ArgumentParser()
+        fdroidserver.common.setup_global_opts(parser)
+        with mock.patch('sys.argv', ['$0']):
+            o = fdroidserver.common.parse_args(parser)
+        self.assertEqual(o, fdroidserver.common.options)
+
+        # No function should set options as a global, and the global
+        # keyword does not create the variable.
+        global options
+        with self.assertRaises(NameError):
+            options
+        self.assertFalse('options' in globals())
+
+    def test_parse_args_without_args(self):
+        """Test that the parsing function works fine when there are no args."""
+        parser = ArgumentParser()
+        fdroidserver.common.setup_global_opts(parser)
+        with mock.patch('sys.argv', ['$0']):
+            o = fdroidserver.common.parse_args(parser)
+        self.assertFalse(o.verbose)
+
+    def test_parse_args_with_args(self):
+        parser = ArgumentParser()
+        fdroidserver.common.setup_global_opts(parser)
+        with mock.patch('sys.argv', ['$0', '-v']):
+            o = fdroidserver.common.parse_args(parser)
+        self.assertTrue(o.verbose)
+
+    def test_get_config(self):
+        """Show how the module-level variables are initialized."""
+        self.assertTrue('config' not in vars() and 'config' not in globals())
+        self.assertIsNone(fdroidserver.common.config)
+        config = fdroidserver.common.read_config()
+        self.assertIsNotNone(fdroidserver.common.config)
+        self.assertEqual(dict, type(config))
+        self.assertEqual(config, fdroidserver.common.config)
+
+    def test_get_config_global(self):
+        """Test assumptions about variable scope using global keyword."""
+        global config
+        self.assertTrue('config' not in vars() and 'config' not in globals())
+        self.assertIsNone(fdroidserver.common.config)
+        c = fdroidserver.common.read_config()
+        self.assertIsNotNone(fdroidserver.common.config)
+        self.assertEqual(dict, type(c))
+        self.assertEqual(c, fdroidserver.common.config)
+        self.assertTrue(
+            'config' not in vars() and 'config' not in globals(),
+            "The config should not be set in the global context, only module-level.",
+        )
+
+
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    parser = ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(CommonTest))

tests/deploy.TestCase

@@ -2,7 +2,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import sys
 import tempfile
@@ -19,7 +18,7 @@ if localmodule not in sys.path:
 import fdroidserver.common
 import fdroidserver.deploy
 from fdroidserver.exception import FDroidException
-from testcommon import TmpCwd, mkdtemp
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 class DeployTest(unittest.TestCase):
@@ -115,11 +114,11 @@ class DeployTest(unittest.TestCase):
         self.maxDiff = None
 
         # setup parameters for this test run
-        fdroidserver.deploy.options = mock.Mock()
-        fdroidserver.deploy.options.no_checksum = True
-        fdroidserver.deploy.options.identity_file = None
-        fdroidserver.deploy.options.verbose = False
-        fdroidserver.deploy.options.quiet = True
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.no_checksum = True
+        fdroidserver.common.options.identity_file = None
+        fdroidserver.common.options.verbose = False
+        fdroidserver.common.options.quiet = True
         fdroidserver.deploy.config = {'make_current_version_link': True}
         url = "example.com:/var/www/fdroid"
         repo_section = 'repo'
@@ -208,12 +207,12 @@ class DeployTest(unittest.TestCase):
 
     def test_update_serverwebroot_with_id_file(self):
         # setup parameters for this test run
-        fdroidserver.deploy.options = mock.Mock()
-        fdroidserver.deploy.options.identity_file = None
-        fdroidserver.deploy.options.no_checksum = True
-        fdroidserver.deploy.options.verbose = True
-        fdroidserver.deploy.options.quiet = False
-        fdroidserver.deploy.options.identity_file = None
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.identity_file = None
+        fdroidserver.common.options.no_checksum = True
+        fdroidserver.common.options.verbose = True
+        fdroidserver.common.options.quiet = False
+        fdroidserver.common.options.identity_file = None
         fdroidserver.deploy.config = {'identity_file': './id_rsa'}
         url = "example.com:/var/www/fdroid"
         repo_section = 'archive'
@@ -290,7 +289,7 @@ class DeployTest(unittest.TestCase):
         not os.getenv('VIRUSTOTAL_API_KEY'), 'VIRUSTOTAL_API_KEY is not set'
     )
     def test_upload_to_virustotal(self):
-        fdroidserver.deploy.options.verbose = True
+        fdroidserver.common.options.verbose = True
         virustotal_apikey = os.getenv('VIRUSTOTAL_API_KEY')
         fdroidserver.deploy.upload_to_virustotal('repo', virustotal_apikey)
 
@@ -308,12 +307,12 @@ class DeployTest(unittest.TestCase):
 
     def test_update_servergitmirrors(self):
         # setup parameters for this test run
-        fdroidserver.deploy.options = mock.Mock()
-        fdroidserver.deploy.options.identity_file = None
-        fdroidserver.deploy.options.no_keep_git_mirror_archive = False
-        fdroidserver.deploy.options.verbose = False
-        fdroidserver.deploy.options.quiet = True
-        fdroidserver.deploy.options.index_only = False
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.identity_file = None
+        fdroidserver.common.options.no_keep_git_mirror_archive = False
+        fdroidserver.common.options.verbose = False
+        fdroidserver.common.options.quiet = True
+        fdroidserver.common.options.index_only = False
 
         config = {}
         fdroidserver.common.fill_config_defaults(config)
@@ -390,15 +389,17 @@ class DeployTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(DeployTest))

tests/dump_internal_metadata_format.py

@@ -56,9 +56,9 @@ def _build_yaml_representer(dumper, data):
 parser = ArgumentParser()
 fdroidserver.common.setup_global_opts(parser)
 fdroidserver.metadata.add_metadata_arguments(parser)
-options = parser.parse_args()
+options = fdroidserver.common.parse_args(parser)
 fdroidserver.metadata.warnings_action = options.W
-fdroidserver.common.read_config(None)
+fdroidserver.common.read_config()
 
 if not os.path.isdir('metadata'):
     print("This script must be run in an F-Droid data folder with a 'metadata' subdir!")

tests/exception.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import inspect
-import optparse
 import os
 import sys
 import unittest
@@ -57,16 +56,18 @@ class ExceptionTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+    from testcommon import parse_args_for_test
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.exception.options, args) = parser.parse_args(['--verbose'])
-    fdroidserver.common.options = fdroidserver.exception.options
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(ExceptionTest))

tests/gpgsign.TestCase

@@ -3,7 +3,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -33,7 +32,7 @@ class GpgsignTest(unittest.TestCase):
         self.repodir.mkdir()
 
         gpgsign.config = None
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['verbose'] = True
         config['gpghome'] = str((self.basedir / 'gnupghome').resolve())
         config['gpgkey'] = '1DBA2E89'
@@ -75,15 +74,17 @@ class GpgsignTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(GpgsignTest))

tests/import_subcommand.TestCase

@@ -4,7 +4,6 @@
 
 import git
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -15,7 +14,6 @@ from unittest import mock
 from pathlib import Path
 
 import requests
-from testcommon import TmpCwd, mkdtemp
 
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
@@ -26,6 +24,7 @@ import fdroidserver.common
 import fdroidserver.import_subcommand
 import fdroidserver.metadata
 from fdroidserver.exception import FDroidException
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 class ImportTest(unittest.TestCase):
@@ -34,8 +33,6 @@ class ImportTest(unittest.TestCase):
     def setUp(self):
         logging.basicConfig(level=logging.DEBUG)
         self.basedir = localmodule / 'tests'
-        fdroidserver.import_subcommand.options = mock.Mock()
-        fdroidserver.import_subcommand.options.rev = None
         os.chdir(self.basedir)
         self._td = mkdtemp()
         self.testdir = self._td.name
@@ -146,7 +143,9 @@ class ImportTest(unittest.TestCase):
             fdroidserver.import_subcommand.main()
 
     @mock.patch('sys.argv', ['fdroid import', '-u', 'https://fake/git/url.git'])
-    @mock.patch('fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a: Path('td'))
+    @mock.patch(
+        'fdroidserver.import_subcommand.clone_to_tmp_dir', lambda a, r: Path('td')
+    )
     def test_main_local_git(self):
         os.chdir(self.testdir)
         git.Repo.init('td')
@@ -161,15 +160,17 @@ class ImportTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(ImportTest))

tests/index.TestCase

@@ -2,9 +2,9 @@
 
 import copy
 import datetime
+import glob
 import inspect
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -25,7 +25,7 @@ if localmodule not in sys.path:
 
 import fdroidserver
 from fdroidserver import common, index, publish, signindex, update
-from testcommon import TmpCwd, mkdtemp
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 from pathlib import Path
 
 
@@ -55,7 +55,7 @@ class IndexTest(unittest.TestCase):
 
         common.config = None
         common.options = Options
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner')
         common.config = config
         signindex.config = config
@@ -418,6 +418,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 +712,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(1413, metadata['icon']['en-US']['size'])
+        self.assertEqual(
+            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'])
@@ -734,7 +751,7 @@ class IndexTest(unittest.TestCase):
             yaml.dump(c, fp)
         os.system('cat config.yml')
         common.config = None
-        common.read_config(Options)
+        common.read_config()
         repodict = {'address': common.config['repo_url']}
         index.add_mirrors_to_repodict('repo', repodict)
         self.assertEqual(
@@ -924,16 +941,17 @@ class AltstoreIndexTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (options, args) = parser.parse_args(["--verbose"])
-    Options.verbose = options.verbose
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(IndexTest))

tests/init.TestCase

@@ -5,7 +5,6 @@
 import inspect
 import logging
 import os
-import optparse
 import shutil
 import sys
 import unittest
@@ -19,7 +18,7 @@ if localmodule not in sys.path:
     sys.path.insert(0, localmodule)
 
 import fdroidserver.init
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class InitTest(unittest.TestCase):
@@ -43,14 +42,14 @@ class InitTest(unittest.TestCase):
             fp.write('keystore: NONE\n')
             fp.write('keypass: mysupersecrets\n')
         os.chmod('config.yml', 0o600)
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertEqual('NONE', config['keystore'])
         self.assertEqual('mysupersecrets', config['keypass'])
         fdroidserver.init.disable_in_config('keypass', 'comment')
         with open(fp.name) as fp:
             self.assertTrue('#keypass:' in fp.read())
         fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         self.assertIsNone(config.get('keypass'))
 
     @unittest.skipIf(os.name == 'nt', "calling main() like this hangs on Windows")
@@ -75,15 +74,17 @@ class InitTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.init.options, args) = parser.parse_args(['--verbose'])
+    fdroidserver.init.options = parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(InitTest))

tests/install.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import inspect
-import optparse
 import os
 import sys
 import unittest
@@ -38,16 +37,18 @@ class InstallTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+    from testcommon import parse_args_for_test
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.install.options, args) = parser.parse_args(['--verbose'])
-    fdroidserver.common.options = fdroidserver.install.options
+    fdroidserver.install.options = parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(InstallTest))

tests/key-tricks.py

@@ -11,7 +11,7 @@ if os.getenv('CI') is None:
     sys.exit(1)
 
 os.chdir(os.path.dirname(__file__))
-config = fdroidserver.common.read_config(common.options)
+config = fdroidserver.common.read_config()
 nightly.PASSWORD = config['keystorepass']
 nightly.KEY_ALIAS = config['repo_keyalias']
 

tests/lint.TestCase

@@ -3,7 +3,6 @@
 # http://www.drdobbs.com/testing/unit-testing-with-python/240165163
 
 import logging
-import optparse
 import os
 import ruamel.yaml
 import shutil
@@ -11,7 +10,6 @@ import sys
 import tempfile
 import unittest
 from pathlib import Path
-from testcommon import mkdtemp
 
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
@@ -22,6 +20,7 @@ import fdroidserver.common
 import fdroidserver.lint
 import fdroidserver.metadata
 from fdroidserver.common import CATEGORIES_CONFIG_NAME
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class LintTest(unittest.TestCase):
@@ -438,6 +437,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):
@@ -488,16 +526,17 @@ class LintAntiFeaturesTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.lint.options, args) = parser.parse_args(['--verbose'])
-    fdroidserver.common.options = fdroidserver.lint.options
+    fdroidserver.lint.options = parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(LintTest))

tests/main.TestCase

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import inspect
-import optparse
 import os
 import sys
 import pkgutil
@@ -9,7 +8,6 @@ import textwrap
 import unittest
 import tempfile
 from unittest import mock
-from testcommon import TmpCwd, TmpPyPath
 
 localmodule = os.path.realpath(
     os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..')
@@ -20,6 +18,7 @@ if localmodule not in sys.path:
 
 from fdroidserver import common
 import fdroidserver.__main__
+from testcommon import TmpCwd, TmpPyPath
 
 
 class MainTest(unittest.TestCase):
@@ -265,15 +264,17 @@ class MainTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(MainTest))

tests/metadata.TestCase

@@ -3,7 +3,6 @@
 import copy
 import io
 import logging
-import optparse
 import os
 import random
 import ruamel.yaml
@@ -16,8 +15,6 @@ from collections import OrderedDict
 from pathlib import Path
 from unittest import mock
 
-from testcommon import TmpCwd, mkdtemp
-
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
 if localmodule not in sys.path:
@@ -27,6 +24,7 @@ import fdroidserver
 from fdroidserver import metadata
 from fdroidserver.exception import MetaDataException
 from fdroidserver.common import DEFAULT_LOCALE
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 def _get_mock_mf(s):
@@ -2447,15 +2445,17 @@ class PostMetadataParseTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(MetadataTest))

tests/net.TestCase

@@ -2,7 +2,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import random
 import requests
@@ -126,15 +125,17 @@ class NetTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(NetTest))

tests/nightly.TestCase

@@ -2,7 +2,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import requests
 import shutil
@@ -70,7 +69,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()
@@ -361,15 +363,17 @@ class NightlyTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(NightlyTest))

tests/parse-fdroiddata-mirror-config.py

@@ -0,0 +1,13 @@
+#!/usr/bin/env python3
+
+import ruamel.yaml
+
+from pathlib import Path
+
+mirrors_yml = Path('/home/hans/code/fdroid/fdroiddata/config/mirrors.yml')
+with mirrors_yml.open() as fp:
+    mirrors_config = ruamel.yaml.YAML(typ='safe').load(fp)
+
+for d in mirrors_config:
+    d['url'] += '/repo'
+    print(d, end=',\n')

tests/publish.TestCase

@@ -13,7 +13,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -34,7 +33,7 @@ from fdroidserver import common
 from fdroidserver import metadata
 from fdroidserver import signatures
 from fdroidserver.exception import FDroidException
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class PublishTest(unittest.TestCase):
@@ -267,7 +266,8 @@ class PublishTest(unittest.TestCase):
 
         os.chdir(self.testdir)
 
-        config = common.read_config(Options)
+        common.options = Options
+        config = common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_sign_then_implant_signature, apksigner not installed!')
         config['repo_keyalias'] = 'sova'
@@ -341,7 +341,8 @@ class PublishTest(unittest.TestCase):
 
         os.chdir(self.testdir)
 
-        config = common.read_config(Options)
+        common.options = Options
+        config = common.read_config()
         if 'apksigner' not in config:
             self.skipTest('SKIPPING test_error_on_failed, apksigner not installed!')
         config['repo_keyalias'] = 'sova'
@@ -413,15 +414,17 @@ class PublishTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(PublishTest))

tests/rewritemeta.TestCase

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -9,14 +8,13 @@ import tempfile
 import textwrap
 from pathlib import Path
 
-from testcommon import TmpCwd, mkdtemp
-
 localmodule = Path(__file__).resolve().parent.parent
 print('localmodule: ' + str(localmodule))
 if localmodule not in sys.path:
     sys.path.insert(0, str(localmodule))
 
 from fdroidserver import common, metadata, rewritemeta
+from testcommon import TmpCwd, mkdtemp
 
 
 class RewriteMetaTest(unittest.TestCase):
@@ -265,15 +263,17 @@ class RewriteMetaTest(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(RewriteMetaTest))

tests/scanner.TestCase

@@ -3,7 +3,6 @@
 import glob
 import inspect
 import logging
-import optparse
 import os
 import re
 import shutil
@@ -31,7 +30,7 @@ import fdroidserver.build
 import fdroidserver.common
 import fdroidserver.metadata
 import fdroidserver.scanner
-from testcommon import TmpCwd, mkdtemp, mock_open_to_str
+from testcommon import TmpCwd, mkdtemp, mock_open_to_str, parse_args_for_test
 
 
 class ScannerTest(unittest.TestCase):
@@ -47,8 +46,8 @@ class ScannerTest(unittest.TestCase):
         self._td.cleanup()
 
     def test_scan_source_files(self):
-        fdroidserver.scanner.options = mock.Mock()
-        fdroidserver.scanner.options.json = False
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.json = False
         source_files = os.path.join(self.basedir, 'source-files')
         projects = {
             'OtakuWorld': 2,
@@ -103,8 +102,8 @@ class ScannerTest(unittest.TestCase):
         """Check for sneaking in banned maven repos"""
         os.chdir(self.testdir)
         fdroidserver.scanner.config = None
-        fdroidserver.scanner.options = mock.Mock()
-        fdroidserver.scanner.options.json = True
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.json = True
         with open('build.gradle', 'w', encoding='utf-8') as fp:
             fp.write(
                 textwrap.dedent(
@@ -136,8 +135,8 @@ class ScannerTest(unittest.TestCase):
         os.chdir(abs_build_dir)
 
         fdroidserver.scanner.config = None
-        fdroidserver.scanner.options = mock.Mock()
-        fdroidserver.scanner.options.json = True
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.json = True
 
         keep = [
             'arg.jar',
@@ -236,7 +235,7 @@ class ScannerTest(unittest.TestCase):
         fdroidserver.build.options.scan_binary = False
         fdroidserver.build.options.notarball = True
         fdroidserver.build.options.skipscan = False
-        fdroidserver.scanner.options = fdroidserver.build.options
+        fdroidserver.common.options = fdroidserver.build.options
 
         app = fdroidserver.metadata.App()
         app.id = 'mocked.app.id'
@@ -315,7 +314,7 @@ class ScannerTest(unittest.TestCase):
         """Check that the scanner can handle scandelete with gradle files with multiple problems"""
         os.chdir(self.testdir)
         fdroidserver.scanner.config = None
-        fdroidserver.scanner.options = mock.Mock()
+        fdroidserver.common.options = mock.Mock()
         build = fdroidserver.metadata.Build()
         build.scandelete = ['build.gradle']
         with open('build.gradle', 'w', encoding='utf-8') as fp:
@@ -733,15 +732,15 @@ class Test_ScannerTool(unittest.TestCase):
             refresh.assert_not_called()
 
     def test_refresh_true(self):
-        fdroidserver.scanner.options = mock.Mock()
-        fdroidserver.scanner.options.refresh_scanner = True
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.refresh_scanner = True
         with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh:
             fdroidserver.scanner.ScannerTool()
             refresh.assert_called_once()
 
     def test_refresh_false(self):
-        fdroidserver.scanner.options = mock.Mock()
-        fdroidserver.scanner.options.refresh_scanner = False
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.refresh_scanner = False
         with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh:
             fdroidserver.scanner.ScannerTool()
             refresh.assert_not_called()
@@ -754,8 +753,8 @@ class Test_ScannerTool(unittest.TestCase):
             refresh.assert_called_once()
 
     def test_refresh_options_overrides_config(self):
-        fdroidserver.scanner.options = mock.Mock()
-        fdroidserver.scanner.options.refresh_scanner = True
+        fdroidserver.common.options = mock.Mock()
+        fdroidserver.common.options.refresh_scanner = True
         os.chdir(self.testdir)
         pathlib.Path('config.yml').write_text('refresh_scanner: false')
         with mock.patch('fdroidserver.scanner.ScannerTool.refresh') as refresh:
@@ -815,15 +814,17 @@ class Test_main(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTests(

tests/signatures.TestCase

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 
 import inspect
-import optparse
 import os
 import sys
 import unittest
@@ -24,7 +23,7 @@ class SignaturesTest(unittest.TestCase):
     def setUp(self):
         logging.basicConfig(level=logging.DEBUG)
         common.config = None
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner')
         config['verbose'] = True
         common.config = config
@@ -59,15 +58,17 @@ class SignaturesTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(SignaturesTest))

tests/signindex.TestCase

@@ -3,7 +3,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import subprocess
@@ -38,7 +37,7 @@ class SignindexTest(unittest.TestCase):
 
     def setUp(self):
         signindex.config = None
-        config = common.read_config(common.options)
+        config = common.read_config()
         config['jarsigner'] = common.find_sdk_tools_cmd('jarsigner')
         config['verbose'] = True
         config['keystore'] = str(self.basedir / 'keystore.jks')
@@ -193,15 +192,17 @@ class SignindexTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(SignindexTest))

tests/testcommon.py

@@ -18,6 +18,10 @@
 import os
 import sys
 import tempfile
+import unittest
+
+from pathlib import Path
+from unittest import mock
 
 
 class TmpCwd:
@@ -60,3 +64,25 @@ 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)
+
+
+def parse_args_for_test(parser, args):
+    """Only send --flags to the ArgumentParser, not test classes, etc."""
+
+    from fdroidserver.common import parse_args
+
+    flags = []
+    for arg in args:
+        if arg[0] == '-':
+            flags.append(flags)
+    with mock.patch('sys.argv', flags):
+        parse_args(parser)

tests/update.TestCase

@@ -9,7 +9,6 @@ import hashlib
 import inspect
 import json
 import logging
-import optparse
 import os
 import random
 import shutil
@@ -24,7 +23,6 @@ import textwrap
 from binascii import hexlify
 from datetime import datetime
 from pathlib import Path
-from testcommon import TmpCwd, mkdtemp
 from unittest import mock
 
 try:
@@ -62,6 +60,7 @@ import fdroidserver.metadata
 import fdroidserver.update
 from fdroidserver.common import CATEGORIES_CONFIG_NAME
 from fdroidserver.looseversion import LooseVersion
+from testcommon import TmpCwd, mkdtemp, parse_args_for_test
 
 
 DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective')
@@ -1208,7 +1207,7 @@ class UpdateTest(unittest.TestCase):
 
         # Set up options
         fdroidserver.common.options = Options
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
+        config = fdroidserver.common.read_config()
         if 'apksigner' not in config:  # TODO remove me for buildserver-bullseye
             self.skipTest('SKIPPING test_update_with_AllowedAPKSigningKeys, apksigner not installed!')
         config['repo_keyalias'] = 'sova'
@@ -2271,15 +2270,17 @@ class TestParseFromPbxproj(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(UpdateTest))

tests/vcs.TestCase

@@ -4,7 +4,6 @@
 
 import inspect
 import logging
-import optparse
 import os
 import sys
 import unittest
@@ -22,7 +21,7 @@ import fdroidserver.build
 import fdroidserver.common
 import fdroidserver.metadata
 import fdroidserver.scanner
-from testcommon import mkdtemp
+from testcommon import mkdtemp, parse_args_for_test
 
 
 class VCSTest(unittest.TestCase):
@@ -88,15 +87,17 @@ class VCSTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
+    parse_args_for_test(parser, sys.argv)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(VCSTest))

tests/verify.TestCase

@@ -3,7 +3,6 @@
 import inspect
 import json
 import logging
-import optparse
 import os
 import shutil
 import sys
@@ -94,15 +93,17 @@ class VerifyTest(unittest.TestCase):
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))
 
-    parser = optparse.OptionParser()
-    parser.add_option(
+    import argparse
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
         "-v",
         "--verbose",
         action="store_true",
         default=False,
         help="Spew out even more information than normal",
     )
-    (common.options, args) = parser.parse_args(['--verbose'])
+    common.options = common.parse_args(parser)
 
     newSuite = unittest.TestSuite()
     newSuite.addTest(unittest.makeSuite(VerifyTest))