-
Notifications
You must be signed in to change notification settings - Fork 146
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Allow usage of keys from ssh-agent that would otherwise not work beca…
…use of missing support for the signature algorithm.
- Loading branch information
1 parent
2eacaa9
commit 41d33dc
Showing
3 changed files
with
79 additions
and
85 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING | |
|
||
package com.jcraft.jsch; | ||
|
||
import java.util.Vector; | ||
import java.util.*; | ||
|
||
class UserAuthPublicKey extends UserAuth{ | ||
|
||
|
@@ -44,75 +44,70 @@ public boolean start(Session session) throws Exception{ | |
return false; | ||
} | ||
|
||
String pkmethods=session.getConfig("PubkeyAcceptedAlgorithms"); | ||
String pkmethodstr=session.getConfig("PubkeyAcceptedAlgorithms"); | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
"Before pruning PubkeyAcceptedAlgorithms = " + pkmethods); | ||
"PubkeyAcceptedAlgorithms = " + pkmethodstr); | ||
} | ||
|
||
String[] not_available_pks = session.getUnavailableSignatures(); | ||
if(not_available_pks!=null && not_available_pks.length>0){ | ||
pkmethods=Util.diffString(pkmethods, not_available_pks); | ||
if(pkmethods==null){ | ||
throw new JSchException("There are not any available sig algorithm."); | ||
String[] not_available_pka = session.getUnavailableSignatures(); | ||
List<String> not_available_pks=(not_available_pka!=null && not_available_pka.length>0 ? | ||
Arrays.asList(not_available_pka) : | ||
Collections.emptyList()); | ||
if(!not_available_pks.isEmpty()){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
"Signature algorithms unavailable for non-agent identities = " + not_available_pks); | ||
} | ||
} | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
"After getUnavailableSignatures PubkeyAcceptedAlgorithms = " + pkmethods); | ||
} | ||
|
||
String[] pkmethoda=Util.split(pkmethods, ","); | ||
if(pkmethoda.length==0){ | ||
List<String> pkmethods=Arrays.asList(Util.split(pkmethodstr, ",")); | ||
if(pkmethods.isEmpty()){ | ||
return false; | ||
} | ||
|
||
String[] server_sig_algs=session.getServerSigAlgs(); | ||
if(server_sig_algs!=null && server_sig_algs.length>0){ | ||
String _known=null; | ||
String _unknown=null; | ||
for(int i=0; i<pkmethoda.length; i++){ | ||
List<String> _known=new ArrayList<>(); | ||
List<String> _unknown=new ArrayList<>(); | ||
for(String pkmethod : pkmethods){ | ||
boolean add=false; | ||
for(int j=0; j<server_sig_algs.length; j++){ | ||
if(pkmethoda[i].equals(server_sig_algs[j])){ | ||
for(String server_sig_alg : server_sig_algs){ | ||
if(pkmethod.equals(server_sig_alg)){ | ||
add=true; | ||
break; | ||
} | ||
} | ||
|
||
if(add){ | ||
if(_known==null) _known=pkmethoda[i]; | ||
else _known+=","+pkmethoda[i]; | ||
_known.add(pkmethod); | ||
} | ||
else{ | ||
if(_unknown==null) _unknown=pkmethoda[i]; | ||
else _unknown+=","+pkmethoda[i]; | ||
_unknown.add(pkmethod); | ||
} | ||
} | ||
|
||
if(_known!=null){ | ||
if(!_known.isEmpty()){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
"PubkeyAcceptedAlgorithms in server-sig-algs = " + _known); | ||
} | ||
} | ||
|
||
if(_unknown!=null){ | ||
if(!_unknown.isEmpty()){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
"PubkeyAcceptedAlgorithms not in server-sig-algs = " + _unknown); | ||
} | ||
} | ||
|
||
if(_known!=null && _unknown!=null){ | ||
String[] _knowna=Util.split(_known, ","); | ||
boolean success=_start(session, identities, _knowna); | ||
if(!_known.isEmpty() && !_unknown.isEmpty()){ | ||
boolean success=_start(session, identities, _known, not_available_pks); | ||
if(success){ | ||
return true; | ||
} | ||
|
||
String[] _unknowna=Util.split(_unknown, ","); | ||
return _start(session, identities, _unknowna); | ||
return _start(session, identities, _unknown, not_available_pks); | ||
} | ||
} | ||
else{ | ||
|
@@ -121,77 +116,74 @@ public boolean start(Session session) throws Exception{ | |
} | ||
} | ||
|
||
return _start(session, identities, pkmethoda); | ||
return _start(session, identities, pkmethods, not_available_pks); | ||
} | ||
} | ||
|
||
private boolean _start(Session session, Vector<Identity> identities, String[] pkmethoda) throws Exception{ | ||
private boolean _start(Session session, List<Identity> identities, List<String> pkmethods, List<String> not_available_pks) throws Exception{ | ||
if(session.auth_failures >= session.max_auth_tries){ | ||
return false; | ||
} | ||
|
||
String rsamethods=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") || | ||
pkmethoda[i].equals("[email protected]") || pkmethoda[i].equals("[email protected]") || | ||
pkmethoda[i].equals("[email protected]") || pkmethoda[i].equals("[email protected]")){ | ||
if(rsamethods==null) rsamethods=pkmethoda[i]; | ||
else rsamethods+=","+pkmethoda[i]; | ||
List<String> rsamethods=new ArrayList<>(); | ||
List<String> nonrsamethods=new ArrayList<>(); | ||
for(String pkmethod : pkmethods){ | ||
if(pkmethod.equals("ssh-rsa") || pkmethod.equals("rsa-sha2-256") || pkmethod.equals("rsa-sha2-512") || | ||
pkmethod.equals("[email protected]") || pkmethod.equals("[email protected]") || | ||
pkmethod.equals("[email protected]") || pkmethod.equals("[email protected]")){ | ||
rsamethods.add(pkmethod); | ||
} | ||
else{ | ||
if(nonrsamethods==null) nonrsamethods=pkmethoda[i]; | ||
else nonrsamethods+=","+pkmethoda[i]; | ||
nonrsamethods.add(pkmethod); | ||
} | ||
} | ||
String[] rsamethoda=Util.split(rsamethods, ","); | ||
String[] nonrsamethoda=Util.split(nonrsamethods, ","); | ||
|
||
byte[] _username=Util.str2byte(username); | ||
|
||
int command; | ||
|
||
iloop: | ||
for(int i=0; i<identities.size(); i++){ | ||
for(Identity identity : identities){ | ||
|
||
if(session.auth_failures >= session.max_auth_tries){ | ||
return false; | ||
} | ||
|
||
Identity identity=identities.elementAt(i); | ||
|
||
//System.err.println("UserAuthPublicKey: identity.isEncrypted()="+identity.isEncrypted()); | ||
decryptKey(session, identity); | ||
//System.err.println("UserAuthPublicKey: identity.isEncrypted()="+identity.isEncrypted()); | ||
|
||
String ipkmethod=identity.getAlgName(); | ||
String[] ipkmethoda=null; | ||
if(ipkmethod.equals("ssh-rsa")){ | ||
ipkmethoda=rsamethoda; | ||
String _ipkmethod=identity.getAlgName(); | ||
List<String> ipkmethods=null; | ||
if(_ipkmethod.equals("ssh-rsa")){ | ||
ipkmethods=rsamethods; | ||
} | ||
else if(nonrsamethoda!=null && nonrsamethoda.length>0){ | ||
for(int j=0; j<nonrsamethoda.length; j++){ | ||
if(ipkmethod.equals(nonrsamethoda[j])){ | ||
ipkmethoda=new String[]{ipkmethod}; | ||
break; | ||
} | ||
} | ||
else if(nonrsamethods.contains(_ipkmethod)){ | ||
ipkmethods=Collections.singletonList(_ipkmethod); | ||
} | ||
if(ipkmethoda==null) { | ||
if(ipkmethods==null) { | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
ipkmethod+" cannot be used as public key type for identity "+identity.getName()); | ||
_ipkmethod+" cannot be used as public key type for identity "+identity.getName()); | ||
} | ||
continue; | ||
} | ||
|
||
byte[] pubkeyblob=identity.getPublicKeyBlob(); | ||
String[] pkmethodsuccess=null; | ||
List<String> pkmethodsuccesses=null; | ||
|
||
if(pubkeyblob!=null){ | ||
command=SSH_MSG_USERAUTH_FAILURE; | ||
loop3: | ||
for(int j=0; j<ipkmethoda.length; j++){ | ||
for(String ipkmethod : ipkmethods){ | ||
if(not_available_pks.contains(ipkmethod) && !(identity instanceof AgentIdentity)){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
ipkmethod+" not available for identity "+identity.getName()); | ||
} | ||
continue loop3; | ||
} | ||
|
||
// send | ||
// byte SSH_MSG_USERAUTH_REQUEST(50) | ||
// string user name | ||
|
@@ -206,7 +198,7 @@ else if(nonrsamethoda!=null && nonrsamethoda.length>0){ | |
buf.putString(Util.str2byte("ssh-connection")); | ||
buf.putString(Util.str2byte("publickey")); | ||
buf.putByte((byte)0); | ||
buf.putString(Util.str2byte(ipkmethoda[j])); | ||
buf.putString(Util.str2byte(ipkmethod)); | ||
buf.putString(pubkeyblob); | ||
session.write(packet); | ||
|
||
|
@@ -218,15 +210,15 @@ else if(nonrsamethoda!=null && nonrsamethoda.length>0){ | |
if(command==SSH_MSG_USERAUTH_PK_OK){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
ipkmethoda[j] + " preauth success"); | ||
ipkmethod + " preauth success"); | ||
} | ||
pkmethodsuccess=new String[]{ipkmethoda[j]}; | ||
pkmethodsuccesses=Collections.singletonList(ipkmethod); | ||
break loop3; | ||
} | ||
else if(command==SSH_MSG_USERAUTH_FAILURE){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
ipkmethoda[j] + " preauth failure"); | ||
ipkmethod + " preauth failure"); | ||
} | ||
continue loop3; | ||
} | ||
|
@@ -245,7 +237,7 @@ else if(command==SSH_MSG_USERAUTH_BANNER){ | |
//throw new JSchException("USERAUTH fail ("+command+")"); | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
ipkmethoda[j] + " preauth failure command (" + command + ")"); | ||
ipkmethod + " preauth failure command (" + command + ")"); | ||
} | ||
continue loop3; | ||
} | ||
|
@@ -265,10 +257,18 @@ else if(command==SSH_MSG_USERAUTH_BANNER){ | |
//System.err.println("UserAuthPublicKey: pubkeyblob="+pubkeyblob); | ||
|
||
if(pubkeyblob==null) continue; | ||
if(pkmethodsuccess==null) pkmethodsuccess=ipkmethoda; | ||
if(pkmethodsuccesses==null) pkmethodsuccesses=ipkmethods; | ||
|
||
loop4: | ||
for(int j=0; j<pkmethodsuccess.length; j++){ | ||
for(String pkmethodsuccess : pkmethodsuccesses){ | ||
if(not_available_pks.contains(pkmethodsuccess) && !(identity instanceof AgentIdentity)){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
pkmethodsuccess+" not available for identity "+identity.getName()); | ||
} | ||
continue loop4; | ||
} | ||
|
||
// send | ||
// byte SSH_MSG_USERAUTH_REQUEST(50) | ||
// string user name | ||
|
@@ -284,7 +284,7 @@ else if(command==SSH_MSG_USERAUTH_BANNER){ | |
buf.putString(Util.str2byte("ssh-connection")); | ||
buf.putString(Util.str2byte("publickey")); | ||
buf.putByte((byte)1); | ||
buf.putString(Util.str2byte(pkmethodsuccess[j])); | ||
buf.putString(Util.str2byte(pkmethodsuccess)); | ||
buf.putString(pubkeyblob); | ||
|
||
// byte[] tmp=new byte[buf.index-5]; | ||
|
@@ -300,11 +300,11 @@ else if(command==SSH_MSG_USERAUTH_BANNER){ | |
tmp[3]=(byte)(sidlen); | ||
System.arraycopy(sid, 0, tmp, 4, sidlen); | ||
System.arraycopy(buf.buffer, 5, tmp, 4+sidlen, buf.index-5); | ||
byte[] signature=identity.getSignature(tmp, pkmethodsuccess[j]); | ||
byte[] signature=identity.getSignature(tmp, pkmethodsuccess); | ||
if(signature==null){ // for example, too long key length. | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
pkmethodsuccess[j] + " signature failure"); | ||
pkmethodsuccess + " signature failure"); | ||
} | ||
continue loop4; | ||
} | ||
|
@@ -319,7 +319,7 @@ else if(command==SSH_MSG_USERAUTH_BANNER){ | |
if(command==SSH_MSG_USERAUTH_SUCCESS){ | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
pkmethodsuccess[j] + " auth success"); | ||
pkmethodsuccess + " auth success"); | ||
} | ||
return true; | ||
} | ||
|
@@ -345,15 +345,15 @@ else if(command==SSH_MSG_USERAUTH_FAILURE){ | |
session.auth_failures++; | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
pkmethodsuccess[j] + " auth failure"); | ||
pkmethodsuccess + " auth failure"); | ||
} | ||
break loop2; | ||
} | ||
//System.err.println("USERAUTH fail ("+command+")"); | ||
//throw new JSchException("USERAUTH fail ("+command+")"); | ||
if(JSch.getLogger().isEnabled(Logger.DEBUG)){ | ||
JSch.getLogger().log(Logger.DEBUG, | ||
pkmethodsuccess[j] + " auth failure command (" + command +")"); | ||
pkmethodsuccess + " auth failure command (" + command +")"); | ||
} | ||
break loop2; | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,16 +120,14 @@ public void testServerSigAlgs() throws Exception { | |
String expectedExtInfo = "SSH_MSG_EXT_INFO received"; | ||
String expectedServerSigAlgs = "server-sig-algs=<ssh-ed25519,ssh-rsa,ssh-dss,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521>"; | ||
String expectedOpenSSH74Bug = "OpenSSH 7.4 detected: adding rsa-sha2-256 & rsa-sha2-512 to server-sig-algs"; | ||
String expectedPubkeysBefore = String.format("After getUnavailableSignatures PubkeyAcceptedAlgorithms = %s", algos); | ||
String expectedPubkeysKnown = "PubkeyAcceptedAlgorithms in server-sig-algs = rsa-sha2-512,rsa-sha2-256,ssh-rsa"; | ||
String expectedPubkeysUnknown = "PubkeyAcceptedAlgorithms not in server-sig-algs = [email protected],[email protected],[email protected],[email protected]"; | ||
String expectedPubkeysKnown = "PubkeyAcceptedAlgorithms in server-sig-algs = \\[rsa-sha2-512, rsa-sha2-256, ssh-rsa\\]"; | ||
String expectedPubkeysUnknown = "PubkeyAcceptedAlgorithms not in server-sig-algs = \\[[email protected], [email protected], [email protected], [email protected]\\]"; | ||
String expectedPreauth = "rsa-sha2-512 preauth success"; | ||
String expectedAuth = "rsa-sha2-512 auth success"; | ||
checkLogs(expectedKex); | ||
checkLogs(expectedExtInfo); | ||
checkLogs(expectedServerSigAlgs); | ||
checkLogs(expectedOpenSSH74Bug); | ||
checkLogs(expectedPubkeysBefore); | ||
checkLogs(expectedPubkeysKnown); | ||
checkLogs(expectedPubkeysUnknown); | ||
checkLogs(expectedPreauth); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,15 +119,13 @@ public void testServerSigAlgs() throws Exception { | |
String expectedKex = "kex: host key algorithm: rsa-sha2-512"; | ||
String expectedExtInfo = "SSH_MSG_EXT_INFO received"; | ||
String expectedServerSigAlgs = "server-sig-algs=<ssh-ed25519,ssh-rsa,rsa-sha2-256,rsa-sha2-512,ssh-dss,ecdsa-sha2-nistp256,ecdsa-sha2-nistp384,ecdsa-sha2-nistp521>"; | ||
String expectedPubkeysBefore = String.format("After getUnavailableSignatures PubkeyAcceptedAlgorithms = %s", algos); | ||
String expectedPubkeysKnown = "PubkeyAcceptedAlgorithms in server-sig-algs = rsa-sha2-512,rsa-sha2-256,ssh-rsa"; | ||
String expectedPubkeysUnknown = "PubkeyAcceptedAlgorithms not in server-sig-algs = [email protected],[email protected],[email protected],[email protected]"; | ||
String expectedPubkeysKnown = "PubkeyAcceptedAlgorithms in server-sig-algs = \\[rsa-sha2-512, rsa-sha2-256, ssh-rsa\\]"; | ||
String expectedPubkeysUnknown = "PubkeyAcceptedAlgorithms not in server-sig-algs = \\[[email protected], [email protected], [email protected], [email protected]\\]"; | ||
String expectedPreauth = "rsa-sha2-512 preauth success"; | ||
String expectedAuth = "rsa-sha2-512 auth success"; | ||
checkLogs(expectedKex); | ||
checkLogs(expectedExtInfo); | ||
checkLogs(expectedServerSigAlgs); | ||
checkLogs(expectedPubkeysBefore); | ||
checkLogs(expectedPubkeysKnown); | ||
checkLogs(expectedPubkeysUnknown); | ||
checkLogs(expectedPreauth); | ||
|
@@ -145,7 +143,6 @@ public void testNoServerSigAlgs() throws Exception { | |
doSftp(session, true); | ||
|
||
String expectedKex = "kex: host key algorithm: rsa-sha2-512"; | ||
String expectedPubkeysBefore = String.format("After getUnavailableSignatures PubkeyAcceptedAlgorithms = %s", algos); | ||
String expectedPubkeysNoServerSigs = String.format("No server-sig-algs found, using PubkeyAcceptedAlgorithms = %s", algos); | ||
String expectedPreauthFail1 = "[email protected] preauth failure"; | ||
String expectedPreauthFail2 = "[email protected] preauth failure"; | ||
|
@@ -154,7 +151,6 @@ public void testNoServerSigAlgs() throws Exception { | |
String expectedPreauth = "rsa-sha2-512 preauth success"; | ||
String expectedAuth = "rsa-sha2-512 auth success"; | ||
checkLogs(expectedKex); | ||
checkLogs(expectedPubkeysBefore); | ||
checkLogs(expectedPreauthFail1); | ||
checkLogs(expectedPreauthFail2); | ||
checkLogs(expectedPreauthFail3); | ||
|
41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this represent a security loophole, since it would allow the SSH client to use less secure algorithms for keys provided by an SSH agent, even if
PubkeyAcceptedAlgorithms
explicitly disallows those algorithms? The reason I ask is that OpenSSH doesn't appear to allow users to bypassPubkeyAcceptedAlgorithms
in this manner.41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose you could view it as a bit of loophole.
However, the JSch user would have to explicitly configure JSch to utilize an ssh-agent (which involves programmatically setting the
IdentityRepository
to anAgentIdentityRepository
), unlike OpenSSH which will typically make use of an ssh-agent by default simply by the mere presence of anSSH_AUTH_SOCK
in the environment.41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the context for my question is that our bastardized implementation of JSch in TurboVNC uses an SSH agent proxy for both ssh-agent and Pageant. I am considering switching to this JSch fork, but in order to do that, I would have to address the various behavioral differences between this implementation and OpenSSH (many of which have already been addressed in our implementation or are in the process of being addressed.) This commit in particular would affect our implementation, so I wanted to understand the ramifications of it. In our implementation, we worked around the same problem by just adding certain algorithms to the default value of
PubkeyAcceptedAlgorithms
, thus allowing users to use (for instance) Ed25519 keys provided by an SSH agent but also to disable the use of that algorithm for specific hosts.41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The primary reason I added this loophole was to provide a way for JSch users to use an ssh-agent to provide support SSH key types (like ssh-ed25519) that aren't natively supported by the version of Java they are using.
I.e., if the user wasn't using Java 15+ (for native EdDSA support), nor did they have Bouncy Castle available, but they did have an ssh-agent available, they could point JSch to the ssh-agent that supports ssh-ed25519 and have it work.
41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured as much, because users have asked me to do the same thing. (Since adopting your implementation of rsa-sha2-256/rsa-sha2-512 that also implemented the
PubkeyAcceptedAlgorithms
keyword, users can no longer use ssh-ed25519 keys provided by ssh-agent unless they explicitly addssh-ed25519
toPubkeyAcceptedAlgorithms
.) However, I am really hesitant to do something that OpenSSH explicitly disallows, because our implementation is much more akin to OpenSSH in that it will use an SSH agent by default if one is available.41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can understand that perspective and am open to suggestions on how we could address it.
It could be difficult, because we want to be careful to not have JSch attempt to try and load a native Signature class that wouldn't work (since that would throw an exception at a really awkward time to deal with, hence all the logic with pruning the unavailable signature algorithms), but at the same time allow a user to coerce JSch to support an ssh key type (like ssh-ed25519) that it natively doesn't support.
41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, a solution that would be cleaner than either your solution or my solution would be to adopt the
+
/-
syntax forPubkeyAcceptedAlgorithms
as well as respond to both a global as well as a host-specific specification of that keyword. Then it would be a simple matter of addingPubkeyAcceptedAlgorithms=+ssh-ed25519
to the top of ~/.ssh/config if a user wants to use that key type (or any other key type that JSch doesn't explicitly support) with an SSH agent. That solution would be consistent with OpenSSH's behavior.41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I believe I misremembered, and this all working correctly and obeying the
PubkeyAcceptedAlgorithms
settings, and there is no loophole.I just tested locally using an ssh-agent that had an
ssh-dss
key loaded, but withPubkeyAcceptedAlgorithms
configured such that it didn't includessh-dss
, and JSch didn't use that key from the ssh-agent.Are you able to somehow coerce JSch into using a key from an ssh-agent who's type isn't allowed by
PubkeyAcceptedAlgorithms
?41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't gotten as far as testing it. At this point, I was just considering whether to adopt this commit in our implementation. But if this commit doesn't allow bypassing
PubkeyAcceptedAlgorithms
for agent-provided keys, then what is its purpose?41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, support for the
+/-
(prepend,append) should be supported already as part of the0.1.72
release (see 41a6749).41d33dc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming you are referring to this code?
If so, what that is doing is pruning out a non-agent Identity instance that happens to be of a signature type that JSch doesn't natively support (i.e., it's an ssh-ed25519 key but we're running on Java 8 and without Bouncy Castle).
But if it is an ssh-agent backed
AgentIdentity
, to go ahead and proceed (since JSch doesn't need to natively support the algorithm).By the time we reach here, signature algorithms that aren't explicitly allowed via
PubkeyAcceptedAlgorithms
have already been pruned.You can see earlier we build a list of algorithms as
ipkmethods
and bail if the Identity's algorithm (identity.getAlgName()
) isn't found inrsamethods
&nonrsamethods
.The
rsamethods
&nonrsamethods
are built earlier like this frompkmethods
:And
pkmethods
is an argument to_start()
that is built earlier in thestart()
method like this, based directly uponPubkeyAcceptedAlgorithms
: