Merge branch 'publish_json' into 'master'

Some publish.py improvements

Closes #820

See merge request fdroid/fdroidserver!787

.gitlab-ci.yml

@@ -6,7 +6,7 @@ variables:
 test:
   image: registry.gitlab.com/fdroid/ci-images-server:latest
   script:
-    - $pip install -e .
+    - $pip install -e .[test]
     - cd tests
     - ./complete-ci-tests
 

fdroidserver/publish.py

@@ -141,13 +141,11 @@ def store_stats_fdroid_signing_key_fingerprints(appids, indent=None):
     sign_sig_key_fingerprint_list(jar_file)
 
 
-def status_update_json(newKeyAliases, generatedKeys, signedApks):
+def status_update_json(generatedKeys, signedApks):
     """Output a JSON file with metadata about this run"""
 
     logging.debug(_('Outputting JSON'))
     output = common.setup_status_output(start_timestamp)
-    if newKeyAliases:
-        output['newKeyAliases'] = newKeyAliases
     if generatedKeys:
         output['generatedKeys'] = generatedKeys
     if signedApks:
@@ -155,15 +153,79 @@ def status_update_json(newKeyAliases, generatedKeys, signedApks):
     common.write_status_json(output)
 
 
-def main():
+def check_for_key_collisions(allapps):
+    """
+    Make sure there's no collision in keyaliases from apps.
+    It was suggested at
+    https://dev.guardianproject.info/projects/bazaar/wiki/FDroid_Audit
+    that a package could be crafted, such that it would use the same signing
+    key as an existing app. While it may be theoretically possible for such a
+    colliding package ID to be generated, it seems virtually impossible that
+    the colliding ID would be something that would be a) a valid package ID,
+    and b) a sane-looking ID that would make its way into the repo.
+    Nonetheless, to be sure, before publishing we check that there are no
+    collisions, and refuse to do any publishing if that's the case...
+    :param allapps a dict of all apps to process
+    :return: a list of all aliases corresponding to allapps
+    """
+    allaliases = []
+    for appid in allapps:
+        m = hashlib.md5()  # nosec just used to generate a keyalias
+        m.update(appid.encode('utf-8'))
+        keyalias = m.hexdigest()[:8]
+        if keyalias in allaliases:
+            logging.error(_("There is a keyalias collision - publishing halted"))
+            sys.exit(1)
+        allaliases.append(keyalias)
+    return allaliases
 
+
+def create_key_if_not_existing(keyalias):
+    """
+    Ensures a signing key with the given keyalias exists
+    :return: boolean, True if a new key was created, false otherwise
+    """
+    # See if we already have a key for this application, and
+    # if not generate one...
+    env_vars = {'LC_ALL': 'C.UTF-8',
+                'FDROID_KEY_STORE_PASS': config['keystorepass'],
+                'FDROID_KEY_PASS': config.get('keypass', "")}
+    cmd = [config['keytool'], '-list',
+           '-alias', keyalias, '-keystore', config['keystore'],
+           '-storepass:env', 'FDROID_KEY_STORE_PASS']
+    if config['keystore'] == 'NONE':
+        cmd += config['smartcardoptions']
+    p = FDroidPopen(cmd, envs=env_vars)
+    if p.returncode != 0:
+        logging.info("Key does not exist - generating...")
+        cmd = [config['keytool'], '-genkey',
+               '-keystore', config['keystore'],
+               '-alias', keyalias,
+               '-keyalg', 'RSA', '-keysize', '2048',
+               '-validity', '10000',
+               '-storepass:env', 'FDROID_KEY_STORE_PASS',
+               '-dname', config['keydname']]
+        if config['keystore'] == 'NONE':
+            cmd += config['smartcardoptions']
+        else:
+            cmd += '-keypass:env', 'FDROID_KEY_PASS'
+        p = FDroidPopen(cmd, envs=env_vars)
+        if p.returncode != 0:
+            raise BuildException("Failed to generate key", p.output)
+        return True
+    else:
+        return False
+
+
+def main():
     global config, options
 
     # Parse command line...
     parser = ArgumentParser(usage="%(prog)s [options] "
-                            "[APPID[:VERCODE] [APPID[:VERCODE] ...]]")
+                                  "[APPID[:VERCODE] [APPID[:VERCODE] ...]]")
     common.setup_global_opts(parser)
-    parser.add_argument("appid", nargs='*', help=_("applicationId with optional versionCode in the form APPID[:VERCODE]"))
+    parser.add_argument("appid", nargs='*',
+                        help=_("applicationId with optional versionCode in the form APPID[:VERCODE]"))
     metadata.add_metadata_arguments(parser)
     options = parser.parse_args()
     metadata.warnings_action = options.W
