Merge branch 'rational-jarsigner-logging' into 'master'

handle jarsigner/apksigner output cleanly for rational logging

Closes #405

See merge request fdroid/fdroidserver!404

.gitignore

@@ -28,6 +28,7 @@ tmp/
 makebuildserver.config.py
 /tests/.fdroid.keypass.txt
 /tests/.fdroid.keystorepass.txt
+/tests/.java.security
 /tests/fdroid-icon.png
 /tests/OBBMainOldVersion.apk
 /tests/OBBMainPatchCurrent.apk

fdroidserver/common.py

@@ -2544,8 +2544,16 @@ def verify_jar_signature(jar):
 
     """
 
-    if subprocess.call([config['jarsigner'], '-strict', '-verify', jar]) != 4:
-        raise VerificationException(_("The repository's index could not be verified."))
+    error = _('JAR signature failed to verify: {path}').format(path=jar)
+    try:
+        output = subprocess.check_output([config['jarsigner'], '-strict', '-verify', jar],
+                                         stderr=subprocess.STDOUT)
+        raise VerificationException(error + '\n' + output.decode('utf-8'))
+    except subprocess.CalledProcessError as e:
+        if e.returncode == 4:
+            logging.debug(_('JAR signature verified: {path}').format(path=jar))
+        else:
+            raise VerificationException(error + '\n' + e.output.decode('utf-8'))
 
 
 def verify_apk_signature(apk, min_sdk_version=None):
@@ -2561,14 +2569,24 @@ def verify_apk_signature(apk, min_sdk_version=None):
         args = [config['apksigner'], 'verify']
         if min_sdk_version:
             args += ['--min-sdk-version=' + min_sdk_version]
-        return subprocess.call(args + [apk]) == 0
+        if options.verbose:
+            args += ['--verbose']
+        try:
+            output = subprocess.check_output(args + [apk])
+            if options.verbose:
+                logging.debug(apk + ': ' + output.decode('utf-8'))
+            return True
+        except subprocess.CalledProcessError as e:
+            logging.error('\n' + apk + ': ' + e.output.decode('utf-8'))
     else:
-        logging.warning("Using Java's jarsigner, not recommended for verifying APKs! Use apksigner")
+        if not config.get('jarsigner_warning_displayed'):
+            config['jarsigner_warning_displayed'] = True
+            logging.warning(_("Using Java's jarsigner, not recommended for verifying APKs! Use apksigner"))
         try:
             verify_jar_signature(apk)
             return True
-        except Exception:
-            pass
+        except Exception as e:
+            logging.error(e)
     return False
 
 
@@ -2589,8 +2607,23 @@ def verify_old_apk_signature(apk):
     with open(_java_security, 'w') as fp:
         fp.write('jdk.jar.disabledAlgorithms=MD2, RSA keySize < 1024')
 
-    return subprocess.call([config['jarsigner'], '-J-Djava.security.properties=' + _java_security,
-                            '-strict', '-verify', apk]) == 4
+    try:
+        cmd = [
+            config['jarsigner'],
+            '-J-Djava.security.properties=' + _java_security,
+            '-strict', '-verify', apk
+        ]
+        output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
+    except subprocess.CalledProcessError as e:
+        if e.returncode != 4:
+            output = e.output
+        else:
+            logging.debug(_('JAR signature verified: {path}').format(path=apk))
+            return True
+
+    logging.error(_('Old APK signature failed to verify: {path}').format(path=apk)
+                  + '\n' + output.decode('utf-8'))
+    return False
 
 
 apk_badchars = re.compile('''[/ :;'"]''')

fdroidserver/index.py

@@ -691,6 +691,7 @@ def download_repo_index(url_str, etag=None, verify_fingerprint=True):
         jar = zipfile.ZipFile(fp)
 
         # verify that the JAR signature is valid
+        logging.debug(_('Verifying index signature:'))
         common.verify_jar_signature(fp.name)
 
         # get public key and its fingerprint from JAR

tests/common.TestCase

@@ -22,6 +22,7 @@ print('localmodule: ' + localmodule)
 if localmodule not in sys.path:
     sys.path.insert(0, localmodule)
 
+import fdroidserver.index
 import fdroidserver.signindex
 import fdroidserver.common
 import fdroidserver.metadata
@@ -275,12 +276,56 @@ class CommonTest(unittest.TestCase):
         config['jarsigner'] = fdroidserver.common.find_sdk_tools_cmd('jarsigner')
         fdroidserver.common.config = config
 
+        self.assertTrue(fdroidserver.common.verify_apk_signature('bad-unicode-πÇÇ现代通用字-български-عربي1.apk'))
+        self.assertFalse(fdroidserver.common.verify_apk_signature('org.bitbucket.tickytacky.mirrormirror_1.apk'))
+        self.assertFalse(fdroidserver.common.verify_apk_signature('org.bitbucket.tickytacky.mirrormirror_2.apk'))
+        self.assertFalse(fdroidserver.common.verify_apk_signature('org.bitbucket.tickytacky.mirrormirror_3.apk'))
+        self.assertFalse(fdroidserver.common.verify_apk_signature('org.bitbucket.tickytacky.mirrormirror_4.apk'))
+        self.assertTrue(fdroidserver.common.verify_apk_signature('org.dyndns.fules.ck_20.apk'))
         self.assertTrue(fdroidserver.common.verify_apk_signature('urzip.apk'))
         self.assertFalse(fdroidserver.common.verify_apk_signature('urzip-badcert.apk'))
         self.assertFalse(fdroidserver.common.verify_apk_signature('urzip-badsig.apk'))
         self.assertTrue(fdroidserver.common.verify_apk_signature('urzip-release.apk'))
         self.assertFalse(fdroidserver.common.verify_apk_signature('urzip-release-unsigned.apk'))
 
+    def test_verify_old_apk_signature(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
+
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('bad-unicode-πÇÇ现代通用字-български-عربي1.apk'))
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_1.apk'))
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_2.apk'))
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_3.apk'))
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_4.apk'))
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.dyndns.fules.ck_20.apk'))
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('urzip.apk'))
+        self.assertFalse(fdroidserver.common.verify_old_apk_signature('urzip-badcert.apk'))
+        self.assertFalse(fdroidserver.common.verify_old_apk_signature('urzip-badsig.apk'))
+        self.assertTrue(fdroidserver.common.verify_old_apk_signature('urzip-release.apk'))
+        self.assertFalse(fdroidserver.common.verify_old_apk_signature('urzip-release-unsigned.apk'))
+
+    def test_verify_jar_signature_succeeds(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
+        source_dir = os.path.join(self.basedir, 'signindex')
+        for f in ('testy.jar', 'guardianproject.jar'):
+            testfile = os.path.join(source_dir, f)
+            fdroidserver.common.verify_jar_signature(testfile)
+
+    def test_verify_jar_signature_fails(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
+        source_dir = os.path.join(self.basedir, 'signindex')
+        testfile = os.path.join(source_dir, 'unsigned.jar')
+        with self.assertRaises(fdroidserver.index.VerificationException):
+            fdroidserver.common.verify_jar_signature(testfile)
+
     def test_verify_apks(self):
         fdroidserver.common.config = None
         config = fdroidserver.common.read_config(fdroidserver.common.options)

tests/index.TestCase

@@ -45,18 +45,6 @@ class IndexTest(unittest.TestCase):
         fdroidserver.common.config = config
         fdroidserver.signindex.config = config
 
-    def test_verify_jar_signature_succeeds(self):
-        source_dir = os.path.join(self.basedir, 'signindex')
-        for f in ('testy.jar', 'guardianproject.jar'):
-            testfile = os.path.join(source_dir, f)
-            fdroidserver.common.verify_jar_signature(testfile)
-
-    def test_verify_jar_signature_fails(self):
-        source_dir = os.path.join(self.basedir, 'signindex')
-        testfile = os.path.join(source_dir, 'unsigned.jar')
-        with self.assertRaises(fdroidserver.index.VerificationException):
-            fdroidserver.common.verify_jar_signature(testfile)
-
     def test_get_public_key_from_jar_succeeds(self):
         source_dir = os.path.join(self.basedir, 'signindex')
         for f in ('testy.jar', 'guardianproject.jar'):