jarsigner: allow weak signatures

openjdk-11 11.0.17 in Debian unstable fails to verify weak signatures:

jarsigner -verbose -strict -verify tests/signindex/guardianproject.jar

         131 Fri Dec 02 20:10:00 CET 2016 META-INF/MANIFEST.MF
         252 Fri Dec 02 20:10:04 CET 2016 META-INF/1.SF
        2299 Fri Dec 02 20:10:04 CET 2016 META-INF/1.RSA
           0 Fri Dec 02 20:09:58 CET 2016 META-INF/
 m  ?  48743 Fri Dec 02 20:09:58 CET 2016 index.xml

  s = signature was verified
  m = entry is listed in manifest
  k = at least one certificate was found in keystore
  ? = unsigned entry

- Signed by "EMAILADDRESS=root@guardianproject.info, CN=guardianproject.info, O=Guardian Project, OU=FDroid Repo, L=New York, ST=New York, C=US"
    Digest algorithm: SHA1 (disabled)
    Signature algorithm: SHA1withRSA (disabled), 4096-bit key

WARNING: The jar will be treated as unsigned, because it is signed with a weak algorithm that is now disabled by the security property:

  jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024, SHA1 denyAfter 2019-01-01, include jdk.disabled.namedCurves

fdroidserver/common.py

@@ -3416,13 +3416,27 @@ def verify_apks(signed_apk, unsigned_apk, tmp_dir, v1_only=None):
     return None
 
 
-def verify_jar_signature(jar):
+def verify_deprecated_jar_signature(jar):
     """Verify 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.
 
+    Also used to verify the signature on an archived APK, supporting deprecated
+    algorithms.
+
+    F-Droid aims to keep every single binary that it ever published.  Therefore,
+    it needs to be able to verify APK signatures that include deprecated/removed
+    algorithms.  For example, jarsigner treats an MD5 signature as unsigned.
+
+    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.
+
     Raises
     ------
     VerificationException
@@ -3430,15 +3444,30 @@ def verify_jar_signature(jar):
 
     """
     error = _('JAR signature failed to verify: {path}').format(path=jar)