@@ -201,29 +263,11 @@ def main():
         logging.error("Config error - missing '{0}'".format(config['keystore']))
         sys.exit(1)
 
-    # It was suggested at
-    #    https://dev.guardianproject.info/projects/bazaar/wiki/FDroid_Audit
-    # that a package could be crafted, such that it would use the same signing
-    # key as an existing app. While it may be theoretically possible for such a
-    # colliding package ID to be generated, it seems virtually impossible that
-    # the colliding ID would be something that would be a) a valid package ID,
-    # and b) a sane-looking ID that would make its way into the repo.
-    # Nonetheless, to be sure, before publishing we check that there are no
-    # collisions, and refuse to do any publishing if that's the case...
     allapps = metadata.read_metadata()
     vercodes = common.read_pkg_args(options.appid, True)
     signed_apks = dict()
-    new_key_aliases = []
     generated_keys = dict()
-    allaliases = []
-    for appid in allapps:
-        m = hashlib.md5()  # nosec just used to generate a keyalias
-        m.update(appid.encode('utf-8'))
-        keyalias = m.hexdigest()[:8]
-        if keyalias in allaliases:
-            logging.error(_("There is a keyalias collision - publishing halted"))
-            sys.exit(1)
-        allaliases.append(keyalias)
+    allaliases = check_for_key_collisions(allapps)
     logging.info(ngettext('{0} app, {1} key aliases',
                           '{0} apps, {1} key aliases', len(allapps)).format(len(allapps), len(allaliases)))
 
@@ -315,58 +359,12 @@ def main():
                     skipsigning = True
 
             # Now we sign with the F-Droid key.
-
-            # Figure out the key alias name we'll use. Only the first 8
-            # characters are significant, so we'll use the first 8 from
-            # the MD5 of the app's ID and hope there are no collisions.
-            # If a collision does occur later, we're going to have to
-            # come up with a new algorithm, AND rename all existing keys
-            # in the keystore!
             if not skipsigning:
-                if appid in config['keyaliases']:
-                    # For this particular app, the key alias is overridden...
-                    keyalias = config['keyaliases'][appid]
-                    if keyalias.startswith('@'):
-                        m = hashlib.md5()  # nosec just used to generate a keyalias
-                        m.update(keyalias[1:].encode('utf-8'))
-                        keyalias = m.hexdigest()[:8]
-                else:
-                    m = hashlib.md5()  # nosec just used to generate a keyalias
-                    m.update(appid.encode('utf-8'))
-                    keyalias = m.hexdigest()[:8]
-                    new_key_aliases.append(keyalias)
+                keyalias = key_alias(appid)
                 logging.info("Key alias: " + keyalias)
 
-                # See if we already have a key for this application, and
-                # if not generate one...
-                env_vars = {'LC_ALL': 'C.UTF-8',
-                            'FDROID_KEY_STORE_PASS': config['keystorepass'],
-                            'FDROID_KEY_PASS': config.get('keypass', "")}
-                cmd = [config['keytool'], '-list',
-                       '-alias', keyalias, '-keystore', config['keystore'],
-                       '-storepass:env', 'FDROID_KEY_STORE_PASS']
-                if config['keystore'] == 'NONE':
-                    cmd += config['smartcardoptions']
-                p = FDroidPopen(cmd, envs=env_vars)
-                if p.returncode != 0:
-                    logging.info("Key does not exist - generating...")
-                    cmd = [config['keytool'], '-genkey',
-                           '-keystore', config['keystore'],
-                           '-alias', keyalias,
-                           '-keyalg', 'RSA', '-keysize', '2048',
-                           '-validity', '10000',
-                           '-storepass:env', 'FDROID_KEY_STORE_PASS',
-                           '-dname', config['keydname']]
-                    if config['keystore'] == 'NONE':
-                        cmd += config['smartcardoptions']
-                    else:
-                        cmd += '-keypass:env', 'FDROID_KEY_PASS'
-                    p = FDroidPopen(cmd, envs=env_vars)
-                    if p.returncode != 0:
-                        raise BuildException("Failed to generate key", p.output)
-                    if appid not in generated_keys:
-                        generated_keys[appid] = set()
-                    generated_keys[appid].add(appid)
+                if create_key_if_not_existing(keyalias):
+                    generated_keys[appid] = keyalias
 
                 signed_apk_path = os.path.join(output_dir, apkfilename)
                 if os.path.exists(signed_apk_path):
@@ -379,13 +377,14 @@ def main():
                 common.sign_apk(apkfile, signed_apk_path, keyalias)
                 if appid not in signed_apks:
                     signed_apks[appid] = []
-                signed_apks[appid].append(apkfile)
+                signed_apks[appid].append({"keyalias": keyalias,
+                                           "filename": apkfile})
 
                 publish_source_tarball(apkfilename, unsigned_dir, output_dir)
                 logging.info('Published ' + apkfilename)
 
     store_stats_fdroid_signing_key_fingerprints(allapps.keys())
