TurboVNC / turbovnc

Main TurboVNC repository
https://TurboVNC.org
GNU General Public License v2.0
773 stars 139 forks source link

Consider switching to the JSch fork at mwiede/jsch #323

Open e6c31d opened 2 years ago

e6c31d commented 2 years ago

JSch hasn't had a release since 2018, and it's missing many features (such as ed25519 keys).

There's a fork at https://github.com/mwiede/jsch and the changelog is quite impressive. Have the TurboVNC project considered it?

dcommander commented 2 years ago

Yes, I noticed that. I will consider switching in the next major release.

dcommander commented 2 years ago

I will also look into merging some of the less disruptive fixes into TurboVNC 3.0.x. For instance, it would be nice to have support for the OpenSSH private key format. Otherwise, generating a new SSH private key and then attempting to use it directly with the TurboVNC Viewer (with the -sshkeyfile command-line argument or the IdentityFile OpenSSH config parameter, as opposed to through an SSH agent) causes JSch to throw an "invalid privatekey" error unless you convert the private key to PEM format (e.g. by invoking ssh-keygen -p -N "" -m pem -f {private-key-file}.) Support for the newer crypto algorithms isn't as important at the moment, but it will eventually become more important.

dcommander commented 2 years ago

The patch that allows JSch to support OpenSSH v1 private keys has been merged into the main branch (which will eventually become TurboVNC 3.0.1.)

dcommander commented 2 years ago

Support for rsa-sha2-256 and rsa-sha2-512 has been merged as well. Those were the two most pressing issues for the TurboVNC 3.0.x release series, since they made our JSch implementation incompatible with recent OpenSSH releases.

samh commented 1 year ago

It seems like perhaps you used a different version of JSch in the v3.0 release? I recently updated to 3.0.2 and was surprised when my ed25519 key no longer worked:

JSch: ssh-ed25519 cannot be used as public key type for identity <name>

I see you said in #331 that "TurboVNC Viewer doesn’t support ED25519 keys" so I was surprised when I retested 3.0 and it worked 😄 (I reinstalled v3.0 to verify that it does indeed work, v3.0.1 and v3.0.2 and the latest prerelease show that error, given the -loglevel 110 option). Anyway, it's no big inconvenience, I generated a new RSA key for now, just wanted to mention it in case somebody else comes across it, as I was confused for a while as to why I had to start typing my password.

Thanks for your work on the SSH support, it's been very nice to use (the authentication and the session manager)!

dcommander commented 1 year ago

@samh See https://github.com/TurboVNC/turbovnc/issues/360

dcommander commented 1 year ago

I am in the process of implementing a number of fixes to our SSH client based on an extensive regression test that I developed for it. In the context of fixing those issues, I have had an opportunity to examine the JSch fork closely, and my impression at the moment is that it still has a lot of quirks-- differences in behavior relative to OpenSSH that I have had to address, with some difficulty, in our own implementation. The JSch fork also relies on JNA for its SSH agent connectors, which is a non-starter for TurboVNC. Of the two features that I have ported from the JSch fork so far (OpenSSH v1 private key support and rsa-sha2-256/rsa-sha2-512 signature support), the latter of those features caused TurboVNC to regress. (See #361.) In order to adopt their code base, I would have to spend a great deal of time submitting upstream pull requests that fix and test the various quirks, as well as figuring out how to integrate as much of their code as possible without integrating the parts of it that would break TurboVNC. Honestly, it would be easier to just port specific features from their code base into ours based on user demand. That would also give me an opportunity to closely examine and regression test each new feature that I adopt.

Thus, please list the features that you would most like to see TurboVNC adopt from the JSch fork.