From f67af4e16b3adc0d2c14fa5e2fdc594975e7fe40 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 7 Nov 2017 08:19:56 +0100 Subject: [PATCH] Work around a Jsch bug: ensure the user name is set from URI JSch unconditionally overrides the user name given in the connection URI by the one found in ~/.ssh/config (if that does specify one for the used host). If the SSH config file has a different user name, we'll end up using the wrong name, which typically results in an authentication failure or in Eclipse/EGit asking for a password for the wrong user. Unfortunately there is no way to prevent or circumvent this Jsch behavior up front; it occurs already in the Session constructor at com.jcraft.jsch.Session() and the Session.applyConfig() method. And while there is a Session.setUserName() that would enable us to correct this, that latter method has package visibility only. So resort to reflection to invoke that setUserName() method to ensure that Jsch uses the user name from the URI, if there is one. Bug: 526778 Change-Id: Ia327099b5210a037380b2750a7fd76ff25c41a5a Signed-off-by: Thomas Wolf --- .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../transport/JschConfigSessionFactory.java | 34 +++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index 0b9a247791..6e8002ae2e 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -608,6 +608,7 @@ sourceIsNotAWildcard=Source is not a wildcard. sourceRefDoesntResolveToAnyObject=Source ref {0} doesn''t resolve to any object. sourceRefNotSpecifiedForRefspec=Source ref not specified for refspec: {0} squashCommitNotUpdatingHEAD=Squash commit -- not updating HEAD +sshUserNameError=Jsch error: failed to set SSH user name correctly to ''{0}''; using ''{1}'' picked up from SSH config file. sslFailureExceptionMessage=Secure connection to {0} could not be stablished because of SSL problems sslFailureInfo=A secure connection to {0}\ncould not be established because the server''s certificate could not be validated. sslFailureCause=SSL reported: {0} diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 4225dba09b..0e1a4f3ac4 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -667,6 +667,7 @@ public class JGitText extends TranslationBundle { /***/ public String sourceRefDoesntResolveToAnyObject; /***/ public String sourceRefNotSpecifiedForRefspec; /***/ public String squashCommitNotUpdatingHEAD; + /***/ public String sshUserNameError; /***/ public String sslFailureExceptionMessage; /***/ public String sslFailureInfo; /***/ public String sslFailureCause; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java index 7fe9066a3d..eadfd69b58 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/JschConfigSessionFactory.java @@ -53,14 +53,19 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.ConnectException; import java.net.UnknownHostException; +import java.text.MessageFormat; import java.util.HashMap; import java.util.Map; import org.eclipse.jgit.errors.TransportException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.util.FS; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.jcraft.jsch.JSch; import com.jcraft.jsch.JSchException; @@ -80,6 +85,10 @@ import com.jcraft.jsch.UserInfo; * to supply appropriate {@link UserInfo} to the session. */ public abstract class JschConfigSessionFactory extends SshSessionFactory { + + private static final Logger LOG = LoggerFactory + .getLogger(JschConfigSessionFactory.class); + private final Map byIdentityFile = new HashMap<>(); private JSch defaultJSch; @@ -177,6 +186,9 @@ public abstract class JschConfigSessionFactory extends SshSessionFactory { FS fs, String user, final String pass, String host, int port, final OpenSshConfig.Host hc) throws JSchException { final Session session = createSession(hc, user, host, port, fs); + // Jsch will have overridden the explicit user by the one from the SSH + // config file... + setUserName(session, user); // We retry already in getSession() method. JSch must not retry // on its own. session.setConfig("MaxAuthTries", "1"); //$NON-NLS-1$ //$NON-NLS-2$ @@ -199,6 +211,28 @@ public abstract class JschConfigSessionFactory extends SshSessionFactory { return session; } + private void setUserName(Session session, String userName) { + // Jsch 0.1.54 picks up the user name from the ssh config, even if an + // explicit user name was given! We must correct that if ~/.ssh/config + // has a different user name. + if (userName == null || userName.isEmpty() + || userName.equals(session.getUserName())) { + return; + } + try { + Class[] parameterTypes = { String.class }; + Method method = Session.class.getDeclaredMethod("setUserName", //$NON-NLS-1$ + parameterTypes); + method.setAccessible(true); + method.invoke(session, userName); + } catch (NullPointerException | IllegalAccessException + | IllegalArgumentException | InvocationTargetException + | NoSuchMethodException | SecurityException e) { + LOG.error(MessageFormat.format(JGitText.get().sshUserNameError, + userName, session.getUserName()), e); + } + } + /** * Create a new remote session for the requested address. * -- 2.39.5