check signature and OpenSSL after APK has proven valid

If working with a random grabbag of APKs, there can be all sorts of
issues like corrupt entries in the ZIP, bad signatures, signatures that
are invalid since they use MD5, etc.  Moving these two checks later means
that the APKs can be renamed still.

This does change how common.getsig() works.  For years, it returned
None if the signature check failed.  Now that I've started working
with giant APK collections gathered from the wild, I can see that
`fdroid update` needs to be able to first index what's there, then
make decisions based on that information.  So that means separating
the getsig() fingerprint fetching from the APK signature verification.

This is not hugely security sensitive, since the APKs still have to
get past the Android checks, e.g. update signature checks.  Plus the
APK hash is already included in the signed index.
This commit is contained in:
Hans-Christoph Steiner 2017-06-01 16:24:31 +02:00
parent 372c8b418d
commit 8776221988
2 changed files with 15 additions and 14 deletions

View File

@ -402,10 +402,6 @@ def getsig(apkpath):
if an error occurred.
"""
# verify the jar signature is correct
if not common.verify_apk_signature(apkpath):
return None
with zipfile.ZipFile(apkpath, 'r') as apk:
certs = [n for n in apk.namelist() if common.CERT_PATH_REGEX.match(n)]
@ -1118,8 +1114,6 @@ def scan_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk):
apk['icons_src'] = {}
apk['icons'] = {}
apk['antiFeatures'] = set()
if has_old_openssl(apkfile):
apk['antiFeatures'].add('KnownVuln')
try:
if SdkToolsPopen(['aapt', 'version'], output=False):
@ -1173,6 +1167,13 @@ def scan_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk):
apk['srcname'] = srcfilename
apk['size'] = os.path.getsize(apkfile)
# verify the jar signature is correct
if not common.verify_apk_signature(apkfile):
return True, None, False
if has_old_openssl(apkfile):
apk['antiFeatures'].add('KnownVuln')
apkzip = zipfile.ZipFile(apkfile, 'r')
# if an APK has files newer than the system time, suggest updating

View File

@ -169,21 +169,21 @@ class UpdateTest(unittest.TestCase):
self.assertTrue(False, 'TypeError!')
def testBadGetsig(self):
"""getsig() should still be able to fetch the fingerprint of bad signatures"""
# config needed to use jarsigner and keytool
config = dict()
fdroidserver.common.fill_config_defaults(config)
fdroidserver.update.config = config
apkfile = os.path.join(os.path.dirname(__file__), 'urzip-badsig.apk')
sig = self.javagetsig(apkfile)
self.assertIsNone(sig, "sig should be None: " + str(sig))
pysig = fdroidserver.update.getsig(apkfile)
self.assertIsNone(pysig, "python sig should be None: " + str(sig))
sig = fdroidserver.update.getsig(apkfile)
self.assertEqual(sig, 'e0ecb5fc2d63088e4a07ae410a127722',
"python sig should be: " + str(sig))
apkfile = os.path.join(os.path.dirname(__file__), 'urzip-badcert.apk')
sig = self.javagetsig(apkfile)
self.assertIsNone(sig, "sig should be None: " + str(sig))
pysig = fdroidserver.update.getsig(apkfile)
self.assertIsNone(pysig, "python sig should be None: " + str(sig))
sig = fdroidserver.update.getsig(apkfile)
self.assertEqual(sig, 'e0ecb5fc2d63088e4a07ae410a127722',
"python sig should be: " + str(sig))
def testScanApksAndObbs(self):
os.chdir(os.path.join(localmodule, 'tests'))