chiark / gitweb /
handle jarsigner/apksigner output cleanly for rational logging
[fdroidserver.git] / fdroidserver / common.py
index 6ed43d4c4e465eda49d752eb7ceea09d7eb6af5a..5d35f3c79d053b80e905e0a8113b9b2ff3c60cb9 100644 (file)
@@ -53,7 +53,8 @@ from distutils.util import strtobool
 
 import fdroidserver.metadata
 from fdroidserver import _
-from fdroidserver.exception import FDroidException, VCSException, BuildException, VerificationException
+from fdroidserver.exception import FDroidException, VCSException, NoSubmodulesException,\
+    BuildException, VerificationException
 from .asynchronousfilereader import AsynchronousFileReader
 
 
@@ -387,8 +388,14 @@ def test_aapt_version(aapt):
             minor = m.group(2)
             bugfix = m.group(3)
             # the Debian package has the version string like "v0.2-23.0.2"
-            if '.' not in bugfix and LooseVersion('.'.join((major, minor, bugfix))) < LooseVersion('0.2.2166767'):
-                logging.warning(_("'{aapt}' is too old, fdroid requires build-tools-23.0.0 or newer!")
+            too_old = False
+            if '.' in bugfix:
+                if LooseVersion(bugfix) < LooseVersion('24.0.0'):
+                    too_old = True
+            elif LooseVersion('.'.join((major, minor, bugfix))) < LooseVersion('0.2.2964546'):
+                too_old = True
+            if too_old:
+                logging.warning(_("'{aapt}' is too old, fdroid requires build-tools-24.0.0 or newer!")
                                 .format(aapt=aapt))
         else:
             logging.warning(_('Unknown version of aapt, might cause problems: ') + output)
@@ -786,7 +793,7 @@ class vcs_git(vcs):
     def clientversioncmd(self):
         return ['git', '--version']
 
-    def GitFetchFDroidPopen(self, gitargs, envs=dict(), cwd=None, output=True):
+    def git(self, args, envs=dict(), cwd=None, output=True):
         '''Prevent git fetch/clone/submodule from hanging at the username/password prompt
 
         While fetch/pull/clone respect the command line option flags,
@@ -795,10 +802,14 @@ class vcs_git(vcs):
         enough.  So we just throw the kitchen sink at it to see what
         sticks.
 
+        Also, because of CVE-2017-1000117, block all SSH URLs.
         '''
-        if cwd is None:
-            cwd = self.local
-        git_config = []
+        #
+        # supported in git >= 2.3
+        git_config = [
+            '-c', 'core.sshCommand=false',
+            '-c', 'url.https://.insteadOf=ssh://',
+        ]
         for domain in ('bitbucket.org', 'github.com', 'gitlab.com'):
             git_config.append('-c')
             git_config.append('url.https://u:p@' + domain + '/.insteadOf=git@' + domain + ':')
@@ -806,15 +817,11 @@ class vcs_git(vcs):
             git_config.append('url.https://u:p@' + domain + '.insteadOf=git://' + domain)
             git_config.append('-c')
             git_config.append('url.https://u:p@' + domain + '.insteadOf=https://' + domain)
-        # add helpful tricks supported in git >= 2.3
-        ssh_command = 'ssh -oBatchMode=yes -oStrictHostKeyChecking=yes'
-        git_config.append('-c')
-        git_config.append('core.sshCommand="' + ssh_command + '"')  # git >= 2.10
         envs.update({
             'GIT_TERMINAL_PROMPT': '0',
-            'GIT_SSH_COMMAND': ssh_command,  # git >= 2.3
+            'GIT_SSH': 'false',  # for git < 2.3
         })
-        return FDroidPopen(['git', ] + git_config + gitargs,
+        return FDroidPopen(['git', ] + git_config + args,
                            envs=envs, cwd=cwd, output=output)
 
     def checkrepo(self):
@@ -833,7 +840,7 @@ class vcs_git(vcs):
     def gotorevisionx(self, rev):
         if not os.path.exists(self.local):
             # Brand new checkout
-            p = FDroidPopen(['git', 'clone', self.remote, self.local], cwd=None)
+            p = self.git(['clone', self.remote, self.local])
             if p.returncode != 0:
                 self.clone_failed = True
                 raise VCSException("Git clone failed", p.output)
@@ -853,10 +860,10 @@ class vcs_git(vcs):
                 raise VCSException(_("Git clean failed"), p.output)
             if not self.refreshed:
                 # Get latest commits and tags from remote
-                p = self.GitFetchFDroidPopen(['fetch', 'origin'])
+                p = self.git(['fetch', 'origin'], cwd=self.local)
                 if p.returncode != 0:
                     raise VCSException(_("Git fetch failed"), p.output)
-                p = self.GitFetchFDroidPopen(['fetch', '--prune', '--tags', 'origin'], output=False)
+                p = self.git(['fetch', '--prune', '--tags', 'origin'], output=False, cwd=self.local)
                 if p.returncode != 0:
                     raise VCSException(_("Git fetch failed"), p.output)
                 # Recreate origin/HEAD as git clone would do it, in case it disappeared
@@ -885,7 +892,7 @@ class vcs_git(vcs):
         self.checkrepo()
         submfile = os.path.join(self.local, '.gitmodules')
         if not os.path.isfile(submfile):
-            raise VCSException(_("No git submodules available"))
+            raise NoSubmodulesException(_("No git submodules available"))
 
         # fix submodules not accessible without an account and public key auth
         with open(submfile, 'r') as f:
@@ -899,7 +906,7 @@ class vcs_git(vcs):
         p = FDroidPopen(['git', 'submodule', 'sync'], cwd=self.local, output=False)
         if p.returncode != 0:
             raise VCSException(_("Git submodule sync failed"), p.output)
-        p = self.GitFetchFDroidPopen(['submodule', 'update', '--init', '--force', '--recursive'])
+        p = self.git(['submodule', 'update', '--init', '--force', '--recursive'], cwd=self.local)
         if p.returncode != 0:
             raise VCSException(_("Git submodule update failed"), p.output)
 
@@ -942,10 +949,23 @@ class vcs_gitsvn(vcs):
         if not result.endswith(self.local):
             raise VCSException('Repository mismatch')
 
+    def git(self, args, envs=dict(), cwd=None, output=True):
+        '''Prevent git fetch/clone/submodule from hanging at the username/password prompt
+        '''
+        # CVE-2017-1000117 block all SSH URLs (supported in git >= 2.3)
+        config = ['-c', 'core.sshCommand=false']
+        envs.update({
+            'GIT_TERMINAL_PROMPT': '0',
+            'GIT_SSH': 'false',  # for git < 2.3
+            'SVN_SSH': 'false',
+        })
+        return FDroidPopen(['git', ] + config + args,
+                           envs=envs, cwd=cwd, output=output)
+
     def gotorevisionx(self, rev):
         if not os.path.exists(self.local):
             # Brand new checkout
-            gitsvn_args = ['git', 'svn', 'clone']
+            gitsvn_args = ['svn', 'clone']
             if ';' in self.remote:
                 remote_split = self.remote.split(';')
                 for i in remote_split[1:]:
@@ -956,13 +976,13 @@ class vcs_gitsvn(vcs):
                     elif i.startswith('branches='):
                         gitsvn_args.extend(['-b', i[9:]])
                 gitsvn_args.extend([remote_split[0], self.local])
-                p = FDroidPopen(gitsvn_args, output=False)
+                p = self.git(gitsvn_args, output=False)
                 if p.returncode != 0:
                     self.clone_failed = True
                     raise VCSException("Git svn clone failed", p.output)
             else:
                 gitsvn_args.extend([self.remote, self.local])
-                p = FDroidPopen(gitsvn_args, output=False)
+                p = self.git(gitsvn_args, output=False)
                 if p.returncode != 0:
                     self.clone_failed = True
                     raise VCSException("Git svn clone failed", p.output)
@@ -970,20 +990,20 @@ class vcs_gitsvn(vcs):
         else:
             self.checkrepo()
             # Discard any working tree changes
-            p = FDroidPopen(['git', 'reset', '--hard'], cwd=self.local, output=False)
+            p = self.git(['reset', '--hard'], cwd=self.local, output=False)
             if p.returncode != 0:
                 raise VCSException("Git reset failed", p.output)
             # Remove untracked files now, in case they're tracked in the target
             # revision (it happens!)
-            p = FDroidPopen(['git', 'clean', '-dffx'], cwd=self.local, output=False)
+            p = self.git(['clean', '-dffx'], cwd=self.local, output=False)
             if p.returncode != 0:
                 raise VCSException("Git clean failed", p.output)
             if not self.refreshed:
                 # Get new commits, branches and tags from repo
-                p = FDroidPopen(['git', 'svn', 'fetch'], cwd=self.local, output=False)
+                p = self.git(['svn', 'fetch'], cwd=self.local, output=False)
                 if p.returncode != 0:
                     raise VCSException("Git svn fetch failed")
-                p = FDroidPopen(['git', 'svn', 'rebase'], cwd=self.local, output=False)
+                p = self.git(['svn', 'rebase'], cwd=self.local, output=False)
                 if p.returncode != 0:
                     raise VCSException("Git svn rebase failed", p.output)
                 self.refreshed = True
@@ -993,7 +1013,7 @@ class vcs_gitsvn(vcs):
             nospaces_rev = rev.replace(' ', '%20')
             # Try finding a svn tag
             for treeish in ['origin/', '']:
-                p = FDroidPopen(['git', 'checkout', treeish + 'tags/' + nospaces_rev], cwd=self.local, output=False)
+                p = self.git(['checkout', treeish + 'tags/' + nospaces_rev], cwd=self.local, output=False)
                 if p.returncode == 0:
                     break
             if p.returncode != 0:
@@ -1014,7 +1034,7 @@ class vcs_gitsvn(vcs):
 
                     svn_rev = svn_rev if svn_rev[0] == 'r' else 'r' + svn_rev
 
-                    p = FDroidPopen(['git', 'svn', 'find-rev', '--before', svn_rev, treeish], cwd=self.local, output=False)
+                    p = self.git(['svn', 'find-rev', '--before', svn_rev, treeish], cwd=self.local, output=False)
                     git_rev = p.output.rstrip()
 
                     if p.returncode == 0 and git_rev:
@@ -1022,17 +1042,17 @@ class vcs_gitsvn(vcs):
 
                 if p.returncode != 0 or not git_rev:
                     # Try a plain git checkout as a last resort
-                    p = FDroidPopen(['git', 'checkout', rev], cwd=self.local, output=False)
+                    p = self.git(['checkout', rev], cwd=self.local, output=False)
                     if p.returncode != 0:
                         raise VCSException("No git treeish found and direct git checkout of '%s' failed" % rev, p.output)
                 else:
                     # Check out the git rev equivalent to the svn rev
-                    p = FDroidPopen(['git', 'checkout', git_rev], cwd=self.local, output=False)
+                    p = self.git(['checkout', git_rev], cwd=self.local, output=False)
                     if p.returncode != 0:
                         raise VCSException(_("Git checkout of '%s' failed") % rev, p.output)
 
         # Get rid of any uncontrolled files left behind
-        p = FDroidPopen(['git', 'clean', '-dffx'], cwd=self.local, output=False)
+        p = self.git(['clean', '-dffx'], cwd=self.local, output=False)
         if p.returncode != 0:
             raise VCSException(_("Git clean failed"), p.output)
 
@@ -1061,7 +1081,7 @@ class vcs_hg(vcs):
 
     def gotorevisionx(self, rev):
         if not os.path.exists(self.local):
-            p = FDroidPopen(['hg', 'clone', self.remote, self.local], output=False)
+            p = FDroidPopen(['hg', 'clone', '--ssh', 'false', self.remote, self.local], output=False)
             if p.returncode != 0:
                 self.clone_failed = True
                 raise VCSException("Hg clone failed", p.output)
@@ -1074,7 +1094,7 @@ class vcs_hg(vcs):
                     raise VCSException("Unexpected output from hg status -uS: " + line)
                 FDroidPopen(['rm', '-rf', line[2:]], cwd=self.local, output=False)
             if not self.refreshed:
-                p = FDroidPopen(['hg', 'pull'], cwd=self.local, output=False)
+                p = FDroidPopen(['hg', 'pull', '--ssh', 'false'], cwd=self.local, output=False)
                 if p.returncode != 0:
                     raise VCSException("Hg pull failed", p.output)
                 self.refreshed = True
@@ -1109,29 +1129,36 @@ class vcs_bzr(vcs):
     def clientversioncmd(self):
         return ['bzr', '--version']
 
+    def bzr(self, args, envs=dict(), cwd=None, output=True):
+        '''Prevent bzr from ever using SSH to avoid security vulns'''
+        envs.update({
+            'BZR_SSH': 'false',
+        })
+        return FDroidPopen(['bzr', ] + args, envs=envs, cwd=cwd, output=output)
+
     def gotorevisionx(self, rev):
         if not os.path.exists(self.local):
-            p = FDroidPopen(['bzr', 'branch', self.remote, self.local], output=False)
+            p = self.bzr(['branch', self.remote, self.local], output=False)
             if p.returncode != 0:
                 self.clone_failed = True
                 raise VCSException("Bzr branch failed", p.output)
         else:
-            p = FDroidPopen(['bzr', 'clean-tree', '--force', '--unknown', '--ignored'], cwd=self.local, output=False)
+            p = self.bzr(['clean-tree', '--force', '--unknown', '--ignored'], cwd=self.local, output=False)
             if p.returncode != 0:
                 raise VCSException("Bzr revert failed", p.output)
             if not self.refreshed:
-                p = FDroidPopen(['bzr', 'pull'], cwd=self.local, output=False)
+                p = self.bzr(['pull'], cwd=self.local, output=False)
                 if p.returncode != 0:
                     raise VCSException("Bzr update failed", p.output)
                 self.refreshed = True
 
         revargs = list(['-r', rev] if rev else [])
-        p = FDroidPopen(['bzr', 'revert'] + revargs, cwd=self.local, output=False)
+        p = self.bzr(['revert'] + revargs, cwd=self.local, output=False)
         if p.returncode != 0:
             raise VCSException("Bzr revert of '%s' failed" % rev, p.output)
 
     def _gettags(self):
-        p = FDroidPopen(['bzr', 'tags'], cwd=self.local, output=False)
+        p = self.bzr(['tags'], cwd=self.local, output=False)
         return [tag.split('   ')[0].strip() for tag in
                 p.output.splitlines()]
 
@@ -1918,6 +1945,22 @@ def get_apk_id_aapt(apkfile):
                           .format(apkfilename=apkfile))
 
 
+def get_minSdkVersion_aapt(apkfile):
+    """Extract the minimum supported Android SDK from an APK using aapt
+
+    :param apkfile: path to an APK file.
+    :returns: the integer representing the SDK version
+    """
+    r = re.compile(r"^sdkVersion:'([0-9]+)'")
+    p = SdkToolsPopen(['aapt', 'dump', 'badging', apkfile], output=False)
+    for line in p.output.splitlines():
+        m = r.match(line)
+        if m:
+            return int(m.group(1))
+    raise FDroidException(_('Reading minSdkVersion failed: "{apkfilename}"')
+                          .format(apkfilename=apkfile))
+
+
 class PopenResult:
     def __init__(self):
         self.returncode = None
@@ -1970,6 +2013,7 @@ def FDroidPopenBytes(commands, cwd=None, envs=None, output=True, stderr_to_stdou
         raise BuildException("OSError while trying to execute " +
                              ' '.join(commands) + ': ' + str(e))
 
+    # TODO are these AsynchronousFileReader threads always exiting?
     if not stderr_to_stdout and options.verbose:
         stderr_queue = Queue()
         stderr_reader = AsynchronousFileReader(p.stderr, stderr_queue)
@@ -2330,7 +2374,7 @@ def apk_strip_signatures(signed_apk, strip_manifest=False):
     """
     with tempfile.TemporaryDirectory() as tmpdir:
         tmp_apk = os.path.join(tmpdir, 'tmp.apk')
-        os.rename(signed_apk, tmp_apk)
+        shutil.move(signed_apk, tmp_apk)
         with ZipFile(tmp_apk, 'r') as in_apk:
             with ZipFile(signed_apk, 'w') as out_apk:
                 for info in in_apk.infolist():
@@ -2391,6 +2435,40 @@ def apk_extract_signatures(apkpath, outdir, manifest=True):
                     out_file.write(in_apk.read(f.filename))
 
 
+def sign_apk(unsigned_path, signed_path, keyalias):
+    """Sign and zipalign an unsigned APK, then save to a new file, deleting the unsigned
+
+    android-18 (4.3) finally added support for reasonable hash
+    algorithms, like SHA-256, before then, the only options were MD5
+    and SHA1 :-/ This aims to use SHA-256 when the APK does not target
+    older Android versions, and is therefore safe to do so.
+
+    https://issuetracker.google.com/issues/36956587
+    https://android-review.googlesource.com/c/platform/libcore/+/44491
+
+    """
+
+    if get_minSdkVersion_aapt(unsigned_path) < 18:
+        signature_algorithm = ['-sigalg', 'SHA1withRSA', '-digestalg', 'SHA1']
+    else:
+        signature_algorithm = ['-sigalg', 'SHA256withRSA', '-digestalg', 'SHA256']
+
+    p = FDroidPopen([config['jarsigner'], '-keystore', config['keystore'],
+                     '-storepass:env', 'FDROID_KEY_STORE_PASS',
+                     '-keypass:env', 'FDROID_KEY_PASS']
+                    + signature_algorithm + [unsigned_path, keyalias],
+                    envs={
+                        'FDROID_KEY_STORE_PASS': config['keystorepass'],
+                        'FDROID_KEY_PASS': config['keypass'], })
+    if p.returncode != 0:
+        raise BuildException(_("Failed to sign application"), p.output)
+
+    p = SdkToolsPopen(['zipalign', '-v', '4', unsigned_path, signed_path])
+    if p.returncode != 0:
+        raise BuildException(_("Failed to zipalign application"))
+    os.remove(unsigned_path)
+
+
 def verify_apks(signed_apk, unsigned_apk, tmp_dir):
     """Verify that two apks are the same
 
@@ -2466,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):
@@ -2483,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
 
 
@@ -2511,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('''[/ :;'"]''')