+    _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:
-        output = subprocess.check_output([config['jarsigner'], '-strict', '-verify', jar],
+        cmd = [
-                                         stderr=subprocess.STDOUT)
+            config['jarsigner'],
+            '-J-Djava.security.properties=' + _java_security,
+            '-strict', '-verify', jar
+        ]
+        output = subprocess.check_output(cmd, 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')) from e
+    finally:
+        if os.path.exists(_java_security):
+            os.chmod(_java_security, 0o600)
+            os.remove(_java_security)
 
 
 def verify_apk_signature(apk, min_sdk_version=None):
@@ -3471,63 +3500,13 @@ def verify_apk_signature(apk, min_sdk_version=None):
             config['jarsigner_warning_displayed'] = True
             logging.warning(_("Using Java's jarsigner, not recommended for verifying APKs! Use apksigner"))
         try:
-            verify_jar_signature(apk)
+            verify_deprecated_jar_signature(apk)
             return True
         except Exception as e:
             logging.error(e)
     return False
 
 
-def verify_old_apk_signature(apk):
-    """Verify the signature on an archived APK, supporting deprecated algorithms.
-
-    F-Droid aims to keep every single binary that it ever published.  Therefore,
-    it needs to be able to verify APK signatures that include deprecated/removed
-    algorithms.  For example, jarsigner treats an MD5 signature as unsigned.
-
-    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 = [
-            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
-    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'))
-    return False
-
-
 apk_badchars = re.compile('''[/ :;'"]''')
 
 
@@ -3749,7 +3728,7 @@ def load_stats_fdroid_signing_key_fingerprints():
     if not os.path.isfile(jar_file):
         return {}
     try:
-        verify_jar_signature(jar_file)
+        verify_deprecated_jar_signature(jar_file)
     except VerificationException as e:
         raise FDroidException("Signature validation of '{}' failed! "
                               "Please run publish again to rebuild this file.".format(jar_file)) from e

fdroidserver/index.py

@@ -1518,7 +1518,7 @@ def get_index_from_jar(jarfile, fingerprint=None):
 
     """
     logging.debug(_('Verifying index signature:'))
-    common.verify_jar_signature(jarfile)
+    common.verify_deprecated_jar_signature(jarfile)
     with zipfile.ZipFile(jarfile) as jar:
         public_key, public_key_fingerprint = get_public_key_from_jar(jar)
         if fingerprint is not None:

fdroidserver/update.py

@@ -50,7 +50,7 @@ from . import _
 from . import common
 from . import index
 from . import metadata
-from .exception import BuildException, FDroidException
+from .exception import BuildException, FDroidException, VerificationException
 
 from PIL import Image, PngImagePlugin
 
@@ -1532,9 +1532,10 @@ def process_apk(apkcache, apkfilename, repodir, knownapks, use_date_from_apk=Fal
         skipapk = False
         if not common.verify_apk_signature(apkfile):
             if repodir == 'archive' or allow_disabled_algorithms:
-                if common.verify_old_apk_signature(apkfile):
+                try:
+                    common.verify_deprecated_jar_signature(apkfile)
                     apk['antiFeatures'].update(['KnownVuln', 'DisabledAlgorithm'])
-                else:
+                except VerificationException:
                     skipapk = True
             else:
                 skipapk = True

tests/common.TestCase

@@ -39,7 +39,8 @@ import fdroidserver.signindex
 import fdroidserver.common
 import fdroidserver.metadata
 from testcommon import TmpCwd
-from fdroidserver.exception import FDroidException, VCSException, MetaDataException
+from fdroidserver.exception import FDroidException, VCSException,\
+    MetaDataException, VerificationException
 
 
 class CommonTest(unittest.TestCase):
@@ -484,34 +485,33 @@ class CommonTest(unittest.TestCase):
         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'))
+        try:
-        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_1.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('bad-unicode-πÇÇ现代通用字-български-عربي1.apk')
-        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_2.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('org.bitbucket.tickytacky.mirrormirror_1.apk')
-        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_3.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('org.bitbucket.tickytacky.mirrormirror_2.apk')
-        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.bitbucket.tickytacky.mirrormirror_4.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('org.bitbucket.tickytacky.mirrormirror_3.apk')
-        self.assertTrue(fdroidserver.common.verify_old_apk_signature('org.dyndns.fules.ck_20.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('org.bitbucket.tickytacky.mirrormirror_4.apk')
-        self.assertTrue(fdroidserver.common.verify_old_apk_signature('urzip.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('org.dyndns.fules.ck_20.apk')
-        self.assertFalse(fdroidserver.common.verify_old_apk_signature('urzip-badcert.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('urzip.apk')
-        self.assertFalse(fdroidserver.common.verify_old_apk_signature('urzip-badsig.apk'))
+            fdroidserver.common.verify_deprecated_jar_signature('urzip-release.apk')
-        self.assertTrue(fdroidserver.common.verify_old_apk_signature('urzip-release.apk'))
+        except VerificationException:
-        self.assertFalse(fdroidserver.common.verify_old_apk_signature('urzip-release-unsigned.apk'))
+            self.fail("failed to jarsigner failed to verify an old apk")
+        self.assertRaises(VerificationException, fdroidserver.common.verify_deprecated_jar_signature, 'urzip-badcert.apk')
+        self.assertRaises(VerificationException, fdroidserver.common.verify_deprecated_jar_signature, 'urzip-badsig.apk')
+        self.assertRaises(VerificationException, fdroidserver.common.verify_deprecated_jar_signature, 'urzip-release-unsigned.apk')
 
-    def test_verify_jar_signature_succeeds(self):
+    def test_verify_deprecated_jar_signature(self):
-        config = fdroidserver.common.read_config(fdroidserver.common.options)
-        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):
         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_deprecated_jar_signature(testfile)
+
         testfile = os.path.join(source_dir, 'unsigned.jar')
         with self.assertRaises(fdroidserver.index.VerificationException):
-            fdroidserver.common.verify_jar_signature(testfile)
+            fdroidserver.common.verify_deprecated_jar_signature(testfile)
 
     def test_verify_apks(self):
         config = fdroidserver.common.read_config(fdroidserver.common.options)

tests/signindex.TestCase

@@ -103,7 +103,7 @@ class SignindexTest(unittest.TestCase):
 
         # index.jar aka v0 must by signed by SHA1withRSA
         f = 'repo/index.jar'
-        common.verify_jar_signature(f)
+        common.verify_deprecated_jar_signature(f)
         self.assertIsNone(apksigcopier.extract_v2_sig(f, expected=False))
         cp = subprocess.run(
             ['jarsigner', '-verify', '-verbose', f], stdout=subprocess.PIPE
@@ -112,7 +112,7 @@ class SignindexTest(unittest.TestCase):
 
         # index-v1.jar must by signed by SHA1withRSA
         f = 'repo/index-v1.jar'
-        common.verify_jar_signature(f)
+        common.verify_deprecated_jar_signature(f)
         self.assertIsNone(apksigcopier.extract_v2_sig(f, expected=False))
         cp = subprocess.run(
             ['jarsigner', '-verify', '-verbose', f], stdout=subprocess.PIPE
@@ -121,7 +121,7 @@ class SignindexTest(unittest.TestCase):
 
         # entry.jar aka index v2 must by signed by a modern algorithm
         f = 'repo/entry.jar'
-        common.verify_jar_signature(f)
+        common.verify_deprecated_jar_signature(f)
         self.assertIsNone(apksigcopier.extract_v2_sig(f, expected=False))
         cp = subprocess.run(
             ['jarsigner', '-verify', '-verbose', f], stdout=subprocess.PIPE