Poor ssh-agent default behaviour, and possible related failure on Mac.

[Parakleta]

Hi,

I have noticed that when TurboVNC Viewer is configured to operate with an agent (ssh-agent or pageant) it automatically loads all of the private keys in the ~/.ssh directory and adds them to the agent. This is not the expected behaviour as by default keys should not be added to the agent.

Unfortunately this also seems to trigger a bug in TurboVNC on Mac using ssh-agent. When it is run for the second time it gets stuck in some kind of authentication loop and eventually fails with the message "SSH_MSG_DISCONNECT: 2 Too many authentication failures". This appears to be due to the same key appearing both in the .ssh directory and in the agent. If I remove either of the now duplicated keys (the file in .ssh or from the agent) TurboVNC then works again without a problem.

I have a similar setup on Windows with pageant but I cannot be sure whether the problem occurs there or not: on the Mac system the local key which is duplicated is not valid for the target server; on the Windows system the local key is valid for the server. This difference (key validity) may be more relevant than platform.

[dcommander]

I have noticed that when TurboVNC Viewer is configured to operate with an agent (ssh-agent or pageant) it automatically loads all of the private keys in the ~/.ssh directory and adds them to the agent. This is not the expected behaviour as by default keys should not be added to the agent.

How did you reach that conclusion? I don't see any such code, and if I do ssh-add -D to remove all keys from ssh-agent, the TurboVNC Viewer prompts me for an SSH password, as I would expect.

[Parakleta]

I add two specific keys to my ssh-agent manually, from a directory other than .ssh. Then I run TurboVNC and the key in my .ssh/id_rsa file then appears in the output of ssh-add -l. If I remove the file .ssh/id_rsa file before running TurboVNC then it isn't added to the agent. The key is not added to the agent when I run the standard ssh command line utility.

I believe the relevant code is here:

https://github.com/TurboVNC/turbovnc/blob/ff35d99e9aebb3905c2d90bea7c3305b63c853cd/java/com/jcraft/jsch/Session.java#L2741

and:

https://github.com/TurboVNC/turbovnc/blob/ff35d99e9aebb3905c2d90bea7c3305b63c853cd/java/com/jcraft/jsch/JSch.java#L500-L505

[Parakleta]

Maybe the behaviour only triggers if the agent is not empty?

[dcommander]

I'll look into that, but I don't think that's the ultimate cause of the failure. I can reproduce a problem that has the same symptoms as what you are describing, and I don't have ~/.ssh/id_rsa. The SSH server on my TurboVNC host has MaxAuthTries set to 6. That means that connecting to the SSH server should work if the private key for my account on the host is in the 6th position or lower in the list of keys offered by ssh-agent. Such is the case with OpenSSH, but with the TurboVNC Viewer, the connection fails if the key is in the 3rd position or higher. If I list all of the identities registered with JSch right before the SSH connection is opened, I only get back the list offered by ssh-agent, and there are no duplicates. However, I think the issue is that, since d6ae34d6f4bd29308acc9835c8eb80f5e4b2143c, the SSH client is trying each of the ssh-rsa, rsa-sha2-256, and rsa-sha2-512 signature schemes in sequence for each key, so there are three auth failures instead of one. I agree also that ~/.ssh/id_rsa shouldn't be duplicated. I'll look into both issues tomorrow.

[Parakleta]

Ahh, that does sound more likely. I suppose I should have checked my server logs to find exactly what was being sent.

Thankyou.

[dcommander]

I think this simple fix will eliminate the key duplication problem:

index c5ecf564..fe5454b7 100644
--- a/java/com/jcraft/jsch/JSch.java
+++ b/java/com/jcraft/jsch/JSch.java
@@ -477,7 +477,7 @@ public class JSch{
          the same fingerprint already exists with a passphrase. */
       Identity decryptedIdentity=findDecryptedIdentity(identity);
       if(decryptedIdentity!=null){
-        identity=decryptedIdentity;
+        return;
       }
     }

However, I'm still struggling with the other problem. I hoped that porting https://github.com/mwiede/jsch/commit/e1fc3602c976c029acb6ff6a98f6f9d0af5c517f into our JSch implementation would fix it, but no such luck. I am still trying to figure out whether the JSch fork experiences the same issue or whether I just need to port some additional code from the fork.

[dcommander]

I've figured it all out. I'm just developing an automated test that iterates through various permutations of private key configurations, to guard against future regression. I should have everything fixed early next week.

[dcommander]

Should now be fixed. GitHub Actions is derping for some reason, so only Windows and macOS pre-release builds are available with the changes at the moment.