updates jsch version and fixes test to match exception correctly

[johnjaylward]

Fixes #301
Fixes #308
Fixes #356

• Bumps the JSCH version from 0.2.5 to the latest 0.2.23. This addresses CVE-2023-48795 which was fixed in jsch-0.2.15 and adds in support for many new algorithms
• Correct test case that fails due to the bump. No implementation code changes needed.
  • This changed in jsch-0.2.7 and appears to be done around the following change note (see updates jsch version and fixes test to match exception correctly #389 (comment) for more details):

    Eliminate KeyPairDeferred and instead change handling of OpenSSH V1 type keys to be more like other KeyPair types

• Bumps gradle build action to new implementation. See notes on repos:
  • Old: https://github.com/gradle/wrapper-validation-action
  • New: https://github.com/gradle/actions/tree/main/wrapper-validation
  • Old: https://github.com/gradle/gradle-build-action
  • New: https://github.com/gradle/actions/tree/main/setup-gradle

Steps to verify the fix

1. Bumped the version
2. Corrected failing test. Was expecting IllegalArgumentException, but with the bump a corrected JSCHException is thrown with the message "AUTH FAIL"
3. ran gradle build and confirmed all tests passed

// Paste the snippet

Backward compatibility

If someone is expecting an "IllegalArgumentException" on wrong passphrases for ssh keys, they will need to change their exception to the JSCHException.

[johnjaylward]

@int128 I believe I fixed the build issue. The actions in my Repo completed successfully.

[johnjaylward]

wait. I missed the release actions file. I will correct that too

[johnjaylward]

ok, that should be all the gradle build actions.

[johnjaylward]

rebased to current master

[elijahfhopp]

My $0.02: looks good. Is that change in exception from jsch/documented by jsch? Worth looking into the code that test covers. Glad this update would fix the CVE.

[johnjaylward]

Based on what I see in the change logs and java docs, all Identity Passphrase related errors should always have thrown JSchException... I'll dig in further to see if it was a bugfix on their side to correct the function, or if there is an API issue that needs to be addressed due to the update.

[johnjaylward]

Findings for change from IllegalArgumentException to JSchException

1. Test started failing at dependency change to com.github.mwiede:jsch:0.2.7
2. Reviewed 0.2.7 changelog for JSchException and "USERAUTH fail" and there are a lot of changes around that.
3. All Javadoc from the initial import (1.5.15) to current 0.2.23 show that passphrase errors should be JSchException
4. I reverted back to the 0.2.5 and got this stack trace for the exception:

   java.lang.IllegalArgumentException: Could not sucessfully decrypt openssh v1 key
   	at com.jcraft.jsch.KeyPairDeferred.decrypt(KeyPairDeferred.java:49)
   	at com.jcraft.jsch.IdentityFile.setPassphrase(IdentityFile.java:64)
   	at com.jcraft.jsch.JSch.addIdentity(JSch.java:551)
   	at com.jcraft.jsch.JSch.addIdentity(JSch.java:496)
   	at com.jcraft.jsch.JSch.addIdentity(JSch.java:476)
   	at org.hidetake.groovy.ssh.connection.UserAuthentication$Trait$Helper.configureUserAuthentication(UserAuthentication.groovy:41)
   

5. The stack trace for the current 0.2.23 is

   Caused by: com.jcraft.jsch.JSchException: USERAUTH fail
   	at app//com.jcraft.jsch.UserAuthPublicKey.decryptKey(UserAuthPublicKey.java:388)
   	at app//com.jcraft.jsch.UserAuthPublicKey._start(UserAuthPublicKey.java:157)
   	at app//com.jcraft.jsch.UserAuthPublicKey.start(UserAuthPublicKey.java:119)
   	at app//com.jcraft.jsch.Session.connect(Session.java:479)
   	at app//com.jcraft.jsch.Session.connect(Session.java:198)
   	at app//org.hidetake.groovy.ssh.connection.ConnectionManager.connectInternal(ConnectionManager.groovy:107)
   

This given the difference in stack traces, this looks like an intentional fix on their part to accomplish 2 goals:

1. Simplify the call stack in general for key handling
2. Bring the code up to match the javadocs

You can see that the new exception is happening later in the process when the connection is established instead of at the configuration point.

[johnjaylward]

also took the opportunity to add the exception message to the test case validation.

[elijahfhopp]

Thank you for the robust documentation. That clears up any uncertainty I have about it. IllegalArgumentException was quite a confusing choice, glad they changed it. +1 to exception message in test.

[johnjaylward]

@int128 , is there anything else I should do for this?

[int128]

Thank you very much!