From c555003a407e4fcb937336d6604387d4d4a6edc5 Mon Sep 17 00:00:00 2001
From: Fabian Henneke <FabianHenneke@users.noreply.github.com>
Date: Thu, 25 Jun 2020 15:21:55 +0200
Subject: [PATCH] Clear passphrase bytes after use

Mimics the behavior of `decrypt()` in `PKCS5KeyFile.java`.
---
 .../keyprovider/OpenSSHKeyV1KeyFile.java      |  3 +
 .../sshj/keyprovider/OpenSSHKeyFileTest.java  | 65 ++++++++++++-------
 2 files changed, 45 insertions(+), 23 deletions(-)

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 d71a5421a..cb8949f73 100644
--- a/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java
+++ b/src/main/java/com/hierynomus/sshj/userauth/keyprovider/OpenSSHKeyV1KeyFile.java
@@ -143,9 +143,12 @@ private void initializeCipher(String kdfName, byte[] kdfOptions, Cipher cipher)
                 CharBuffer charBuffer = CharBuffer.wrap(pwdf.reqPassword(null));
                 ByteBuffer byteBuffer = Charset.forName("UTF-8").encode(charBuffer);
                 passphrase = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit());
+                Arrays.fill(charBuffer.array(), '\u0000');
+                Arrays.fill(byteBuffer.array(), (byte) 0);
             }
             byte[] keyiv = new byte[48];
             new BCrypt().pbkdf(passphrase, opts.readBytes(), opts.readUInt32AsInt(), keyiv);
+            Arrays.fill(passphrase, (byte) 0);
             byte[] key = Arrays.copyOfRange(keyiv, 0, 32);
             byte[] iv = Arrays.copyOfRange(keyiv, 32, 48);
             cipher.init(Cipher.Mode.Decrypt, key, iv);
diff --git a/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java b/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java
index 8457521df..0d6a4092e 100644
--- a/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java
+++ b/src/test/java/net/schmizz/sshj/keyprovider/OpenSSHKeyFileTest.java
@@ -47,10 +47,7 @@
 import java.text.DateFormat;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
-import java.util.Arrays;
-import java.util.Date;
-import java.util.Map;
-import java.util.Scanner;
+import java.util.*;
 
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -70,6 +67,44 @@ public class OpenSSHKeyFileTest {
     final char[] correctPassphrase = "test_passphrase".toCharArray();
     final char[] incorrectPassphrase = new char[]{' '};
 
+    private static class WipeTrackingPasswordFinder implements PasswordFinder {
+        private int reqCounter = 0;
+
+        final private String password;
+        final private boolean withRetry;
+        final private ArrayList<char[]> toWipe = new ArrayList<>();
+
+        WipeTrackingPasswordFinder(String password, Boolean withRetry) {
+            this.password = password;
+            this.withRetry = withRetry;
+        }
+
+        @Override
+        public char[] reqPassword(Resource<?> resource) {
+            char[] passwordChars;
+            if (withRetry && reqCounter < 3) {
+                reqCounter++;
+                // Return an incorrect password three times before returning the correct one.
+                passwordChars = (password + "incorrect").toCharArray();
+            } else {
+                passwordChars = password.toCharArray();
+            }
+            toWipe.add(passwordChars);
+            return passwordChars;
+        }
+
+        @Override
+        public boolean shouldRetry(Resource<?> resource) {
+            return withRetry && reqCounter <= 3;
+        }
+
+        public void assertWiped() {
+            for (char[] passwordChars : toWipe) {
+                assertArrayEquals(new char[passwordChars.length], passwordChars);
+            }
+        }
+    };
+
     final PasswordFinder onlyGivesWhenReady = new PasswordFinder() {
         @Override
         public char[] reqPassword(Resource resource) {
@@ -249,27 +284,11 @@ public void shouldLoadECDSAPrivateKeyAsOpenSSHV1() throws IOException {
 
     private void checkOpenSSHKeyV1(String key, final String password, boolean withRetry) throws IOException {
         OpenSSHKeyV1KeyFile keyFile = new OpenSSHKeyV1KeyFile();
-        keyFile.init(new File(key), new PasswordFinder() {
-            private int reqCounter = 0;
-
-            @Override
-            public char[] reqPassword(Resource<?> resource) {
-                if (withRetry && reqCounter < 3) {
-                    reqCounter++;
-                    // Return an incorrect password three times before returning the correct one.
-                    return (password + "incorrect").toCharArray();
-                } else {
-                    return password.toCharArray();
-                }
-            }
-
-            @Override
-            public boolean shouldRetry(Resource<?> resource) {
-                return withRetry && reqCounter <= 3;
-            }
-        });
+        WipeTrackingPasswordFinder pwf = new WipeTrackingPasswordFinder(password, withRetry);
+        keyFile.init(new File(key), pwf);
         PrivateKey aPrivate = keyFile.getPrivate();
         assertThat(aPrivate.getAlgorithm(), equalTo("EdDSA"));
+        pwf.assertWiped();
     }
 
     @Test