use apksigner to sign index-v2 with modern, supported algorithms

The current signing method uses apksigner to sign the JAR so that it
will automatically select algorithms that are compatible with Android
SDK 23, which added the most recent algorithms:
https://developer.android.com/reference/java/security/Signature

This signing method uses then inherits the default signing algothim
settings, since Java and Android both maintain those.  That helps
avoid a repeat of being stuck on an old signing algorithm.  That means
specifically that this call to apksigner does not specify any of the
algorithms.

The old indexes must be signed by SHA1withRSA otherwise they will no
longer be compatible with old Androids.

apksigner 30.0.0+ is available in Debian/bullseye, Debian/buster-backports,
Ubuntu 21.10, and Ubuntu 20.04 from the fdroid PPA.  Here's a quick way to
test:

for f in `ls -1 /opt/android-sdk/build-tools/*/apksigner | sort ` /usr/bin/apksigner; do printf "$f : "; $f sign --v4-signing-enabled false; done

closes #1005

.gitlab-ci.yml

@@ -125,7 +125,7 @@ ubuntu_lts_ppa:
     - ./run-tests
 
 
-# Test using Ubuntu/bionic LTS (supported til 2022) with all depends
+# Test using Ubuntu/bionic LTS (supported til April, 2023) with all depends
 # from pypi.  The venv is used to isolate the dist tarball generation
 # environment from the clean install environment.
 ubuntu_bionic_pip:
@@ -145,6 +145,11 @@ ubuntu_bionic_pip:
     - tar tzf dist/fdroidserver-*.tar.gz
     # back to bare machine to act as user's install machine
     - $pip install --upgrade pip setuptools wheel  # make this go away: "error: invalid command 'bdist_wheel'"
+
+    - export ANDROID_HOME=/opt/android-sdk
+    - $pip install sdkmanager
+    - sdkmanager 'build-tools;30.0.0'
+
     - $pip install dist/fdroidserver-*.tar.gz
     - tar xzf dist/fdroidserver-*.tar.gz
     - cd fdroidserver-*

fdroidserver/common.py

@@ -87,9 +87,10 @@ FDROID_PATH = os.path.realpath(os.path.join(os.path.dirname(__file__), '..'))
 # this is the build-tools version, aapt has a separate version that
 # has to be manually set in test_aapt_version()
 MINIMUM_AAPT_BUILD_TOOLS_VERSION = '26.0.0'
+# 30.0.0 is the first version to support --v4-signing-enabled.
 # 26.0.2 is the first version recognizing md5 based signatures as valid again
 # (as does android, so we want that)
-MINIMUM_APKSIGNER_BUILD_TOOLS_VERSION = '26.0.2'
+MINIMUM_APKSIGNER_BUILD_TOOLS_VERSION = '30.0.0'
 
 VERCODE_OPERATION_RE = re.compile(r'^([ 0-9/*+-]|%c)+$')
 
@@ -3412,6 +3413,18 @@ def get_min_sdk_version(apk):
         return 1
 
 
+def get_apksigner_smartcardoptions(smartcardoptions):
+    if '-providerName' in smartcardoptions.copy():
+        pos = smartcardoptions.index('-providerName')
+        # remove -providerName and it's argument
+        del smartcardoptions[pos]
+        del smartcardoptions[pos]
+    replacements = {'-storetype': '--ks-type',
+                    '-providerClass': '--provider-class',
+                    '-providerArg': '--provider-arg'}
+    return [replacements.get(n, n) for n in smartcardoptions]
+
+
 def sign_apk(unsigned_path, signed_path, keyalias):
     """Sign and zipalign an unsigned APK, then save to a new file, deleting the unsigned.
 
@@ -3429,16 +3442,7 @@ def sign_apk(unsigned_path, signed_path, keyalias):
 
     """
     if config['keystore'] == 'NONE':
