GH-827: known_hosts: skip and log invalid lines

Previous code would throw an exception on the first error encountered.

Author: tomaswolf

CHANGES.md

@@ -31,6 +31,7 @@
 
 * [GH-807](https://github.com/apache/mina-sshd/issues/807) Handle "verified" flag for sk-* keys
 * [GH-809](https://github.com/apache/mina-sshd/pull/809) Fix server-side authentication for FIDO/U2F sk-* keys with flags in `authorized_keys`
+* [GH-827](https://github.com/apache/mina-sshd/pull/827) Don't fail on invalid `known_hosts` lines; log and skip them
 
 ## New Features
 

sshd-common/src/main/java/org/apache/sshd/client/config/hosts/KnownHostEntry.java

@@ -24,7 +24,6 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
-import java.io.StreamCorruptedException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -34,13 +33,14 @@
 import java.util.Collections;
 import java.util.List;
 
-import org.apache.sshd.common.config.ConfigFileReaderSupport;
 import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
 import org.apache.sshd.common.config.keys.PublicKeyEntry;
 import org.apache.sshd.common.util.GenericUtils;
 import org.apache.sshd.common.util.ValidateUtils;
 import org.apache.sshd.common.util.io.input.NoCloseInputStream;
 import org.apache.sshd.common.util.io.input.NoCloseReader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Contains a representation of an entry in the <code>known_hosts</code> file
@@ -59,6 +59,8 @@ public class KnownHostEntry extends HostPatternsHolder {
      */
     public static final String STD_HOSTS_FILENAME = "known_hosts";
 
+    private static final Logger LOG = LoggerFactory.getLogger(KnownHostEntry.class);
+
     private static final class LazyDefaultConfigFileHolder {
         private static final Path HOSTS_FILE = PublicKeyEntry.getDefaultKeysFolderPath().resolve(STD_HOSTS_FILENAME);
 
@@ -183,7 +185,7 @@ public static List<KnownHostEntry> readKnownHostEntries(BufferedReader rdr) thro
                 continue;
             }
 
-            int pos = line.indexOf(ConfigFileReaderSupport.COMMENT_CHAR);
+            int pos = line.indexOf(PublicKeyEntry.COMMENT_CHAR);
             if (pos == 0) {
                 continue;
             }
@@ -203,9 +205,8 @@ public static List<KnownHostEntry> readKnownHostEntries(BufferedReader rdr) thro
                     entries = new ArrayList<>();
                 }
                 entries.add(entry);
-            } catch (RuntimeException | Error e) { // TODO consider consulting a user callback
-                throw new StreamCorruptedException("Failed (" + e.getClass().getSimpleName() + ") to parse line #"
-                                                   + lineNumber + " '" + line + "': " + e.getMessage());
+            } catch (RuntimeException e) { // TODO consider consulting a user callback
+                LOG.warn("Invalid known_hosts line #" + lineNumber + " '" + line + "': " + e.getMessage());
             }
         }
 
@@ -217,13 +218,16 @@ public static List<KnownHostEntry> readKnownHostEntries(BufferedReader rdr) thro
     }
 
     public static KnownHostEntry parseKnownHostEntry(String line) {
-        return parseKnownHostEntry(GenericUtils.isEmpty(line) ? null : new KnownHostEntry(), line);
+        return GenericUtils.isEmpty(line) ? null : parseKnownHostEntry(new KnownHostEntry(), line);
     }
 
     public static <E extends KnownHostEntry> E parseKnownHostEntry(E entry, String data) {
+        if (data == null) {
+            return null;
+        }
         String line = GenericUtils.replaceWhitespaceAndTrim(data);
         if (GenericUtils.isEmpty(line) || (line.charAt(0) == PublicKeyEntry.COMMENT_CHAR)) {
-            return entry;
+            return null;
         }
 
         entry.setConfigLine(line);
@@ -254,6 +258,9 @@ public static <E extends KnownHostEntry> E parseKnownHostEntry(E entry, String d
         }
         AuthorizedKeyEntry key = PublicKeyEntry.parsePublicKeyEntry(new AuthorizedKeyEntry(),
                 ValidateUtils.checkNotNullAndNotEmpty(line, "No valid key entry recovered from line=%s", data));
+        if (key == null) {
+            return null;
+        }
         entry.setKeyEntry(key);
         return entry;
     }

sshd-core/src/test/java/org/apache/sshd/client/keyverifier/KnownHostsUnsupportedKeysTest.java

@@ -70,6 +70,23 @@ void unknownExistingKey() throws Exception {
         assertTrue(invokeVerifier(verifier, new SshdSocketAddress("127.0.0.1", 2222), key));
     }
 
+    @Test
+    void invalidLineIgnored() throws Exception {
+        Path knownHosts = tmp.resolve("known_hosts");
+        String entry
+                = "[127.0.0.1]:2222 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCbZVVpqEHGLNWMqMeyU1VbWb91XteoamVcgpy4yxNVbZffb5IDdbo1ons/y9KAhcub6LZeLrvXzVUZbXCZiUkg=";
+        List<String> lines = new ArrayList<>();
+        lines.add(entry + entry);
+        lines.add(entry);
+        Files.write(knownHosts, lines);
+        KnownHostsServerKeyVerifier verifier = new KnownHostsServerKeyVerifier(RejectAllServerKeyVerifier.INSTANCE, knownHosts);
+        KnownHostEntry knownHost = KnownHostEntry.parseKnownHostEntry(lines.get(1));
+        AuthorizedKeyEntry keyEntry = knownHost.getKeyEntry();
+        assertNotNull(keyEntry);
+        PublicKey key = keyEntry.resolvePublicKey(null, PublicKeyEntryResolver.FAILING);
+        assertTrue(invokeVerifier(verifier, new SshdSocketAddress("127.0.0.1", 2222), key));
+    }
+
     @Test
     void unknownNewKey() throws Exception {
         KeyPair kp = CommonTestSupportUtils.generateKeyPair(KeyUtils.RSA_ALGORITHM, 1024);