only process supported Putty v3 keys + minor optimizations (#729)

Author: jpstotz

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

@@ -100,9 +100,16 @@ public KeyType getType() throws IOException {
         return KeyType.UNKNOWN;
     }
 
-    public boolean isEncrypted() {
-        // Currently the only supported encryption types are "aes256-cbc" and "none".
-        return "aes256-cbc".equals(headers.get("Encryption"));
+    public boolean isEncrypted() throws IOException {
+        // Currently, the only supported encryption types are "aes256-cbc" and "none".
+        String encryption = headers.get("Encryption");
+        if ("none".equals(encryption)) {
+            return false;
+        }
+        if ("aes256-cbc".equals(encryption)) {
+            return true;
+        }
+        throw new IOException(String.format("Unsupported encryption: %s", encryption));
     }
 
     private Map<String, String> payload = new HashMap<String, String>();
@@ -116,8 +123,9 @@ protected KeyPair readKeyPair() throws IOException {
         this.parseKeyPair();
         final Buffer.PlainBuffer publicKeyReader = new Buffer.PlainBuffer(publicKey);
         final Buffer.PlainBuffer privateKeyReader = new Buffer.PlainBuffer(privateKey);
+        final KeyType keyType = this.getType();
         publicKeyReader.readBytes(); // The first part of the payload is a human-readable key format name.
-        if (KeyType.RSA.equals(this.getType())) {
+        if (KeyType.RSA.equals(keyType)) {
             // public key exponent
             BigInteger e = publicKeyReader.readMPInt();
             // modulus
@@ -139,7 +147,7 @@ protected KeyPair readKeyPair() throws IOException {
                 throw new IOException(i.getMessage(), i);
             }
         }
-        if (KeyType.DSA.equals(this.getType())) {
+        if (KeyType.DSA.equals(keyType)) {
             BigInteger p = publicKeyReader.readMPInt();
             BigInteger q = publicKeyReader.readMPInt();
             BigInteger g = publicKeyReader.readMPInt();
@@ -161,14 +169,14 @@ protected KeyPair readKeyPair() throws IOException {
                 throw new IOException(e.getMessage(), e);
             }
         }
-        if (KeyType.ED25519.equals(this.getType())) {
+        if (KeyType.ED25519.equals(keyType)) {
             EdDSANamedCurveSpec ed25519 = EdDSANamedCurveTable.getByName("Ed25519");
             EdDSAPublicKeySpec publicSpec = new EdDSAPublicKeySpec(publicKeyReader.readBytes(), ed25519);
             EdDSAPrivateKeySpec privateSpec = new EdDSAPrivateKeySpec(privateKeyReader.readBytes(), ed25519);
             return new KeyPair(new EdDSAPublicKey(publicSpec), new EdDSAPrivateKey(privateSpec));
         }
         final String ecdsaCurve;
-        switch (this.getType()) {
+        switch (keyType) {
             case ECDSA256:
                 ecdsaCurve = "P-256";
                 break;
@@ -190,7 +198,7 @@ protected KeyPair readKeyPair() throws IOException {
             ECPrivateKeySpec pks = new ECPrivateKeySpec(s, ecCurveSpec);
             try {
                 PrivateKey privateKey = SecurityUtils.getKeyFactory(KeyAlgorithm.ECDSA).generatePrivate(pks);
-                return new KeyPair(getType().readPubKeyFromBuffer(publicKeyReader), privateKey);
+                return new KeyPair(keyType.readPubKeyFromBuffer(publicKeyReader), privateKey);
             } catch (GeneralSecurityException e) {
                 throw new IOException(e.getMessage(), e);
             }
@@ -252,6 +260,12 @@ protected void parseKeyPair() throws IOException {
      * This is used to decrypt the private key when it's encrypted.
      */
     private byte[] toKey(final String passphrase) throws IOException {
+        // The field Key-Derivation has been introduced with Putty v3 key file format
+        // The only available formats are "Argon2i" "Argon2d" and "Argon2id"
+        String keyDerivation = headers.get("Key-Derivation");
+        if (keyDerivation != null) {
+            throw new IOException(String.format("Unsupported key derivation function: %s", keyDerivation));
+        }
         try {
             MessageDigest digest = MessageDigest.getInstance("SHA-1");
 
@@ -283,7 +297,7 @@ private byte[] toKey(final String passphrase) throws IOException {
      */
     private void verify(final String passphrase) throws IOException {
         try {
-            // The key to the MAC is itself a SHA-1 hash of:
+            // The key to the MAC is itself a SHA-1 hash of (v1/v2 key only):
             MessageDigest digest = MessageDigest.getInstance("SHA-1");
             digest.update("putty-private-key-file-mac-key".getBytes());
             if (passphrase != null) {
@@ -297,8 +311,9 @@ private void verify(final String passphrase) throws IOException {
             final ByteArrayOutputStream out = new ByteArrayOutputStream();
             final DataOutputStream data = new DataOutputStream(out);
             // name of algorithm
-            data.writeInt(this.getType().toString().length());
-            data.writeBytes(this.getType().toString());
+            String keyType = this.getType().toString();
+            data.writeInt(keyType.length());
+            data.writeBytes(keyType);
 
             data.writeInt(headers.get("Encryption").length());
             data.writeBytes(headers.get("Encryption"));

src/test/java/net/schmizz/sshj/keyprovider/PuTTYKeyFileTest.java

@@ -25,10 +25,13 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
+import java.security.PrivateKey;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class PuTTYKeyFileTest {
 
@@ -236,7 +239,39 @@ public class PuTTYKeyFileTest {
     		"Private-Lines: 1\n" + 
     		"AAAAIEblmwyKaGuvc6dLgNeHsc1BuZeQORTSxBF5SBLNyjYc\n" + 
     		"Private-MAC: e1aed15a209f48fdaa5228640f1109a7740340764a96f97ec6023da7f92d07ea";
-    
+
+    final static String v3_rsa_encrypted = "PuTTY-User-Key-File-3: ssh-rsa\n" +
+            "Encryption: aes256-cbc\n" +
+            "Comment: rsa-key-20210926\n" +
+            "Public-Lines: 6\n" +
+            "AAAAB3NzaC1yc2EAAAADAQABAAABAQCBjWQHMpKAQnU3vZZF/iHn4RA867Ox+U03\n" +
+            "/GOHivW0SgGIQbhKcSSWvTzYOE+GQdtX9T2KJxr76z/lB4nghkcWkpLoQW91gNBf\n" +
+            "PUagMvaBxKXC8cNqaMm99uw5KpRg8SpTJWxwYPlQtzmyxav0PRFeOMSsiRsnjNuX\n" +
+            "polMDSu6vmkkuKrPzvinPZbsXoZeMybcm1gn2Zq+7ik4us0icaGxRJRuF+nVqYag\n" +
+            "EmO9jmQoytyqoNWzvPYEh/dh85hESwtIKXiaMOjQg52dW5BuELPGV7ZxaKRK7Znw\n" +
+            "RGW6CtoGYulo0mJz5IZslDrRK/EK2bSGDbrlAcYaajROB6aBDyaJ\n" +
+            "Key-Derivation: Argon2id\n" +
+            "Argon2-Memory: 8192\n" +
+            "Argon2-Passes: 21\n" +
+            "Argon2-Parallelism: 1\n" +
+            "Argon2-Salt: baf1530601433715467614d044c0e4a5\n" +
+            "Private-Lines: 14\n" +
+            "QAJl3mq/QJc8/of4xWbgBuE09GdgIuVhRYGAV5yC5C0dpuiJ+yF/6h7mk36s5E3Q\n" +
+            "k32l+ZoWHG/kBc8s6N9rTQnIgC/eieNlN5FK3OSSoI9PBvoAtNEVWsR2T4U6ZkAG\n" +
+            "FbyF3vRWq2h9Ux8flZusySqafQ2AhXP79pr13wvMziv1QbPkPFHWaR1Uvq9w0GJq\n" +
+            "rfR+M6t8/6aPKhnsCTy8MiAoIcjeZmHiG/vOMIBBoWI7KtpD5IrbO4pIgzRK8m9Z\n" +
+            "JqQvgWCPnddwCeiDFOZwf/Bm6g+duQYId4upB1IxSs34j21a7ZkMSExDZyV0d13S\n" +
+            "G59U9pReZ7mHyIjORqeY7ssr/L9aJPPa7YCu4J5a/Bn/ARf/X5XmMnueFZ6H806M\n" +
+            "ZUtHzeG2sZGoHULpwEaY1zRQs1JD5UAeaFzgDpzD4oeaD8v+FS3RdNlgj2gtWNcl\n" +
+            "h8nvWD60XbylR0BdbB553xGuC8HC0482xQCCJUc8SMHZ/k2+FKTaf2m2p4dLyKkk\n" +
+            "Qrw43QcmkgypUPRHKvnVs+6qUYMDHkwtPR1ZGFqHQzlHozvO9NdY/ZXTln/qfPZA\n" +
+            "5w5TKvy0/GvofhISJCMocnPbkqGR6fDcKbpUjAS/RDgsCKKS5hxf6nhsYUgrXA4G\n" +
+            "hXIgqGnMefLemjRG7dD/3XE8NmF6Q8mjIideEOBeP4tRCaDC2n90rZ3yChP9bsel\n" +
+            "yg/TeKxj7OLk+X3ocP3yw2lsp3zOPsptSNtGI7g9VaIPGtxGaqRaIuObdLbBxCeR\n" +
+            "ZgKSIuWtz8W1kT0aWuZ0aXMPagGao0ZsffmroyVpGbzW3QaI9633Krmf7EyphZoy\n" +
+            "6tV3Z/GJ5aQJFeMYPOq69ktXRLAWr800822NwEStcxtQHTWbaTk7dxh8+0xwlCgI\n" +
+            "Private-MAC: 582dea09758afd93a8e248abce358287d384e5ee36d21515ffcc0d42d8c5d86a\n";
+
     @Test
     public void test2048() throws Exception {
         PuTTYKeyFile key = new PuTTYKeyFile();
@@ -359,7 +394,36 @@ public void testV3Key() throws Exception {
         assertNotNull(key.getPrivate());
         assertNotNull(key.getPublic());
     }
-    
+
+    /**
+     * Reading an encrypted Putty v3 key requires an Argon2i/Argon2d/Argon2id
+     * implementation.
+     * Putty v3 keys additionally use a different algorithm for generating the "Private-MAC"
+     */
+    @Test
+    public void testRSAv3EncryptedKey() throws Exception {
+        PuTTYKeyFile key = new PuTTYKeyFile();
+        key.init(new StringReader(v3_rsa_encrypted), new PasswordFinder() {
+            @Override
+            public char[] reqPassword(Resource<?> resource) {
+                return "changeit".toCharArray();
+            }
+
+            @Override
+            public boolean shouldRetry(Resource<?> resource) {
+                return false;
+            }
+        });
+        try {
+            PrivateKey privateKey = key.getPrivate();
+            fail("IOException expected as encrypted Putty v3 keys are not yet supported");
+        } catch (IOException e) {
+            assertTrue(e.getMessage().startsWith("Unsupported key derivation function"));
+        }
+        // assertNotNull(key.getPrivate());
+        // assertNotNull(key.getPublic());
+    }
+
     @Test
     public void testCorrectPassphraseRsa() throws Exception {
         PuTTYKeyFile key = new PuTTYKeyFile();