diff --git a/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java b/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java index 3fbd1401..e400b8c0 100644 --- a/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java +++ b/src/main/java/net/schmizz/sshj/transport/verification/OpenSSHKnownHosts.java @@ -199,14 +199,21 @@ public class OpenSSHKnownHosts return new CommentEntry(line); } - final String[] split = line.split(" "); + final String[] split = line.split("\\s+"); + if(split.length < 3) { + log.error("Error reading entry `{}`", line); + return new BadHostEntry(line); + } int i = 0; + if (split[i].isEmpty()) { + i++; + } final Marker marker = Marker.fromString(split[i]); if (marker != null) { i++; } - if(split.length < 3) { + if(split.length < i + 3) { log.error("Error reading entry `{}`", line); return new BadHostEntry(line); } diff --git a/src/main/java/net/schmizz/sshj/userauth/keyprovider/OpenSSHKeyFile.java b/src/main/java/net/schmizz/sshj/userauth/keyprovider/OpenSSHKeyFile.java index da379e63..b9907583 100644 --- a/src/main/java/net/schmizz/sshj/userauth/keyprovider/OpenSSHKeyFile.java +++ b/src/main/java/net/schmizz/sshj/userauth/keyprovider/OpenSSHKeyFile.java @@ -93,13 +93,21 @@ public class OpenSSHKeyFile private void initPubKey(Reader publicKey) throws IOException { final BufferedReader br = new BufferedReader(publicKey); try { - final String keydata = br.readLine(); - if (keydata != null) { - String[] parts = keydata.trim().split(" "); - assert parts.length >= 2; - type = KeyType.fromString(parts[0]); - pubKey = new Buffer.PlainBuffer(Base64.decode(parts[1])).readPublicKey(); + String keydata; + while ((keydata = br.readLine()) != null) { + keydata = keydata.trim(); + if (!keydata.isEmpty()) { + String[] parts = keydata.trim().split("\\s+"); + if (parts.length >= 2) { + type = KeyType.fromString(parts[0]); + pubKey = new Buffer.PlainBuffer(Base64.decode(parts[1])).readPublicKey(); + } else { + throw new IOException("Got line with only one column"); + } + return; + } } + throw new IOException("Public key file is blank"); } finally { br.close(); } diff --git a/src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy b/src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy index ec0029c0..12c32706 100644 --- a/src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy +++ b/src/test/groovy/com/hierynomus/sshj/transport/verification/OpenSSHKnownHostsSpec.groovy @@ -144,6 +144,47 @@ host1 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBL toStringValue == "OpenSSHKnownHosts{khFile='" + f + "'}" } + def "should forgive redundant spaces like OpenSSH does"() { + given: + def key = "AAAAC3NzaC1lZDI1NTE5AAAAIIRsJi92NJJTQwXHZiRiARoEy4n1jYsNTQePHFTSl7tG" + def f = knownHosts(""" + |host1 ssh-ed25519 $key + | + | host2 ssh-ed25519 $key ,./gargage\\., + |\t\t\t\t\t + |\t@revoked host3\tssh-ed25519\t \t$key\t + """.stripMargin()) + def pk = new Buffer.PlainBuffer(Base64.decode(key)).readPublicKey() + + when: + def knownhosts = new OpenSSHKnownHosts(f) + + then: + ["host1", "host2", "host3"].forEach { + knownhosts.verify(it, 22, pk) + } + } + + def "should not throw errors while parsing corrupted records"() { + given: + def key = "AAAAC3NzaC1lZDI1NTE5AAAAIIRsJi92NJJTQwXHZiRiARoEy4n1jYsNTQePHFTSl7tG" + def f = knownHosts( + "\n" // empty line + + " \n" // blank line + + "bad-host1\n" // absent key type and key contents + + "bad-host2 ssh-ed25519\n" // absent key contents + + " bad-host3 ssh-ed25519\n" // absent key contents, with leading spaces + + "@revoked bad-host5 ssh-ed25519\n" // absent key contents, with marker + + "good-host ssh-ed25519 $key" // the only good host at the end + ) + + when: + def knownhosts = new OpenSSHKnownHosts(f) + + then: + knownhosts.verify("good-host", 22, new Buffer.PlainBuffer(Base64.decode(key)).readPublicKey()) + } + def knownHosts(String s) { def f = temp.newFile("known_hosts") f.write(s) diff --git a/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java b/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java index 3dbe1bdc..9e29bc49 100644 --- a/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java +++ b/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java @@ -26,10 +26,18 @@ import net.schmizz.sshj.userauth.password.PasswordUtils; import net.schmizz.sshj.userauth.password.Resource; import net.schmizz.sshj.util.KeyUtil; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import java.io.BufferedReader; +import java.io.BufferedWriter; import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStreamReader; +import java.io.OutputStreamWriter; import java.math.BigInteger; import java.security.GeneralSecurityException; import java.security.PrivateKey; @@ -95,6 +103,9 @@ public class OpenSSHKeyFileTest { } }; + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Test public void blankingOut() throws IOException, GeneralSecurityException { @@ -305,6 +316,47 @@ public class OpenSSHKeyFileTest { assertEquals("", certificate.getExtensions().get("permit-pty")); } + /** + * Sometimes users copy-pastes private and public keys in text editors. It leads to redundant + * spaces and newlines. OpenSSH can easily read such keys, so users expect from SSHJ the same. + */ + @Test + public void notTrimmedKeys() throws IOException { + File initialPrivateKey = new File("src/test/resources/id_rsa"); + File initialPublicKey = new File("src/test/resources/id_rsa.pub"); + File corruptedPrivateKey = new File(temporaryFolder.newFolder(), "id_rsa"); + File corruptedPublicKey = new File(corruptedPrivateKey.getParent(), "id_rsa.pub"); + + BufferedReader reader = new BufferedReader(new InputStreamReader(new FileInputStream(initialPrivateKey))); + BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(corruptedPrivateKey))); + String line; + while ((line = reader.readLine()) != null) { + writer.write(line); + writer.write("\n"); + } + writer.write("\n\n"); + reader.close(); + writer.close(); + + reader = new BufferedReader(new InputStreamReader(new FileInputStream(initialPublicKey))); + writer = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(corruptedPublicKey))); + writer.write("\n\n \t "); + writer.write(reader.readLine().replace(" ", " \t ")); + writer.write("\n\n"); + reader.close(); + writer.close(); + + FileKeyProvider initialKeyFile = new OpenSSHKeyFile(); + FileKeyProvider corruptedKeyFile = new OpenSSHKeyFile(); + initialKeyFile.init(initialPrivateKey); + corruptedKeyFile.init(corruptedPrivateKey); + + assertEquals(initialKeyFile.getPrivate(), + corruptedKeyFile.getPrivate()); + assertEquals(initialKeyFile.getPublic(), + corruptedKeyFile.getPublic()); + } + @Before public void checkBCRegistration() { if (!SecurityUtils.isBouncyCastleRegistered()) {