From 3b95d3de64231fae86f1fb7608166b8d3b291372 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 20 Jul 2021 13:31:27 -0700 Subject: [PATCH] update: AllowedAPKSigningKeys metadata to enforce APK signers This field lets you specify which signing certificates should be trusted for APKs in a binary repo. --- .gitlab-ci.yml | 1 + fdroidserver/metadata.py | 16 +++ fdroidserver/update.py | 46 ++++++++- tests/metadata/dump/com.politedroid.yaml | 1 + tests/metadata/dump/org.adaway.yaml | 1 + .../dump/org.smssecure.smssecure.yaml | 1 + tests/metadata/dump/org.videolan.vlc.yaml | 1 + tests/update.TestCase | 99 +++++++++++++++++++ 8 files changed, 165 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index eae0a7df..3fca1897 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -47,6 +47,7 @@ metadata_v0: - cd fdroiddata - ../tests/dump_internal_metadata_format.py - sed -i + -e '/AllowedAPKSigningKeys:/d' -e '/Liberapay:/d' -e '/OpenCollective/d' metadata/dump_*/*.yaml diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index d0722dc4..12b8e1e5 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -90,6 +90,8 @@ yaml_app_field_order = [ '\n', 'Builds', '\n', + 'AllowedAPKSigningKeys', + '\n', 'MaintainerNotes', '\n', 'ArchivePolicy', @@ -145,6 +147,7 @@ class App(dict): self.RepoType = '' self.Repo = '' self.Binaries = None + self.AllowedAPKSigningKeys = [] self.MaintainerNotes = '' self.ArchivePolicy = None self.AutoUpdateMode = 'None' @@ -199,6 +202,7 @@ fieldtypes = { 'MaintainerNotes': TYPE_MULTILINE, 'Categories': TYPE_LIST, 'AntiFeatures': TYPE_LIST, + 'AllowedAPKSigningKeys': TYPE_LIST, 'Build': TYPE_BUILD, } @@ -433,6 +437,10 @@ valuetypes = { r'^http[s]?://', ["Binaries"]), + FieldValidator("AllowedAPKSigningKeys", + r'^[a-fA-F0-9]{64}$', + ["AllowedAPKSigningKeys"]), + FieldValidator("Archive Policy", r'^[0-9]+ versions$', ["ArchivePolicy"]), @@ -927,6 +935,14 @@ def write_yaml(mf, app): cm.update({field: _builds_to_yaml(app)}) elif field == 'CurrentVersionCode': cm.update({field: _field_to_yaml(TYPE_INT, getattr(app, field))}) + elif field == 'AllowedAPKSigningKeys': + value = getattr(app, field) + if value: + value = [str(i).lower() for i in value] + if len(value) == 1: + cm.update({field: _field_to_yaml(TYPE_STRING, value[0])}) + else: + cm.update({field: _field_to_yaml(TYPE_LIST, value)}) else: cm.update({field: _field_to_yaml(fieldtype(field), getattr(app, field))}) diff --git a/fdroidserver/update.py b/fdroidserver/update.py index c60abfd7..f8b895d8 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -36,6 +36,8 @@ import yaml import copy from datetime import datetime from argparse import ArgumentParser +from pathlib import Path + try: from yaml import CSafeLoader as SafeLoader except ImportError: @@ -51,6 +53,7 @@ from . import metadata from .exception import BuildException, FDroidException from PIL import Image, PngImagePlugin + if hasattr(Image, 'DecompressionBombWarning'): warnings.simplefilter('error', Image.DecompressionBombWarning) Image.MAX_IMAGE_PIXELS = 0xffffff # 4096x4096 @@ -688,7 +691,6 @@ def insert_obbs(repodir, apps, apks): list of current, valid apps apks current information on all APKs - """ def obbWarnDelete(f, msg): logging.warning(msg + ' ' + f) @@ -2223,6 +2225,31 @@ def get_apps_with_packages(apps, apks): return appsWithPackages +def get_apks_without_allowed_signatures(app, apk): + """Check the APK or package has been signed by one of the allowed signing certificates. + + The fingerprint of the signing certificate is the standard X.509 + SHA-256 fingerprint as a hex string. It can be fetched from an + APK using: + + apksigner verify --print-certs my.apk | grep SHA-256 + + Parameters + ---------- + app + The app which declares the AllowedSigningKey + apk + The APK to check + """ + if not app or not apk: + return + allowed_signer_keys = app.get('AllowedAPKSigningKeys', []) + if not allowed_signer_keys: + return + if apk['signer'] not in allowed_signer_keys: + return apk['apkName'] + + def prepare_apps(apps, apks, repodir): """Encapsulate all necessary preparation steps before we can build an index out of apps and apks. @@ -2372,6 +2399,23 @@ def main(): appid_has_apks = set() appid_has_repo_files = set() for apk in apks: + to_remove = get_apks_without_allowed_signatures(apps.get(apk['packageName']), apk) + if to_remove: + apks.remove(apk) + logging.warning( + _('"{path}" is signed by a key that is not allowed:').format( + path=to_remove + ) + + '\n' + + apk['signer'] + ) + if options.delete_unknown: + for d in repodirs: + path = Path(d) / to_remove + if path.exists(): + logging.warning(_('Removing {path}"').format(path=path)) + path.unlink() + if apk['apkName'].endswith('.apk'): appid_has_apks.add(apk['packageName']) else: diff --git a/tests/metadata/dump/com.politedroid.yaml b/tests/metadata/dump/com.politedroid.yaml index 28c089ad..6f83af0a 100644 --- a/tests/metadata/dump/com.politedroid.yaml +++ b/tests/metadata/dump/com.politedroid.yaml @@ -1,3 +1,4 @@ +AllowedAPKSigningKeys: [] AntiFeatures: [] ArchivePolicy: 4 versions AuthorEmail: null diff --git a/tests/metadata/dump/org.adaway.yaml b/tests/metadata/dump/org.adaway.yaml index b5cca8cd..95bca2ce 100644 --- a/tests/metadata/dump/org.adaway.yaml +++ b/tests/metadata/dump/org.adaway.yaml @@ -1,3 +1,4 @@ +AllowedAPKSigningKeys: [] AntiFeatures: [] ArchivePolicy: null AuthorEmail: null diff --git a/tests/metadata/dump/org.smssecure.smssecure.yaml b/tests/metadata/dump/org.smssecure.smssecure.yaml index 4af66030..c30f6c79 100644 --- a/tests/metadata/dump/org.smssecure.smssecure.yaml +++ b/tests/metadata/dump/org.smssecure.smssecure.yaml @@ -1,3 +1,4 @@ +AllowedAPKSigningKeys: [] AntiFeatures: [] ArchivePolicy: null AuthorEmail: null diff --git a/tests/metadata/dump/org.videolan.vlc.yaml b/tests/metadata/dump/org.videolan.vlc.yaml index 89e60be7..c6f928e3 100644 --- a/tests/metadata/dump/org.videolan.vlc.yaml +++ b/tests/metadata/dump/org.videolan.vlc.yaml @@ -1,3 +1,4 @@ +AllowedAPKSigningKeys: [] AntiFeatures: [] ArchivePolicy: 9 versions AuthorEmail: null diff --git a/tests/update.TestCase b/tests/update.TestCase index 72315b98..fd08d6ed 100755 --- a/tests/update.TestCase +++ b/tests/update.TestCase @@ -62,6 +62,7 @@ DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective') class Options: allow_disabled_algorithms = False clean = False + nosign = False pretty = True rename_apks = False @@ -1026,6 +1027,104 @@ class UpdateTest(unittest.TestCase): self.assertIsNone(apk) self.assertFalse(cachechanged) + def test_get_apks_without_allowed_signatures(self): + """Test when no AllowedAPKSigningKeys is specified""" + config = dict() + fdroidserver.common.fill_config_defaults(config) + fdroidserver.common.config = config + fdroidserver.common.options = Options + + app = fdroidserver.metadata.App() + knownapks = fdroidserver.common.KnownApks() + apkfile = 'v1.v2.sig_1020.apk' + (skip, apk, cachechanged) = fdroidserver.update.process_apk( + {}, apkfile, 'repo', knownapks, False + ) + + r = fdroidserver.update.get_apks_without_allowed_signatures(app, apk) + self.assertIsNone(r) + + def test_get_apks_without_allowed_signatures_allowed(self): + """Test when the APK matches the specified AllowedAPKSigningKeys""" + config = dict() + fdroidserver.common.fill_config_defaults(config) + fdroidserver.common.config = config + fdroidserver.common.options = Options + fdroidserver.update.options = fdroidserver.common.options + + app = fdroidserver.metadata.App( + { + 'AllowedAPKSigningKeys': '32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6' + } + ) + knownapks = fdroidserver.common.KnownApks() + apkfile = 'v1.v2.sig_1020.apk' + (skip, apk, cachechanged) = fdroidserver.update.process_apk( + {}, apkfile, 'repo', knownapks, False + ) + + r = fdroidserver.update.get_apks_without_allowed_signatures(app, apk) + self.assertIsNone(r) + + def test_get_apks_without_allowed_signatures_blocked(self): + """Test when the APK does not match any specified AllowedAPKSigningKeys""" + config = dict() + fdroidserver.common.fill_config_defaults(config) + fdroidserver.common.config = config + fdroidserver.common.options = Options + fdroidserver.update.options = fdroidserver.common.options + + app = fdroidserver.metadata.App( + { + 'AllowedAPKSigningKeys': 'fa4edeadfa4edeadfa4edeadfa4edeadfa4edeadfa4edeadfa4edeadfa4edead' + } + ) + knownapks = fdroidserver.common.KnownApks() + apkfile = 'v1.v2.sig_1020.apk' + (skip, apk, cachechanged) = fdroidserver.update.process_apk( + {}, apkfile, 'repo', knownapks, False + ) + + r = fdroidserver.update.get_apks_without_allowed_signatures(app, apk) + self.assertEqual(apkfile, r) + + def test_update_with_AllowedAPKSigningKeys(self): + """Test that APKs without allowed signatures get deleted.""" + testdir = tempfile.mkdtemp( + prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir + ) + os.chdir(testdir) + os.mkdir('repo') + testapk = os.path.join('repo', 'com.politedroid_6.apk') + shutil.copy(os.path.join(self.basedir, testapk), testapk) + os.mkdir('metadata') + metadatafile = os.path.join('metadata', 'com.politedroid.yml') + shutil.copy(os.path.join(self.basedir, metadatafile), metadatafile) + with open(metadatafile, 'a') as fp: + fp.write( + '\n\nAllowedAPKSigningKeys: 32a23624c201b949f085996ba5ed53d40f703aca4989476949cae891022e0ed6\n' + ) + + fdroidserver.common.options = Options + config = fdroidserver.common.read_config(fdroidserver.common.options) + config['repo_keyalias'] = 'sova' + config['keystorepass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI=' + config['keypass'] = 'r9aquRHYoI8+dYz6jKrLntQ5/NJNASFBacJh7Jv2BlI=' + config['keystore'] = os.path.join(self.basedir, 'keystore.jks') + + self.assertTrue(os.path.exists(testapk)) + with mock.patch('sys.argv', ['fdroid update', '--delete-unknown']): + fdroidserver.update.main() + self.assertTrue(os.path.exists(testapk)) + + with open(metadatafile, 'a') as fp: + fp.write( + '\n\nAllowedAPKSigningKeys: fa4edeadfa4edeadfa4edeadfa4edeadfa4edeadfa4edeadfa4edeadfa4edead\n' + ) + with mock.patch('sys.argv', ['fdroid update', '--delete-unknown']): + fdroidserver.update.main() + self.assertFalse(os.path.exists(testapk)) + def test_translate_per_build_anti_features(self): os.chdir(os.path.join(localmodule, 'tests')) testdir = tempfile.mkdtemp(