From: Hans-Christoph Steiner Date: Tue, 27 Jun 2017 19:40:39 +0000 (+0200) Subject: update: allow_disabled_algorithms option to keep MD5 sigs in repo X-Git-Tag: 0.8~29^2~5 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=746d4bd4cf33cc480e429d459f93c280a4306cf3;p=fdroidserver.git update: allow_disabled_algorithms option to keep MD5 sigs in repo The new policy is to move APKs with invalid signatures to the archive, and only add those APKs to the archive's index if they have valid MD5 signatures. closes #323 closes #292 --- 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: + skipapk = True + + if skipapk: + if archive_bad_sig: logging.warning('Archiving "' + apkfilename + '" with invalid signature!') - move_apk_between_sections('repo', 'archive', apk) - return True, None, False + move_apk_between_sections(repodir, 'archive', apk) + else: + logging.warning('Skipping "' + apkfilename + '" with invalid signature!') + return True, None, False - if has_known_vulnerability(apkfile): - apk['antiFeatures'].add('KnownVuln') + 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':