Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
devvthedev opened this issue Jul 18, 2024 · 4 comments

Comments

@devvthedev
Copy link

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 [email protected]: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", "[email protected]"))
    .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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants