eclipse-jgit / jgit

JGit, the Java implementation of git
https://www.eclipse.org/jgit/
Other
92 stars 31 forks source link

SSH-Agent-Proxy can't be used for users with .ssh/config IdentityFile entry #23

Closed realdadfish closed 5 months ago

realdadfish commented 5 months ago

Version

6.8.0.202311291450-r

Operating System

MacOS

Bug description

If JGit is configured to use a custom JschConfigSessionFactory to connect to a locally running ssh-agent instance via proxy like this:

val factory = object : JschConfigSessionFactory() {
    @Throws(JSchException::class)
    override fun createDefaultJSch(fs: FS): JSch {
        val jsch: JSch = super.createDefaultJSch(fs)
        if (SSHAgentConnector.isConnectorAvailable()) {
            val factory = ConnectorFactory.getDefault()
            val identityRepository = RemoteIdentityRepository(factory.createConnector())
            jsch.identityRepository = identityRepository
        }
        return jsch
    }
}
SshSessionFactory.setInstance(factory)

then this RemoteIdentityRepository will not be used if the user who is running a program with JGit has a local .ssh/config file that contains one or more references to IdentityFiles:

IdentityFile ~/.ssh/id_first
IdentityFile ~/.ssh/id_second

The reason for that misbehavior is the following code in org.eclipse.jgit.transport.ssh.jsch.JschConfigSessionFactory with inline comments what's going on:

protected JSch getJSch(OpenSshConfig.Host hc, FS fs) throws JSchException {
    if (defaultJSch == null) {
        defaultJSch = createDefaultJSch(fs);
        if (defaultJSch.getConfigRepository() == null) {
            defaultJSch.setConfigRepository(
                    new JschBugFixingConfigRepository(config));
        }
        for (Object name : defaultJSch.getIdentityNames())
            byIdentityFile.put((String) name, defaultJSch);
                    // ^^^ the identities coming from the agent are identified by their name (comment), e.g. "my-first-identity"
    }

    final File identityFile = hc.getIdentityFile();
             // ^^^ here the first identity file from .ssh/config is returned, i.e. /absolute/path/to/.ssh/id_first
    if (identityFile == null)
        return defaultJSch;
                     // ^^^ no identity file, no problem, our JSch instance with the remote identity repository is used

    final String identityKey = identityFile.getAbsolutePath();
    JSch jsch = byIdentityFile.get(identityKey);
             // ^^^ now that can never succeed, because the comment of the identity is of course unequal to the path of it's file
    if (jsch == null) {
        jsch = new JSch();
        configureJSch(jsch);
        if (jsch.getConfigRepository() == null) {
            jsch.setConfigRepository(defaultJSch.getConfigRepository());
        }
        jsch.setHostKeyRepository(defaultJSch.getHostKeyRepository());
        jsch.addIdentity(identityKey);
        byIdentityFile.put(identityKey, jsch);
    }
    return jsch;
}

Actual behavior

The overridden JSch instance is thrown away.

Expected behavior

The overriden JSch instance should take precedence.

Relevant log output

No response

Other information

No response

realdadfish commented 5 months ago

Hrm... I just figured that getJSch is protected, so I could overwrite the complete implementation there for my use case. Oh well. Guess this can be closed then.

msohn commented 5 months ago

Please note that jsch is pretty much unmaintained, hence better use the Apache mina-sshd client. At some point in time we will remove support for jsch.

realdadfish commented 3 months ago

Since it was not totally obvious for me, it all works fine out-of-the box with the said Apache MINA SSHD-Client via service loading when the artifacts org.eclipse.jgit:org.eclipse.jgit.ssh.apache and org.eclipse.jgit:org.eclipse.jgit.ssh.apache.agent are in the runtime classpath. Thanks again!

realdadfish commented 3 months ago

And one more follow-up, to disable SSH Host Key verification (because in the CI Setup I'm using I can't make the default implementation recognize my ~/.ssh/known_hosts file) the following code was needed:

SshSessionFactory.setInstance(object : SshdSessionFactory() {
    override fun createServerKeyDatabase(homeDir: File?, sshDir: File?): ServerKeyDatabase {
        return object : ServerKeyDatabase {
            override fun lookup(
                connectAddress: String?,
                remoteAddress: InetSocketAddress?,
                config: ServerKeyDatabase.Configuration?
            ): MutableList<PublicKey> = mutableListOf()

            // accept any checked key
            override fun accept(
                connectAddress: String?,
                remoteAddress: InetSocketAddress?,
                serverKey: PublicKey?,
                config: ServerKeyDatabase.Configuration?,
                provider: CredentialsProvider?
            ): Boolean = true
        }
    }
})