From: Hans-Christoph Steiner Date: Mon, 19 Dec 2016 15:54:32 +0000 (+0100) Subject: verify: ensure only a single signature is in compared APK X-Git-Tag: 0.8~94^2~4 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=998b6245e93ce35ba0a7ff85f64b720cb5674691;p=fdroidserver.git verify: ensure only a single signature is in compared APK The ZIP format allows multiple entries with the exact same filename, and on top of that, it does not allow deleting or updating entries. To make the `fdroid verify` procedure failsafe, it needs to create a new temporary APK that is made up on the contents of the "unsigned APK" and the signature from the "signed APK". Since it would be possible to give a signed APK as in the unsigned one's position, `fdroid verify` was not able to update the signature since it was just adding the new signature to the end of the ZIP file. When reading a ZIP, the first entry is used. --- diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 31c97380..103ce449 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -2012,28 +2012,43 @@ def verify_apks(signed_apk, unsigned_apk, tmp_dir): One of the inputs is signed, the other is unsigned. The signature metadata is transferred from the signed to the unsigned apk, and then jarsigner is used to verify that the signature from the signed apk is also varlid for - the unsigned one. + the unsigned one. If the APK given as unsigned actually does have a + signature, it will be stripped out and ignored. :param signed_apk: Path to a signed apk file :param unsigned_apk: Path to an unsigned apk file expected to match it :param tmp_dir: Path to directory for temporary files :returns: None if the verification is successful, otherwise a string describing what went wrong. """ - with ZipFile(signed_apk) as signed_apk_as_zip: - meta_inf_files = ['META-INF/MANIFEST.MF'] - for f in signed_apk_as_zip.namelist(): - if apk_sigfile.match(f): - meta_inf_files.append(f) - if len(meta_inf_files) < 3: - return "Signature files missing from {0}".format(signed_apk) - signed_apk_as_zip.extractall(tmp_dir, meta_inf_files) - with ZipFile(unsigned_apk, mode='a') as unsigned_apk_as_zip: - for meta_inf_file in meta_inf_files: - unsigned_apk_as_zip.write(os.path.join(tmp_dir, meta_inf_file), arcname=meta_inf_file) - - if subprocess.call([config['jarsigner'], '-verify', unsigned_apk]) != 0: - logging.info("...NOT verified - {0}".format(signed_apk)) - return compare_apks(signed_apk, unsigned_apk, tmp_dir) + + signed = ZipFile(signed_apk, 'r') + meta_inf_files = ['META-INF/MANIFEST.MF'] + for f in signed.namelist(): + if apk_sigfile.match(f): + meta_inf_files.append(f) + if len(meta_inf_files) < 3: + return "Signature files missing from {0}".format(signed_apk) + + tmp_apk = os.path.join(tmp_dir, 'sigcp_' + os.path.basename(unsigned_apk)) + unsigned = ZipFile(unsigned_apk, 'r') + # only read the signature from the signed APK, everything else from unsigned + with ZipFile(tmp_apk, 'w') as tmp: + for filename in meta_inf_files: + tmp.writestr(signed.getinfo(filename), signed.read(filename)) + for info in unsigned.infolist(): + if info.filename in meta_inf_files: + logging.warning('Ignoring ' + info.filename + ' from ' + unsigned_apk) + continue + if info.filename in tmp.namelist(): + return "duplicate filename found: " + info.filename + tmp.writestr(info, unsigned.read(info.filename)) + unsigned.close() + signed.close() + + if subprocess.call([config['jarsigner'], '-verify', tmp_apk]) != 0: + logging.info("...NOT verified - {0}".format(unsigned_apk)) + return compare_apks(signed_apk, tmp_apk, tmp_dir) + logging.info("...successfully verified") return None diff --git a/tests/common.TestCase b/tests/common.TestCase index 98939985..63550e47 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -10,6 +10,7 @@ import shutil import sys import tempfile import unittest +from zipfile import ZipFile localmodule = os.path.realpath( os.path.join(os.path.dirname(inspect.getfile(inspect.currentframe())), '..')) @@ -177,6 +178,46 @@ class CommonTest(unittest.TestCase): # these should be resigned, and therefore different self.assertNotEqual(open(sourcefile, 'rb').read(), open(testfile, 'rb').read()) + def test_verify_apks(self): + fdroidserver.common.config = None + config = fdroidserver.common.read_config(fdroidserver.common.options) + config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner') + fdroidserver.common.config = config + + basedir = os.path.dirname(__file__) + sourceapk = os.path.join(basedir, 'urzip.apk') + + tmpdir = os.path.join(basedir, '..', '.testfiles') + if not os.path.exists(tmpdir): + os.makedirs(tmpdir) + testdir = tempfile.mkdtemp(prefix='test_verify_apks', dir=tmpdir) + print('testdir', testdir) + + copyapk = os.path.join(testdir, 'urzip-copy.apk') + shutil.copy(sourceapk, copyapk) + self.assertTrue(fdroidserver.common.verify_apk_signature(copyapk)) + self.assertIsNone(fdroidserver.common.verify_apks(sourceapk, copyapk, tmpdir)) + + unsignedapk = os.path.join(testdir, 'urzip-unsigned.apk') + with ZipFile(sourceapk, 'r') as apk: + with ZipFile(unsignedapk, 'w') as testapk: + for info in apk.infolist(): + if not info.filename.startswith('META-INF/'): + testapk.writestr(info, apk.read(info.filename)) + self.assertIsNone(fdroidserver.common.verify_apks(sourceapk, unsignedapk, tmpdir)) + + twosigapk = os.path.join(testdir, 'urzip-twosig.apk') + otherapk = ZipFile(os.path.join(basedir, 'urzip-release.apk'), 'r') + with ZipFile(sourceapk, 'r') as apk: + with ZipFile(twosigapk, 'w') as testapk: + for info in apk.infolist(): + testapk.writestr(info, apk.read(info.filename)) + if info.filename.startswith('META-INF/'): + testapk.writestr(info, otherapk.read(info.filename)) + otherapk.close() + self.assertFalse(fdroidserver.common.verify_apk_signature(twosigapk)) + self.assertIsNone(fdroidserver.common.verify_apks(sourceapk, twosigapk, tmpdir)) + if __name__ == "__main__": parser = optparse.OptionParser()