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

[SECURITY] JSch library is outdated and unmaintained, switch to mweide implementation #5608

Closed
jonbartels opened this issue Jan 3, 2023 · 6 comments
Labels
Internal-Issue-Created An issue has been created in NextGen's internal issue tracker RS-9938 Security triaged
Milestone

Comments

@jonbartels
Copy link
Contributor

Describe the security issue
JSCH has not published a release since 2018 http://www.jcraft.com/jsch/ChangeLog . Several changes in openssh (the most common SSH implementation) will no longer allow JSCH to connect with older, unsupported algorithms.

Suggested remediation

Additional context
Explanation of why jsch was forked

@jonbartels
Copy link
Contributor Author

Fortunately JSCH only appears in two SFTP classes in the file connector.

This means that changing JSch implementations can be isolated to just the file connector. It can be built as its own extension, deployed, tested, etc.

Whatever internal tickets, tasks, and testing were done for #4080 which was part of MC 4.1.0 can be repeated using the mweide JSch fork. @joaryche were the JSch updates for 4.1.0 just simply swapping JARs or was there more to it?

@joaryche
Copy link
Collaborator

I created a docker container which uses an image (atmoz/sftp) for SSH/SFTP. then ran my connector tests against the container.

@lmillergithub lmillergithub added triaged RS-9938 Internal-Issue-Created An issue has been created in NextGen's internal issue tracker labels Jan 11, 2023
@tonygermano
Copy link
Collaborator

@lmillergithub Is there a reason that Jsch is in server-lib instead of with the File Reader/Writer extensions?

@jonbartels
Copy link
Contributor Author

The user "Joe Clark" did an experiment in Slack swapping in the mweide JAR. Joe posted some errors:
image

This is actually GOOD.

  1. The library appears to "just work"
  2. See how the mweide errors show more detail about the problem? This additional detail is an improvement over old jsch because it shows information about keys

Joe has not done a complete, successful experiment but I think Joe has shown that the mweide JAR has merit.

https://mirthconnect.slack.com/archives/C02SW0K4D/p1675982566199079?thread_ts=1675722960.061319&cid=C02SW0K4D

@jonbartels
Copy link
Contributor Author

https://github.com/mwiede/jsch/releases/tag/jsch-0.2.8 just released and mwiede/jsch#287 would be helpful in Mirth for cases where connections fail due to algo negotiation.

@jonbartels
Copy link
Contributor Author

https://github.com/jonbartels/connect/tree/5608-update-jsch-impl-to-mweide

Have not tested yet but the change seems as simple as swapping JARs. I intend to follow John Andersons testing strategy.

I could not find any other references where jsch was explicitly loaded.

I kind of wanted to add a lib directory specific to the file connector but that seemed like a bigger change to the project layout and out of scope.

pladesma added a commit that referenced this issue Nov 30, 2023
@pladesma pladesma added this to the 4.5.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal-Issue-Created An issue has been created in NextGen's internal issue tracker RS-9938 Security triaged
Projects
None yet
Development

No branches or pull requests

5 participants