Merge branch 'signing' into 'master'

sign using apksigner

Closes #827 and #634

See merge request fdroid/fdroidserver!736

examples/config.py

@@ -135,7 +135,7 @@ The repository of older versions of applications from the main demo repository.
 
 # You should not need to change these at all, unless you have a very
 # customized setup for using smartcards in Java with keytool/jarsigner
-# smartcardoptions = "-storetype PKCS11 -providerName SunPKCS11-OpenSC \
+# smartcardoptions = "-storetype PKCS11 \
 #    -providerClass sun.security.pkcs11.SunPKCS11 \
 #    -providerArg opensc-fdroid.cfg"
 

fdroidserver/common.py

@@ -71,7 +71,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_VERSION = '26.0.0'
+MINIMUM_AAPT_BUILD_TOOLS_VERSION = '26.0.0'
+# 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'
 
 VERCODE_OPERATION_RE = re.compile(r'^([ 0-9/*+-]|%c)+$')
 
@@ -111,7 +114,7 @@ default_config = {
         'r16b': None,
     },
     'cachedir': os.path.join(os.getenv('HOME'), '.cache', 'fdroidserver'),
-    'build_tools': MINIMUM_AAPT_VERSION,
+    'build_tools': MINIMUM_AAPT_BUILD_TOOLS_VERSION,
     'force_build_tools': False,
     'java_paths': None,
     'scan_binary': False,
@@ -321,8 +324,7 @@ def read_config(opts, config_file='config.py'):
         config['smartcardoptions'] = re.sub(r'\s+', r' ', config['smartcardoptions']).split(' ')
     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',
-                                      'SunPKCS11-OpenSC', '-providerClass',
+        config['smartcardoptions'] = ['-storetype', 'PKCS11', '-providerClass',
                                       'sun.security.pkcs11.SunPKCS11',
                                       '-providerArg', 'opensc-fdroid.cfg']
 
@@ -416,6 +418,28 @@ def assert_config_keystore(config):
                               + "you can create one using: fdroid update --create-key")
 
 
+def find_apksigner():
+    """
+    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'):
+        return config['apksigner']
+    build_tools_path = os.path.join(config['sdk_path'], 'build-tools')
+    for f in sorted(os.listdir(build_tools_path), reverse=True):
+        if LooseVersion(f) < LooseVersion(MINIMUM_AAPT_BUILD_TOOLS_VERSION):
+            return None
+        if os.path.exists(os.path.join(build_tools_path, f, 'apksigner')):
+            apksigner = os.path.join(build_tools_path, f, 'apksigner')
+            logging.info("Using %s " % apksigner)
+            # memoize result
+            config['apksigner'] = apksigner
+            return config['apksigner']
+
+
 def find_sdk_tools_cmd(cmd):
     '''find a working path to a tool from the Android SDK'''
 
@@ -466,13 +490,13 @@ def test_aapt_version(aapt):
             # the Debian package has the version string like "v0.2-23.0.2"
             too_old = False
             if '.' in bugfix:
-                if LooseVersion(bugfix) < LooseVersion(MINIMUM_AAPT_VERSION):
+                if LooseVersion(bugfix) < LooseVersion(MINIMUM_AAPT_BUILD_TOOLS_VERSION):
                     too_old = True
             elif LooseVersion('.'.join((major, minor, bugfix))) < LooseVersion('0.2.4062713'):
                 too_old = True
             if too_old:
                 logging.warning(_("'{aapt}' is too old, fdroid requires build-tools-{version} or newer!")
-                                .format(aapt=aapt, version=MINIMUM_AAPT_VERSION))
+                                .format(aapt=aapt, version=MINIMUM_AAPT_BUILD_TOOLS_VERSION))
         else:
             logging.warning(_('Unknown version of aapt, might cause problems: ') + output)
 
@@ -2333,7 +2357,7 @@ def _get_androguard_APK(apkfile):
     try:
         from androguard.core.bytecodes.apk import APK
     except ImportError:
-        raise FDroidException("androguard library is not installed and aapt not present")
+        raise FDroidException("androguard library is not installed")
 
     return APK(apkfile)
 
@@ -2510,21 +2534,6 @@ def get_native_code(apkfile):
     return sorted(list(archset))
 
 
-def get_minSdkVersion(apkfile):
-    """Extract the minimum supported Android SDK from an APK using androguard
-
-    :param apkfile: path to an APK file.
-    :returns: the integer representing the SDK version
-    """
-
-    try:
-        apk = _get_androguard_APK(apkfile)
-    except FileNotFoundError:
-        raise FDroidException(_('Reading minSdkVersion failed: "{apkfilename}"')
-                              .format(apkfilename=apkfile))
-    return int(apk.get_min_sdk_version())
-
-
 class PopenResult:
     def __init__(self):
         self.returncode = None
@@ -2954,7 +2963,7 @@ def metadata_find_developer_signing_files(appid, vercode):
         return None
 
 
-def apk_strip_signatures(signed_apk, strip_manifest=False):
+def apk_strip_v1_signatures(signed_apk, strip_manifest=False):
     """Removes signatures from APK.
 
     :param signed_apk: path to apk file.