-    status_update_json(new_key_aliases, generated_keys, signed_apks)
+    status_update_json(generated_keys, signed_apks)
     logging.info('published list signing-key fingerprints')
 
 

setup.py

@@ -52,7 +52,6 @@ def get_data_files():
 with open("README.md", "r") as fh:
     long_description = fh.read()
 
-
 setup(name='fdroidserver',
       version='1.2a',
       description='F-Droid Server Tools',
@@ -88,6 +87,9 @@ setup(name='fdroidserver',
           'requests >= 2.5.2, != 2.11.0, != 2.12.2, != 2.18.0',
           'yamllint',
       ],
+      extras_require={
+          'test': ['pyjks'],
+      },
       classifiers=[
           'Development Status :: 4 - Beta',
           'Intended Audience :: Developers',

tests/publish.TestCase

@@ -50,6 +50,9 @@ class PublishTest(unittest.TestCase):
         self.assertEqual('dc3b169e', publish.key_alias('org.test.testy'))
         self.assertEqual('78688a0f', publish.key_alias('org.org.org'))
 
+        self.assertEqual('ee8807d2', publish.key_alias("org.schabi.newpipe"))
+        self.assertEqual('b53c7e11', publish.key_alias("de.grobox.liberario"))
+
         publish.config = {'keyaliases': {'yep.app': '@org.org.org',
                                          'com.example.app': '1a2b3c4d'}}
         self.assertEqual('78688a0f', publish.key_alias('yep.app'))
@@ -162,6 +165,62 @@ class PublishTest(unittest.TestCase):
         with mock.patch.object(sys, 'argv', ['fdroid fakesubcommand']):
             publish.main()
 
+    def test_check_for_key_collisions(self):
+        from fdroidserver.metadata import App
+        common.config = {}
+        common.fill_config_defaults(common.config)
+        publish.config = common.config
+
+        randomappids = [
+            "org.fdroid.fdroid",
+            "a.b.c",
+            "u.v.w.x.y.z",
+            "lpzpkgqwyevnmzvrlaazhgardbyiyoybyicpmifkyrxkobljoz",
+            "vuslsm.jlrevavz.qnbsenmizhur.lprwbjiujtu.ekiho",
+            "w.g.g.w.p.v.f.v.gvhyz",
+            "nlozuqer.ufiinmrbjqboogsjgmpfks.dywtpcpnyssjmqz",
+        ]
+        allapps = {}
+        for appid in randomappids:
+            allapps[appid] = App()
+        allaliases = publish.check_for_key_collisions(allapps)
+        self.assertEqual(len(randomappids), len(allaliases))
+
+        allapps = {'tof.cv.mpp': App(), 'j6mX276h': App()}
+        self.assertEqual(publish.key_alias('tof.cv.mpp'), publish.key_alias('j6mX276h'))
+        self.assertRaises(SystemExit, publish.check_for_key_collisions, allapps)
+
+    def test_create_key_if_not_existing(self):
+        try:
+            import jks
+            import jks.util
+        except ImportError:
+            self.skipTest("pyjks not installed")
+        common.config = {}
+        common.fill_config_defaults(common.config)
+        publish.config = common.config
+        publish.config['keystorepass'] = '123456'
+        publish.config['keypass'] = '654321'
+        publish.config['keystore'] = "keystore.jks"
+        publish.config['keydname'] = 'CN=Birdman, OU=Cell, O=Alcatraz, L=Alcatraz, S=California, C=US'
+        testdir = tempfile.mkdtemp(prefix=inspect.currentframe().f_code.co_name, dir=self.tmpdir)
+        os.chdir(testdir)
+        keystore = jks.KeyStore.new("jks", [])
+        keystore.save(publish.config['keystore'], publish.config['keystorepass'])
+
+        self.assertTrue(publish.create_key_if_not_existing("newalias"))
+        # The second time we try that, a new key should not be created
+        self.assertFalse(publish.create_key_if_not_existing("newalias"))
+        self.assertTrue(publish.create_key_if_not_existing("anotheralias"))
+
+        keystore = jks.KeyStore.load(publish.config['keystore'], publish.config['keystorepass'])
+        self.assertCountEqual(keystore.private_keys, ["newalias", "anotheralias"])
+        for alias, pk in keystore.private_keys.items():
+            self.assertFalse(pk.is_decrypted())
+            pk.decrypt(publish.config['keypass'])
+            self.assertTrue(pk.is_decrypted())
+            self.assertEqual(jks.util.RSA_ENCRYPTION_OID, pk.algorithm_oid)
+
 
 if __name__ == "__main__":
     os.chdir(os.path.dirname(__file__))