Clear passphrase bytes after use (#609)

Mimics the behavior of `decrypt()` in `PKCS5KeyFile.java`.
hierynomus · Jul 1, 2020 · 7bde5c1 · 7bde5c1
1 parent 3c85b86
commit 7bde5c1
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 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
@@ -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
@@ -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