Merge branch 'apksigner-entry-jar' into 'master'

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

Closes #1005

See merge request fdroid/fdroidserver!1134

.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()
+        signing_args = get_apksigner_smartcardoptions(config['smartcardoptions'])
-        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]
     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):
+    This signing method uses then inherits the default signing
-    """Sign a JAR file with Java's jarsigner.
+    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:
+    if use_old_algs:
-        args = [
-            config['jarsigner'],
-            '-keystore',
-            config['keystore'],
-            '-storepass:env',
-            'FDROID_KEY_STORE_PASS',
-            '-digestalg',
-            'SHA-256',
-            '-sigalg',
-            'SHA256withRSA',
-            jar,
-            config['repo_keyalias'],
-        ]
-    else:
         # 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.
@@ -78,16 +75,46 @@ def sign_jar(jar, hash_algorithm=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/run-tests

@@ -202,6 +202,17 @@ if use_apksigner; then
     grep -F '<application id=' repo/index.xml > /dev/null
 fi
 
+
+#------------------------------------------------------------------------------#
+echo_header "TODO remove once buildserver image is upgraded to bullseye with apksigner"
+
+if java -version 2>&1 | grep -F 1.8.0; then
+    echo "Skipping the rest because they require apksigner 30.0.0+ which does not run on Java8"
+    echo SUCCESS
+    exit
+fi
+
+
 #------------------------------------------------------------------------------#
 echo_header "test UTF-8 metadata"
 

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
 
@@ -69,6 +70,8 @@ class SignindexTest(unittest.TestCase):
             signindex.sign_index(str(self.repodir), 'index-v1.json')
 
     def test_signindex(self):
+        if common.find_apksigner({}) is None:  # TODO remove me for buildserver-bullseye
+            self.skipTest('SKIPPING test_signindex, apksigner not installed!')
         os.mkdir('archive')
         metadata = Path('metadata')
         metadata.mkdir()
@@ -96,6 +99,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__":

tests/update.TestCase

@@ -1165,6 +1165,8 @@ class UpdateTest(unittest.TestCase):
 
         fdroidserver.common.options = Options
         config = fdroidserver.common.read_config(fdroidserver.common.options)
+        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'
         config['keystorepass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='
         config['keypass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='