mwiede / jsch

fork of the popular jsch library
Other
705 stars 129 forks source link

Flag to enable "legacy" algos on initialization? #59

Open Hr0bar opened 3 years ago

Hr0bar commented 3 years ago

Hi, I see a lot of users are expecing full in-place compatibility with old Jsch, but that does not happen due to the retirement of some now insecure algos:

https://github.com/mwiede/jsch/issues/48 https://github.com/mwiede/jsch/issues/47 https://github.com/mwiede/jsch/issues/45 https://github.com/mwiede/jsch/issues/40 https://github.com/mwiede/jsch/issues/37

I propose adding a flag that would enable all algos that were previously enabled in the legacy Jsch, something like:

new com.jcraft.jsch.JSch(enable_insecure = "true");

Of course, they would be the least preferred/offered option in the negotiation list.

Reason: we use Jsch to connect to all kinds of versions (some 10+ years old) of below OSes, and its simply impossible to live without these legacy algos. I know we can enable them via configuration as in the example existing issues, but a simple flag that would enable all of them at once is definitely a cleaner way.

mwiede commented 3 years ago

in general, I would be open to support such a flag, the only thing is, that we should keep the API of original Jsch, so that exchanging the library does not break anything. Feel free to send in a PR.

norrisjeremy commented 3 years ago

I'm not sure if I see adding a new constructor flag as especially valuable, as it still requires users to actively change the code to their applications using the JSch library. If they are already in a position to make such a code change, they can just as easily adjust their code to use a different set of crypto algorithms that are compatible with the SSH server with which they are interacting. Additionally, such a change would no longer be backwards compatible at an API level with the original JSch library, since it would not have a similarly defined constructor to the JSch class.

As an alternate approach, what if we instead added some system properties that users could utilize to adjust the various crypto algorithms? This would allow users to do something like java -Djsch.kex="..." -Djsch.server_host_key="..." -Djsch.cipher="..." -Djsch.mac="..." when launching their applications. The value in this approach to the issue is that a user would no longer need to make changes to their application code: they could just define system properties containing the set of crypto algorithms that are compatible with their SSH servers.

norrisjeremy commented 3 years ago

FYI, #61 includes proposal to allow usage of system properties to override the various crypto algorithm defaults.

norrisjeremy commented 3 years ago

Hi @Hr0bar,

In the recent 0.1.65 release we added several System Properties that can be administratively set by users to manipulate the default crypto algorithms used. This should allow users to set them without necessarily requiring them to modify their application code as we have had to suggest in the prior issues that were raised. Hopefully this will make it a bit easier to support older OS's that only support legacy/insecure crypto algorithms.

Thanks, Jeremy

mendhak commented 2 years ago

A 'legacy or insecure = true' flag is very useful where application developers are allowing users to supply both the server details as well as keys to connect. In other words, where the server details are not known during the time of development.

As this library gets future updates, the list of algorithms deprecated will change, and many applications using this library will miss out on what flags and algorithms to allow for their usages to 'keep up' with the times. So the legacy=true flag becomes a clean convenience function, instead of having to wait for users to say something isn't working which causes friction and a poor application experience. It also it serves as a little reminder to them that what they're doing is not a good practice. As OP has said, although things gets deprecated, deprecated setups still exist, and will continue to exist.

In the case of my app, it's on Android so there aren't any application flags that can be set. I'd probably provide a UI toggle which if enabled, sets this legacy variable to true.