Add support for verifying multiple host entries (Fixes #405)

README.adoc

@@ -107,6 +107,9 @@ Google Group: http://groups.google.com/group/sshj-users
 Fork away!
 
 == Release history
+SSHJ 0.24.0 (2018-??-??)::
+* Added support for hmac-ripemd160
+
 SSHJ 0.23.0 (2017-10-13)::
 * Merged https://github.com/hierynomus/sshj/pulls/372[#372]: Upgrade to 'net.i2p.crypto:eddsa:0.2.0'
 * Fixed https://github.com/hierynomus/sshj/issues/355[#355] and https://github.com/hierynomus/sshj/issues/354[#354]: Correctly decode signature bytes

src/main/java/net/schmizz/sshj/common/Buffer.java

@@ -460,10 +460,13 @@ public T putSensitiveString(char[] str) {
 
     public PublicKey readPublicKey()
             throws BufferException {
+        KeyType keyType = KeyType.fromString(readString());
         try {
-            return KeyType.fromString(readString()).readPubKeyFromBuffer(this);
+            return keyType.readPubKeyFromBuffer(this);
         } catch (GeneralSecurityException e) {
             throw new SSHRuntimeException(e);
+        } catch (UnsupportedOperationException uoe) {
+            throw new BufferException("Could not decode keytype " + keyType);
         }
     }
 

src/main/java/net/schmizz/sshj/transport/verification/ConsoleKnownHostsVerifier.java

@@ -41,7 +41,7 @@ public ConsoleKnownHostsVerifier(File khFile, Console console)
     protected boolean hostKeyUnverifiableAction(String hostname, PublicKey key) {
         final KeyType type = KeyType.fromKey(key);
         console.printf("The authenticity of host '%s' can't be established.\n" +
-                               "%s key fingerprint is %s.\n", hostname, type, SecurityUtils.getFingerprint(key));
+                "%s key fingerprint is %s.\n", hostname, type, SecurityUtils.getFingerprint(key));
         String response = console.readLine("Are you sure you want to continue connecting (yes/no)? ");
         while (!(response.equalsIgnoreCase(YES) || response.equalsIgnoreCase(NO))) {
             response = console.readLine("Please explicitly enter yes/no: ");
@@ -60,7 +60,7 @@ protected boolean hostKeyUnverifiableAction(String hostname, PublicKey key) {
     }
 
     @Override
-    protected boolean hostKeyChangedAction(KnownHostEntry entry, String hostname, PublicKey key) {
+    protected boolean hostKeyChangedAction(String hostname, PublicKey key) {
         final KeyType type = KeyType.fromKey(key);
         final String fp = SecurityUtils.getFingerprint(key);
         final String path = getFile().getAbsolutePath();

src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java

@@ -87,14 +87,23 @@ public boolean verify(final String hostname, final int port, final PublicKey key
 
         final String adjustedHostname = (port != 22) ? "[" + hostname + "]:" + port : hostname;
 
+        boolean foundApplicableHostEntry = false;
         for (KnownHostEntry e : entries) {
             try {
-                if (e.appliesTo(type, adjustedHostname))
-                    return e.verify(key) || hostKeyChangedAction(e, adjustedHostname, key);
+                if (e.appliesTo(type, adjustedHostname)) {
+                    foundApplicableHostEntry = true;
+                    if (e.verify(key)) {
+                        return true;
+                    }
+                }
             } catch (IOException ioe) {
                 log.error("Error with {}: {}", e, ioe);
                 return false;
             }
+
+        }
+        if (foundApplicableHostEntry) {
+            return hostKeyChangedAction(adjustedHostname, key);
         }
 
         return hostKeyUnverifiableAction(adjustedHostname, key);
@@ -104,7 +113,7 @@ protected boolean hostKeyUnverifiableAction(String hostname, PublicKey key) {
         return false;
     }
 
-    protected boolean hostKeyChangedAction(KnownHostEntry entry, String hostname, PublicKey key) {
+    protected boolean hostKeyChangedAction(String hostname, PublicKey key) {
         log.warn("Host key for `{}` has changed!", hostname);
         return false;
     }
@@ -199,7 +208,7 @@ public KnownHostEntry parseEntry(String line)
             }
             if(split.length < 3) {
                 log.error("Error reading entry `{}`", line);
-                return null;
+                return new BadHostEntry(line);
             }
             final String hostnames = split[i++];
             final String sType = split[i++];
@@ -209,7 +218,13 @@ public KnownHostEntry parseEntry(String line)
 
             if (type != KeyType.UNKNOWN) {
                 final String sKey = split[i++];
-                key = new Buffer.PlainBuffer(Base64.decode(sKey)).readPublicKey();
+                try {
+                    byte[] keyBytes = Base64.decode(sKey);
+                    key = new Buffer.PlainBuffer(keyBytes).readPublicKey();
+                } catch (IOException ioe) {
+                    log.warn("Error decoding Base64 key bytes", ioe);
+                    return new BadHostEntry(line);
+                }
             } else if (isBits(sType)) {
                 type = KeyType.RSA;
                 // int bits = Integer.valueOf(sType);
@@ -220,11 +235,11 @@ public KnownHostEntry parseEntry(String line)
                     key = keyFactory.generatePublic(new RSAPublicKeySpec(n, e));
                 } catch (Exception ex) {
                     log.error("Error reading entry `{}`, could not create key", line, ex);
-                    return null;
+                    return new BadHostEntry(line);
                 }
             } else {
                 log.error("Error reading entry `{}`, could not determine type", line);
-                return null;
+                return new BadHostEntry(line);
             }
 
             return new HostEntry(marker, hostnames, type, key);
@@ -364,6 +379,44 @@ protected String getHostPart() {
         }
     }
 
+    public static class BadHostEntry implements KnownHostEntry {
+        private String line;
+
+        public BadHostEntry(String line) {
+            this.line = line;
+        }
+
+        @Override
+        public KeyType getType() {
+            return KeyType.UNKNOWN;
+        }
+
+        @Override
+        public String getFingerprint() {
+            return null;
+        }
+
+        @Override
+        public boolean appliesTo(String host) throws IOException {
+            return false;
+        }
+
+        @Override
+        public boolean appliesTo(KeyType type, String host) throws IOException {
+            return false;
+        }
+
+        @Override
+        public boolean verify(PublicKey key) throws IOException {
+            return false;
+        }
+
+        @Override
+        public String getLine() {
+            return line;
+        }
+    }
+
     public enum Marker {
         CA_CERT("@cert-authority"),
         REVOKED("@revoked");

src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy

@@ -0,0 +1,114 @@
+/*
+ * Copyright (C)2009 - SSHJ Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.hierynomus.sshj.transport.verification
+
+import net.schmizz.sshj.common.Base64
+import net.schmizz.sshj.common.Buffer
+import net.schmizz.sshj.transport.verification.OpenSSHKnownHosts
+import org.junit.Rule
+import org.junit.rules.TemporaryFolder
+import spock.lang.Specification
+import spock.lang.Unroll
+
+import static org.hamcrest.CoreMatchers.equalTo
+import static org.hamcrest.CoreMatchers.instanceOf
+import static org.junit.Assert.assertThat
+import static org.junit.Assert.assertThat
+
+class OpenSSHKnownHostsSpec extends Specification {
+
+    @Rule
+    TemporaryFolder folder
+
+    def "should check all host entries for key"() {
+        given:
+        def f = knownHosts("""
+host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCiYp2IDgzDFhl8T4TRLIhEljvEixz1YN0XWh4dYh0REGK9T4QKiyb28EztPMdcOtz1uyX5rUGYXX9hj99S4SiU=
+host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck=
+""")
+        def pk = new Buffer.PlainBuffer(Base64.decode("AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck=")).readPublicKey()
+        when:
+        def knownhosts = new OpenSSHKnownHosts(f)
+
+        then:
+        knownhosts.verify("host1", 22, pk)
+    }
+
+    def "should not fail on bad base64 entry"() {
+        given:
+        def f = knownHosts("""
+host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTIDgzDFhl8T4TRLIhEljvEixz1YN0XWh4dYh0REGK9T4QKiyb28EztPMdcOtz1uyX5rUGYXX9hj99S4SiU=
+host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck=
+""")
+        def pk = new Buffer.PlainBuffer(Base64.decode("AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBLTjA7hduYGmvV9smEEsIdGLdghSPD7kL8QarIIOkeXmBh+LTtT/T1K+Ot/rmXCZsP8hoUXxbvN+Tks440Ci0ck=")).readPublicKey()
+        when:
+        def knownhosts = new OpenSSHKnownHosts(f)
+
+        then:
+        knownhosts.verify("host1", 22, pk)
+    }
+
+    def "should mark bad line and not fail"() {
+        given:
+        def f = knownHosts("M36Lo+Ik5ukNugvvoNFlpnyiHMmtKxt3FpyEfYuryXjNqMNWHn/ARVnpUIl5jRLTB7WBzyLYMG7X5nuoFL9zYqKGtHxChbDunxMVbspw5WXI9VN+qxcLwmITmpEvI9ApyS/Ox2ZyN7zw==\n")
+
+        when:
+        def knownhosts = new OpenSSHKnownHosts(f)
+
+        then:
+        knownhosts.entries().size() == 1
+        knownhosts.entries().get(0) instanceof OpenSSHKnownHosts.BadHostEntry
+    }
+
+    @Unroll
+    def "should add comment for #type line"() {
+        given:
+        def f = knownHosts(s)
+
+        when:
+        def knownHosts = new OpenSSHKnownHosts(f)
+
+        then:
+        knownHosts.entries().size() == 1
+        knownHosts.entries().get(0) instanceof OpenSSHKnownHosts.CommentEntry
+
+        where:
+        type << ["empty", "comment"]
+        s << ["\n", "#comment\n"]
+    }
+
+    @Unroll
+    def "should match any host name from multi-host line"() {
+        given:
+        def f = knownHosts("schmizz.net,69.163.155.180 ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ==")
+        def pk = new Buffer.PlainBuffer(Base64.decode("AAAAB3NzaC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ==")).readPublicKey()
+
+        when:
+        def knownHosts = new OpenSSHKnownHosts(f)
+
+        then:
+        knownHosts.verify(h, 22, pk)
+
+        where:
+        h << ["schmizz.net", "69.163.155.180"]
+    }
+
+    def knownHosts(String s) {
+        def f = folder.newFile("known_hosts")
+        f.write(s)
+        return f
+    }
+}

src/test/resources/known_hosts

@@ -1,4 +1,11 @@
+# Line 1: bogus line, with invalid base64 string
+# Line 2: syntactically correct line, but incorrect base64 string
+# Line 3: correct line (that should match) Above we have a plain line
+# Line 4: hashed line
+# Line 5: v1 line
+# Comment lines like these should be ignored
+schmizz.net,69.163.155.180 ssh-rsa #BOGUS#aC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ==
+schmizz.net,69.163.155.180 ssh-rsa AAAAAAAAAC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ==
 schmizz.net,69.163.155.180 ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA6P9Hlwdahh250jGZYKg2snRq2j2lFJVdKSHyxqbJiVy9VX9gTkN3K2MD48qyrYLYOyGs3vTttyUk+cK++JMzURWsrP4piby7LpeOT+3Iq8CQNj4gXZdcH9w15Vuk2qS11at6IsQPVHpKD9HGg9//EFUccI/4w06k4XXLm/IxOGUwj6I2AeWmEOL3aDi+fe07TTosSdLUD6INtR0cyKsg0zC7Da24ixoShT8Oy3x2MpR7CY3PQ1pUVmvPkr79VeA+4qV9F1JM09WdboAMZgWQZ+XrbtuBlGsyhpUHSCQOya+kOJ+bYryS+U7A+6nmTW3C9FX4FgFqTF89UHOC7V0zZQ==
-# Above we have a plain line, Below we have a hashed line, Last is a v1 line, This is a garbage line.
 |1|dy7xSefq6NmJms6AzANG3w45W28=|SSCTlHs4pZbc2uaRoPvjyEAHE1g= ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAu64GJcCkdtckPGt8uKTyhG1ShT1Np1kh10eE49imQ4Nh9Y/IrSPzDtYUAazQ88ABc2NffuOKkdn2qtUwZ1ulfcdNfN3oTim3BiVHqa041pKG0L+onQe8Bo+CaG5KBLy/C24eNGM9EcfQvDQOnq1eD3lnR/l8fFckldzjfxZgar0yT9Bb3pwp50oN+1wSEINJEHOgMIW8kZBQmyNr/B+b7yX+Y1s1vuYIP/i4WimCVmkdi9G87Ga8w7GxKalRD2QOG6Xms2YWRQDN6M/MOn4tda3EKolbWkctEWcQf/PcVJffTH4Wv5f0RjVyrQv4ha4FZcNAv6RkRd9WkiCsiTKioQ==
 test.com,1.1.1.1 2048 35 22017496617994656680820635966392838863613340434802393112245951008866692373218840197754553998457793202561151141246686162285550121243768846314646395880632789308110750881198697743542374668273149584280424505890648953477691795864456749782348425425954366277600319096366690719901119774784695056100331902394094537054256611668966698242432417382422091372756244612839068092471592121759862971414741954991375710930168229171638843329213652899594987626853020377726482288618521941129157643483558764875338089684351824791983007780922947554898825663693324944982594850256042689880090306493029526546183035567296830604572253312294059766327