From 26472c22cecaab2f70cef2af67ec37f14fdd07e5 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Tue, 18 Apr 2023 14:38:58 +0200 Subject: [PATCH] build: check `AllowedAPKSigningKeys` in reproducible build scenario The builder should check the `AllowedAPKSigningKeys` at build time, so that the CI can check if somebody gives a wrong value that doesn't match a compared RB binary. In the event it fails, it gives useful information, and in the event it succeeds, it makes it clear that this build has verification back to the developer's original key. Also, add tests for this to the test suite. --- fdroidserver/build.py | 28 ++++++++++++++++++++++++ tests/build.TestCase | 51 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/fdroidserver/build.py b/fdroidserver/build.py index 83065dbb..9078cd04 100644 --- a/fdroidserver/build.py +++ b/fdroidserver/build.py @@ -1112,6 +1112,34 @@ def main(): 'supplied reference binary ' 'successfully') + used_key = common.apk_signer_fingerprint(of) + expected_keys = app['AllowedAPKSigningKeys'] + if used_key is None: + logging.warn(_('reference binary missing ' + 'signature')) + elif len(expected_keys) == 0: + logging.warn(_('AllowedAPKSigningKeys missing ' + 'but reference binary supplied')) + elif used_key not in expected_keys: + if options.test: + logging.warning(_('Keeping failed build "{apkfilename}"') + .format(apkfilename=unsigned_apk)) + else: + logging.debug('removing %s', unsigned_apk) + os.remove(unsigned_apk) + logging.debug('removing %s', of) + os.remove(of) + raise FDroidException('supplied reference ' + 'binary signed with ' + '{signer} instead of ' + 'with {expected}'. + format(signer=used_key, + expected=expected_keys)) + else: + logging.info(_('supplied reference binary has ' + 'allowed signer {signer}'). + format(signer=used_key)) + build_succeeded_ids.append([app['id'], build.versionCode]) if not options.onserver: diff --git a/tests/build.TestCase b/tests/build.TestCase index a00f4f75..0e4906f5 100755 --- a/tests/build.TestCase +++ b/tests/build.TestCase @@ -627,6 +627,9 @@ class BuildTest(unittest.TestCase): } ) app['Builds'] = [build] + expected_key = 'a' * 64 + bogus_key = 'b' * 64 + app['AllowedAPKSigningKeys'] = [expected_key] fdroidserver.metadata.write_metadata(metadata_file, app) os.makedirs(os.path.join('unsigned', 'binaries')) @@ -657,13 +660,32 @@ class BuildTest(unittest.TestCase): a, b, c, d, e, f # silence linters' "unused" warnings with mock.patch('sys.argv', ['fdroid build', appid]): - # successful comparison + # successful comparison, successful signer open(production_result, 'w').close() open(production_compare_file, 'w').close() - with mock.patch('fdroidserver.common.verify_apks', lambda *args: None): + with mock.patch( + 'fdroidserver.common.verify_apks', lambda *args: None + ) as g, mock.patch( + 'fdroidserver.common.apk_signer_fingerprint', + lambda *args: expected_key, + ) as h: + g, h fdroidserver.build.main() self.assertTrue(os.path.exists(production_result)) self.assertTrue(os.path.exists(production_compare_file)) + # successful comparison, failed signer + open(production_result, 'w').close() + open(production_compare_file, 'w').close() + with mock.patch( + 'fdroidserver.common.verify_apks', lambda *args: None + ) as g, mock.patch( + 'fdroidserver.common.apk_signer_fingerprint', + lambda *args: bogus_key, + ) as h: + g, h + fdroidserver.build.main() + self.assertFalse(os.path.exists(production_result)) + self.assertFalse(os.path.exists(production_compare_file)) # failed comparison open(production_result, 'w').close() open(production_compare_file, 'w').close() @@ -675,15 +697,36 @@ class BuildTest(unittest.TestCase): self.assertFalse(os.path.exists(production_compare_file)) with mock.patch('sys.argv', ['fdroid build', '--test', appid]): - # successful comparison + # successful comparison, successful signer open(test_result, 'w').close() open(test_compare_file, 'w').close() - with mock.patch('fdroidserver.common.verify_apks', lambda *args: None): + with mock.patch( + 'fdroidserver.common.verify_apks', lambda *args: None + ) as g, mock.patch( + 'fdroidserver.common.apk_signer_fingerprint', + lambda *args: expected_key, + ) as h: + g, h fdroidserver.build.main() self.assertTrue(os.path.exists(test_result)) self.assertTrue(os.path.exists(test_compare_file)) self.assertFalse(os.path.exists(production_result)) self.assertFalse(os.path.exists(production_compare_file)) + # successful comparison, failed signer + open(test_result, 'w').close() + open(test_compare_file, 'w').close() + with mock.patch( + 'fdroidserver.common.verify_apks', lambda *args: None + ) as g, mock.patch( + 'fdroidserver.common.apk_signer_fingerprint', + lambda *args: bogus_key, + ) as h: + g, h + fdroidserver.build.main() + self.assertTrue(os.path.exists(test_result)) + self.assertFalse(os.path.exists(test_compare_file)) + self.assertFalse(os.path.exists(production_result)) + self.assertFalse(os.path.exists(production_compare_file)) # failed comparison open(test_result, 'w').close() open(test_compare_file, 'w').close()