Skip to content

Sftp lacks support for ssh-ed25519 due to unmaintained library #3572

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

Closed
Brutus5000 opened this issue May 31, 2021 · 12 comments · Fixed by #3892
Closed

Sftp lacks support for ssh-ed25519 due to unmaintained library #3572

Brutus5000 opened this issue May 31, 2021 · 12 comments · Fixed by #3892

Comments

@Brutus5000
Copy link

Brutus5000 commented May 31, 2021

In what version(s) of Spring Integration are you seeing this issue?

For example:
5.5.0 and all previous version

Describe the bug

The DefaultSftpSessionFactory does not support host keys of type ssh-ed25519.
If it tries to validate one it will return with 2 messages:

INFO: The authenticity of host '****' can't be established.\nRSA key fingerprint is ****.\nAre you sure you want to continue connecting?
ERROR: MessagingException Failed to execute on session; nested exception is java.lang.IllegalStateException: failed to create SFTP Session

However: If all unknown keys are accepted, the connection can be established. This might cause people to rather not validate keys because of convenience and would open the door for vulnerabilities.

To Reproduce

Start an SFTP server using ssh-ed25519 based host keys. In my case I'm using the docker image atmoz/sftp.
Connect to the server via sftp cli and accept the key.
Use the known hosts to instantiate a SftRemoteFileTemplate.

Expected behavior

The host key is properly validated and used.

Background information
The reason for this issue is the lack of support for this key type in the JSch library. Furthermore the library seems to be no longer maintained as nobody answers on maintenance requests on the mailing list.
The lack of key type support will be a security issue in the future. More details can also be found here: http://www.matez.de/index.php/2020/06/22/the-future-of-jsch-without-ssh-rsa/

Several forks such as https://github.com/mwiede/jsch have appeared that resolve these issues. Maybe it's possible to swap the dependency while keeping compatibility.

@Brutus5000 Brutus5000 added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels May 31, 2021
@artembilan
Copy link
Member

According to README for the project you mention: https://github.com/mwiede/jsch, it is just "drop in replacement".
So, you go to your dependency management, exclude com.jcraft:jsch from the spring-integration-sftp and add com.github.mwiede:jsch with respective version to support your requirements.

We probably will think for the future version to switch such a dependency: technically you should not care what SSH library we use underneath, but unfortunately we can't do that in the current point release.
So, we can treat this as a question with respective answer in that blog post and close.
Or we can treat it as a Feature Request and will resolve in the future since it is going to be some breaking change.

But definitely this is not a bug.

WDYT?

Thank you for understanding!

@artembilan artembilan added in: sftp type: enhancement status: waiting-for-reporter Needs a feedback from the reporter and removed status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Jun 21, 2021
@Brutus5000
Copy link
Author

Thanks for the tip with swapping dependencies.

I have no hard feeling about categorization of this issue. For keeping it on the radar I'd rather keep it open as a feature request.
I'm not sure what the plans for the sftp module are and what the general Spring handling of unmaintained dependencies works.

But is there even a need to break any compatibility if the official Spring module could avoid this by just swapping the dependency? Or are there other sftp changes planned anyway justifying a breaking change?

@artembilan
Copy link
Member

Right, as I said: there is a workaround for the current version.
If there is strong feeling that we need to change underlying library for our API, we can consider to do so.
But even a simple dependency change without any code is still considered as breaking change. Therefore only the next version.
At the moment I don't have plans for intermediate 5.6 version, so such a change if we do is going to be available in the next 6.0 next year.
any current 5.5.x is a point release where we can change the code only to fix bug or introduce some minor feature which does not affect existing components too much.
Since changing dependency is that part where it affects the whole SFTP component as an artifact from Maven Central, we treat it as a breaking change and can't do in the current 5.5.x.

Hope that all clears things.
Turning this into a Feature Request to be fixed in the next version: I find your argument about missing original library support as a trigger to look into other SSH solution. Or just plain switch to the mentioned fork!

@artembilan artembilan removed the status: waiting-for-reporter Needs a feedback from the reporter label Jun 21, 2021
@artembilan artembilan added this to the 6.0.x milestone Jun 21, 2021
@gbaso
Copy link

gbaso commented Oct 27, 2021

Openssh latest release has officially dropped support for RSA-SHA1.

Please reconsider releasing a 5.6 version with a mantained ssh library (like the aforementioned com.github.mwiede:jsch) if release 6.0 is still a while away.

@artembilan
Copy link
Member

As I said before: there is a workaround which does not break the framework code and does not require a ode change on your side: you just exclude com.jcraft:jsch dependency and add om.github.mwiede:jsch.
It does not justify a whole version to release on our side.

Thank for understanding.

@damienhollis
Copy link

