Skip to content

Commit

Permalink
JSch: Fix detection of supported RSA sig schemes
Browse files Browse the repository at this point in the history
KeyExchange.guess() only returns the first server/client match for each
category, and the client algorithms are taken from the HostKeyAlgorithms
OpenSSH config file keyword rather than the PubkeyAcceptedAlgorithms
keyword.  Thus, fd34df2 effectively
made it so that an RSA signature scheme could only be used if it was
the first server-supported algorithm listed with the HostKeyAlgorithms
keyword.

Instead, set Session.supportedRSAMethods to the list of RSA signature
schemes that the server supports, and attempt to use the first one of
those algorithms that is specified with PubkeyAcceptedAlgorithms.  This
fulfills the intent of fd34df2 and
emulates the behavior of OpenSSH.
  • Loading branch information
dcommander committed Oct 16, 2024
1 parent 06c6c2c commit b632a9c
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 18 deletions.
5 changes: 5 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ advertises support for the pseudo-encoding. (The next major release of the
TurboVNC Server will forego using the extension unless the VNC viewer
advertises support for it.)

5. Fixed an issue in the TurboVNC Viewer's built-in SSH client whereby the
`ssh-rsa` and `rsa-sha2-512` signature schemes could not be used unless they
were specified as the first argument for the `HostKeyAlgorithms` OpenSSH config
file keyword or the server did not support `rsa-sha2-256`.


3.1.2
=====
Expand Down
17 changes: 15 additions & 2 deletions java/com/jcraft/jsch/KeyExchange.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* -*-mode:java; c-basic-offset:2; indent-tabs-mode:nil -*- */
/*
Copyright (c) 2002-2018 ymnk, JCraft,Inc. All rights reserved.
Copyright (c) 2018 D. R. Commander. All rights reserved.
Copyright (c) 2018, 2024 D. R. Commander. All rights reserved.
Copyright (c) 2020 Jeremy Norris. All rights reserved.
Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -31,6 +31,8 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING

package com.jcraft.jsch;

import java.util.Vector;

public abstract class KeyExchange{

static final int PROPOSAL_KEX_ALGS=0;
Expand Down Expand Up @@ -91,7 +93,7 @@ public String getKeyAlgorithName() {
return key_alg_name;
}

protected static String[] guess(byte[]I_S, byte[]I_C){
protected static String[] guess(Session session, byte[]I_S, byte[]I_C){
String[] guess=new String[PROPOSAL_MAX];
Buffer sb=new Buffer(I_S); sb.setOffSet(17);
Buffer cb=new Buffer(I_C); cb.setOffSet(17);
Expand All @@ -115,6 +117,17 @@ protected static String[] guess(byte[]I_S, byte[]I_C){
int j=0;
int k=0;

if(i==PROPOSAL_SERVER_HOST_KEY_ALGS) {
String smethods=new String(sp);

if(smethods.matches("(^|.*,)ssh-rsa(,.*|$)"))
session.supportedRSAMethods.addElement("ssh-rsa");
if(smethods.matches("(^|.*,)rsa-sha2-256(,.*|$)"))
session.supportedRSAMethods.addElement("rsa-sha2-256");
if(smethods.matches("(^|.*,)rsa-sha2-512(,.*|$)"))
session.supportedRSAMethods.addElement("rsa-sha2-512");
}

loop:
while(j<cp.length){
while(j<cp.length && cp[j]!=',')j++;
Expand Down
7 changes: 1 addition & 6 deletions java/com/jcraft/jsch/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -588,15 +588,10 @@ private KeyExchange receive_kexinit(Buffer buf) throws Exception {
send_kexinit();
}

guess=KeyExchange.guess(I_S, I_C);
guess=KeyExchange.guess(this, I_S, I_C);
if(guess==null){
throw new JSchException("Algorithm negotiation fail");
}
for(String method : guess){
if(method.equals("ssh-rsa") || method.equals("rsa-sha2-256") || method.equals("rsa-sha2-512")){
supportedRSAMethods.addElement(method);
}
}

if(!isAuthed &&
(guess[KeyExchange.PROPOSAL_ENC_ALGS_CTOS].equals("none") ||
Expand Down
17 changes: 7 additions & 10 deletions java/com/jcraft/jsch/UserAuthPublicKey.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Copyright (c) 2002-2018 ymnk, JCraft,Inc. All rights reserved.
Copyright (c) 2020 Jeremy Norris. All rights reserved.
Copyright (c) 2020-2021 Matthias Wiedemann. All rights reserved.
Copyright (c) 2023 D. R. Commander. All rights reserved.
Copyright (c) 2023-2024 D. R. Commander. All rights reserved.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:
Expand Down Expand Up @@ -56,23 +56,20 @@ public boolean start(Session session) throws Exception{
return false;
}

String rsamethods=null;
String rsamethod=null;
String nonrsamethods=null;
for(int i=0; i<pkmethoda.length; i++){
if(pkmethoda[i].equals("ssh-rsa") || pkmethoda[i].equals("rsa-sha2-256") || pkmethoda[i].equals("rsa-sha2-512")){
if(session.supportedRSAMethods.contains(pkmethoda[i])){
if(rsamethods==null) rsamethods=pkmethoda[i];
else rsamethods+=","+pkmethoda[i];
}
if(session.supportedRSAMethods.contains(pkmethoda[i]) &&
rsamethod==null)
rsamethod=pkmethoda[i];
}
else{
if(nonrsamethods==null) nonrsamethods=pkmethoda[i];
else nonrsamethods+=","+pkmethoda[i];
}
}
String[] rsamethoda=Util.split(rsamethods, ",");
String[] nonrsamethoda=Util.split(nonrsamethods, ",");

_username=Util.str2byte(username);

iloop:
Expand All @@ -97,8 +94,8 @@ public boolean start(Session session) throws Exception{

String ipkmethod=identity.getAlgName();
String[] ipkmethoda=null;
if(ipkmethod.equals("ssh-rsa")){
ipkmethoda=rsamethoda;
if(ipkmethod.equals("ssh-rsa") && rsamethod!=null){
ipkmethoda=new String[]{rsamethod};
}
else if(nonrsamethoda!=null && nonrsamethoda.length>0){
for(int j=0; j<nonrsamethoda.length; j++){
Expand Down

0 comments on commit b632a9c

Please sign in to comment.