switch to preferring apksigner, requiring for fdroid publish

fdroidserver/common.py

@@ -262,6 +262,11 @@ def fill_config_defaults(thisconfig):
     if 'keytool' not in thisconfig and shutil.which('keytool'):
         thisconfig['keytool'] = shutil.which('keytool')
 
+    # enable apksigner by default so v2/v3 APK signatures validate
+    find_apksigner(thisconfig)
+    if not thisconfig.get('apksigner'):
+        logging.warning(_('apksigner not found! Cannot sign or verify modern APKs'))
+
     for k in ['ndk_paths', 'java_paths']:
         d = thisconfig[k]
         for k2 in d.copy():
@@ -456,26 +461,35 @@ def assert_config_keystore(config):
                               + "you can create one using: fdroid update --create-key")
 
 
-def find_apksigner():
+def find_apksigner(config):
-    """
+    """Searches for the best version apksigner and adds it to the config
+
     Returns the best version of apksigner following this algorithm:
     * use config['apksigner'] if set
     * try to find apksigner in path
     * find apksigner in build-tools starting from newest installed
       going down to MINIMUM_APKSIGNER_BUILD_TOOLS_VERSION
     :return: path to apksigner or None if no version is found
+
     """
-    if set_command_in_config('apksigner'):
+    command = 'apksigner'
-        return config['apksigner']
+    if command in config:
-    build_tools_path = os.path.join(config['sdk_path'], 'build-tools')
+        return
+
+    tmp = find_command(command)
+    if tmp is not None:
+        config[command] = tmp
+        return
+
+    build_tools_path = os.path.join(config.get('sdk_path', ''), 'build-tools')
     if not os.path.isdir(build_tools_path):
-        return None
+        return
     for f in sorted(os.listdir(build_tools_path), reverse=True):
         if not os.path.isdir(os.path.join(build_tools_path, f)):
             continue
         try:
             if LooseVersion(f) < LooseVersion(MINIMUM_APKSIGNER_BUILD_TOOLS_VERSION):
-                return None
+                return
         except TypeError:
             continue
         if os.path.exists(os.path.join(build_tools_path, f, 'apksigner')):
@@ -483,7 +497,6 @@ def find_apksigner():
             logging.info("Using %s " % apksigner)
             # memoize result
             config['apksigner'] = apksigner
-            return config['apksigner']
 
 
 def find_sdk_tools_cmd(cmd):
@@ -3087,9 +3100,10 @@ def sign_apk(unsigned_path, signed_path, keyalias):
         signing_args = [replacements.get(n, n) for n in apksigner_smartcardoptions]
     else:
         signing_args = ['--key-pass', 'env:FDROID_KEY_PASS']
-    if not find_apksigner():
+    apksigner = config.get('apksigner', '')
+    if not shutil.which(apksigner):
         raise BuildException(_("apksigner not found, it's required for signing!"))
-    cmd = [find_apksigner(), 'sign',
+    cmd = [apksigner, 'sign',
            '--ks', config['keystore'],
            '--ks-pass', 'env:FDROID_KEY_STORE_PASS']
     cmd += signing_args

fdroidserver/init.py

@@ -146,12 +146,6 @@ def main():
     # now that we have a local config.yml, read configuration...
     config = common.read_config(options)
 
-    # enable apksigner by default so v2/v3 APK signatures validate
-    if common.find_apksigner() is not None:
-        apksigner = common.find_apksigner()
-        test_config['apksigner'] = apksigner
-        common.write_to_config(test_config, 'apksigner', apksigner)
-
     # the NDK is optional and there may be multiple versions of it, so it's
     # left for the user to configure
 

tests/build.TestCase

@@ -37,6 +37,8 @@ class BuildTest(unittest.TestCase):
         if not os.path.exists(self.tmpdir):
             os.makedirs(self.tmpdir)
         os.chdir(self.basedir)
+        fdroidserver.common.config = None
+        fdroidserver.build.config = None
 
     def test_get_apk_metadata(self):
         config = dict()

tests/common.TestCase

@@ -619,12 +619,9 @@ class CommonTest(unittest.TestCase):
     def test_sign_apk(self):
         fdroidserver.common.config = None
         config = fdroidserver.common.read_config(fdroidserver.common.options)
-        try:
+        if 'apksigner' not in config:
-            fdroidserver.common.find_sdk_tools_cmd('zipalign')
+            self.skipTest('SKIPPING test_sign_apk, apksigner not installed!')
-        except fdroidserver.exception.FDroidException:
-            self.skipTest('SKIPPING test_sign_apk, zipalign not installed!')
 
-        config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         config['keyalias'] = 'sova'
         config['keystorepass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='
         config['keypass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='
@@ -656,18 +653,6 @@ class CommonTest(unittest.TestCase):
         self.assertTrue(fdroidserver.common.verify_apk_signature(signed))
         self.assertEqual('18', fdroidserver.common._get_androguard_APK(signed).get_min_sdk_version())
 
-    def test_sign_apk_targetsdk_30(self):
-        fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
-        if not fdroidserver.common.find_apksigner():
-            self.skipTest('SKIPPING as apksigner is not installed!')
-        config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
-        config['keyalias'] = 'sova'
-        config['keystorepass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='
-        config['keypass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='
-        config['keystore'] = os.path.join(self.basedir, 'keystore.jks')
-        testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
-
         shutil.copy(os.path.join(self.basedir, 'minimal_targetsdk_30_unsigned.apk'), testdir)
         unsigned = os.path.join(testdir, 'minimal_targetsdk_30_unsigned.apk')
         signed = os.path.join(testdir, 'minimal_targetsdk_30.apk')
@@ -681,18 +666,6 @@ class CommonTest(unittest.TestCase):
         # verify it has a v2 signature
         self.assertTrue(fdroidserver.common._get_androguard_APK(signed).is_signed_v2())
 
-    def test_sign_no_targetsdk(self):
-        fdroidserver.common.config = None
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
-        if not fdroidserver.common.find_apksigner():
-            self.skipTest('SKIPPING as apksigner is not installed!')
-        config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
-        config['keyalias'] = 'sova'
-        config['keystorepass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='
-        config['keypass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI='
-        config['keystore'] = os.path.join(self.basedir, 'keystore.jks')
-        testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
-
         shutil.copy(os.path.join(self.basedir, 'no_targetsdk_minsdk30_unsigned.apk'), testdir)
         unsigned = os.path.join(testdir, 'no_targetsdk_minsdk30_unsigned.apk')
         signed = os.path.join(testdir, 'no_targetsdk_minsdk30_signed.apk')
@@ -1442,7 +1415,7 @@ class CommonTest(unittest.TestCase):
         self.assertFalse(os.path.exists('config.yml'))
         self.assertFalse(os.path.exists('config.py'))
         config = fdroidserver.common.read_config(fdroidserver.common.options)
-        self.assertEqual(None, config.get('apksigner'))
+        self.assertIsNone(config.get('stats_server'))
         self.assertIsNotNone(config.get('char_limits'))
 
     def test_with_config_yml(self):

tests/init.TestCase

@@ -10,7 +10,6 @@ import shutil
 import sys
 import tempfile
 import unittest
-import yaml
 
 
 localmodule = os.path.realpath(
@@ -68,11 +67,7 @@ class InitTest(unittest.TestCase):
 
         sys.argv = ['fdroid init', '--keystore', 'keystore.jks', '--repo-keyalias=sova']
         fdroidserver.init.main()
-        with open('config.yml') as fp:
+        self.assertEqual(apksigner, fdroidserver.init.config.get('apksigner'))
-            config = yaml.safe_load(fp)
-        self.assertTrue(os.path.exists(config['keystore']))
-        self.assertTrue(os.path.exists(config['apksigner']))
-        self.assertEqual(apksigner, config['apksigner'])
 
 
 if __name__ == "__main__":

tests/run-tests

@@ -72,11 +72,17 @@ have_git_2_3() {
 
 is_MD5_disabled() {
     javac $WORKSPACE/tests/IsMD5Disabled.java && java -cp $WORKSPACE/tests IsMD5Disabled
-    return $?
 }
 
 use_apksigner() {
-    test -x "`sed -En 's,^ *apksigner: +,,p' config.yml`"
+    python3 -c "
+import sys
+sys.path.insert(0, '$WORKSPACE')
+from fdroidserver import common
+c = {'sdk_path': '$ANDROID_HOME'}
+common.find_apksigner(c)
+exit(c.get('apksigner') is None)
+"
 }
 
 #------------------------------------------------------------------------------#
@@ -169,7 +175,7 @@ $fdroid --version
 #------------------------------------------------------------------------------#
 echo_header 'run process when building and signing are on separate machines'
 
-if which zipalign || ls -1 $ANDROID_HOME/build-tools/*/zipalign; then
+if use_apksigner; then
     REPOROOT=`create_test_dir`
     cd $REPOROOT
     cp $WORKSPACE/tests/keystore.jks $REPOROOT/
@@ -325,8 +331,9 @@ else
     test `grep '<package>' repo/index.xml | wc -l` -eq 7
 fi
 
+
 #------------------------------------------------------------------------------#
-if ! which apksigner; then
+if ! use_apksigner; then
     echo_header 'test per-app "Archive Policy"'
 
     REPOROOT=`create_test_dir`
@@ -534,7 +541,7 @@ test -e repo/org.bitbucket.tickytacky.mirrormirror_4.apk
 test -e archive/urzip-badsig.apk
 
 sed -i.tmp '/apksigner:/d' config.yml
-if ! which apksigner; then
+if ! use_apksigner; then
     $sed -i.tmp '/allow_disabled_algorithms/d' config.yml
     $fdroid update --pretty --nosign
     test `grep '<package>' archive/index.xml | wc -l` -eq 5
@@ -1240,6 +1247,7 @@ fi
 #------------------------------------------------------------------------------#
 echo_header 'test extracting and publishing with developer signature'
 
+if use_apksigner; then
     REPOROOT=`create_test_dir`
     cd $REPOROOT
     fdroid_init_with_prebuilt_keystore
@@ -1257,11 +1265,12 @@ test -f metadata/com.politedroid/signatures/6/RELEASE.SF
     ! test -f repo/com.politedroid_6.apk
     $fdroid publish
     test -f repo/com.politedroid_6.apk
+    if which apksigner; then
+	apksigner verify repo/com.politedroid_6.apk
+    fi
     if which jarsigner; then
 	jarsigner -verify repo/com.politedroid_6.apk
     fi
-if which apksigner; then
-    apksigner verify repo/com.politedroid_6.apk
 fi
 
 

tests/update.TestCase

@@ -656,9 +656,7 @@ class UpdateTest(unittest.TestCase):
         fdroidserver.update.config = config
         os.chdir(os.path.join(localmodule, 'tests'))
 
-        apksigner = fdroidserver.common.find_apksigner()
+        if 'apksigner' in config:
-        if apksigner:
-            config['apksigner'] = apksigner
             apk_info = fdroidserver.update.scan_apk('v2.only.sig_2.apk')
             self.assertIsNone(apk_info.get('maxSdkVersion'))
             self.assertEqual(apk_info.get('versionName'), 'v2-only')