Replace PKCS5 Key File Class with PKCS8 (#793)

* Replaced PKCS5 parsing with PKCS8

- Moved tests for PEM-encoded PKCS1 files to PKCS8
- Removed PKCS5 Key File implementation

* Added PKCS8 test to retry password after initial failure

Co-authored-by: Jeroen van Erp <[email protected]>

Author: exceptionfactory

src/main/java/net/schmizz/sshj/DefaultConfig.java

@@ -36,11 +36,9 @@
 import net.schmizz.sshj.transport.kex.DHGexSHA1;
 import net.schmizz.sshj.transport.kex.DHGexSHA256;
 import net.schmizz.sshj.transport.kex.ECDHNistP;
-import net.schmizz.sshj.transport.random.BouncyCastleRandom;
 import net.schmizz.sshj.transport.random.JCERandom;
 import net.schmizz.sshj.transport.random.SingletonRandomFactory;
 import net.schmizz.sshj.userauth.keyprovider.OpenSSHKeyFile;
-import net.schmizz.sshj.userauth.keyprovider.PKCS5KeyFile;
 import net.schmizz.sshj.userauth.keyprovider.PKCS8KeyFile;
 import net.schmizz.sshj.userauth.keyprovider.PuTTYKeyFile;
 import org.slf4j.Logger;
@@ -162,7 +160,6 @@ protected void initFileKeyProviderFactories(boolean bouncyCastleRegistered) {
             setFileKeyProviderFactories(
                     new OpenSSHKeyV1KeyFile.Factory(),
                     new PKCS8KeyFile.Factory(),
-                    new PKCS5KeyFile.Factory(),
                     new OpenSSHKeyFile.Factory(),
                     new PuTTYKeyFile.Factory());
         }

src/main/java/net/schmizz/sshj/SSHClient.java

@@ -551,7 +551,7 @@ public KeyProvider loadKeys(String location, char[] passphrase)
      * Creates a {@link KeyProvider} instance from given location on the file system. Currently the following private key files are supported:
      * <ul>
      *     <li>PKCS8 (OpenSSH uses this format)</li>
-     *     <li>PKCS5</li>
+     *     <li>PEM-encoded PKCS1</li>
      *     <li>Putty keyfile</li>
      *     <li>openssh-key-v1 (New OpenSSH keyfile format)</li>
      * </ul>

src/main/java/net/schmizz/sshj/userauth/keyprovider/KeyFormat.java

@@ -16,10 +16,9 @@
 package net.schmizz.sshj.userauth.keyprovider;
 
 /**
-* @version $Id:$
+* Key File Formats
 */
 public enum KeyFormat {
-    PKCS5,
     PKCS8,
     OpenSSH,
     OpenSSHv1,

src/main/java/net/schmizz/sshj/userauth/keyprovider/KeyProviderUtil.java

@@ -27,9 +27,9 @@ public class KeyProviderUtil {
      * <p/>
      * Return values are consistent with the {@code NamedFactory} implementations in the {@code keyprovider} package.
      *
-     * @param location
+     * @param location File Path to key
      * @return name of the key file format
-     * @throws java.io.IOException
+     * @throws java.io.IOException Thrown on file processing failures
      */
     public static KeyFormat detectKeyFileFormat(File location)
             throws IOException {
@@ -45,7 +45,7 @@ public static KeyFormat detectKeyFileFormat(File location)
      * @param privateKey     Private key stored in a string
      * @param separatePubKey Is the public key stored separately from the private key
      * @return name of the key file format
-     * @throws java.io.IOException
+     * @throws java.io.IOException Thrown on file processing failures
      */
     public static KeyFormat detectKeyFileFormat(String privateKey, boolean separatePubKey)
             throws IOException {
@@ -60,7 +60,7 @@ public static KeyFormat detectKeyFileFormat(String privateKey, boolean separateP
      * @param privateKey     Private key accessible through a {@code Reader}
      * @param separatePubKey Is the public key stored separately from the private key
      * @return name of the key file format
-     * @throws java.io.IOException
+     * @throws java.io.IOException Thrown on file processing failures
      */
     public static KeyFormat detectKeyFileFormat(Reader privateKey, boolean separatePubKey)
             throws IOException {
@@ -94,10 +94,8 @@ private static KeyFormat keyFormatFromHeader(String header, boolean separatePubK
             } else if (separatePubKey) {
                 // Can delay asking for password since have unencrypted pubkey
                 return KeyFormat.OpenSSH;
-            } else if (header.contains("BEGIN PRIVATE KEY") || header.contains("BEGIN ENCRYPTED PRIVATE KEY")) {
-                return KeyFormat.PKCS8;
             } else {
-                return KeyFormat.PKCS5;
+                return KeyFormat.PKCS8;
             }
         } else if (header.startsWith("PuTTY-User-Key-File-")) {
             return KeyFormat.PuTTY;

src/main/java/net/schmizz/sshj/userauth/keyprovider/PKCS5KeyFile.java

@@ -39,7 +39,9 @@
 import java.io.IOException;
 import java.security.KeyPair;
 
-/** Represents a PKCS8-encoded key file. This is the format used by (old-style) OpenSSH and OpenSSL. */
+/**
+ * Key File implementation supporting PEM-encoded PKCS8 and PKCS1 formats with or without password-based encryption
+ */
 public class PKCS8KeyFile extends BaseFileKeyProvider {
 
     public static class Factory

src/main/java/net/schmizz/sshj/userauth/keyprovider/PKCS8KeyFile.java

@@ -60,7 +60,7 @@ class FileKeyProviderSpec extends Specification {
 
     where:
     format | keyfile
-    KeyFormat.PKCS5 | "src/test/resources/keyformats/pkcs5"
+    KeyFormat.PKCS8 | "src/test/resources/keyformats/pkcs8"
     KeyFormat.OpenSSH | "src/test/resources/keyformats/openssh"
   }
 }

src/test/groovy/com/hierynomus/sshj/userauth/keyprovider/FileKeyProviderSpec.groovy

@@ -35,9 +35,9 @@ public void testOpenSsh() throws IOException {
     }
 
     @Test
-    public void testPkcs5() throws IOException {
-        KeyFormat format = KeyProviderUtil.detectKeyFileFormat(new File(ROOT, "pkcs5"));
-        assertEquals(KeyFormat.PKCS5, format);
+    public void testPkcs1Rsa() throws IOException {
+        KeyFormat format = KeyProviderUtil.detectKeyFileFormat(new File(ROOT, "pkcs1-rsa"));
+        assertEquals(KeyFormat.PKCS8, format);
     }
 
     @Test