diff --git a/fdroidserver/build.py b/fdroidserver/build.py index 83065dbb..71fd20bc 100644 --- a/fdroidserver/build.py +++ b/fdroidserver/build.py @@ -826,6 +826,15 @@ def force_halt_build(timeout): vm.destroy() +def keep_when_not_allowed(): + """Control if APKs signed by keys not in AllowedAPKSigningKeys are removed.""" + return ( + (options is not None and options.keep_when_not_allowed) + or (config is not None and config.get('keep_when_not_allowed')) + or common.default_config['keep_when_not_allowed'] + ) + + def parse_commandline(): """Parse the command line. @@ -863,6 +872,8 @@ def parse_commandline(): help=_("Force build of disabled apps, and carries on regardless of scan problems. Only allowed in test mode.")) parser.add_argument("-a", "--all", action="store_true", default=False, help=_("Build all applications available")) + parser.add_argument("--keep-when-not-allowed", default=False, action="store_true", + help=argparse.SUPPRESS) parser.add_argument("-w", "--wiki", default=False, action="store_true", help=argparse.SUPPRESS) metadata.add_metadata_arguments(parser) @@ -1112,6 +1123,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 or keep_when_not_allowed(): + 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/fdroidserver/common.py b/fdroidserver/common.py index a6039dcb..550e7b5a 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -128,6 +128,7 @@ default_config = { 'gradle': os.path.join(FDROID_PATH, 'gradlew-fdroid'), 'sync_from_local_copy_dir': False, 'allow_disabled_algorithms': False, + 'keep_when_not_allowed': False, 'per_app_repos': False, 'make_current_version_link': False, 'current_version_name_source': 'Name', diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index bc165f30..fb94e258 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -696,6 +696,25 @@ def check_updates_ucm_http_aum_pattern(app): # noqa: D403 yield _("AutoUpdateMode with UpdateCheckMode: HTTP must have a pattern.") +def check_certificate_pinned_binaries(app): + if len(app.get('AllowedAPKSigningKeys')) > 0: + return + if app.get('Binaries') is not None: + yield _( + 'App has Binaries but does not have corresponding AllowedAPKSigningKeys to pin certificate.' + ) + return + builds = app.get('Builds') + if builds is None: + return + for build in builds: + if build.get('binary') is not None: + yield _( + 'App version has binary but does not have corresponding AllowedAPKSigningKeys to pin certificate.' + ) + return + + def main(): global config, options @@ -803,6 +822,7 @@ def main(): check_current_version_code, check_updates_expected, check_updates_ucm_http_aum_pattern, + check_certificate_pinned_binaries, ] for check_func in app_check_funcs: diff --git a/tests/build.TestCase b/tests/build.TestCase index a00f4f75..128b3f4c 100755 --- a/tests/build.TestCase +++ b/tests/build.TestCase @@ -40,6 +40,10 @@ class FakeProcess: print('FakeFDroidPopen', args, kwargs) +class Options: + keep_when_not_allowed = False + + class BuildTest(unittest.TestCase): '''fdroidserver/build.py''' @@ -51,6 +55,7 @@ class BuildTest(unittest.TestCase): os.chdir(self.basedir) fdroidserver.common.config = None fdroidserver.build.config = None + fdroidserver.build.options = None self._td = mkdtemp() self.testdir = self._td.name @@ -604,9 +609,9 @@ class BuildTest(unittest.TestCase): sdk_path = os.path.join(self.testdir, 'android-sdk') self.create_fake_android_home(sdk_path) with open('config.yml', 'w') as fp: - yaml.dump({'sdk_path': sdk_path}, fp) + yaml.dump({'sdk_path': sdk_path, 'keep_when_not_allowed': True}, fp) os.chmod('config.yml', 0o600) - fdroidserver.common.build = fdroidserver.common.read_config() + fdroidserver.build.config = fdroidserver.common.read_config() os.mkdir('metadata') appid = 'info.guardianproject.checkey' @@ -696,6 +701,139 @@ class BuildTest(unittest.TestCase): self.assertFalse(os.path.exists(production_result)) self.assertFalse(os.path.exists(production_compare_file)) + def test_failed_allowedapksigningkeys_are_not_in_unsigned(self): + os.chdir(self.testdir) + os.mkdir('metadata') + appid = 'info.guardianproject.checkey' + metadata_file = os.path.join('metadata', appid + '.yml') + shutil.copy(os.path.join(self.basedir, metadata_file), 'metadata') + with open(metadata_file) as fp: + app = fdroidserver.metadata.App(yaml.safe_load(fp)) + app['RepoType'] = 'git' + app[ + 'Binaries' + ] = 'https://example.com/fdroid/repo/info.guardianproject.checkey_%v.apk' + build = fdroidserver.metadata.Build( + { + 'versionCode': 123, + 'versionName': '1.2.3', + 'commit': '1.2.3', + 'disable': False, + } + ) + 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')) + production_result = os.path.join( + 'unsigned', '%s_%d.apk' % (appid, build['versionCode']) + ) + production_compare_file = os.path.join( + 'unsigned', 'binaries', '%s_%d.binary.apk' % (appid, build['versionCode']) + ) + os.makedirs(os.path.join('tmp', 'binaries')) + test_result = os.path.join('tmp', '%s_%d.apk' % (appid, build['versionCode'])) + test_compare_file = os.path.join( + 'tmp', 'binaries', '%s_%d.binary.apk' % (appid, build['versionCode']) + ) + with mock.patch( + 'fdroidserver.common.force_exit', lambda *args: None + ) as a, mock.patch( + 'fdroidserver.common.get_android_tools_version_log', lambda: 'fake' + ) as b, mock.patch( + 'fdroidserver.common.FDroidPopen', FakeProcess + ) as c, mock.patch( + 'fdroidserver.build.FDroidPopen', FakeProcess + ) as d, mock.patch( + 'fdroidserver.build.trybuild', lambda *args: True + ) as e, mock.patch( + 'fdroidserver.net.download_file', lambda *args, **kwargs: None + ) as f: + a, b, c, d, e, f # silence linters' "unused" warnings + + with mock.patch('sys.argv', ['fdroid build', appid]): + # 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 + ) 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() + with mock.patch( + 'fdroidserver.common.verify_apks', lambda *args: 'failed' + ): + fdroidserver.build.main() + self.assertFalse(os.path.exists(production_result)) + self.assertFalse(os.path.exists(production_compare_file)) + + with mock.patch('sys.argv', ['fdroid build', '--test', appid]): + # 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 + ) 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() + with mock.patch( + 'fdroidserver.common.verify_apks', lambda *args: 'failed' + ): + 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)) + @mock.patch('fdroidserver.vmtools.get_build_vm') @mock.patch('fdroidserver.vmtools.get_clean_builder') @mock.patch('paramiko.SSHClient') @@ -780,6 +918,51 @@ class BuildTest(unittest.TestCase): test_flag = ('--skip-scan', True) fdroidserver.build.build_server(app, build, vcs, '', '', '', False) + def test_keep_when_not_allowed_default(self): + self.assertFalse(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_config_true(self): + fdroidserver.build.config = {'keep_when_not_allowed': True} + self.assertTrue(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_config_false(self): + fdroidserver.build.config = {'keep_when_not_allowed': False} + self.assertFalse(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_options_true(self): + fdroidserver.build.options = Options + fdroidserver.build.options.keep_when_not_allowed = True + self.assertTrue(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_options_false(self): + fdroidserver.build.options = Options + fdroidserver.build.options.keep_when_not_allowed = False + self.assertFalse(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_options_true_override_config(self): + fdroidserver.build.options = Options + fdroidserver.build.options.keep_when_not_allowed = True + fdroidserver.build.config = {'keep_when_not_allowed': False} + self.assertTrue(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_options_default_does_not_override(self): + fdroidserver.build.options = Options + fdroidserver.build.options.keep_when_not_allowed = False + fdroidserver.build.config = {'keep_when_not_allowed': True} + self.assertTrue(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_all_true(self): + fdroidserver.build.options = Options + fdroidserver.build.options.keep_when_not_allowed = True + fdroidserver.build.config = {'keep_when_not_allowed': True} + self.assertTrue(fdroidserver.build.keep_when_not_allowed()) + + def test_keep_when_not_allowed_all_false(self): + fdroidserver.build.options = Options + fdroidserver.build.options.keep_when_not_allowed = False + fdroidserver.build.config = {'keep_when_not_allowed': False} + self.assertFalse(fdroidserver.build.keep_when_not_allowed()) + if __name__ == "__main__": os.chdir(os.path.dirname(__file__))