Merge branch 'fix_new_jdk' into 'master'

Skip jarsigner test due to weak signatures

See merge request fdroid/fdroidserver!1239

MANIFEST.in

@@ -558,9 +558,6 @@ include tests/get_android_tools_versions/android-sdk/patcher/v4/source.propertie
 include tests/get_android_tools_versions/android-sdk/platforms/android-30/source.properties
 include tests/get_android_tools_versions/android-sdk/skiaparser/1/source.properties
 include tests/get_android_tools_versions/android-sdk/tools/source.properties
-include tests/getsig/getsig.java
-include tests/getsig/make.sh
-include tests/getsig/run.sh
 include tests/gnupghome/pubring.gpg
 include tests/gnupghome/random_seed
 include tests/gnupghome/secring.gpg

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('''[/ :;'"]''')
 
 
@@ -3748,11 +3727,11 @@ def load_stats_fdroid_signing_key_fingerprints():
     jar_file = os.path.join('stats', 'publishsigkeys.jar')
     if not os.path.isfile(jar_file):
         return {}
-    cmd = [config['jarsigner'], '-strict', '-verify', jar_file]
+    try:
-    p = FDroidPopen(cmd, output=False)
+        verify_deprecated_jar_signature(jar_file)
-    if p.returncode != 4:
+    except VerificationException as e:
         raise FDroidException("Signature validation of '{}' failed! "
-                              "Please run publish again to rebuild this file.".format(jar_file))
+                              "Please run publish again to rebuild this file.".format(jar_file)) from e
 
     jar_sigkey = apk_signer_fingerprint(jar_file)
     repo_key_sig = config.get('repo_key_sha256')

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/init.py

@@ -264,7 +264,7 @@ def main():
         c = dict(test_config)
         c['keystorepass'] = password
         c['keypass'] = password
-        c['repo_keyalias'] = socket.getfqdn()
+        c['repo_keyalias'] = repo_keyalias or socket.getfqdn()
         c['keydname'] = 'CN=' + c['repo_keyalias'] + ', OU=F-Droid'
         common.write_to_config(test_config, 'keystorepass', password)
         common.write_to_config(test_config, 'keypass', password)

fdroidserver/signindex.py

@@ -110,8 +110,8 @@ def sign_jar(jar, use_old_algs=False):
         'FDROID_KEY_PASS': config.get('keypass', ""),
     }
     p = common.FDroidPopen(args, envs=env_vars)
-    if p.returncode != 0:
+    if not use_old_algs and p.returncode != 0:
-        # workaround for buster-backports apksigner on f-droid.org publish server
+        # workaround for apksigner v30 on f-droid.org publish server
         v4 = args.index("--v4-signing-enabled")
         del args[v4 + 1]
         del args[v4]

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/getsig/getsig.java

@@ -1,105 +0,0 @@
-import java.io.IOException;
-import java.io.InputStream;
-import java.math.BigInteger;
-import java.security.Signature;
-import java.security.cert.*;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
-import java.util.Enumeration;
-import java.util.jar.JarEntry;
-import java.util.jar.JarFile;
-
-public class getsig {
-
-    public static void main(String[] args) {
-
-        String apkPath = null;
-        boolean full = false;
-
-        if(args.length == 1) {
-            apkPath = args[0];
-        } else if (args.length == 2) {
-            if(!args[0].equals("-f")) {
-                System.out.println("Only -f is supported");
-                System.exit(1);
-            }
-            apkPath = args[1];
-            full = true;
-        } else {
-            System.out.println("Specify the APK file to get the signature from!");
-            System.exit(1);
-        }
-
-        try {
-
-            JarFile apk = new JarFile(apkPath);
-            java.security.cert.Certificate[] certs = null;
-
-            Enumeration entries = apk.entries();
-            while (entries.hasMoreElements()) {
-                JarEntry je = (JarEntry) entries.nextElement();
-                if (!je.isDirectory() && !je.getName().startsWith("META-INF/")) {
-                    // Just need to read the stream (discarding the data) to get
-                    // it to process the certificate...
-                    byte[] b = new byte[4096];
-                    InputStream is = apk.getInputStream(je);
-                    while (is.read(b, 0, b.length) != -1);
-                    is.close();
-                    certs = je.getCertificates();
-                    if(certs != null)
-                        break;
-                }
-            }
-            apk.close();
-
-            if (certs == null) {
-                System.out.println("Not signed");
-                System.exit(1);
-            }
-            if (certs.length != 1) {
-                System.out.println("One signature expected");
-                System.exit(1);
-            }
-
-            // Get the signature in the same form that is returned by
-            // android.content.pm.Signature.toCharsString() (but in the
-            // form of a byte array so we can pass it to the MD5 function)...
-            byte[] sig = certs[0].getEncoded();
-            byte[] csig = new byte[sig.length * 2];
-            for (int j=0; j<sig.length; j++) {
-                byte v = sig[j];
-                int d = (v>>4)&0xf;
-                csig[j*2] = (byte)(d >= 10 ? ('a' + d - 10) : ('0' + d));
-                d = v&0xf;
-                csig[j*2+1] = (byte)(d >= 10 ? ('a' + d - 10) : ('0' + d));
-            }
-
-            String result;
-            if(full) {
-                result = new String(csig);
-            } else {
-                // Get the MD5 sum...
-                MessageDigest md;
-                md = MessageDigest.getInstance("MD5");
-                byte[] md5sum = new byte[32];
-                md.update(csig);
-                md5sum = md.digest();
-                BigInteger bigInt = new BigInteger(1, md5sum);
-                String md5hash = bigInt.toString(16);
-                while (md5hash.length() < 32)
-                    md5hash = "0" + md5hash;
-                result = md5hash;
-            }
-
-            System.out.println("Result:" + result);
-            System.exit(0);
-
-        } catch (Exception e) {
-            System.out.println("Exception:" + e);
-            System.exit(1);
-        }
-    }
-
-}
-
-

