support get new format fingerprint from HostKey

[eshizhan]

The PR make HostKey.getFingerPrint method support new format fingerprint, and keeping original method return old format.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[norrisjeremy]

Hi @eshizhan,

What exactly do you mean by "new format fingerprint"?

Thanks,
Jeremy

[eshizhan]

@norrisjeremy ref: http:https://www.openssh.com/txt/release-6.8

 Add FingerprintHash option to ssh(1) and sshd(8), and equivalent
 command-line flags to the other tools to control algorithm used
 for key fingerprints. The default changes from MD5 to SHA256 and
 format from hex to base64.

 Fingerprints now have the hash algorithm prepended. An example of
 the new format: SHA256:mVPwvezndPv/ARoIadVY98vAC0g+P/5633yTC4d/wXE
 Please note that visual host keys will also be different.

[eshizhan]

The old fingerprint format like: 9d:38:5b:83:a9:17:52:92:56:1a:5e:c4:d4:81:8e:0a:ca:51:a2:64:f1:74:20:11:2e:f8:8a:c3:a1:39:49:8f, the new fingerprint format like: SHA256:nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8.
The Util.getFingerPrint in original code is support two format, just HostKey.getFingerPrint using old format before.

[norrisjeremy]

Hi @eshizhan,

I believe the only effect that your change has is to force the fingerprint has to prepend the MD5: or SHA256: string to the front of the string hash that it returns (depending upon which algorithm is configured): otherwise nothing else is different.

I'm not really sure that is thus worthwhile to introduce an unwieldy method overload (with a confusing boolean newFormat argument) to the public API.

Can you provide more details as to how this change will solve a functional issue you are having?

Thanks,
Jeremy

[norrisjeremy]

Hi @eshizhan,

What would you think of something like this instead, to just force HostKey.getFingerPrint() into always using the more modern format introduced in OpenSSH 6.8?

diff --git a/src/main/java/com/jcraft/jsch/HostKey.java b/src/main/java/com/jcraft/jsch/HostKey.java
index 0f9922b..112478a 100644
--- a/src/main/java/com/jcraft/jsch/HostKey.java
+++ b/src/main/java/com/jcraft/jsch/HostKey.java
@@ -128,7 +128,7 @@ public class HostKey {
         jsch.getInstanceLogger().log(Logger.ERROR, "getFingerPrint: " + e.getMessage(), e);
       }
     }
