Host keyword in ssh config should be case-insensitive

[jsundmannorberg]

Hi,

According to man ssh_config keywords should be case-insensitive and arguments should be case-sensitive.

However, it appears that the "Host" keyword is case-sensitive in the current implementation (on line 127 in OpenSSHConfig.java we have a case-sensitive equality check if(key_value[0].equals("Host")){ )

We have seen cases where this leads to confusing bugs when users assume that the config keywords are case-insensitive, so it would probably be good to change the code to match the specification.

Regards,
Johan Sundman Norberg

[norrisjeremy]

Hi @jsundmannorberg,

Are you mostly concerned with treatment of the "Host" keyword itself? Or is handling other keywords important as well?

While fixing the instance for the "Host" keyword you pointed out is a simple one-line change, there are other ssh_config type keywords that are are used in other classes (like JSch.java, Session.java, etc.).
This would likely require more signficant changes, such as converting the internal config Map from using Hashtable<String, String> to TreeMap<String, String> with a String.CASE_INSENSITIVE_ORDER type Comparator<String>, which would in turn require careful analysis of internal locking, since TreeMap isn't thread-safe like Hashtable.

Thanks,
Jeremy

[jsundmannorberg]

Hi @norrisjeremy,

The Host keyword is the one we have encountered. In this case it is made confusing by the fact that other keywords in the same class are case-insensitive (the find method converts all keys to uppercase before searching).

I haven't really looked at how other keywords are treated, but at least they have not caused any issues for us so far.

Regards,
Johan Sundman Norberg

[norrisjeremy]

Hi @jsundmannorberg,

The new 0.1.68 release includes a fix to make the "Host" keyword case-insensitive.

Thanks,
Jeremy

[jsundmannorberg]

Hi @norrisjeremy,

Thanks! Unfortunately it appears that there is a bug in the new version. Now it checks that key_value[0].toUpperCase().equals("Host") which will never match.

Regards,
Johan

[norrisjeremy]

Hi @jsundmannorberg,

Well that is an embarrassing blunder. Would something like this look correct to you?

diff --git a/src/main/java/com/jcraft/jsch/OpenSSHConfig.java b/src/main/java/com/jcraft/jsch/OpenSSHConfig.java
index a24eb56..8068784 100644
--- a/src/main/java/com/jcraft/jsch/OpenSSHConfig.java
+++ b/src/main/java/com/jcraft/jsch/OpenSSHConfig.java
@@ -124,7 +124,7 @@ public class OpenSSHConfig implements ConfigRepository {
       if(key_value.length <= 1)
         continue;
 
-      if(key_value[0].toUpperCase().equals("Host")){
+      if(key_value[0].equalsIgnoreCase("Host")){
         config.put(host, kv);
         hosts.addElement(host);
         host = key_value[1];

Thanks,
Jeremy

[jsundmannorberg]

Hi @norrisjeremy,

That looks like it should work. This kind of bug is surprisingly hard to catch, no worries 😄

Thanks,
Johan

[norrisjeremy]

Hi @jsundmannorberg,

I believe this issued should now hopefully be fully resolved now in the new 0.1.69 release.

Thanks,
Jeremy

[jsundmannorberg]

The new version works perfectly. Thanks!