tests/getsig/make.sh

@@ -1,2 +0,0 @@
-#!/bin/sh
-javac getsig.java

tests/getsig/run.sh

@@ -1,2 +0,0 @@
-#!/bin/sh
-java getsig $1 $2 $3

tests/run-tests

@@ -151,10 +151,7 @@ test -x ./hooks/pre-commit && ./hooks/pre-commit
 
 
 #------------------------------------------------------------------------------#
-echo_header "test python getsig replacement"
+echo_header "run unit tests"
-
-cd $WORKSPACE/tests/getsig
-./make.sh
 
 cd $WORKSPACE/tests
 for testcase in $WORKSPACE/tests/*.TestCase; do

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

tests/update.TestCase

@@ -20,7 +20,6 @@ import unittest
 import yaml
 import zipfile
 import textwrap
-from binascii import unhexlify
 from datetime import datetime
 from distutils.version import LooseVersion
 from testcommon import TmpCwd
@@ -53,7 +52,6 @@ import fdroidserver.common
 import fdroidserver.exception
 import fdroidserver.metadata
 import fdroidserver.update
-from fdroidserver.common import FDroidPopen
 
 
 DONATION_FIELDS = ('Donate', 'Liberapay', 'OpenCollective')
@@ -550,48 +548,6 @@ class UpdateTest(unittest.TestCase):
         self.assertEqual(app['localized']['en-US']['name'], 'Goguma')
         self.assertEqual(app['localized']['en-US']['summary'], 'An IRC client for mobile devices')
 
-    def javagetsig(self, apkfile):
-        getsig_dir = 'getsig'
-        if not os.path.exists(os.path.join(getsig_dir, "getsig.class")):
-            logging.critical("getsig.class not found. To fix: cd '%s' && ./make.sh" % getsig_dir)
-            sys.exit(1)
-        # FDroidPopen needs some config to work
-        config = dict()
-        fdroidserver.common.fill_config_defaults(config)
-        fdroidserver.common.config = config
-        p = FDroidPopen(['java', '-cp', 'getsig', 'getsig', apkfile])
-        sig = None
-        for line in p.output.splitlines():
-            if line.startswith('Result:'):
-                sig = line[7:].strip()
-                break
-        if p.returncode == 0:
-            return sig
-        else:
-            return None
-
-    def testGoodGetsig(self):
-        # config needed to use jarsigner and keytool
-        config = dict()
-        fdroidserver.common.fill_config_defaults(config)
-        fdroidserver.common.options = Options
-        fdroidserver.update.config = config
-        apkfile = 'urzip.apk'
-        sig = self.javagetsig(apkfile)
-        self.assertIsNotNone(sig, "sig is None")
-        pysig = fdroidserver.update.getsig(apkfile)
-        self.assertIsNotNone(pysig, "pysig is None")
-        self.assertEqual(sig, fdroidserver.update.getsig(apkfile),
-                         "python sig not equal to java sig!")
-        self.assertEqual(len(sig), len(pysig),
-                         "the length of the two sigs are different!")
-        try:
-            self.assertEqual(unhexlify(sig), unhexlify(pysig),
-                             "the length of the two sigs are different!")
-        except TypeError as e:
-            print(e)
-            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