-    return Util.getFingerPrint(hash, key, false, true);
+    return Util.getFingerPrint(hash, key, true, false);
   }
 
   public String getComment() {
diff --git a/src/test/java/com/jcraft/jsch/KnownHostsTest.java b/src/test/java/com/jcraft/jsch/KnownHostsTest.java
index c74df31..fb7be14 100644
--- a/src/test/java/com/jcraft/jsch/KnownHostsTest.java
+++ b/src/test/java/com/jcraft/jsch/KnownHostsTest.java
@@ -454,9 +454,8 @@ class KnownHostsTest {
         "|1|AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=|mie6rcAf1aPGk6d+HxnkpvO4HaOAH/Y6YWegs+Xog/s=",
         hhk.getHost(), "host mismatch");
     assertEquals("", hhk.getMarker(), "marker mismatch");
-    assertEquals(
-        "9c:fb:7f:99:79:01:6d:46:68:87:39:15:4f:f5:cc:9d:71:7a:8b:5a:4a:c1:c7:4b:9c:20:a5:91:c2:6a:ff:5a",
-        hhk.getFingerPrint(jsch));
+    assertEquals("SHA256:nPt/mXkBbUZohzkVT/XMnXF6i1pKwcdLnCClkcJq/1o", hhk.getFingerPrint(jsch),
+        "fingerprint mismatch");
     assertEquals(null, hhk.getComment(), "comment mismatch");
     assertEquals("ICAgIHNzaC1yc2E=", hhk.getKey(), "key mismatch");
     assertEquals("ssh-rsa", hhk.getType(), "type mismatch");
@@ -1006,9 +1005,8 @@ class KnownHostsTest {
       String expectedType, String expectedComment) {
     assertEquals(expctedHost, hhk.getHost(), "host mismatch");
     assertEquals(expectedMarker, hhk.getMarker(), "marker mismatch");
-    assertEquals(
-        "1e:b5:70:92:65:6e:6a:f9:d6:7a:a9:43:00:40:a2:e7:c8:51:35:df:ee:60:19:b7:4b:18:1d:eb:46:48:28:4b",
-        hhk.getFingerPrint(jsch));
+    assertEquals("SHA256:HrVwkmVuavnWeqlDAECi58hRNd/uYBm3Sxgd60ZIKEs", hhk.getFingerPrint(jsch),
+        "fingerprint mismatch");
     assertEquals(expectedComment, hhk.getComment(), "comment mismatch");
     assertEquals("ICAgIHNzaC1kc2E=", hhk.getKey(), "key mismatch");
     assertEquals(expectedType, hhk.getType(), "type mismatch");
@@ -1022,9 +1020,8 @@ class KnownHostsTest {
     assertEquals("|1|AAECAwQFBgcICQoLDA0ODxAREhM=|/pE4peaossRYDRp6bEWa348eFLI=", hhk.getHost(),
         "host mismatch");
     assertEquals(expectedMarker, hhk.getMarker(), "marker mismatch");
-    assertEquals(
-        "1e:b5:70:92:65:6e:6a:f9:d6:7a:a9:43:00:40:a2:e7:c8:51:35:df:ee:60:19:b7:4b:18:1d:eb:46:48:28:4b",
-        hhk.getFingerPrint(jsch));
+    assertEquals("SHA256:HrVwkmVuavnWeqlDAECi58hRNd/uYBm3Sxgd60ZIKEs", hhk.getFingerPrint(jsch),
+        "fingerprint mismatch");
     assertEquals(expectedComment, hhk.getComment(), "comment mismatch");
     assertEquals("ICAgIHNzaC1kc2E=", hhk.getKey(), "key mismatch");
     assertEquals(expectedType, hhk.getType(), "type mismatch");
@@ -1054,9 +1051,8 @@ class KnownHostsTest {
     assertEquals(1, keys.length, "1 key expected");
     HostKey key = keys[0];
     assertEquals("some comment", key.getComment(), "comment mismatch");
-    assertEquals(
-        "9d:38:5b:83:a9:17:52:92:56:1a:5e:c4:d4:81:8e:0a:ca:51:a2:64:f1:74:20:11:2e:f8:8a:c3:a1:39:49:8f",
-        key.getFingerPrint(jsch), "fingerprint mismatch");
+    assertEquals("SHA256:nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8", key.getFingerPrint(jsch),
+        "fingerprint mismatch");
     assertEquals(expectedHostResult, key.getHost(), "host mismatch");
     assertEquals(rsaKey, key.getKey(), "key mismatch");
     assertEquals(expectedMarker, key.getMarker(), "marker mismatch");

Thanks,
Jeremy

[eshizhan]

@norrisjeremy Thank you for the review.
I have considered two use cases: 1. Comparing whether the HostKey hash value is correct. 2. Logging to help with auditing and troubleshooting. These are also the functions of the fingerprint.

Initially, I had the same idea as you, to enforce the use of the new fingerprint format. But considering that this library is a well-known library in Java and is used by many other libraries and applications, I worry that forcing the transition to the new format will cause incompatibility. For example, someone might persist the HostKey hash to be compared in the old format.

This new format has actually been released for a long time. Switching to the new format is definitely a good choice. I chose the clumsy way of overloading, simply because of concerns about compatibility.

[norrisjeremy]

Hi @eshizhan,

Yes, considering how long ago OpenSSH switched to the new format (6.8 was released 9 years ago), I think I'd be more inclined to just change the format of the existing HostKey.getFingerPrint() method to match instead of introducing a method overload, but let me think on it some more.
@mwiede What's your opinion on this?

Thanks,
Jeremy