apache / mina-sshd

Apache MINA sshd is a comprehensive Java library for client- and server-side SSH.
https://mina.apache.org/sshd-project/
Apache License 2.0
880 stars 356 forks source link

GitSshdSessionFactory - why do I need a credentialsProvider if I've configured the SshSessionFactory? #532

Open devvthedev opened 2 months ago

devvthedev commented 2 months ago

Hello,

I'm using the following JGit and Apache MINA dependencies in my project:

implementation "org.eclipse.jgit:org.eclipse.jgit:6.10.0.202406032230-r"
implementation "org.eclipse.jgit:org.eclipse.jgit.ssh.apache:6.10.0.202406032230-r"
implementation "org.apache.sshd:sshd-git:2.13.1"

I've created a default org.apache.sshd.client.SshClient with: SshClient sshClient = SshClient.setUpDefaultClient();

and then set an instance of the GitSshdSessionFactory on JGits SshSessionFactory: SshSessionFactory.setInstance(new GitSshdSessionFactory(sshClient));


By default the sshClient is reading the SSH keys I have stored in the ~/.ssh folder where the config file has the following configuration:

Host github.com
  IdentityFile ~/.ssh/my-ssh-key

I am using a passphrase-less private SSH key.



The question I have is around JGits Git push command call when talking back to the remote repository.

For example,

  1. Given I have cloned a repository on my machine with SSH in a terminal:

git clone git@github.com:devvthedev/my-project.git

  1. Made some changes to the repo

  2. Opened the repo using JGit with Git.open(...) and get a Git object back

  3. Staged the changes I made and committed them:

git.add()
    .addFilepattern(".")
    .call();
git.commit()
    .setCommitter(new PersonIdent("me", "me@example.com"))
    .setMessage("message")
    .call();
  1. When I try to push those changes with JGit:
git.push()
    .call();

I get the following error:

Caused by: org.eclipse.jgit.errors.TransportException: Unable to connect
        at org.apache.sshd.git.transport.GitSshdSessionFactory.getSession(GitSshdSessionFactory.java:134)
        at org.eclipse.jgit.transport.SshTransport.getSession(SshTransport.java:107)
        at org.eclipse.jgit.transport.TransportGitSsh$SshPushConnection.<init>(TransportGitSsh.java:356)
        at org.eclipse.jgit.transport.TransportGitSsh.openPush(TransportGitSsh.java:157)
        at org.eclipse.jgit.transport.PushProcess.execute(PushProcess.java:140)
        at org.eclipse.jgit.transport.Transport.push(Transport.java:1555)
        at org.eclipse.jgit.api.PushCommand.call(PushCommand.java:158)
        ... 2 more
Caused by: java.lang.NullPointerException: Cannot invoke "org.eclipse.jgit.transport.CredentialsProvider.isInteractive()" because "credentialsProvider" is null
        at org.apache.sshd.git.transport.GitSshdSession.<init>(GitSshdSession.java:55)
        at org.apache.sshd.git.transport.GitSshdSessionFactory$1.<init>(GitSshdSessionFactory.java:90)
        at org.apache.sshd.git.transport.GitSshdSessionFactory.getSession(GitSshdSessionFactory.java:90)
        ... 8 more


If I provide a CredentialsProvider with the git user and no password:

git.push()
    .setCredentialsProvider(new UsernamePasswordCredentialsProvider("git", ""))
    .call();

It works!

So my question is - why is a CredentialsProvider required given I have configured a SshSessionFactory instance?

I've also raised this to JGit maintainers here

Thank you

tomaswolf commented 2 months ago

I don't know why the GitSshdSession asks for a password up front. It should set appropriate FilePasswordProvider and UserInteraction implementations based on the JGit CredentialsProvider instead (and they should be null-safe).

Note that the JGit CredentialsProvider is not just for getting credentials. It is a general interface for user interaction similar to javax.security.auth.callback.CallbackHandler. In JGit it is used for instance also to prompt the user what to do when an SSH server's host key is not known yet. (If CredentialsProvider.isInteractive() == true.) So it might be a good idea anyway to provide a general CredentialsProvider implementation. (Though I don't see what this GitSshdSession does with it further on -- apparently nothing.)

As I mentioned in the JGit issue, another idea might be to use JGit's own binding to Apache MINA sshd.

devvthedev commented 3 weeks ago

Hi @tomaswolf

I've moved to using JGit's own binding to Apache MINA sshd as suggested but have some questions on using the SshdSessionFactoryBuilder.

I'm building a SshdSessionFactory where I supply a key pair programmatically (.setDefaultKeysProvider). However, the factory builder requires me to set both HomeDirectory and SshDirectory despite the Javadoc stating:

may be null, in which case the home directory as defined by FS.userHome() is assumed

