diff --git a/examples/config.py b/examples/config.py index 0a0558b3..974f8ee1 100644 --- a/examples/config.py +++ b/examples/config.py @@ -71,6 +71,15 @@ archive_description = """ The repository of older versions of applications from the main demo repository. """ +# This allows a specific kind of insecure APK to be included in the +# 'repo' section. Since April 2017, APK signatures that use MD5 are +# no longer considered valid, jarsigner and apksigner will return an +# error when verifying. `fdroid update` will move APKs with these +# disabled signatures to the archive. This option stops that +# behavior, and lets those APKs stay part of 'repo'. +# +# allow_disabled_algorithms = True + # Normally, all apps are collected into a single app repository, like on # https://f-droid.org. For certain situations, it is better to make a repo # that is made up of APKs only from a single app. For example, an automated diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 884fea4b..76888cce 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -85,6 +85,7 @@ default_config = { 'gradle': 'gradle', 'accepted_formats': ['txt', 'yml'], 'sync_from_local_copy_dir': False, + 'allow_disabled_algorithms': False, 'per_app_repos': False, 'make_current_version_link': True, 'current_version_name_source': 'Name', diff --git a/fdroidserver/update.py b/fdroidserver/update.py index 6ef9c126..8488574f 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -1082,7 +1082,8 @@ def scan_apk_androguard(apk, apkfile): apk['features'].append(feature) -def scan_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk): +def scan_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk=False, + allow_disabled_algorithms=False, archive_bad_sig=False): """Scan the apk with the given filename in the given repo directory. This also extracts the icons. @@ -1093,6 +1094,9 @@ def scan_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk): :param knownapks: known apks info :param use_date_from_apk: use date from APK (instead of current date) for newly added APKs + :param allow_disabled_algorithms: allow APKs with valid signatures that include + disabled algorithms in the signature (e.g. MD5) + :param archive_bad_sig: move APKs with a bad signature to the archive :returns: (skip, apk, cachechanged) where skip is a boolean indicating whether to skip this apk, apk is the scanned apk information, and cachechanged is True if the apkcache got changed. """ @@ -1186,19 +1190,27 @@ def scan_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk): # verify the jar signature is correct, allow deprecated # algorithms only if the APK is in the archive. + skipapk = False if not common.verify_apk_signature(apkfile): - if repodir == 'archive': + if repodir == 'archive' or allow_disabled_algorithms: if common.verify_old_apk_signature(apkfile): - apk['antiFeatures'].add('KnownVuln') + apk['antiFeatures'].update(['KnownVuln', 'DisabledAlgorithm']) else: - return True, None, False + skipapk = True else: - logging.warning('Archiving "' + apkfilename + '" with invalid signature!') - move_apk_between_sections('repo', 'archive', apk) - return True, None, False + skipapk = True - if has_known_vulnerability(apkfile): - apk['antiFeatures'].add('KnownVuln') + if skipapk: + if archive_bad_sig: + logging.warning('Archiving "' + apkfilename + '" with invalid signature!') + move_apk_between_sections(repodir, 'archive', apk) + else: + logging.warning('Skipping "' + apkfilename + '" with invalid signature!') + return True, None, False + + if 'KnownVuln' not in apk['antiFeatures']: + if has_known_vulnerability(apkfile): + apk['antiFeatures'].add('KnownVuln') apkzip = zipfile.ZipFile(apkfile, 'r') @@ -1372,7 +1384,9 @@ def scan_apks(apkcache, repodir, knownapks, use_date_from_apk=False): apks = [] for apkfile in sorted(glob.glob(os.path.join(repodir, '*.apk'))): apkfilename = apkfile[len(repodir) + 1:] - (skip, apk, cachechanged) = scan_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk) + ada = options.allow_disabled_algorithms or config['allow_disabled_algorithms'] + (skip, apk, cachechanged) = scan_apk(apkcache, apkfilename, repodir, knownapks, + use_date_from_apk, ada, True) if skip: continue apks.append(apk) @@ -1459,12 +1473,16 @@ def archive_old_apks(apps, apks, archapks, repodir, archivedir, defaultkeepversi current_app_archapks = filter_apk_list_sorted(archapks) if len(current_app_apks) < keepversions and len(current_app_archapks) > 0: - required = keepversions - len(apks) - # Move forward the ones we want again. - for apk in current_app_archapks[:required]: - move_apk_between_sections(archivedir, repodir, apk) - archapks.remove(apk) - apks.append(apk) + kept = 0 + # Move forward the ones we want again, except DisableAlgorithm + for apk in current_app_archapks: + if 'DisabledAlgorithm' not in apk['antiFeatures']: + move_apk_between_sections(archivedir, repodir, apk) + archapks.remove(apk) + apks.append(apk) + kept += 1 + if kept == keepversions: + break def move_apk_between_sections(from_dir, to_dir, apk): @@ -1477,6 +1495,9 @@ def move_apk_between_sections(from_dir, to_dir, apk): to_path = os.path.join(to_dir, filename) shutil.move(from_path, to_path) + if from_dir == to_dir: + return + logging.info("Moving %s from %s to %s" % (apk['apkName'], from_dir, to_dir)) _move_file(from_dir, to_dir, apk['apkName'], False) _move_file(from_dir, to_dir, apk['apkName'] + '.asc', True) @@ -1550,6 +1571,8 @@ def main(): help="Use date from apk instead of current time for newly added apks") parser.add_argument("--rename-apks", action="store_true", default=False, help="Rename APK files that do not match package.name_123.apk") + 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() metadata.warnings_action = options.W diff --git a/tests/org.bitbucket.tickytacky.mirrormirror_1.apk b/tests/org.bitbucket.tickytacky.mirrormirror_1.apk new file mode 100644 index 00000000..6ec4272f Binary files /dev/null and b/tests/org.bitbucket.tickytacky.mirrormirror_1.apk differ diff --git a/tests/org.bitbucket.tickytacky.mirrormirror_2.apk b/tests/org.bitbucket.tickytacky.mirrormirror_2.apk new file mode 100644 index 00000000..26474277 Binary files /dev/null and b/tests/org.bitbucket.tickytacky.mirrormirror_2.apk differ diff --git a/tests/org.bitbucket.tickytacky.mirrormirror_3.apk b/tests/org.bitbucket.tickytacky.mirrormirror_3.apk new file mode 100644 index 00000000..af2a1fe8 Binary files /dev/null and b/tests/org.bitbucket.tickytacky.mirrormirror_3.apk differ diff --git a/tests/org.bitbucket.tickytacky.mirrormirror_4.apk b/tests/org.bitbucket.tickytacky.mirrormirror_4.apk new file mode 100644 index 00000000..a5f52eda Binary files /dev/null and b/tests/org.bitbucket.tickytacky.mirrormirror_4.apk differ diff --git a/tests/run-tests b/tests/run-tests index 5f523ea4..b8514eef 100755 --- a/tests/run-tests +++ b/tests/run-tests @@ -240,6 +240,35 @@ sed -i --expression='s,timestamp="[0-9]*",timestamp="1480431575",' repo/index.xm diff -uw $WORKSPACE/tests/repo/index.xml repo/index.xml +#------------------------------------------------------------------------------# +echo_header 'test moving lots of APKs to the archive' + +REPOROOT=`create_test_dir` +cd $REPOROOT +cp $WORKSPACE/tests/keystore.jks $REPOROOT/ +$fdroid init --keystore keystore.jks --repo-keyalias=sova +echo 'keystorepass = "r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI="' >> config.py +echo 'keypass = "r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI="' >> config.py +echo "accepted_formats = ['txt']" >> config.py +sed -i '/allow_disabled_algorithms/d' config.py +test -d metadata || mkdir metadata +cp $WORKSPACE/tests/metadata/*.txt metadata/ +echo 'Summary:good test version of urzip' > metadata/info.guardianproject.urzip.txt +echo 'Summary:good MD5 sig, which is disabled algorithm' > metadata/org.bitbucket.tickytacky.mirrormirror.txt +sed -i '/Archive Policy:/d' metadata/*.txt +test -d repo || mkdir repo +cp $WORKSPACE/tests/urzip.apk \ + $WORKSPACE/tests/org.bitbucket.tickytacky.mirrormirror_[0-9].apk \ + $WORKSPACE/tests/repo/com.politedroid_[0-9].apk \ + $WORKSPACE/tests/repo/obb.main.twoversions_110161[357].apk \ + repo/ +sed -i 's,archive_older = [0-9],archive_older = 3,' config.py + +$fdroid update --pretty --nosign +test `grep '' archive/index.xml | wc -l` -eq 5 +test `grep '' repo/index.xml | wc -l` -eq 7 + + #------------------------------------------------------------------------------# echo_header 'test per-app "Archive Policy"' @@ -383,6 +412,77 @@ test -e repo/com.politedroid_5.apk ! test -e repo/com.politedroid_6.apk +#------------------------------------------------------------------------------# +echo_header 'test allowing disabled signatures in repo and archive' + +REPOROOT=`create_test_dir` +cd $REPOROOT +cp $WORKSPACE/tests/keystore.jks $REPOROOT/ +$fdroid init --keystore keystore.jks --repo-keyalias=sova +echo 'keystorepass = "r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI="' >> config.py +echo 'keypass = "r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI="' >> config.py +echo "accepted_formats = ['txt']" >> config.py +echo 'allow_disabled_algorithms = True' >> config.py +sed -i 's,archive_older = [0-9],archive_older = 3,' config.py +test -d metadata || mkdir metadata +cp $WORKSPACE/tests/metadata/com.politedroid.txt metadata/ +echo 'Summary:good test version of urzip' > metadata/info.guardianproject.urzip.txt +echo 'Summary:good MD5 sig, disabled algorithm' > metadata/org.bitbucket.tickytacky.mirrormirror.txt +sed -i '/Archive Policy:/d' metadata/*.txt +test -d repo || mkdir repo +cp $WORKSPACE/tests/repo/com.politedroid_[0-9].apk \ + $WORKSPACE/tests/org.bitbucket.tickytacky.mirrormirror_[0-9].apk \ + $WORKSPACE/tests/urzip-badsig.apk \ + repo/ + +$fdroid update --pretty --nosign +test `grep '' archive/index.xml | wc -l` -eq 2 +test `grep '' repo/index.xml | wc -l` -eq 6 +grep -F com.politedroid_3.apk archive/index.xml +grep -F com.politedroid_4.apk repo/index.xml +grep -F com.politedroid_5.apk repo/index.xml +grep -F com.politedroid_6.apk repo/index.xml +grep -F org.bitbucket.tickytacky.mirrormirror_1.apk archive/index.xml +grep -F org.bitbucket.tickytacky.mirrormirror_2.apk repo/index.xml +grep -F org.bitbucket.tickytacky.mirrormirror_3.apk repo/index.xml +grep -F org.bitbucket.tickytacky.mirrormirror_4.apk repo/index.xml +! grep -F urzip-badsig.apk repo/index.xml +! grep -F urzip-badsig.apk archive/index.xml +test -e archive/com.politedroid_3.apk +test -e repo/com.politedroid_4.apk +test -e repo/com.politedroid_5.apk +test -e repo/com.politedroid_6.apk +test -e archive/org.bitbucket.tickytacky.mirrormirror_1.apk +test -e repo/org.bitbucket.tickytacky.mirrormirror_2.apk +test -e repo/org.bitbucket.tickytacky.mirrormirror_3.apk +test -e repo/org.bitbucket.tickytacky.mirrormirror_4.apk +test -e archive/urzip-badsig.apk + +sed -i '/allow_disabled_algorithms/d' config.py +$fdroid update --pretty --nosign +test `grep '' archive/index.xml | wc -l` -eq 5 +test `grep '' repo/index.xml | wc -l` -eq 3 +grep -F org.bitbucket.tickytacky.mirrormirror_1.apk archive/index.xml +grep -F org.bitbucket.tickytacky.mirrormirror_2.apk archive/index.xml +grep -F org.bitbucket.tickytacky.mirrormirror_3.apk archive/index.xml +grep -F org.bitbucket.tickytacky.mirrormirror_4.apk archive/index.xml +grep -F com.politedroid_3.apk archive/index.xml +grep -F com.politedroid_4.apk repo/index.xml +grep -F com.politedroid_5.apk repo/index.xml +grep -F com.politedroid_6.apk repo/index.xml +! grep -F urzip-badsig.apk repo/index.xml +! grep -F urzip-badsig.apk archive/index.xml +test -e archive/org.bitbucket.tickytacky.mirrormirror_1.apk +test -e archive/org.bitbucket.tickytacky.mirrormirror_2.apk +test -e archive/org.bitbucket.tickytacky.mirrormirror_3.apk +test -e archive/org.bitbucket.tickytacky.mirrormirror_4.apk +test -e archive/com.politedroid_3.apk +test -e archive/urzip-badsig.apk +test -e repo/com.politedroid_4.apk +test -e repo/com.politedroid_5.apk +test -e repo/com.politedroid_6.apk + + #------------------------------------------------------------------------------# echo_header 'rename apks with `fdroid update --rename-apks`, --nosign for speed' diff --git a/tests/update.TestCase b/tests/update.TestCase index cd47e020..91406a35 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -201,6 +201,7 @@ class UpdateTest(unittest.TestCase): fdroidserver.update.options.clean = True fdroidserver.update.options.delete_unknown = True fdroidserver.update.options.rename_apks = False + fdroidserver.update.options.allow_disabled_algorithms = False apps = fdroidserver.metadata.read_metadata(xref=True) knownapks = fdroidserver.common.KnownApks() @@ -250,7 +251,7 @@ class UpdateTest(unittest.TestCase): config = dict() fdroidserver.common.fill_config_defaults(config) fdroidserver.update.config = config - os.chdir(os.path.dirname(__file__)) + os.chdir(os.path.join(localmodule, 'tests')) if os.path.basename(os.getcwd()) != 'tests': raise Exception('This test must be run in the "tests/" subdir') @@ -263,6 +264,7 @@ class UpdateTest(unittest.TestCase): fdroidserver.update.options.clean = True fdroidserver.update.options.rename_apks = False fdroidserver.update.options.delete_unknown = True + fdroidserver.update.options.allow_disabled_algorithms = False for icon_dir in fdroidserver.update.get_all_icon_dirs('repo'): if not os.path.exists(icon_dir): @@ -290,6 +292,87 @@ class UpdateTest(unittest.TestCase): self.maxDiff = None self.assertEqual(apk, frompickle) + def test_scan_apk_signed_by_disabled_algorithms(self): + os.chdir(os.path.join(localmodule, 'tests')) + if os.path.basename(os.getcwd()) != 'tests': + raise Exception('This test must be run in the "tests/" subdir') + + config = dict() + fdroidserver.common.fill_config_defaults(config) + fdroidserver.update.config = config + + config['ndk_paths'] = dict() + config['accepted_formats'] = ['json', 'txt', 'yml'] + fdroidserver.common.config = config + fdroidserver.update.config = config + + fdroidserver.update.options = type('', (), {})() + fdroidserver.update.options.clean = True + fdroidserver.update.options.verbose = True + fdroidserver.update.options.rename_apks = False + fdroidserver.update.options.delete_unknown = True + fdroidserver.update.options.allow_disabled_algorithms = False + + knownapks = fdroidserver.common.KnownApks() + apksourcedir = os.getcwd() + tmpdir = os.path.join(localmodule, '.testfiles') + if not os.path.exists(tmpdir): + os.makedirs(tmpdir) + tmptestsdir = tempfile.mkdtemp(prefix='test_scan_apk_signed_by_disabled_algorithms-', dir=tmpdir) + print('tmptestsdir', tmptestsdir) + os.chdir(tmptestsdir) + os.mkdir('repo') + os.mkdir('archive') + # setup the repo, create icons dirs, etc. + fdroidserver.update.scan_apks({}, 'repo', knownapks) + fdroidserver.update.scan_apks({}, 'archive', knownapks) + + disabledsigs = ['org.bitbucket.tickytacky.mirrormirror_2.apk', ] + for apkName in disabledsigs: + shutil.copy(os.path.join(apksourcedir, apkName), + os.path.join(tmptestsdir, 'repo')) + + skip, apk, cachechanged = fdroidserver.update.scan_apk({}, apkName, 'repo', knownapks, + allow_disabled_algorithms=True, + archive_bad_sig=False) + self.assertFalse(skip) + self.assertIsNotNone(apk) + self.assertTrue(cachechanged) + self.assertFalse(os.path.exists(os.path.join('archive', apkName))) + self.assertTrue(os.path.exists(os.path.join('repo', apkName))) + + # this test only works on systems with fully updated Java/jarsigner + # that has MD5 listed in jdk.jar.disabledAlgorithms in java.security + skip, apk, cachechanged = fdroidserver.update.scan_apk({}, apkName, 'repo', knownapks, + allow_disabled_algorithms=False, + archive_bad_sig=True) + self.assertTrue(skip) + self.assertIsNone(apk) + self.assertFalse(cachechanged) + self.assertTrue(os.path.exists(os.path.join('archive', apkName))) + self.assertFalse(os.path.exists(os.path.join('repo', apkName))) + + skip, apk, cachechanged = fdroidserver.update.scan_apk({}, apkName, 'archive', knownapks, + allow_disabled_algorithms=False, + archive_bad_sig=False) + self.assertFalse(skip) + self.assertIsNotNone(apk) + self.assertTrue(cachechanged) + self.assertTrue(os.path.exists(os.path.join('archive', apkName))) + self.assertFalse(os.path.exists(os.path.join('repo', apkName))) + + badsigs = ['urzip-badcert.apk', 'urzip-badsig.apk', 'urzip-release-unsigned.apk', ] + for apkName in badsigs: + shutil.copy(os.path.join(apksourcedir, apkName), + os.path.join(tmptestsdir, 'repo')) + + skip, apk, cachechanged = fdroidserver.update.scan_apk({}, apkName, 'repo', knownapks, + allow_disabled_algorithms=False, + archive_bad_sig=False) + self.assertTrue(skip) + self.assertIsNone(apk) + self.assertFalse(cachechanged) + def test_scan_invalid_apk(self): os.chdir(os.path.join(localmodule, 'tests')) if os.path.basename(os.getcwd()) != 'tests':