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

@@ -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

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))})
 

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:

tests/metadata/dump/com.politedroid.yaml

@@ -1,3 +1,4 @@
+AllowedAPKSigningKeys: []
 AntiFeatures: []
 ArchivePolicy: 4 versions
 AuthorEmail: null

tests/metadata/dump/org.adaway.yaml

@@ -1,3 +1,4 @@
+AllowedAPKSigningKeys: []
 AntiFeatures: []
 ArchivePolicy: null
 AuthorEmail: null

tests/metadata/dump/org.smssecure.smssecure.yaml

@@ -1,3 +1,4 @@
+AllowedAPKSigningKeys: []
 AntiFeatures: []
 ArchivePolicy: null
 AuthorEmail: null

tests/metadata/dump/org.videolan.vlc.yaml

@@ -1,3 +1,4 @@
+AllowedAPKSigningKeys: []
 AntiFeatures: []
 ArchivePolicy: 9 versions
 AuthorEmail: null

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(