diff --git a/fdroidserver/publish.py b/fdroidserver/publish.py index 43a7fbaf..8a6a3e81 100644 --- a/fdroidserver/publish.py +++ b/fdroidserver/publish.py @@ -153,6 +153,33 @@ def status_update_json(generatedKeys, signedApks): common.write_status_json(output) +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 main(): global config, options @@ -199,29 +226,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))) @@ -313,26 +322,8 @@ 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 diff --git a/tests/publish.TestCase b/tests/publish.TestCase index 88cdc48a..41863482 100755 --- a/tests/publish.TestCase +++ b/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,31 @@ 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 + + # We cannot really test the negative case here as there doesn't seem + # to be a md5 collision with text input around. + # So we just test we don't trigger an exception here and get back the same number + # of aliases as we put in apps. + 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)) + if __name__ == "__main__": os.chdir(os.path.dirname(__file__))