From cc1e10a37ae16b849f3e64ad126dcd7264ce8188 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 3 May 2018 13:46:36 +0200 Subject: [PATCH 1/3] delete .java.security after checking MD5 signatures This file is written freshly each time before use, so it does not need to be ekpt around. It was the only file making the fdroiddata.git repo dirty on the f-droid.org infrastructure. This also adds stricter file permissions to avoid an attacker changing those settings during operation. --- fdroidserver/common.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index ca50ceea..68b7e79e 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -2665,12 +2665,20 @@ def verify_old_apk_signature(apk): jarsigner passes unsigned APKs as "verified"! So this has to turn on -strict then check for result 4. + Just to be safe, this never reuses the file, and locks down the + file permissions while in use. That should prevent a bad actor + from changing the settings during operation. + :returns: boolean whether the APK was verified + """ _java_security = os.path.join(os.getcwd(), '.java.security') + if os.path.exists(_java_security): + os.remove(_java_security) with open(_java_security, 'w') as fp: fp.write('jdk.jar.disabledAlgorithms=MD2, RSA keySize < 1024') + os.chmod(_java_security, 0o400) try: cmd = [ @@ -2685,6 +2693,10 @@ def verify_old_apk_signature(apk): else: logging.debug(_('JAR signature verified: {path}').format(path=apk)) return True + finally: + if os.path.exists(_java_security): + os.chmod(_java_security, 0o600) + os.remove(_java_security) logging.error(_('Old APK signature failed to verify: {path}').format(path=apk) + '\n' + output.decode('utf-8')) From 98a2f70e38605450446857fd7064e702f9f55b73 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 3 May 2018 13:30:03 +0200 Subject: [PATCH 2/3] fix intermittent test failure For some reason, the parser stopped working intermittently, even though the format has been the same since aapt 23 or earlier. Then also, some of the test cases pointed to symlinks that were no longer generated, and one test app now has a blank versionName. Strange that this wasn't caught in the gitlab-ci runs. !484 FAIL: test_get_api_id_aapt (__main__.CommonTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "./common.TestCase", line 578, in testA_get_api_id_aapt self.assertEqual(versionName, vn) AssertionError: '0.1' != "0.1' platformBuildVersionName='4.3.1-1425645" - 0.1 + 0.1' platformBuildVersionName='4.3.1-1425645 --- fdroidserver/common.py | 4 ++-- tests/common.TestCase | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 68b7e79e..c0e49027 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -1995,12 +1995,12 @@ def is_apk_and_debuggable(apkfile): def get_apk_id_aapt(apkfile): - """Extrat identification information from APK using aapt. + """Extract identification information from APK using aapt. :param apkfile: path to an APK file. :returns: triplet (appid, version code, version name) """ - r = re.compile("^package: name='(?P.*)' versionCode='(?P.*)' versionName='(?P.*)'.*") + r = re.compile("^package: name='(?P.*)' versionCode='(?P.*)' versionName='(?P.*?)'(?: platformBuildVersionName='.*')?") p = SdkToolsPopen(['aapt', 'dump', 'badging', apkfile], output=False) for line in p.output.splitlines(): m = r.match(line) diff --git a/tests/common.TestCase b/tests/common.TestCase index 55af2558..c01369f4 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -543,8 +543,6 @@ class CommonTest(unittest.TestCase): testcases = [ ('repo/obb.main.twoversions_1101613.apk', 'obb.main.twoversions', '1101613', '0.1'), - ('OBBMainPatchCurrent.apk', 'obb.mainpatch.current', '1619', '0.1'), - ('OBBMainTwoVersions.apk', 'obb.main.twoversions', '1101613', '0.1'), ('org.bitbucket.tickytacky.mirrormirror_1.apk', 'org.bitbucket.tickytacky.mirrormirror', '1', '1.0'), ('org.bitbucket.tickytacky.mirrormirror_2.apk', 'org.bitbucket.tickytacky.mirrormirror', '2', '1.0.1'), ('org.bitbucket.tickytacky.mirrormirror_3.apk', 'org.bitbucket.tickytacky.mirrormirror', '3', '1.0.2'), @@ -555,12 +553,11 @@ class CommonTest(unittest.TestCase): ('urzip-badsig.apk', 'info.guardianproject.urzip', '100', '0.1'), ('urzip-release.apk', 'info.guardianproject.urzip', '100', '0.1'), ('urzip-release-unsigned.apk', 'info.guardianproject.urzip', '100', '0.1'), - ('urzip-πÇÇπÇÇ现代汉语通用字-български-عربي1234.apk', 'info.guardianproject.urzip', '100', '0.1'), ('repo/com.politedroid_3.apk', 'com.politedroid', '3', '1.2'), ('repo/com.politedroid_4.apk', 'com.politedroid', '4', '1.3'), ('repo/com.politedroid_5.apk', 'com.politedroid', '5', '1.4'), ('repo/com.politedroid_6.apk', 'com.politedroid', '6', '1.5'), - ('repo/duplicate.permisssions_9999999.apk', 'duplicate.permisssions', '9999999', '0.3-7-gb817ac8'), + ('repo/duplicate.permisssions_9999999.apk', 'duplicate.permisssions', '9999999', ''), ('repo/info.zwanenburg.caffeinetile_4.apk', 'info.zwanenburg.caffeinetile', '4', '1.3'), ('repo/obb.main.oldversion_1444412523.apk', 'obb.main.oldversion', '1444412523', '0.1'), ('repo/obb.mainpatch.current_1619_another-release-key.apk', 'obb.mainpatch.current', '1619', '0.1'), From 27a5cce832cb4c2d42e3ebd91562c00043b02721 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 3 May 2018 13:33:37 +0200 Subject: [PATCH 3/3] implement common.get_apk_id() using androguard --- fdroidserver/common.py | 57 +++++++++++++++++++++++++++++++++----- fdroidserver/signatures.py | 2 +- fdroidserver/update.py | 25 ++--------------- tests/common.TestCase | 19 +++++++++---- 4 files changed, 66 insertions(+), 37 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index c0e49027..bb1de220 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -1955,6 +1955,36 @@ def use_androguard(): use_androguard.show_path = True +def _get_androguard_APK(apkfile): + try: + from androguard.core.bytecodes.apk import APK + except ImportError: + raise FDroidException("androguard library is not installed and aapt not present") + + return APK(apkfile) + + +def ensure_final_value(packageName, arsc, value): + """Ensure incoming value is always the value, not the resid + + androguard will sometimes return the Android "resId" aka + Resource ID instead of the actual value. This checks whether + the value is actually a resId, then performs the Android + Resource lookup as needed. + + """ + if value: + returnValue = value + if value[0] == '@': + try: # can be a literal value or a resId + res_id = int('0x' + value[1:], 16) + res_id = arsc.get_id(packageName, res_id)[1] + returnValue = arsc.get_string(packageName, res_id)[1] + except ValueError: + pass + return returnValue + + def is_apk_and_debuggable_aapt(apkfile): p = SdkToolsPopen(['aapt', 'dump', 'xmltree', apkfile, 'AndroidManifest.xml'], output=False) @@ -1967,12 +1997,7 @@ def is_apk_and_debuggable_aapt(apkfile): def is_apk_and_debuggable_androguard(apkfile): - try: - from androguard.core.bytecodes.apk import APK - except ImportError: - raise FDroidException("androguard library is not installed and aapt not present") - - apkobject = APK(apkfile) + apkobject = _get_androguard_APK(apkfile) if apkobject.is_valid_APK(): debuggable = apkobject.get_element("application", "debuggable") if debuggable is not None: @@ -1994,12 +2019,30 @@ def is_apk_and_debuggable(apkfile): return is_apk_and_debuggable_aapt(apkfile) -def get_apk_id_aapt(apkfile): +def get_apk_id(apkfile): """Extract identification information from APK using aapt. :param apkfile: path to an APK file. :returns: triplet (appid, version code, version name) """ + if use_androguard(): + return get_apk_id_androguard(apkfile) + else: + return get_apk_id_aapt(apkfile) + + +def get_apk_id_androguard(apkfile): + if not os.path.exists(apkfile): + raise FDroidException(_("Reading packageName/versionCode/versionName failed, APK invalid: '{apkfilename}'") + .format(apkfilename=apkfile)) + a = _get_androguard_APK(apkfile) + versionName = ensure_final_value(a.package, a.get_android_resources(), a.get_androidversion_name()) + if not versionName: + versionName = '' # versionName is expected to always be a str + return a.package, a.get_androidversion_code(), versionName + + +def get_apk_id_aapt(apkfile): r = re.compile("^package: name='(?P.*)' versionCode='(?P.*)' versionName='(?P.*?)'(?: platformBuildVersionName='.*')?") p = SdkToolsPopen(['aapt', 'dump', 'badging', apkfile], output=False) for line in p.output.splitlines(): diff --git a/fdroidserver/signatures.py b/fdroidserver/signatures.py index c5b115b8..7d82ad14 100644 --- a/fdroidserver/signatures.py +++ b/fdroidserver/signatures.py @@ -36,7 +36,7 @@ def extract_signature(apkpath): raise FDroidException("no valid signature in '{}'".format(apkpath)) logging.debug('signature okay: %s', apkpath) - appid, vercode, _ignored = common.get_apk_id_aapt(apkpath) + appid, vercode, _ignored = common.get_apk_id(apkpath) sigdir = common.metadata_get_sigdir(appid, vercode) if not os.path.exists(sigdir): os.makedirs(sigdir) diff --git a/fdroidserver/update.py b/fdroidserver/update.py index ebd29a2c..b1aac66a 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -1181,27 +1181,6 @@ def scan_apk_aapt(apk, apkfile): apk['icons_src'] = _get_apk_icons_src(apkfile, icon_name) -def _ensure_final_value(packageName, arsc, value): - """Ensure incoming value is always the value, not the resid - - androguard will sometimes return the Android "resId" aka - Resource ID instead of the actual value. This checks whether - the value is actually a resId, then performs the Android - Resource lookup as needed. - - """ - if value: - returnValue = value - if value[0] == '@': - try: # can be a literal value or a resId - res_id = int(value.replace("@", "0x"), 16) - res_id = arsc.get_id(packageName, res_id)[1] - returnValue = arsc.get_string(packageName, res_id)[1] - except ValueError: - pass - return returnValue - - def _sanitize_sdk_version(value): """Sanitize the raw values from androguard to handle bad values @@ -1250,8 +1229,8 @@ def scan_apk_androguard(apk, apkfile): apk['versionCode'] = int(apkobject.get_androidversion_code()) apk['name'] = apkobject.get_app_name() - apk['versionName'] = _ensure_final_value(apk['packageName'], arsc, - apkobject.get_androidversion_name()) + apk['versionName'] = common.ensure_final_value(apk['packageName'], arsc, + apkobject.get_androidversion_name()) minSdkVersion = _sanitize_sdk_version(apkobject.get_min_sdk_version()) if minSdkVersion is not None: diff --git a/tests/common.TestCase b/tests/common.TestCase index c01369f4..03acff65 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -533,7 +533,7 @@ class CommonTest(unittest.TestCase): self.assertTrue(fdroidserver.common.verify_apk_signature(signed)) self.assertEqual(18, fdroidserver.common.get_minSdkVersion_aapt(signed)) - def test_get_api_id_aapt(self): + def test_get_api_id(self): config = dict() fdroidserver.common.fill_config_defaults(config) @@ -568,13 +568,20 @@ class CommonTest(unittest.TestCase): ('repo/urzip-; Рахма́нинов, [rɐxˈmanʲɪnəf] سيرجي_رخمانينوف 谢尔盖·.apk', 'info.guardianproject.urzip', '100', '0.1'), ] for apkfilename, appid, versionCode, versionName in testcases: - a, vc, vn = fdroidserver.common.get_apk_id_aapt(apkfilename) - self.assertEqual(appid, a) - self.assertEqual(versionCode, vc) - self.assertEqual(versionName, vn) + print('\n\nAPKFILENAME\n', apkfilename) + if 'aapt' in config: + a, vc, vn = fdroidserver.common.get_apk_id_aapt(apkfilename) + self.assertEqual(appid, a) + self.assertEqual(versionCode, vc) + self.assertEqual(versionName, vn) + if fdroidserver.common.use_androguard(): + a, vc, vn = fdroidserver.common.get_apk_id_androguard(apkfilename) + self.assertEqual(appid, a) + self.assertEqual(versionCode, vc) + self.assertEqual(versionName, vn) with self.assertRaises(FDroidException): - fdroidserver.common.get_apk_id_aapt('nope') + fdroidserver.common.get_apk_id('nope') def test_get_minSdkVersion_aapt(self):