Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear passphrase bytes after use #609

Merged
merged 1 commit into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down