eclipse-jgit / jgit

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

Unable to configure SshdSessionFactory without specifying files (SshDirectory & HomeDirectory) using SshdSessionFactoryBuilder #89

Open devvthedev opened 2 weeks ago

devvthedev commented 2 weeks ago

Version

6.10.0.202406032230-r

Operating System

MacOS

Bug description

I want to build a SshdSessionFactory using the SshdSessionFactoryBuilder specifying an SSH key pair programatically without needing to set what Home Directory and SSH folder are.

"It should be possible to run this without any files"

As mentioned in https://github.com/apache/mina-sshd/issues/532 specifically here, I am unable to build a SshdSessionFactory using the SshdSessionFactoryBuilder without being forced to specify the File location for both HomeDirectory and SshDirectory.


This can be reproduced by building a SshdSessionFactory using the SshdSessionFactoryBuilder.


When constructing an SshdSessionFactoryBuilder with: .setHomeDirectory(null) and .setSshDirectory(null)

(or omitting entirely)

e.g.

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(null)
    .setSshDirectory(null)
    .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);

I am required to set both HomeDirectory and SshDirectory to some directory despite providing my key pair through the .setDefaultKeysProvider(ignoredSshDirBecauseWeUseAnInMemorySetOfKeyPairs -> keyPairs) builder method.

Actual behavior

A null pointer exception is thrown saying that the HomeDirectory and SshDirectory cannot be null.

When invoking .build on the builder, the following setters for these directories are called:

SshdSessionFactory build(KeyCache cache) {
    SshdSessionFactory factory = new SessionFactory(cache, 
        proxyDataFactory);
    factory.setHomeDirectory(homeDirectory);
    factory.setSshDirectory(sshDirectory);
    return factory;
}

Both the SSH directory and HomeDirectory are required to be non null:

@NonNull File homeDir
@NonNull File sshDir

SshdSessionFactory.setHomeDirectory SshdSessionFactory.setSshDirectory

Expected behavior

To not have to provide an SSH Directory or Home Directory to the builder.

I provide the following to the builder:

Which should mean these directories are not required to be specified as they are not used when creating a session.

Relevant log output

If I do not provide a HomeDirectory or do `.setHomeDirectory(null)`

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.io.File.isAbsolute()" because "homeDir" is null
        at org.eclipse.jgit.transport.sshd.SshdSessionFactory.setHomeDirectory(SshdSessionFactory.java:319)
        at org.eclipse.jgit.transport.sshd.SshdSessionFactoryBuilder$State.build(SshdSessionFactoryBuilder.java:341)
        at org.eclipse.jgit.transport.sshd.SshdSessionFactoryBuilder.build(SshdSessionFactoryBuilder.java:289)

If I do not provide a SshDirectory or do `.setSshDirectory(null)`

Exception in thread "main" java.lang.NullPointerException: Cannot invoke "java.io.File.isAbsolute()" because "sshDir" is null
        at org.eclipse.jgit.transport.sshd.SshdSessionFactory.setSshDirectory(SshdSessionFactory.java:342)
        at org.eclipse.jgit.transport.sshd.SshdSessionFactoryBuilder$State.build(SshdSessionFactoryBuilder.java:342)
        at org.eclipse.jgit.transport.sshd.SshdSessionFactoryBuilder.build(SshdSessionFactoryBuilder.java:289)

Other information

Reference: https://github.com/apache/mina-sshd/issues/532

CC: @tomaswolf

tomaswolf commented 2 weeks ago

It definitely should be possible to run without SSH directory.

The home directory is a bit more difficult since it is used in various places to do ~ substitution. It should fall back to FS.DETECTED.userHome(), which in turn should use the Java user.home, or in the very worst case the current directory at the time the application was started.

Explicitly setting the home directory to null should probably be forbidden.

I'll take a look, but the fix will not make it into the upcoming 7.0 release.

devvthedev commented 2 weeks ago

Hi @tomaswolf

It definitely should be possible to run without SSH directory.

Apologies, are you saying it's possible right now to configure it to run without SSH directory?

When I tried to exclude it in the factory builder by setting it to null or omitting it, the factory.setSshDirectory(sshDirectory); prevents that?

If it is currently possible, could you give me an example of how to configure it?


I'll take a look, but the fix will not make it into the upcoming 7.0 release.

Thank you ☺️ How would you suggest for now I configure the HomeDirectory until the fix makes it in?

Thank You!

tomaswolf commented 2 weeks ago

It definitely should be possible to run without SSH directory.

Apologies, are you saying it's possible right now to configure it to run without SSH directory?

I'm saying that was the intent.

How would you suggest for now I configure the HomeDirectory until the fix makes it in?

Set it to any suitable directory. But choose one where you don't get files downloaded from the Internet.

devvthedev commented 2 weeks ago

I'm saying that was the intent.

I've looked through the code and places that use the sshDir will have been overridden by other config on the builder we discussed earlier. (setServerKeyDatabase, ConfigStoreFactory etc.)

Are we saying that the setter of the ssh directory on the sshhSessionFactory should/will be made nullable?


Set it to any suitable directory.

Would a temporary directory work?

Will this temporary fix also be the case for setting the SshDirectory on the builder? So I should set both HomeDirectory and SshDirectory on the builder to this other suitable directory that is not the Git root directory?

Thanks