@@ -3037,36 +3046,76 @@ def apk_extract_signatures(apkpath, outdir, manifest=True):
 def sign_apk(unsigned_path, signed_path, keyalias):
     """Sign and zipalign an unsigned APK, then save to a new file, deleting the unsigned
 
-    android-18 (4.3) finally added support for reasonable hash
-    algorithms, like SHA-256, before then, the only options were MD5
-    and SHA1 :-/ This aims to use SHA-256 when the APK does not target
+    Use apksigner for making v2 and v3 signature for apks with targetSDK >=30 as
+    otherwise they won't be installable on Android 11/R.
+
+    Otherwise use jarsigner for v1 only signatures until we have apksig v2/v3
+    signature transplantig support.
+
+    When using jarsigner we need to manually select the hash algorithm,
+    apksigner does this automatically. Apksigner also does the zipalign for us.
+
+    SHA-256 support was added in android-18 (4.3), before then, the only options were MD5
+    and SHA1. This aims to use SHA-256 when the APK does not target
     older Android versions, and is therefore safe to do so.
 
     https://issuetracker.google.com/issues/36956587
     https://android-review.googlesource.com/c/platform/libcore/+/44491
 
     """
-
-    if get_minSdkVersion(unsigned_path) < 18:
-        signature_algorithm = ['-sigalg', 'SHA1withRSA', '-digestalg', 'SHA1']
+    apk = _get_androguard_APK(unsigned_path)
+    if int(apk.get_target_sdk_version()) >= 30:
+        if config['keystore'] == 'NONE':
+            # NOTE: apksigner doesn't like -providerName/--provider-name at all, don't use
+            #       apksigner documents the options as --ks-provider-class and --ks-provider-arg
+            #       those seem to be accepted but fail when actually making a signature with
+            #       weird internal exceptions. Those options actually work.
+            #       From: https://geoffreymetais.github.io/code/key-signing/#scripting
+            replacements = {'-storetype': '--ks-type',
+                            '-providerClass': '--provider-class',
+                            '-providerArg': '--provider-arg'}
+            signing_args = [replacements.get(n, n) for n in config['smartcardoptions']]
+        else:
+            signing_args = ['--key-pass', 'env:FDROID_KEY_PASS']
+        if not find_apksigner():
+            raise BuildException(_("apksigner not found, it's required for signing!"))
+        cmd = [find_apksigner(), 'sign',
+               '--ks', config['keystore'],
+               '--ks-pass', 'env:FDROID_KEY_STORE_PASS']
+        cmd += signing_args
+        cmd += ['--ks-key-alias', keyalias,
+                '--in', unsigned_path,
+                '--out', signed_path]
+        p = FDroidPopen(cmd, envs={
+            'FDROID_KEY_STORE_PASS': config['keystorepass'],
+            'FDROID_KEY_PASS': config.get('keypass', "")})
+        if p.returncode != 0:
+            raise BuildException(_("Failed to sign application"), p.output)
+        os.remove(unsigned_path)
     else:
-        signature_algorithm = ['-sigalg', 'SHA256withRSA', '-digestalg', 'SHA-256']
 
-    cmd = [config['jarsigner'], '-keystore', config['keystore'],
-           '-storepass:env', 'FDROID_KEY_STORE_PASS']
-    if config['keystore'] == 'NONE':
-        cmd += config['smartcardoptions']
-    else:
-        cmd += '-keypass:env', 'FDROID_KEY_PASS'
-    p = FDroidPopen(cmd + signature_algorithm + [unsigned_path, keyalias],
-                    envs={
-                        'FDROID_KEY_STORE_PASS': config['keystorepass'],
-                        'FDROID_KEY_PASS': config.get('keypass', "")})
-    if p.returncode != 0:
-        raise BuildException(_("Failed to sign application"), p.output)
+        if int(apk.get_min_sdk_version()) < 18:
+            signature_algorithm = ['-sigalg', 'SHA1withRSA', '-digestalg', 'SHA1']
+        else:
+            signature_algorithm = ['-sigalg', 'SHA256withRSA', '-digestalg', 'SHA-256']
+        if config['keystore'] == 'NONE':
+            signing_args = config['smartcardoptions']
+        else:
+            signing_args = ['-keypass:env', 'FDROID_KEY_PASS']
 
-    _zipalign(unsigned_path, signed_path)
-    os.remove(unsigned_path)
+        cmd = [config['jarsigner'], '-keystore', config['keystore'],
+               '-storepass:env', 'FDROID_KEY_STORE_PASS']
+        cmd += signing_args
+        cmd += signature_algorithm
+        cmd += [unsigned_path, keyalias]
+        p = FDroidPopen(cmd, envs={
+            'FDROID_KEY_STORE_PASS': config['keystorepass'],
+            'FDROID_KEY_PASS': config.get('keypass', "")})
+        if p.returncode != 0:
+            raise BuildException(_("Failed to sign application"), p.output)
+
+        _zipalign(unsigned_path, signed_path)
+        os.remove(unsigned_path)
 
 
 def verify_apks(signed_apk, unsigned_apk, tmp_dir):

fdroidserver/init.py

@@ -194,7 +194,7 @@ def main():
         common.write_to_config(test_config, 'repo_keyalias', '1')  # seems to be the default
         disable_in_config('keypass', 'never used with smartcard')
         common.write_to_config(test_config, 'smartcardoptions',
-                               ('-storetype PKCS11 -providerName SunPKCS11-OpenSC '
+                               ('-storetype PKCS11 '
                                 + '-providerClass sun.security.pkcs11.SunPKCS11 '
                                 + '-providerArg opensc-fdroid.cfg'))
         # find opensc-pkcs11.so

fdroidserver/nightly.py

@@ -268,7 +268,7 @@ Last updated: {date}'''.format(repo_git_base=repo_git_base,
                     os.chmod(apkfilename, 0o644)
                     logging.debug(_('Resigning {apkfilename} with provided debug.keystore')
                                   .format(apkfilename=os.path.basename(apkfilename)))
-                    common.apk_strip_signatures(apkfilename, strip_manifest=True)
+                    common.apk_strip_v1_signatures(apkfilename, strip_manifest=True)
                     common.sign_apk(apkfilename, destapk, KEY_ALIAS)
 
         if options.verbose:

tests/common.TestCase

@@ -633,15 +633,13 @@ class CommonTest(unittest.TestCase):
                              fdroidserver.common.apk_signer_fingerprint_short(apkfile))
 
     def test_sign_apk(self):
-        try:
-            fdroidserver.common.find_sdk_tools_cmd('aapt')
-            fdroidserver.common.find_sdk_tools_cmd('zipalign')
-        except fdroidserver.exception.FDroidException:
-            print('\n\nSKIPPING test_sign_apk, zipalign is not installed!\n')
-            return
-
         fdroidserver.common.config = None
         config = fdroidserver.common.read_config(fdroidserver.common.options)
+        try:
+            fdroidserver.common.find_sdk_tools_cmd('zipalign')
+        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='
@@ -653,10 +651,10 @@ class CommonTest(unittest.TestCase):
         testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
         unsigned = os.path.join(testdir, 'urzip-release-unsigned.apk')
         signed = os.path.join(testdir, 'urzip-release.apk')
+        shutil.copy(os.path.join(self.basedir, 'urzip-release-unsigned.apk'), testdir)
 
         self.assertFalse(fdroidserver.common.verify_apk_signature(unsigned))
 
-        shutil.copy(os.path.join(self.basedir, 'urzip-release-unsigned.apk'), testdir)
         fdroidserver.common.sign_apk(unsigned, signed, config['keyalias'])
         self.assertTrue(os.path.isfile(signed))
         self.assertFalse(os.path.isfile(unsigned))
@@ -667,20 +665,37 @@ class CommonTest(unittest.TestCase):
         signed = os.path.join(testdir, 'duplicate.permisssions_9999999.apk')
         shutil.copy(os.path.join(self.basedir, 'repo', 'duplicate.permisssions_9999999.apk'),
                     os.path.join(unsigned))
-        fdroidserver.common.apk_strip_signatures(unsigned, strip_manifest=True)
+        fdroidserver.common.apk_strip_v1_signatures(unsigned, strip_manifest=True)
         fdroidserver.common.sign_apk(unsigned, signed, config['keyalias'])
         self.assertTrue(os.path.isfile(signed))
         self.assertFalse(os.path.isfile(unsigned))
         self.assertTrue(fdroidserver.common.verify_apk_signature(signed))
-        try:
-            fdroidserver.common.find_sdk_tools_cmd('aapt')
-            self.assertEqual(18, fdroidserver.common.get_minSdkVersion(signed))
-        except fdroidserver.exception.FDroidException:
-            print('\n\nSKIPPING test_sign_apk min SDK check, aapt is not installed!\n')
-            return
+        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)
+        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')
+
+        self.assertFalse(fdroidserver.common.verify_apk_signature(unsigned))
+        fdroidserver.common.sign_apk(unsigned, signed, config['keyalias'])
+
+        self.assertTrue(os.path.isfile(signed))
+        self.assertFalse(os.path.isfile(unsigned))
+        self.assertTrue(fdroidserver.common.verify_apk_signature(signed))
+        # verify it has a v2 signature
+        self.assertTrue(fdroidserver.common._get_androguard_APK(signed).is_signed_v2())
 
     def test_get_apk_id(self):
-
         config = dict()
         fdroidserver.common.fill_config_defaults(config)
         fdroidserver.common.config = config
@@ -775,54 +790,40 @@ class CommonTest(unittest.TestCase):
             nc = fdroidserver.common.get_native_code(apkfilename)
             self.assertEqual(native_code, nc)
 
-    def test_get_minSdkVersion_androguard(self):
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('bad-unicode-πÇÇ现代通用字-български-عربي1.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_1.apk')
-        self.assertEqual(14, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_2.apk')
-        self.assertEqual(14, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_3.apk')
-        self.assertEqual(14, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_4.apk')
-        self.assertEqual(14, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('org.dyndns.fules.ck_20.apk')
-        self.assertEqual(7, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('urzip.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('urzip-badcert.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('urzip-badsig.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('urzip-release.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('urzip-release-unsigned.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/com.politedroid_3.apk')
-        self.assertEqual(3, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/com.politedroid_4.apk')
-        self.assertEqual(3, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/com.politedroid_5.apk')
-        self.assertEqual(3, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/com.politedroid_6.apk')
-        self.assertEqual(14, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/obb.main.oldversion_1444412523.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/obb.mainpatch.current_1619_another-release-key.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/obb.mainpatch.current_1619.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/obb.main.twoversions_1101613.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/obb.main.twoversions_1101615.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/obb.main.twoversions_1101617.apk')
-        self.assertEqual(4, minSdkVersion)
-        minSdkVersion = fdroidserver.common.get_minSdkVersion('repo/urzip-; Рахма́, [rɐxˈmanʲɪnəf] سيرجي_رخمانينوف 谢·.apk')
-        self.assertEqual(4, minSdkVersion)
+    def test_get_sdkversions_androguard(self):
+        """This is a sanity test that androguard isn't broken"""
+        def get_minSdkVersion(apkfile):
+            apk = fdroidserver.common._get_androguard_APK(apkfile)
+            return int(apk.get_min_sdk_version())
 
-        with self.assertRaises(FDroidException):
-            fdroidserver.common.get_minSdkVersion('nope')
+        def get_targetSdkVersion(apkfile):
+            apk = fdroidserver.common._get_androguard_APK(apkfile)
+            return int(apk.get_target_sdk_version())
+
+        self.assertEqual(4, get_minSdkVersion('bad-unicode-πÇÇ现代通用字-български-عربي1.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'))
+        self.assertEqual(14, get_minSdkVersion('org.bitbucket.tickytacky.mirrormirror_4.apk'))
+        self.assertEqual(7, get_minSdkVersion('org.dyndns.fules.ck_20.apk'))
+        self.assertEqual(4, get_minSdkVersion('urzip.apk'))
+        self.assertEqual(4, get_minSdkVersion('urzip-badcert.apk'))
+        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(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'))
+        self.assertEqual(14, get_minSdkVersion('repo/com.politedroid_6.apk'))
+        self.assertEqual(4, get_minSdkVersion('repo/obb.main.oldversion_1444412523.apk'))
+        self.assertEqual(4, get_minSdkVersion('repo/obb.mainpatch.current_1619_another-release-key.apk'))
+        self.assertEqual(4, get_minSdkVersion('repo/obb.mainpatch.current_1619.apk'))
+        self.assertEqual(4, get_minSdkVersion('repo/obb.main.twoversions_1101613.apk'))
+        self.assertEqual(4, get_minSdkVersion('repo/obb.main.twoversions_1101615.apk'))
+        self.assertEqual(4, get_minSdkVersion('repo/obb.main.twoversions_1101617.apk'))
+        self.assertEqual(4, get_minSdkVersion('repo/urzip-; Рахма́, [rɐxˈmanʲɪnəf] سيرجي_رخمانينوف 谢·.apk'))
+
+        self.assertEqual(30, get_targetSdkVersion('minimal_targetsdk_30_unsigned.apk'))
 
     def test_apk_release_name(self):
         appid, vercode, sigfp = fdroidserver.common.apk_parse_release_filename('com.serwylo.lexica_905.apk')