Forgive redundant spaces in OpenSSHv2 public keys and known_hosts (#524)

* Forgive redundant spaces in OpenSSHv2 public keys and known_hosts

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.

* Fixed bugs in OpenSSH key file and known_hosts parsers

* OpenSSHKnownHosts should not throw errors while parsing corrupted records
This commit is contained in:
Vladimir Lagunov
2019-09-18 20:14:45 +07:00
committed by Jeroen van Erp
parent d5c045defd
commit 327a4c4c5b
4 changed files with 116 additions and 8 deletions

View File

@@ -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);
}

View File

@@ -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;
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();
}

View File

@@ -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)

View File

@@ -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()) {