Discussion:
[JSch-users] Deadlock in KnownHosts
Vladimir Kvashin
2016-02-10 08:38:24 UTC
Permalink
Hi,

Recently a classical deadlock was found in KnownHosts.

It is described in a bug that was filed against NetBeans
https://netbeans.org/bugzilla/show_bug.cgi?id=257800
but the reason is obviously in jsch itself.

The questions are:
- is this problem already known?
- are there any plans to fix it?
- does jsch accept contributions? if yes, I can try fixing this and
propose a patch

The below is jsut a copy-paste of some info from the bug:

"SFTP" thread
- has locked KnownHosts instance
- then tries to lock KnownHosts.pool

"ConnectionManager queue" thread
- has locked KnownHosts.pool
- then tries to lock KnownHosts instance

Below are minimal thread fgarments that illustrate this (note that there are no NB code in these stacks):

"ConnectionManager queue" Id=371 in BLOCKED on lock=***@24a91105
owned by SFTP: : Uploading /home/kiss/netbeans-8.1/cnd/bin/Linux-x86_64/rfs_controller ***@11.44.33.55 <mailto:***@11.44.33.55>:/var/tmp/dlight_ikiss/6b88f630/tools/Linux-x86_64/rfs_controller Id=388
at com.jcraft.jsch.KnownHosts.getHMACSHA1(KnownHosts.java:486)
at com.jcraft.jsch.KnownHosts.access$000(KnownHosts.java:35)
at com.jcraft.jsch.KnownHosts$HashedHostKey.isMatched(KnownHosts.java:540)
at com.jcraft.jsch.KnownHosts.getHostKey(KnownHosts.java:361)
- locked ***@79ee48d
at com.jcraft.jsch.Session.checkHost(Session.java:809)
at com.jcraft.jsch.Session.connect(Session.java:342)

"SFTP: : Uploading /home/kiss/netbeans-8.1/cnd/bin/Linux-x86_64/rfs_controller ***@11.44.33.55 <mailto:***@11.44.33.55>:/var/tmp/dlight_ikiss/6b88f630/tools/Linux-x86_64/rfs_controller" Id=388 in BLOCKED on lock=***@79ee48d
owned by ConnectionManager queue Id=371
at com.jcraft.jsch.KnownHosts.check(KnownHosts.java:263)
at com.jcraft.jsch.Session.checkHost(Session.java:732)
- locked ***@24a91105
at com.jcraft.jsch.Session.connect(Session.java:342)


Thank you!
Vladimir
Lothar Kimmeringer
2016-02-11 12:13:50 UTC
Permalink
Hi,
Post by Vladimir Kvashin
- does jsch accept contributions? if yes, I can try fixing this
and propose a patch
I asked this question as well in the past and have never
received an answer.
Post by Vladimir Kvashin
- are there any plans to fix it?
FWIW, here is a patch, that should fix the issue. The
Digester instance in KnownHost is used in a global way
and needed quite some synchronization. I changed it to
a new instance every time the digester is retrieved,
making the synchronization as a whole unnecessary.


Cheers, Lothar


--------------- snip
### Eclipse Workspace Patch 1.0
#P JSCH_EBD
Index: src/main/java/com/jcraft/jsch/KnownHosts.java
===================================================================
--- src/main/java/com/jcraft/jsch/KnownHosts.java (revision 24760)
+++ src/main/java/com/jcraft/jsch/KnownHosts.java (working copy)
@@ -39,7 +39,7 @@
private String known_hosts=null;
private java.util.Vector pool=null;

- private MAC hmacsha1=null;
+ private Class hmacsha1_class = null;

KnownHosts(JSch jsch){
super();
@@ -482,17 +482,18 @@
return hosts;
}

- private synchronized MAC getHMACSHA1(){
- if(hmacsha1==null){
+ private MAC getHMACSHA1(){
try{
- Class c=Class.forName(jsch.getConfig("hmac-sha1"));
- hmacsha1=(MAC)(c.newInstance());
+ if (hmacsha1_class == null){
+ String hmacsha1_classname = JSch.getConfig("hmac-sha1");
+ hmacsha1_class = Class.forName(hmacsha1_classname);
+ }
+ return (MAC) hmacsha1_class.newInstance();
}
catch(Exception e){
System.err.println("hmacsha1: "+e);
}
- }
- return hmacsha1;
+ return null;
}

HostKey createHashedHostKey(String host, byte[]key) throws JSchException {
@@ -539,14 +540,12 @@
}
MAC macsha1=getHMACSHA1();
try{
- synchronized(macsha1){
macsha1.init(salt);
byte[] foo=Util.str2byte(_host);
macsha1.update(foo, 0, foo.length);
byte[] bar=new byte[macsha1.getBlockSize()];
macsha1.doFinal(bar, 0);
return Util.array_equals(hash, bar);
- }
}
catch(Exception e){
System.out.println(e);
@@ -570,13 +569,11 @@
}
}
try{
- synchronized(macsha1){
macsha1.init(salt);
byte[] foo=Util.str2byte(host);
macsha1.update(foo, 0, foo.length);
hash=new byte[macsha1.getBlockSize()];
macsha1.doFinal(hash, 0);
- }
}
catch(Exception e){
}
--------------- snip

Loading...