From 5dcb48831feee272a9da4a564c52383d3c952d04 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 19 Sep 2017 15:07:19 +0200 Subject: [PATCH] index: always use jarsigner for verifying JAR signatures apksigner v0.7 (build-tools 26.0.1), Google made it require that the AndroidManifest.xml was present in the archive before it verifies the signature. So this needs to stick with the jarsigner hack for JARs. --- fdroidserver/common.py | 37 +++++++++++++++++++++++++++---------- fdroidserver/index.py | 12 +----------- tests/index.TestCase | 4 ++-- tests/run-tests | 1 + 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 4d897653..807974d6 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -50,7 +50,7 @@ from distutils.util import strtobool import fdroidserver.metadata from fdroidserver import _ -from fdroidserver.exception import FDroidException, VCSException, BuildException +from fdroidserver.exception import FDroidException, VCSException, BuildException, VerificationException from .asynchronousfilereader import AsynchronousFileReader @@ -2077,24 +2077,41 @@ def verify_apks(signed_apk, unsigned_apk, tmp_dir): return None -def verify_apk_signature(apk, jar=False): +def verify_jar_signature(jar): + """Verifies the signature of a given JAR file. + + jarsigner is very shitty: unsigned JARs pass as "verified"! So + this has to turn on -strict then check for result 4, since this + does not expect the signature to be from a CA-signed certificate. + + :raises: VerificationException() if the JAR's signature could not be verified + + """ + + if subprocess.call([config['jarsigner'], '-strict', '-verify', jar]) != 4: + raise VerificationException(_("The repository's index could not be verified.")) + + +def verify_apk_signature(apk, min_sdk_version=None): """verify the signature on an APK Try to use apksigner whenever possible since jarsigner is very - shitty: unsigned APKs pass as "verified"! So this has to turn on - -strict then check for result 4. - - You can set :param: jar to True if you want to use this method - to verify jar signatures. + shitty: unsigned APKs pass as "verified"! Warning, this does + not work on JARs with apksigner >= 0.7 (build-tools 26.0.1) """ if set_command_in_config('apksigner'): args = [config['apksigner'], 'verify'] - if jar: - args += ['--min-sdk-version=1'] + if min_sdk_version: + args += ['--min-sdk-version=' + min_sdk_version] return subprocess.call(args + [apk]) == 0 else: logging.warning("Using Java's jarsigner, not recommended for verifying APKs! Use apksigner") - return subprocess.call([config['jarsigner'], '-strict', '-verify', apk]) == 4 + try: + verify_jar_signature(apk) + return True + except: + pass + return False def verify_old_apk_signature(apk): diff --git a/fdroidserver/index.py b/fdroidserver/index.py index 59139e17..79322d55 100644 --- a/fdroidserver/index.py +++ b/fdroidserver/index.py @@ -632,7 +632,7 @@ def download_repo_index(url_str, etag=None, verify_fingerprint=True): jar = zipfile.ZipFile(fp) # verify that the JAR signature is valid - verify_jar_signature(fp.name) + common.verify_jar_signature(fp.name) # get public key and its fingerprint from JAR public_key, public_key_fingerprint = get_public_key_from_jar(jar) @@ -652,16 +652,6 @@ def download_repo_index(url_str, etag=None, verify_fingerprint=True): return index, new_etag -def verify_jar_signature(file): - """ - Verifies the signature of a given JAR file. - - :raises: VerificationException() if the JAR's signature could not be verified - """ - if not common.verify_apk_signature(file, jar=True): - raise VerificationException(_("The repository's index could not be verified.")) - - def get_public_key_from_jar(jar): """ Get the public key and its fingerprint from a JAR file. diff --git a/tests/index.TestCase b/tests/index.TestCase index 2798e781..31593523 100755 --- a/tests/index.TestCase +++ b/tests/index.TestCase @@ -39,14 +39,14 @@ class IndexTest(unittest.TestCase): source_dir = os.path.join(basedir, 'signindex') for f in ('testy.jar', 'guardianproject.jar'): testfile = os.path.join(source_dir, f) - fdroidserver.index.verify_jar_signature(testfile) + fdroidserver.common.verify_jar_signature(testfile) def test_verify_jar_signature_fails(self): basedir = os.path.dirname(__file__) source_dir = os.path.join(basedir, 'signindex') testfile = os.path.join(source_dir, 'unsigned.jar') with self.assertRaises(fdroidserver.index.VerificationException): - fdroidserver.index.verify_jar_signature(testfile) + fdroidserver.common.verify_jar_signature(testfile) def test_get_public_key_from_jar_succeeds(self): basedir = os.path.dirname(__file__) diff --git a/tests/run-tests b/tests/run-tests index 608c1e2e..c58942bd 100755 --- a/tests/run-tests +++ b/tests/run-tests @@ -266,6 +266,7 @@ cp $WORKSPACE/tests/urzip.apk \ sed -i 's,archive_older = [0-9],archive_older = 3,' config.py $fdroid update --pretty --nosign +echo "This will fail when jarsigner allows MD5 for APK signatures" test `grep '' archive/index.xml | wc -l` -eq 5 test `grep '' repo/index.xml | wc -l` -eq 7 -- 2.30.2