If I pass null or omit these setters from the builder, I get a null pointer exception for both directories:

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.io.File.isAbsolute()" because "homeDir" is null

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.io.File.isAbsolute()" because "sshDir" is null


Since I am providing the key pair programatically, I don't want to use the ssh directory on disk.

This has led to me having to specify a directory for these:

.setHomeDirectory(directory) // Need to set despite Javadoc
.setSshDirectory(directory) // Need to set despite Javadoc


Full code example setting key and passphrase on builder:

String privateKeyContent = "private key contents"
String passphrase "private key passphrase contents"
Iterable<KeyPair> keyPairs = SecurityUtils.loadKeyPairIdentities(null,
                    null, 
                    new ByteArrayInputStream(privateKeyContent.getBytes()), 
                    (session, resourceKey, retryIndex) -> passphrase);

SshdSessionFactory sshSessionFactory = new SshdSessionFactoryBuilder()
    .setPreferredAuthentications("publickey")
    .setDefaultKeysProvider(ignoredSshDirBecauseWeUseAnInMemorySetOfKeyPairs -> keyPairs)
    .setHomeDirectory(directory) // Need to set despite Javadoc
    .setSshDirectory(directory) // Need to set despite Javadoc
    .build(null);

I've seen some examples across GitHub where the directory specified in .setHomeDirectory(directory) and .setSshDirectory(directory) is the directory that is being cloned to but I'm not convinced this is the correct approach? e.g. newSshdSessionFactory method: https://github.com/thingsboard/thingsboard/blob/02d3382731a5540f60e530f9c9c90a935b131c67/common/version-control/src/main/java/org/thingsboard/server/service/sync/vc/GitRepository.java#L503-L504

where directory is the repository directory: https://github.com/thingsboard/thingsboard/blob/master/common/version-control/src/main/java/org/thingsboard/server/service/sync/vc/DefaultGitRepositoryService.java#L295

What would you suggest here? Is there a way to omit setting the ssh folder and home directory?

Should I use a temporary directory instead?

Thanks

tomaswolf commented 3 weeks ago

I don't know what other libraries using this do. Setting the ssh dir to a git working tree directory is a bit strange. I'd never set it to any directory where files may be written from somewhere over the internet, which is what a git fetch does.

It should be possible to run this without any files, but it's possible that we overlooked something with that home directory.

You would have to set a ServerKeyDatabase that does not rely on ~/.ssh/known_hosts, and the SshConfigStore should not rely on ~/.ssh/config. If it then still fails without setting these directories, open a bug in JGit.

devvthedev commented 3 weeks ago

Hi @tomaswolf

Thanks for getting back to me.

I've tried what you suggested:

String privateKeyContent = "private key contents"
String passphrase "private key passphrase contents"
Iterable<KeyPair> keyPairs = SecurityUtils.loadKeyPairIdentities(null,
                    null, 
                    new ByteArrayInputStream(privateKeyContent.getBytes()), 
                    (session, resourceKey, retryIndex) -> passphrase);

SshdSessionFactory sshSessionFactory = new SshdSessionFactoryBuilder()
    .setPreferredAuthentications("publickey")
    .setDefaultKeysProvider(ignoredSshDirBecauseWeUseAnInMemorySetOfKeyPairs -> keyPairs)
    .setHomeDirectory(directory) // Need to set despite setting ConfigStoreFactory and ServerKeyDatabase up
    .setSshDirectory(directory) // Need to set despite setting ConfigStoreFactory and ServerKeyDatabase up
    .setConfigStoreFactory((ignoredHomeDir, ignoredConfigFile, ignoredLocalUserName) -> null)
    .setServerKeyDatabase((ignoredHomeDir, ignoredSshDir) -> new ServerKeyDatabase() {
        @Override
        public List<PublicKey> lookup(String connectAddress, InetSocketAddress remoteAddress, Configuration config) {
            return Collections.emptyList();
        }

        @Override
        public boolean accept(String connectAddress, InetSocketAddress remoteAddress, PublicKey serverKey, Configuration config, CredentialsProvider provider) {
             // Work out whether to accept the server key or not

             // example: accept all server keys
             return true;
        }
     })
    .build(null);

When the build method is invoked, it then sets the following on the SshdSessionFactory:

factory.setHomeDirectory(homeDirectory);
factory.setSshDirectory(sshDirectory);


The method signatures both specify @NonNull e.g.

public void setHomeDirectory(@NonNull File homeDir) {
    ...
}

and

public void setSshDirectory(@NonNull File sshDir) {
    ...
}

https://github.com/eclipse-jgit/jgit/blob/master/org.eclipse.jgit.ssh.apache/src/org/eclipse/jgit/transport/sshd/SshdSessionFactory.java#L312-L324

I believe if these were nullable that would alleviate this? What do you think?

I will open a Bug issue in JGit describing this in more detail.

Thanks