-        apksigner_smartcardoptions = config['smartcardoptions'].copy()
-        if '-providerName' in apksigner_smartcardoptions:
-            pos = config['smartcardoptions'].index('-providerName')
-            # remove -providerName and it's argument
-            del apksigner_smartcardoptions[pos]
-            del apksigner_smartcardoptions[pos]
-        replacements = {'-storetype': '--ks-type',
-                        '-providerClass': '--provider-class',
-                        '-providerArg': '--provider-arg'}
-        signing_args = [replacements.get(n, n) for n in apksigner_smartcardoptions]
+        signing_args = get_apksigner_smartcardoptions(config['smartcardoptions'])
     else:
         signing_args = ['--key-pass', 'env:FDROID_KEY_PASS']
     apksigner = config.get('apksigner', '')

fdroidserver/index.py

@@ -858,7 +858,7 @@ def make_v2(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_
         logging.debug(_('index-v2 must have a signature, use `fdroid signindex` to create it!'))
     else:
         signindex.config = common.config
-        signindex.sign_index(repodir, json_name, signindex.HashAlg.SHA256)
+        signindex.sign_index(repodir, json_name)
 
 
 def make_v1(apps, packages, repodir, repodict, requestsdict, fdroid_signing_key_fingerprints):
@@ -1345,7 +1345,7 @@ def make_v0(apps, apks, repodir, repodict, requestsdict, fdroid_signing_key_fing
                 os.remove(signed)
         else:
             signindex.config = common.config
-            signindex.sign_jar(signed)
+            signindex.sign_jar(signed, use_old_algs=True)
 
     # Copy the repo icon into the repo directory...
     icon_dir = os.path.join(repodir, 'icons')

fdroidserver/signindex.py

@@ -21,7 +21,6 @@ import os
 import time
 import zipfile
 from argparse import ArgumentParser
-from enum import Enum
 import logging
 
 from . import _
@@ -34,29 +33,27 @@ options = None
 start_timestamp = time.gmtime()
 
 
-HashAlg = Enum("SHA1", "SHA256")
+def sign_jar(jar, use_old_algs=False):
+    """Sign a JAR file with the best available algorithm.
 
+    The current signing method uses apksigner to sign the JAR so that
+    it will automatically select algorithms that are compatible with
+    Android SDK 23, which added the most recent algorithms:
+    https://developer.android.com/reference/java/security/Signature
 
-def sign_jar(jar, hash_algorithm=None):
-    """Sign a JAR file with Java's jarsigner.
+    This signing method uses then inherits the default signing
+    algothim settings, since Java and Android both maintain those.
+    That helps avoid a repeat of being stuck on an old signing
+    algorithm.  That means specifically that this call to apksigner
+    does not specify any of the algorithms.
+
+    The old indexes must be signed by SHA1withRSA otherwise they will
+    no longer be compatible with old Androids.
 
     This method requires a properly initialized config object.
+
     """
-    if hash_algorithm == HashAlg.SHA256:
-        args = [
-            config['jarsigner'],
-            '-keystore',
-            config['keystore'],
-            '-storepass:env',
-            'FDROID_KEY_STORE_PASS',
-            '-digestalg',
-            'SHA-256',
-            '-sigalg',
-            'SHA256withRSA',
-            jar,
-            config['repo_keyalias'],
-        ]
-    else:
+    if use_old_algs:
         # This does use old hashing algorithms, i.e. SHA1, but that's not
         # broken yet for file verification.  This could be set to SHA256,
         # but then Android < 4.3 would not be able to verify it.
@@ -74,20 +71,50 @@ def sign_jar(jar, hash_algorithm=None):
             jar,
             config['repo_keyalias'],
         ]
-    if config['keystore'] == 'NONE':
-        args += config['smartcardoptions']
-    else:  # smardcards never use -keypass
-        args += ['-keypass:env', 'FDROID_KEY_PASS']
+        if config['keystore'] == 'NONE':
+            args += config['smartcardoptions']
+        else:  # smardcards never use -keypass
+            args += ['-keypass:env', 'FDROID_KEY_PASS']
+    else:
+        # https://developer.android.com/studio/command-line/apksigner
+        args = [
+            config['apksigner'],
+            'sign',
+            '--min-sdk-version',
+            '23',  # enable all current algorithms
+            '--max-sdk-version',
+            '24',  # avoid future incompatible algorithms
+            # disable all APK signature types, only use JAR sigs aka v1
+            '--v1-signing-enabled',
+            'true',
+            '--v2-signing-enabled',
+            'false',
+            '--v3-signing-enabled',
+            'false',
+            '--v4-signing-enabled',
+            'false',
+            '--ks',
+            config['keystore'],
+            '--ks-pass',
+            'env:FDROID_KEY_STORE_PASS',
+            '--ks-key-alias',
+            config['repo_keyalias'],
+        ]
+        if config['keystore'] == 'NONE':
+            args += common.get_apksigner_smartcardoptions(config['smartcardoptions'])
+        else:  # smardcards never use --key-pass
+            args += ['--key-pass', 'env:FDROID_KEY_PASS']
+        args += [jar]
     env_vars = {
         'FDROID_KEY_STORE_PASS': config['keystorepass'],
         'FDROID_KEY_PASS': config.get('keypass', ""),
     }
     p = common.FDroidPopen(args, envs=env_vars)
     if p.returncode != 0:
-        raise FDroidException("Failed to sign %s!" % jar)
+        raise FDroidException("Failed to sign %s: %s" % (jar, p.output))
 
 
-def sign_index(repodir, json_name, hash_algorithm=None):
+def sign_index(repodir, json_name):
     """Sign index-v1.json to make index-v1.jar.
 
     This is a bit different than index.jar: instead of their being index.xml
@@ -109,7 +136,11 @@ def sign_index(repodir, json_name, hash_algorithm=None):
     jar_file = os.path.join(repodir, name + '.jar')
     with zipfile.ZipFile(jar_file, 'w', zipfile.ZIP_DEFLATED) as jar:
         jar.write(index_file, json_name)
-    sign_jar(jar_file, hash_algorithm)
+
+    if json_name in ('index.xml', 'index-v1.json'):
+        sign_jar(jar_file, use_old_algs=True)
+    else:
+        sign_jar(jar_file)
 
 
 def status_update_json(signed):
@@ -165,7 +196,7 @@ def main():
         json_name = 'entry.json'
         index_file = os.path.join(output_dir, json_name)
         if os.path.exists(index_file):
-            sign_index(output_dir, json_name, HashAlg.SHA256)
+            sign_index(output_dir, json_name)
             logging.info('Signed ' + index_file)
             signed.append(index_file)
 

tests/common.TestCase

@@ -450,7 +450,7 @@ class CommonTest(unittest.TestCase):
             sourcefile = os.path.join(sourcedir, f)
             testfile = os.path.join(testsdir, f)
             shutil.copy(sourcefile, testsdir)
-            fdroidserver.signindex.sign_jar(testfile)
+            fdroidserver.signindex.sign_jar(testfile, use_old_algs=True)
             # these should be resigned, and therefore different
             self.assertNotEqual(
                 open(sourcefile, 'rb').read(), open(testfile, 'rb').read()
@@ -872,6 +872,9 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(os.path.isfile(unsigned))
         self.assertTrue(fdroidserver.common.verify_apk_signature(signed))
 
+    @unittest.skipUnless(
+        os.path.exists('tests/SystemWebView-repack.apk'), "file too big for sdist"
+    )
     def test_resign_apk(self):
         """When using apksigner, it should resign signed APKs"""
         config = fdroidserver.common.read_config(fdroidserver.common.options)
@@ -2455,8 +2458,42 @@ class CommonTest(unittest.TestCase):
                 self.assertTrue(os.path.exists(f), f + ' was created')
                 self.assertFalse(is_repo_file(f), f + ' not repo file')
 
+    def test_get_apksigner_smartcardoptions(self):
+        testdir = tempfile.mkdtemp(
+            prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir
+        )
+        os.chdir(testdir)
+        with open('config.yml', 'w') as fp:
+            d = {
+                'smartcardoptions': '-storetype PKCS11'
+                ' -providerName SunPKCS11-OpenSC'
+                ' -providerClass sun.security.pkcs11.SunPKCS11'
+                ' -providerArg opensc-fdroid.cfg'
+            }
+            yaml.dump(d, fp)
+        config = fdroidserver.common.read_config()
+        fdroidserver.common.config = config
+        self.assertTrue(isinstance(d['smartcardoptions'], str))
+        self.assertTrue(isinstance(config['smartcardoptions'], list))
+        self.assertEqual(
+            [
+                '--ks-type',
+                'PKCS11',
+                '--provider-class',
+                'sun.security.pkcs11.SunPKCS11',
+                '--provider-arg',
+                'opensc-fdroid.cfg',
+            ],
+            fdroidserver.common.get_apksigner_smartcardoptions(
+                config['smartcardoptions']
+            ),
+        )
+
     def test_get_smartcardoptions_list(self):
-        os.chdir(self.tmpdir)
+        testdir = tempfile.mkdtemp(
+            prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir
+        )
+        os.chdir(testdir)
         with open('config.yml', 'w') as fp:
             fp.write(
                 textwrap.dedent(
@@ -2491,7 +2528,10 @@ class CommonTest(unittest.TestCase):
         )
 
     def test_get_smartcardoptions_spaces(self):
-        os.chdir(self.tmpdir)
+        testdir = tempfile.mkdtemp(
+            prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir
+        )
+        os.chdir(testdir)
         with open('config.yml', 'w') as fp:
             fp.write(
                 textwrap.dedent(
@@ -2519,7 +2559,10 @@ class CommonTest(unittest.TestCase):
         )
 
     def test_get_smartcardoptions_config_py(self):
-        os.chdir(self.tmpdir)
+        testdir = tempfile.mkdtemp(
+            prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir
+        )
+        os.chdir(testdir)
         with open('config.py', 'w') as fp:
             fp.write(
                 textwrap.dedent(

tests/signindex.TestCase

@@ -6,6 +6,7 @@ import logging
 import optparse
 import os
 import shutil
+import subprocess
 import sys
 import tempfile
 import unittest
@@ -17,7 +18,7 @@ print('localmodule: ' + localmodule)
 if localmodule not in sys.path:
     sys.path.insert(0, localmodule)
 
-from fdroidserver import common, signindex, update
+from fdroidserver import apksigcopier, common, signindex, update
 from pathlib import Path
 from unittest.mock import patch
 
@@ -96,6 +97,34 @@ class SignindexTest(unittest.TestCase):
             signindex.main()
         for f in index_files:
             self.assertTrue(f.exists(), '%s should exist!' % f)
+        self.assertFalse(os.path.exists('index-v2.jar'))  # no JAR version of this file
+
+        # index.jar aka v0 must by signed by SHA1withRSA
+        f = 'repo/index.jar'
+        common.verify_jar_signature(f)
+        self.assertIsNone(apksigcopier.extract_v2_sig(f, expected=False))
+        cp = subprocess.run(
+            ['jarsigner', '-verify', '-verbose', f], stdout=subprocess.PIPE
+        )
+        self.assertTrue(b'SHA1withRSA' in cp.stdout)
+
+        # index-v1.jar must by signed by SHA1withRSA
+        f = 'repo/index-v1.jar'
+        common.verify_jar_signature(f)
+        self.assertIsNone(apksigcopier.extract_v2_sig(f, expected=False))
+        cp = subprocess.run(
+            ['jarsigner', '-verify', '-verbose', f], stdout=subprocess.PIPE
+        )
+        self.assertTrue(b'SHA1withRSA' in cp.stdout)
+
+        # entry.jar aka index v2 must by signed by a modern algorithm
+        f = 'repo/entry.jar'
+        common.verify_jar_signature(f)
+        self.assertIsNone(apksigcopier.extract_v2_sig(f, expected=False))
+        cp = subprocess.run(
+            ['jarsigner', '-verify', '-verbose', f], stdout=subprocess.PIPE
+        )
+        self.assertFalse(b'SHA1withRSA' in cp.stdout)
 
 
 if __name__ == "__main__":