Integrate SECURITY-1468 fix

README.adoc

@@ -106,6 +106,16 @@ Windows Credentials Manager works very well for interactive users on the Windows
 Windows Credentials Manager does not work as well for batch processing in the git client plugin.
 It is best to disable Windows Credentials Manager when installing Git on Jenkins agents running Windows.
 
+[#ssh-host-key-verification]
+== Ssh Host Key verification
+
+Git Client plugin provides various options to verify the SSH keys presented by Git repository host servers.
+By default, Git Client plugin uses "Accept first connection" strategy, which automatically adds keys for hosts that have not been seen before to `known_hosts`, and does not allow connections to previously-seen hosts with modified keys.
+Configure the host key verification strategy from "Manage Jenkins" >> "Configure Global Security" >> "Git Host Key Verification Configuration".
+
+image::images/ssh-host-key-verification.png[Configure Ssh host key verification]
+
+
 [#bug-reports]
 == Bug Reports
 

pom.xml

@@ -59,7 +59,7 @@
   </scm>
 
   <properties>
-    <revision>3.11.1</revision>
+    <revision>3.11.2</revision>
     <changelist>-SNAPSHOT</changelist>
     <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>

src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java

@@ -2006,6 +2006,7 @@ private String launchCommandWithCredentials(ArgumentListBuilder args, File workD
         File usernameFile = null;
         File passwordFile = null;
         File passphrase = null;
+        File knownHostsTemp = null;
         EnvVars env = environment;
         if (!PROMPT_FOR_AUTHENTICATION && isAtLeastVersion(2, 3, 0, 0)) {
             env = new EnvVars(env);
@@ -2027,11 +2028,12 @@ private String launchCommandWithCredentials(ArgumentListBuilder args, File workD
                     userName = sshUser.getUsername();
                 }
                 passphrase = createPassphraseFile(sshUser);
+                knownHostsTemp = createTempFile("known_hosts","");
                 if (launcher.isUnix()) {
-                    ssh =  createUnixGitSSH(key, userName);
+                    ssh =  createUnixGitSSH(key, userName, knownHostsTemp);
                     askpass =  createUnixSshAskpass(sshUser, passphrase);
                 } else {
-                    ssh = createWindowsGitSSH(key, userName);
+                    ssh = createWindowsGitSSH(key, userName, knownHostsTemp);
                     askpass =  createWindowsSshAskpass(sshUser, passphrase);
                 }
 
@@ -2103,6 +2105,7 @@ private String launchCommandWithCredentials(ArgumentListBuilder args, File workD
             deleteTempFile(passphrase);
             deleteTempFile(usernameFile);
             deleteTempFile(passwordFile);
+            deleteTempFile(knownHostsTemp);
         }
     }
 
@@ -2538,20 +2541,20 @@ public File getSSHExecutable() {
         throw new RuntimeException("ssh executable not found. The git plugin only supports official git client https://git-scm.com/download/win");
     }
 
-    private File createWindowsGitSSH(File key, String user) throws IOException {
+    private File createWindowsGitSSH(File key, String user, File knownHosts) throws IOException {
         File ssh = createTempFile("ssh", ".bat");
 
         File sshexe = getSSHExecutable();
 
         try (PrintWriter w = new PrintWriter(ssh, encoding)) {
             w.println("@echo off");
-            w.println("\"" + sshexe.getAbsolutePath() + "\" -i \"" + key.getAbsolutePath() +"\" -l \"" + user + "\" -o StrictHostKeyChecking=no %* ");
+            w.println("\"" + sshexe.getAbsolutePath() + "\" -i \"" + key.getAbsolutePath() +"\" -l \"" + user + "\" " + getHostKeyFactory().forCliGit(listener).getVerifyHostKeyOption(knownHosts) + " %* ");
         }
         ssh.setExecutable(true, true);
         return ssh;
     }
 
-    private File createUnixGitSSH(File key, String user) throws IOException {
+    private File createUnixGitSSH(File key, String user, File knownHosts) throws IOException {
         File ssh = createTempFile("ssh", ".sh");
         File ssh_copy = new File(ssh.toString() + "-copy");
         boolean isCopied = false;
@@ -2562,7 +2565,7 @@ private File createUnixGitSSH(File key, String user) throws IOException {
             w.println("  DISPLAY=:123.456");
             w.println("  export DISPLAY");
             w.println("fi");
-            w.println("ssh -i \"" + key.getAbsolutePath() + "\" -l \"" + user + "\" -o StrictHostKeyChecking=no \"$@\"");
+            w.println("ssh -i \"" + key.getAbsolutePath() + "\" -l \"" + user + "\" " + getHostKeyFactory().forCliGit(listener).getVerifyHostKeyOption(knownHosts) + " \"$@\"");
         }
         ssh.setExecutable(true, true);
         //JENKINS-48258 git client plugin occasionally fails with "text file busy" error

src/main/java/org/jenkinsci/plugins/gitclient/Git.java

@@ -10,12 +10,16 @@
 import org.jenkinsci.plugins.gitclient.jgit.PreemptiveAuthHttpClientConnectionFactory;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import org.jenkinsci.plugins.gitclient.verifier.HostKeyVerifierFactory;
+import org.jenkinsci.plugins.gitclient.verifier.NoHostKeyVerificationStrategy;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.Serializable;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Git repository access class. Provides local and remote access to a git
@@ -34,6 +38,9 @@
  * @author <a href="mailto:nicolas.deloof@gmail.com">Nicolas De Loof</a>
  */
 public class Git implements Serializable {
+
+    private static final Logger LOGGER = Logger.getLogger(Git.class.getName());
+
     private FilePath repository;
     private TaskListener listener;
     private EnvVars env;
@@ -122,7 +129,14 @@ public Git using(String exe) {
      * @throws java.lang.InterruptedException if interrupted.
      */
     public GitClient getClient() throws IOException, InterruptedException {
-        jenkins.MasterToSlaveFileCallable<GitClient> callable = new GitAPIMasterToSlaveFileCallable();
+        HostKeyVerifierFactory hostKeyFactory;
+        if (Jenkins.getInstanceOrNull() == null) {
+            LOGGER.log(Level.FINE, "No Jenkins instance, skipping host key checking by default");
+            hostKeyFactory = new NoHostKeyVerificationStrategy().getVerifier();
+        } else {
+            hostKeyFactory = GitHostKeyVerificationConfiguration.get().getSshHostKeyVerificationStrategy().getVerifier();
+        }
+        jenkins.MasterToSlaveFileCallable<GitClient> callable = new GitAPIMasterToSlaveFileCallable(hostKeyFactory);
         GitClient git = (repository!=null ? repository.act(callable) : callable.invoke(null,null));
         Jenkins jenkinsInstance = Jenkins.getInstanceOrNull();
         if (jenkinsInstance != null && git != null)
@@ -152,6 +166,13 @@ private GitClient initMockClient(String className, String exe, EnvVars env, File
     private static final long serialVersionUID = 1L;
 
     private class GitAPIMasterToSlaveFileCallable extends jenkins.MasterToSlaveFileCallable<GitClient> {
+
+        private final HostKeyVerifierFactory hostKeyFactory;
+
+        public GitAPIMasterToSlaveFileCallable(HostKeyVerifierFactory hostKeyFactory) {
+            this.hostKeyFactory = hostKeyFactory;
+        }
+
         public GitClient invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
             if (listener == null) listener = TaskListener.NULL;
             if (env == null) env = new EnvVars();
@@ -162,15 +183,17 @@ public GitClient invoke(File f, VirtualChannel channel) throws IOException, Inte
             }
 
             if (exe == null || JGitTool.MAGIC_EXENAME.equalsIgnoreCase(exe)) {
-                return new JGitAPIImpl(f, listener);
+                return new JGitAPIImpl(f, listener, null, hostKeyFactory);
             }
 
             if (JGitApacheTool.MAGIC_EXENAME.equalsIgnoreCase(exe)) {
                 final PreemptiveAuthHttpClientConnectionFactory factory = new PreemptiveAuthHttpClientConnectionFactory();
-                return new JGitAPIImpl(f, listener, factory);
+                return new JGitAPIImpl(f, listener, factory, hostKeyFactory);
             }
             // Ensure we return a backward compatible GitAPI, even API only claim to provide a GitClient
-            return new GitAPI(exe, f, listener, env);
+            GitAPI gitAPI = new GitAPI(exe, f, listener, env);
+            gitAPI.setHostKeyFactory(hostKeyFactory);
+            return gitAPI;
         }
     }
 }

src/main/java/org/jenkinsci/plugins/gitclient/GitHostKeyVerificationConfiguration.java

@@ -0,0 +1,35 @@
+package org.jenkinsci.plugins.gitclient;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.Extension;
+import hudson.model.PersistentDescriptor;
+import jenkins.model.GlobalConfiguration;
+import jenkins.model.GlobalConfigurationCategory;
+import org.jenkinsci.plugins.gitclient.verifier.HostKeyVerifierFactory;
+import org.jenkinsci.plugins.gitclient.verifier.AcceptFirstConnectionStrategy;
+import org.jenkinsci.plugins.gitclient.verifier.SshHostKeyVerificationStrategy;
+
+@Extension
+public class GitHostKeyVerificationConfiguration extends GlobalConfiguration implements PersistentDescriptor {
+
+    private SshHostKeyVerificationStrategy<? extends HostKeyVerifierFactory> sshHostKeyVerificationStrategy;
+
+    @Override
+    public @NonNull
+    GlobalConfigurationCategory getCategory() {
+        return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Security.class);
+    }
+
+    public SshHostKeyVerificationStrategy<? extends HostKeyVerifierFactory> getSshHostKeyVerificationStrategy() {
+        return sshHostKeyVerificationStrategy != null ? sshHostKeyVerificationStrategy : new AcceptFirstConnectionStrategy();
+    }
+
+    public void setSshHostKeyVerificationStrategy(SshHostKeyVerificationStrategy<? extends HostKeyVerifierFactory> sshHostKeyVerificationStrategy) {
+        this.sshHostKeyVerificationStrategy = sshHostKeyVerificationStrategy;
+        save();
+    }
+
+    public static @NonNull GitHostKeyVerificationConfiguration get() {
+        return GlobalConfiguration.all().getInstance(GitHostKeyVerificationConfiguration.class);
+    }
+}

src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java

@@ -123,6 +123,7 @@
 import hudson.plugins.git.GitObject;
 import org.eclipse.jgit.api.RebaseCommand.Operation;
 import org.eclipse.jgit.api.RebaseResult;
+import org.jenkinsci.plugins.gitclient.verifier.HostKeyVerifierFactory;
 
 /**
  * GitClient pure Java implementation using JGit.
@@ -148,15 +149,20 @@ public class JGitAPIImpl extends LegacyCompatibleGitAPIImpl {
         this(workspace, listener, null);
     }
 
+    @Deprecated
     JGitAPIImpl(File workspace, TaskListener listener, final PreemptiveAuthHttpClientConnectionFactory httpConnectionFactory) {
+        this(workspace, listener, httpConnectionFactory, null);
+    }
+
+    JGitAPIImpl(File workspace, TaskListener listener, final PreemptiveAuthHttpClientConnectionFactory httpConnectionFactory, HostKeyVerifierFactory hostKeyFactory) {
         /* If workspace is null, then default to current directory to match 
          * CliGitAPIImpl behavior */
-        super(workspace == null ? new File(".") : workspace);
+        super(workspace == null ? new File(".") : workspace, hostKeyFactory);
         this.listener = listener;
 
         // to avoid rogue plugins from clobbering what we use, always
         // make a point of overwriting it with ours.
-        SshSessionFactory.setInstance(new TrileadSessionFactory());
+        SshSessionFactory.setInstance(new TrileadSessionFactory(hostKeyFactory, listener));
 
         if (httpConnectionFactory != null) {
             httpConnectionFactory.setCredentialsProvider(asSmartCredentialsProvider());

src/main/java/org/jenkinsci/plugins/gitclient/LegacyCompatibleGitAPIImpl.java

@@ -25,6 +25,7 @@
 import java.util.Map;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import org.jenkinsci.plugins.gitclient.verifier.HostKeyVerifierFactory;
 
 /**
  * Partial implementation of {@link IGitAPI} by delegating to {@link GitClient} APIs.
@@ -37,6 +38,8 @@
  */
 abstract class LegacyCompatibleGitAPIImpl extends AbstractGitAPIImpl implements IGitAPI {
 
+    private HostKeyVerifierFactory hostKeyFactory;
+
     /**
      * isBareRepository.
      *
@@ -56,10 +59,24 @@ public boolean isBareRepository() throws GitException, InterruptedException {
      *
      * @param workspace a {@link java.io.File} object.
      */
+    @Deprecated
     protected LegacyCompatibleGitAPIImpl(File workspace) {
         this.workspace = workspace;
     }
 
+    protected LegacyCompatibleGitAPIImpl(File workspace, HostKeyVerifierFactory hostKeyFactory) {
+        this.workspace = workspace;
+        this.hostKeyFactory = hostKeyFactory;
+    }
+
+    public HostKeyVerifierFactory getHostKeyFactory() {
+        return hostKeyFactory;
+    }
+
+    public void setHostKeyFactory(HostKeyVerifierFactory verifier) {
+        this.hostKeyFactory = verifier;
+    }
+
     /** {@inheritDoc} */
     @Deprecated
     public boolean hasGitModules(String treeIsh) throws GitException {

src/main/java/org/jenkinsci/plugins/gitclient/trilead/JGitConnection.java

@@ -0,0 +1,26 @@
+package org.jenkinsci.plugins.gitclient.trilead;
+
+import com.trilead.ssh2.Connection;
+import com.trilead.ssh2.ConnectionInfo;
+import com.trilead.ssh2.ServerHostKeyVerifier;
+import org.jenkinsci.plugins.gitclient.verifier.AbstractJGitHostKeyVerifier;
+
+import java.io.IOException;
+
+public class JGitConnection extends Connection {
+
+    public JGitConnection(String hostname, int port) {
+        super(hostname, port);
+    }
+
+    @Override
+    public ConnectionInfo connect(ServerHostKeyVerifier verifier) throws IOException {
+        if (verifier instanceof AbstractJGitHostKeyVerifier) {
+            String[] serverHostKeyAlgorithms = ((AbstractJGitHostKeyVerifier) verifier).getServerHostKeyAlgorithms(this);
+            if (serverHostKeyAlgorithms != null && serverHostKeyAlgorithms.length > 0) {
+                setServerHostKeyAlgorithms(serverHostKeyAlgorithms);
+            }
+        }
+        return super.connect(verifier);
+    }
+}

src/main/java/org/jenkinsci/plugins/gitclient/trilead/TrileadSessionFactory.java

@@ -2,31 +2,60 @@
 
 import com.cloudbees.jenkins.plugins.sshcredentials.SSHAuthenticator;
 import com.trilead.ssh2.Connection;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import hudson.model.TaskListener;
 import org.eclipse.jgit.errors.TransportException;
 import org.eclipse.jgit.errors.UnsupportedCredentialItem;
 import org.eclipse.jgit.transport.CredentialsProvider;
 import org.eclipse.jgit.transport.RemoteSession;
 import org.eclipse.jgit.transport.SshSessionFactory;
 import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.util.FS;
+import org.jenkinsci.plugins.gitclient.verifier.AcceptFirstConnectionVerifier;
+import org.jenkinsci.plugins.gitclient.verifier.HostKeyVerifierFactory;
 
 import java.io.IOException;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * Makes JGit uses Trilead for connectivity.
  *
  * @author Kohsuke Kawaguchi
  */
 public class TrileadSessionFactory extends SshSessionFactory {
+
+    private static final ReentrantLock JGIT_ACCEPT_FIRST_LOCK = new ReentrantLock();
+
+    private final HostKeyVerifierFactory hostKeyVerifierFactory;
+    private final TaskListener listener;
+
+    @SuppressFBWarnings(value = "EI_EXPOSE_REP2")
+    public TrileadSessionFactory(HostKeyVerifierFactory hostKeyVerifierFactory, TaskListener listener) {
+        this.hostKeyVerifierFactory = hostKeyVerifierFactory;
+        this.listener = listener;
+    }
+
     /** {@inheritDoc} */
     @Override
     public RemoteSession getSession(URIish uri, CredentialsProvider credentialsProvider, FS fs, int tms) throws TransportException {
         try {
             int p = uri.getPort();
             if (p<0)    p = 22;
-            Connection con = new Connection(uri.getHost(), p);
+            JGitConnection con = new JGitConnection(uri.getHost(), p);
             con.setTCPNoDelay(true);
-            con.connect();  // TODO: host key check
+            if (hostKeyVerifierFactory instanceof AcceptFirstConnectionVerifier) {
+                // Accept First connection behavior need to be synchronized, because it's the only verifier
+                // which could change (populate) known hosts dynamically, in other words AcceptFirstConnectionVerifier
+                // should be able to see and read if any known hosts was added during parallel connection.
+                JGIT_ACCEPT_FIRST_LOCK.lock();
+                try {
+                    con.connect(hostKeyVerifierFactory.forJGit(listener));
+                } finally {
+                    JGIT_ACCEPT_FIRST_LOCK.unlock();
+                }
+            } else {
+                con.connect(hostKeyVerifierFactory.forJGit(listener));
+            }
 
             boolean authenticated;
             if (credentialsProvider instanceof SmartCredentialsProvider) {

src/main/java/org/jenkinsci/plugins/gitclient/verifier/AbstractCliGitHostKeyVerifier.java

@@ -0,0 +1,18 @@
+package org.jenkinsci.plugins.gitclient.verifier;
+
+import org.jenkinsci.remoting.SerializableOnlyOverRemoting;
+
+import java.io.File;
+import java.io.IOException;
+
+public interface AbstractCliGitHostKeyVerifier extends SerializableOnlyOverRemoting {
+
+    /**
+     * Specifies Git command-line options that control the logic of this verifier.
+     * @param tempKnownHosts a temporary file that has already been created and may be used.
+     * @return the command-line options
+     * @throws IOException
+     */
+    String getVerifyHostKeyOption(File tempKnownHosts) throws IOException;
+
+}