From 63086334b13e081f137df411eece686af7d2a4d1 Mon Sep 17 00:00:00 2001 From: exceptionfactory Date: Sat, 7 Oct 2023 21:40:33 -0500 Subject: [PATCH] Improved malformed file handling for OpenSSH Private Keys --- .../keyprovider/OpenSSHKeyV1KeyFile.java | 48 ++++++++++++------- .../sshj/keyprovider/OpenSSHKeyFileTest.java | 19 +++++++- .../keytypes/test_ed25519_missing_footer | 6 +++ 3 files changed, 53 insertions(+), 20 deletions(-) create mode 100644 src/test/resources/keytypes/test_ed25519_missing_footer diff --git a/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java b/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java index 499e954e..3848750b 100644 --- a/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java +++ b/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java @@ -98,19 +98,19 @@ public void init(File location) { @Override protected KeyPair readKeyPair() throws IOException { - BufferedReader reader = new BufferedReader(resource.getReader()); + final BufferedReader reader = new BufferedReader(resource.getReader()); try { - if (!checkHeader(reader)) { - throw new IOException("This key is not in 'openssh-key-v1' format"); + if (checkHeader(reader)) { + final String encodedPrivateKey = readEncodedKey(reader); + byte[] decodedPrivateKey = Base64.getDecoder().decode(encodedPrivateKey); + final PlainBuffer bufferedPrivateKey = new PlainBuffer(decodedPrivateKey); + return readDecodedKeyPair(bufferedPrivateKey); + } else { + final String message = String.format("File header not found [%s%s]", BEGIN, OPENSSH_PRIVATE_KEY); + throw new IOException(message); } - - String keyFile = readKeyFile(reader); - byte[] decode = Base64.getDecoder().decode(keyFile); - PlainBuffer keyBuffer = new PlainBuffer(decode); - return readDecodedKeyPair(keyBuffer); - - } catch (GeneralSecurityException e) { - throw new SSHRuntimeException(e); + } catch (final GeneralSecurityException e) { + throw new SSHRuntimeException("Read OpenSSH Version 1 Key failed", e); } finally { IOUtils.closeQuietly(reader); } @@ -149,7 +149,7 @@ private KeyPair readDecodedKeyPair(final PlainBuffer keyBuffer) throws IOExcepti logger.debug("Reading unencrypted keypair"); return readUnencrypted(privateKeyBuffer, publicKey); } else { - logger.info("Keypair is encrypted with: " + cipherName + ", " + kdfName + ", " + Arrays.toString(kdfOptions)); + logger.info("Keypair is encrypted with: {}, {}, {}", cipherName, kdfName, Arrays.toString(kdfOptions)); while (true) { PlainBuffer decryptionBuffer = new PlainBuffer(privateKeyBuffer); PlainBuffer decrypted = decryptBuffer(decryptionBuffer, cipherName, kdfName, kdfOptions); @@ -209,14 +209,26 @@ private PublicKey readPublicKey(final PlainBuffer plainBuffer) throws Buffer.Buf return KeyType.fromString(plainBuffer.readString()).readPubKeyFromBuffer(plainBuffer); } - private String readKeyFile(final BufferedReader reader) throws IOException { - StringBuilder sb = new StringBuilder(); + private String readEncodedKey(final BufferedReader reader) throws IOException { + final StringBuilder builder = new StringBuilder(); + + boolean footerFound = false; String line = reader.readLine(); - while (!line.startsWith(END)) { - sb.append(line); + while (line != null) { + if (line.startsWith(END)) { + footerFound = true; + break; + } + builder.append(line); line = reader.readLine(); } - return sb.toString(); + + if (footerFound) { + return builder.toString(); + } else { + final String message = String.format("File footer not found [%s%s]", END, OPENSSH_PRIVATE_KEY); + throw new IOException(message); + } } private boolean checkHeader(final BufferedReader reader) throws IOException { @@ -244,7 +256,7 @@ private KeyPair readUnencrypted(final PlainBuffer keyBuffer, final PublicKey pub // The private key section contains both the public key and the private key String keyType = keyBuffer.readString(); // string keytype KeyType kt = KeyType.fromString(keyType); - logger.info("Read key type: {}", keyType, kt); + KeyPair kp; switch (kt) { case ED25519: diff --git a/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java b/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java index 0834d4c1..e120447d 100644 --- a/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java +++ b/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java @@ -25,7 +25,6 @@ import net.schmizz.sshj.userauth.password.PasswordUtils; import net.schmizz.sshj.userauth.password.Resource; import net.schmizz.sshj.util.KeyUtil; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -226,6 +225,22 @@ public void shouldLoadED25519PrivateKey() throws IOException { assertThat(aPrivate.getAlgorithm(), equalTo("EdDSA")); } + @Test + public void shouldHandlePrivateKeyMissingHeader() { + OpenSSHKeyV1KeyFile keyFile = new OpenSSHKeyV1KeyFile(); + keyFile.init(new File("src/test/resources/keyformats/pkcs8")); + final IOException exception = assertThrows(IOException.class, keyFile::getPrivate); + assertTrue(exception.getMessage().contains("header not found")); + } + + @Test + public void shouldHandlePrivateKeyMissingFooter() { + final OpenSSHKeyV1KeyFile keyFile = new OpenSSHKeyV1KeyFile(); + keyFile.init(new File("src/test/resources/keytypes/test_ed25519_missing_footer")); + final IOException exception = assertThrows(IOException.class, keyFile::getPrivate); + assertTrue(exception.getMessage().contains("footer not found")); + } + @Test public void shouldLoadProtectedED25519PrivateKeyAes256CTR() throws IOException { checkOpenSSHKeyV1("src/test/resources/keytypes/ed25519_protected", "sshjtest", false); @@ -245,7 +260,7 @@ public void shouldLoadProtectedED25519PrivateKeyAes128CBC() throws IOException { } @Test - public void shouldFailOnIncorrectPassphraseAfterRetries() throws IOException { + public void shouldFailOnIncorrectPassphraseAfterRetries() { assertThrows(KeyDecryptionFailedException.class, () -> { OpenSSHKeyV1KeyFile keyFile = new OpenSSHKeyV1KeyFile(); keyFile.init(new File("src/test/resources/keytypes/ed25519_aes256cbc.pem"), new PasswordFinder() { diff --git a/src/test/resources/keytypes/test_ed25519_missing_footer b/src/test/resources/keytypes/test_ed25519_missing_footer new file mode 100644 index 00000000..9aed6026 --- /dev/null +++ b/src/test/resources/keytypes/test_ed25519_missing_footer @@ -0,0 +1,6 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW +QyNTUxOQAAACAwHSYkZJATPMgvLHkxKAJ9j38Gyyq5HGoWdMcT6FiAiQAAAJDimgR84poE +fAAAAAtzc2gtZWQyNTUxOQAAACAwHSYkZJATPMgvLHkxKAJ9j38Gyyq5HGoWdMcT6FiAiQ +AAAECmsckQycWnfGQK6XtQpaMGODbAkMQOdJNK6XJSipB7dDAdJiRkkBM8yC8seTEoAn2P +fwbLKrkcahZ0xxPoWICJAAAACXJvb3RAc3NoagECAwQ=