Hi @artembilan I see that version 6.0 is progressing towards a release. Is anything happening with this issue? We are currently using the workaround for jsch but I notice other projects are using to Apache MINA (e.g. spring-cloud/spring-cloud-config#1901). What does spring-integration plan to do?

@artembilan
Copy link
Member

I didn't look into an Apache MINA for SSH support, but if that what is done in Spring Cloud Config and looks like they did that instead of migrating to a forked lib, then we indeed can go ahead and fix this issue same way - replace JSCH with Apache MINA.
Contribution is welcome!

@artembilan
Copy link
Member

It looks like we already have an org.apache.sshd:sshd-sftp dependency in our SFTP module.
Perhaps we just have to override the whole module to be based on the org.apache.sshd.sftp.client.SftpClient API...

@artembilan artembilan self-assigned this Sep 23, 2022
@artembilan artembilan modified the milestones: 6.0.x, 6.0.0-RC1 Sep 23, 2022
artembilan added a commit to artembilan/spring-integration that referenced this issue Sep 23, 2022
Fixes spring-projects#3572

* Rework SFTP module from the JSch API to more modern `sshd-sftp`
* Migrate generics of most API from `ChannelSftp.LsEntry` to the `SftpClient.DirEntry`
* Rework `DefaultSftpSessionFactory` to deal with an `SshClient` and create `SftpClient`
wrapped to the `SftpSession`
* Implement a `ResourceKnownHostsServerKeyVerifier` to load `known-hosts` from any possible resource
* Implement an expected `SftpSession.list()` with just file name to take or pattern matching
* Remove some unused tests and their config
* Remove tests for custom `UserInfo` since we don't provide any custom out-of-the-box
* Test a new `ResourceKnownHostsServerKeyVerifier` against default `known-hosts` file
artembilan added a commit to artembilan/spring-integration that referenced this issue Sep 27, 2022
Fixes spring-projects#3572

* Rework SFTP module from the JSch API to more modern `sshd-sftp`
* Migrate generics of most API from `ChannelSftp.LsEntry` to the `SftpClient.DirEntry`
* Rework `DefaultSftpSessionFactory` to deal with an `SshClient` and create `SftpClient`
wrapped to the `SftpSession`
* Implement a `ResourceKnownHostsServerKeyVerifier` to load `known-hosts` from any possible resource
* Implement an expected `SftpSession.list()` with just file name to take or pattern matching
* Remove some unused tests and their config
* Remove tests for custom `UserInfo` since we don't provide any custom out-of-the-box
* Test a new `ResourceKnownHostsServerKeyVerifier` against default `known-hosts` file
garyrussell pushed a commit that referenced this issue Sep 27, 2022
* GH-3572: Migrate SFTP from `jsch` to `sshd-sftp`

Fixes #3572

* Rework SFTP module from the JSch API to more modern `sshd-sftp`
* Migrate generics of most API from `ChannelSftp.LsEntry` to the `SftpClient.DirEntry`
* Rework `DefaultSftpSessionFactory` to deal with an `SshClient` and create `SftpClient`
wrapped to the `SftpSession`
* Implement a `ResourceKnownHostsServerKeyVerifier` to load `known-hosts` from any possible resource
* Implement an expected `SftpSession.list()` with just file name to take or pattern matching
* Remove some unused tests and their config
* Remove tests for custom `UserInfo` since we don't provide any custom out-of-the-box
* Test a new `ResourceKnownHostsServerKeyVerifier` against default `known-hosts` file

* * Some tests improvements

* * Improve generics handling for `FileUtils`
@gauravbrills
Copy link

Is there a migration guide on how to migrate the jsch features to mina sshd somewhere (maybe not SI as its more of an abstraction) as seems right now for us easier to move to the drop in replacement com.github.mwiede:jsch. Concerns mainly on customizations which were possible in jsch for jump host proxy , certain session configs etc .

@rishiraj88
Copy link

Interesting Thread.

@artembilan
Copy link
Member

We are not experts here of the SFTP protocol, neither those JSCH and MINA projects.
It is probably better to ask Apache MINA community about that jump proxy support.
We have something mentioned in the doc those: https://docs.spring.io/spring-integration/docs/current/reference/html/sftp.html#sftp-session-factory-properties

hostConfig::An org.apache.sshd.client.config.hosts.HostConfigEntry instance as an alternative for the user/host/port options. Can be configured with a proxy jump property.

We also had some conversion like this before though: #8559

@gauravbrills
Copy link

Thanks yes in jsch we had a custom sftpProxyCommand to do the jump host call . Sure will check in mina forums on this and trying via hostConfig (though not found a good example yet will post if works) .

Also yes thanks these threads helped to work with com.github.mwiede:jsch for the meantime as now unfortunately our platform needs to support both old and new sftp servers .But yes long term plan will be to move to a stable Apache project.

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

Successfully merging a pull request may close this issue.

6 participants