From 9ce9eb96c0436627300b59f690cdb0cfc2f1227d Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Thu, 1 Nov 2018 15:35:40 +0100 Subject: [PATCH 01/12] First version of obfuscated password --- .../ArmadilloBcryptKeyStretcher.java | 23 ++------ .../armadillo/ByteArrayRuntimeObfuscator.java | 33 +++++++++-- .../armadillo/DefaultEncryptionProtocol.java | 40 +++++++++++++- .../lib/armadillo/EncryptionFingerprint.java | 11 ++++ .../lib/armadillo/EncryptionProtocol.java | 9 +++ .../armadillo/SecureSharedPreferences.java | 36 ++++++------ .../ASecureSharedPreferencesTest.java | 40 ++++++++------ .../armadillo/TestEncryptionFingerprint.java | 17 ++++++ .../armadillo/ByteArrayObfuscatorTest.java | 55 +++++++++++++++++++ .../DefaultEncryptionProtocolTest.java | 29 ++++++++++ 10 files changed, 234 insertions(+), 59 deletions(-) create mode 100644 armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java create mode 100644 armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java create mode 100644 armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ArmadilloBcryptKeyStretcher.java b/armadillo/src/main/java/at/favre/lib/armadillo/ArmadilloBcryptKeyStretcher.java index 0781235..3e731ef 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ArmadilloBcryptKeyStretcher.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ArmadilloBcryptKeyStretcher.java @@ -1,12 +1,6 @@ package at.favre.lib.armadillo; import android.os.StrictMode; -import android.support.annotation.Nullable; - -import java.nio.ByteBuffer; -import java.nio.CharBuffer; -import java.nio.charset.Charset; -import java.nio.charset.StandardCharsets; import at.favre.lib.bytes.Bytes; import at.favre.lib.crypto.HKDF; @@ -69,23 +63,16 @@ public byte[] stretch(byte[] salt, char[] password, int outLengthByte) { * @param logRounds log2(Iterations). e.g. 12 ==> 2^12 = 4,096 iterations * @return the Bcrypt hash of the password */ - private static byte[] bcrypt(byte[] salt, @Nullable char[] password, int logRounds) { + private static byte[] bcrypt(byte[] salt, char[] password, int logRounds) { StrictMode.noteSlowCall("bcrypt is a very expensive call and should not be done on the main thread"); - byte[] passwordBytes = null; + Bytes passwordBytes = Bytes.empty(); try { - passwordBytes = charArrayToByteArray(password, StandardCharsets.UTF_8); + passwordBytes = Bytes.from(password); return BCrypt.with(BCrypt.Version.VERSION_2A).hashRaw(logRounds, HKDF.fromHmacSha256().expand(salt, "bcrypt-salt".getBytes(), 16), - HKDF.fromHmacSha256().expand(passwordBytes, "bcrypt-pw".getBytes(), 71)).rawHash; + HKDF.fromHmacSha256().expand(passwordBytes.array(), "bcrypt-pw".getBytes(), 71)).rawHash; } finally { - Bytes.wrapNullSafe(passwordBytes).mutable().secureWipe(); + passwordBytes.mutable().secureWipe(); } } - - private static byte[] charArrayToByteArray(char[] charArray, Charset charset) { - ByteBuffer bb = charset.encode(CharBuffer.wrap(charArray)); - byte[] bytes = new byte[bb.remaining()]; - bb.get(bytes); - return bytes; - } } diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java index 8af5313..cba1dfc 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java @@ -22,18 +22,43 @@ public interface ByteArrayRuntimeObfuscator { */ byte[] getBytes(); - final class Default implements ByteArrayRuntimeObfuscator { + /** + * Wipe and overwrite the internal byte arrays + */ + void wipe(); + final class Default implements ByteArrayRuntimeObfuscator { private final byte[][] data; Default(byte[] array, SecureRandom secureRandom) { - byte[] key = Bytes.random(array.length, secureRandom).array(); - this.data = new byte[][]{key, Bytes.wrap(array).mutable().xor(key).array()}; + int len = (int) (Math.abs(Bytes.random(8).toLong()) % 9) + 1; + this.data = new byte[len + 1][]; + for (int i = 0; i < data.length - 1; i++) { + byte[] key = Bytes.random(array.length, secureRandom).array(); + this.data[i] = key; + Bytes.wrap(array).mutable().xor(key); + } + this.data[data.length - 1] = array; } @Override public byte[] getBytes() { - return Bytes.wrap(data[0]).xor(data[1]).array(); + Bytes b = Bytes.empty(); + for (int i = data.length - 1; i >= 0; i--) { + if (b.isEmpty()) { + b = Bytes.wrap(data[i]).mutable(); + continue; + } + b.xor(data[i]); + } + return b.array(); + } + + @Override + public void wipe() { + for (byte[] arr : data) { + Bytes.wrap(arr).mutable().secureWipe(); + } } } } diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java b/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java index b986e92..2000ae3 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java @@ -4,6 +4,8 @@ import android.support.annotation.Nullable; import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; import java.security.SecureRandom; import java.text.Normalizer; import java.util.Objects; @@ -106,8 +108,8 @@ public byte[] encrypt(@NonNull String contentKey, @Nullable char[] password, byt private byte[] encode(byte[] contentSalt, byte[] encrypted) { ByteBuffer buffer = ByteBuffer.allocate(PROTOCOL_VERSION_LENGTH_BYTES - + CONTENT_SALT_SIZE_LENGTH_BYTES + contentSalt.length - + ENCRYPTED_CONTENT_SIZE_LENGTH_BYTES + encrypted.length); + + CONTENT_SALT_SIZE_LENGTH_BYTES + contentSalt.length + + ENCRYPTED_CONTENT_SIZE_LENGTH_BYTES + encrypted.length); buffer.putInt(protocolVersion); buffer.put((byte) contentSalt.length); buffer.put(contentSalt); @@ -171,6 +173,27 @@ public KeyStretchingFunction getKeyStretchingFunction() { return keyStretchingFunction; } + @Nullable + @Override + public ByteArrayRuntimeObfuscator obfuscatePassword(@Nullable char[] password) { + return obfuscatePasswordInternal(password, secureRandom); + } + + @Nullable + @Override + public char[] deobfuscatePassword(@Nullable ByteArrayRuntimeObfuscator obfuscated) { + if (obfuscated == null) return null; + + CharBuffer charBuffer = StandardCharsets.UTF_8.decode(ByteBuffer.wrap(obfuscated.getBytes())); + + if (charBuffer.capacity() != charBuffer.limit()) { + char[] compacted = new char[charBuffer.remaining()]; + charBuffer.get(compacted); + return compacted; + } + return charBuffer.array(); + } + private byte[] keyDerivationFunction(String contentKey, byte[] fingerprint, byte[] contentSalt, byte[] preferenceSalt, @Nullable char[] password) { Bytes ikm = Bytes.from(fingerprint, contentSalt, Bytes.from(contentKey, Normalizer.Form.NFKD).array()); @@ -195,7 +218,7 @@ public static final class Factory implements EncryptionProtocol.Factory { private final Compressor compressor; Factory(int protocolVersion, EncryptionFingerprint fingerprint, StringMessageDigest stringMessageDigest, - AuthenticatedEncryption authenticatedEncryption, int keyStrength, + AuthenticatedEncryption authenticatedEncryption, @AuthenticatedEncryption.KeyStrength int keyStrength, KeyStretchingFunction keyStretchingFunction, DataObfuscator.Factory dataObfuscatorFactory, SecureRandom secureRandom, @Nullable Compressor compressor) { this.protocolVersion = protocolVersion; @@ -228,5 +251,16 @@ public DataObfuscator createDataObfuscator() { public SecureRandom getSecureRandom() { return secureRandom; } + + @Nullable + @Override + public ByteArrayRuntimeObfuscator obfuscatePassword(@Nullable char[] password) { + return obfuscatePasswordInternal(password, secureRandom); + } + } + + private static ByteArrayRuntimeObfuscator obfuscatePasswordInternal(@Nullable char[] password, SecureRandom secureRandom) { + if (password == null) return null; + return new ByteArrayRuntimeObfuscator.Default(Bytes.from(password).array(), secureRandom); } } diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprint.java b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprint.java index 0a1b12f..fc93ccf 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprint.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprint.java @@ -23,8 +23,14 @@ */ public interface EncryptionFingerprint { + byte[] getBytes(); + /** + * Wipe and overwrite the internal byte arrays + */ + void wipe(); + /** * Default implementation of a {@link EncryptionFingerprint} with simple internal in-memory * data obfuscation. @@ -41,5 +47,10 @@ public Default(byte[] array) { public byte[] getBytes() { return holder.getBytes(); } + + @Override + public void wipe() { + holder.wipe(); + } } } diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java index 1ffd398..1e7396f 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java @@ -86,6 +86,12 @@ interface EncryptionProtocol { */ KeyStretchingFunction getKeyStretchingFunction(); + @Nullable + ByteArrayRuntimeObfuscator obfuscatePassword(@Nullable char[] password); + + @Nullable + char[] deobfuscatePassword(@Nullable ByteArrayRuntimeObfuscator obfuscated); + /** * Factory creating a new instance of {@link EncryptionProtocol} */ @@ -119,5 +125,8 @@ interface Factory { * @return random gen */ SecureRandom getSecureRandom(); + + @Nullable + ByteArrayRuntimeObfuscator obfuscatePassword(@Nullable char[] password); } } diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/SecureSharedPreferences.java b/armadillo/src/main/java/at/favre/lib/armadillo/SecureSharedPreferences.java index ed5c7bb..8798721 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/SecureSharedPreferences.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/SecureSharedPreferences.java @@ -29,7 +29,7 @@ * * @author Patrick Favre-Bulle */ -@SuppressWarnings({"unused", "WeakerAccess", "UnusedReturnValue"}) +@SuppressWarnings( {"unused", "WeakerAccess", "UnusedReturnValue"}) public final class SecureSharedPreferences implements ArmadilloSharedPreferences { private static final String PREFERENCES_SALT_KEY = "at.favre.lib.securepref.KEY_RANDOM"; @@ -41,7 +41,7 @@ public final class SecureSharedPreferences implements ArmadilloSharedPreferences private final RecoveryPolicy recoveryPolicy; @Nullable - private char[] password; + private ByteArrayRuntimeObfuscator password; private boolean supportVerifyPassword; private String prefSaltContentKey; @@ -54,7 +54,7 @@ public SecureSharedPreferences(Context context, String preferenceName, Encryptio public SecureSharedPreferences(Context context, String preferenceName, EncryptionProtocol.Factory encryptionProtocol, RecoveryPolicy recoveryPolicy, @Nullable char[] password, boolean supportVerifyPassword) { this(context.getSharedPreferences(encryptionProtocol.getStringMessageDigest().derive(preferenceName, "prefName"), Context.MODE_PRIVATE), - encryptionProtocol, recoveryPolicy, password, supportVerifyPassword); + encryptionProtocol, recoveryPolicy, password, supportVerifyPassword); } public SecureSharedPreferences(SharedPreferences sharedPreferences, EncryptionProtocol.Factory encryptionProtocolFactory, @@ -63,7 +63,7 @@ public SecureSharedPreferences(SharedPreferences sharedPreferences, EncryptionPr this.sharedPreferences = sharedPreferences; this.factory = encryptionProtocolFactory; this.recoveryPolicy = recoveryPolicy; - this.password = password; + this.password = factory.obfuscatePassword(password); this.supportVerifyPassword = supportVerifyPassword; init(); } @@ -75,9 +75,9 @@ public SecureSharedPreferences(SharedPreferences sharedPreferences, EncryptionPr */ private void init() { this.preferencesSalt = getPreferencesSalt( - factory.getStringMessageDigest(), - factory.createDataObfuscator(), - factory.getSecureRandom()); + factory.getStringMessageDigest(), + factory.createDataObfuscator(), + factory.getSecureRandom()); this.encryptionProtocol = factory.create(preferencesSalt); if (supportVerifyPassword && !hasValidationValue()) { storePasswordValidationValue(preferencesSalt); @@ -295,9 +295,9 @@ public void changePassword(@Nullable char[] newPassword, @Nullable KeyStretching } if (password != null) { - Arrays.fill(password, (char) 0); + password.wipe(); } - password = newPassword; + password = encryptionProtocol.obfuscatePassword(newPassword); } /** @@ -311,6 +311,7 @@ public void changePassword(@Nullable char[] newPassword, @Nullable KeyStretching */ private boolean reencryptStringType(@Nullable char[] newPassword, Editor editor, String keyHash, @Nullable KeyStretchingFunction newKsFunction) { try { + final ByteArrayRuntimeObfuscator pw = encryptionProtocol.obfuscatePassword(newPassword); final String encryptedValue = sharedPreferences.getString(keyHash, null); if (encryptedValue == null) { @@ -326,14 +327,13 @@ private boolean reencryptStringType(@Nullable char[] newPassword, Editor editor, encryptionProtocol.setKeyStretchingFunction(newKsFunction); } - editor.putEncryptedBase64(keyHash, encryptToBase64(keyHash, newPassword, bytes)); + editor.putEncryptedBase64(keyHash, encryptToBase64(keyHash, pw, bytes)); return true; } catch (ClassCastException e) { return false; } } - /** * Re-encrypts StringSet stored with given key hash using the new provided password. * @@ -344,6 +344,7 @@ private boolean reencryptStringType(@Nullable char[] newPassword, Editor editor, * @return returns true if the StringSet was successfully re-encrypted. */ private boolean reencryptStringSetType(char[] newPassword, Editor editor, String keyHash, @Nullable KeyStretchingFunction newKsFunction) { + final ByteArrayRuntimeObfuscator pw = encryptionProtocol.obfuscatePassword(newPassword); final Set encryptedSet = sharedPreferences.getStringSet(keyHash, null); if (encryptedSet == null) { return false; @@ -364,7 +365,7 @@ private boolean reencryptStringSetType(char[] newPassword, Editor editor, String final Set encryptedValues = new HashSet<>(decryptedSet.size()); for (String value : decryptedSet) { - encryptedValues.add(encryptToBase64(keyHash, newPassword, Bytes.from(value).array())); + encryptedValues.add(encryptToBase64(keyHash, pw, Bytes.from(value).array())); } editor.putEncryptedStringSet(keyHash, encryptedValues); return true; @@ -373,7 +374,7 @@ private boolean reencryptStringSetType(char[] newPassword, Editor editor, String @Override public void close() { if (password != null) { - Arrays.fill(password, (char) 0); + password.wipe(); } password = null; if (preferencesSalt != null) { @@ -503,18 +504,18 @@ private void handlePossibleClear() { } @NonNull - private String encryptToBase64(String keyHash, @Nullable char[] password, byte[] content) { + private String encryptToBase64(String keyHash, @Nullable ByteArrayRuntimeObfuscator password, byte[] content) { try { - return Bytes.wrap(encryptionProtocol.encrypt(keyHash, password, content)).encodeBase64(); + return Bytes.wrap(encryptionProtocol.encrypt(keyHash, encryptionProtocol.deobfuscatePassword(password), content)).encodeBase64(); } catch (EncryptionProtocolException e) { throw new IllegalStateException(e); } } @Nullable - private byte[] decrypt(String keyHash, char[] password, @NonNull String base64Encrypted) { + private byte[] decrypt(String keyHash, @Nullable ByteArrayRuntimeObfuscator password, @NonNull String base64Encrypted) { try { - return encryptionProtocol.decrypt(keyHash, password, Bytes.parseBase64(base64Encrypted).array()); + return encryptionProtocol.decrypt(keyHash, encryptionProtocol.deobfuscatePassword(password), Bytes.parseBase64(base64Encrypted).array()); } catch (EncryptionProtocolException e) { if (recoveryPolicy.shouldRemoveBrokenContent()) { sharedPreferences.edit().remove(keyHash).apply(); @@ -525,4 +526,5 @@ private byte[] decrypt(String keyHash, char[] password, @NonNull String base64En } return null; } + } diff --git a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java index 302bafe..37e3c1e 100644 --- a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java +++ b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java @@ -228,6 +228,12 @@ public void simpleStringGetWithBrokenBcryptPassword() { .keyStretchingFunction(new BrokenBcryptKeyStretcher(6)).build()); } + @Test + public void simpleStringGetWithUnicodePw() { + preferenceSmokeTest(create("withPw", "ö案äü_ß²áèµÿA2ijʥ".toCharArray()) + .keyStretchingFunction(new FastKeyStretcher()).build()); + } + @Test public void simpleStringGetWithFastKDF() { preferenceSmokeTest(create("withPw", "superSecret".toCharArray()) @@ -244,7 +250,7 @@ public void testWithDifferentFingerprint() { preferenceSmokeTest(create("fingerprint", null) .encryptionFingerprint(Bytes.random(16).array()).build()); preferenceSmokeTest(create("fingerprint2", null) - .encryptionFingerprint(() -> new byte[16]).build()); + .encryptionFingerprint(new TestEncryptionFingerprint()).build()); } @Test @@ -359,14 +365,14 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open new shared pref and add some data ArmadilloSharedPreferences pref = create(name, clonePassword(currentPassword)) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); pref.edit().putString("k1", "string1").putInt("k2", 2).putStringSet("set", testSet) - .putBoolean("k3", true).commit(); + .putBoolean("k3", true).commit(); pref.close(); // open again and check if can be used pref = create(name, clonePassword(currentPassword)) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); assertTrue(pref.getBoolean("k3", false)); @@ -375,7 +381,7 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open with old pw and change to new one, all the values should be accessible pref = create(name, clonePassword(currentPassword)) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); pref.changePassword(clonePassword(newPassword)); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); @@ -385,7 +391,7 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open with new pw, should be accessible pref = create(name, clonePassword(newPassword)) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); assertTrue(pref.getBoolean("k3", false)); @@ -394,14 +400,14 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open with old pw, should throw exception, since cannot decrypt pref = create(name, clonePassword(currentPassword)) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); try { pref.getString("k1", null); fail("should throw exception, since cannot decrypt"); } catch (SecureSharedPreferenceCryptoException ignored) { } } - + private char[] clonePassword(char[] password) { return password == null ? null : password.clone(); } @@ -415,14 +421,14 @@ public void testChangePasswordAndKeyStretchingFunction() { // open new shared pref and add some data ArmadilloSharedPreferences pref = create("testChangePassword", "pw1".toCharArray()) - .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); + .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); pref.edit().putString("k1", "string1").putInt("k2", 2).putStringSet("set", testSet) - .putBoolean("k3", true).commit(); + .putBoolean("k3", true).commit(); pref.close(); // open again and check if can be used pref = create("testChangePassword", "pw1".toCharArray()) - .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); + .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); assertTrue(pref.getBoolean("k3", false)); @@ -431,7 +437,7 @@ public void testChangePasswordAndKeyStretchingFunction() { // open with old pw and old ksFn; change to new one, all the values should be accessible pref = create("testChangePassword", "pw1".toCharArray()) - .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); + .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); pref.changePassword("pw2".toCharArray(), new ArmadilloBcryptKeyStretcher(8)); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); @@ -450,7 +456,7 @@ public void testChangePasswordAndKeyStretchingFunction() { // open with new pw and old ksFn, should throw exception, since cannot decrypt pref = create("testChangePassword", "pw2".toCharArray()) - .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); + .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); try { pref.getString("k1", null); fail("should throw exception, since cannot decrypt"); @@ -459,7 +465,7 @@ public void testChangePasswordAndKeyStretchingFunction() { // open with old pw and old ksFn, should throw exception, since cannot decrypt pref = create("testChangePassword", "pw1".toCharArray()) - .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); + .keyStretchingFunction(new BrokenBcryptKeyStretcher(8)).build(); try { pref.getString("k1", null); fail("should throw exception, since cannot decrypt"); @@ -471,13 +477,13 @@ public void testChangePasswordAndKeyStretchingFunction() { public void testInvalidPasswordShouldNotBeAccessible() { // open new shared pref and add some data ArmadilloSharedPreferences pref = create("testInvalidPassword", "pw1".toCharArray()) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); pref.edit().putString("k1", "string1").putInt("k2", 2).putBoolean("k3", true).commit(); pref.close(); // open again and check if can be used pref = create("testInvalidPassword", "pw1".toCharArray()) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); assertTrue(pref.getBoolean("k3", false)); @@ -485,7 +491,7 @@ public void testInvalidPasswordShouldNotBeAccessible() { // open with invalid pw, should throw exception, since cannot decrypt pref = create("testInvalidPassword", "pw2".toCharArray()) - .keyStretchingFunction(new FastKeyStretcher()).build(); + .keyStretchingFunction(new FastKeyStretcher()).build(); try { pref.getString("k1", null); fail("should throw exception, since cannot decrypt"); diff --git a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java new file mode 100644 index 0000000..f99379e --- /dev/null +++ b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java @@ -0,0 +1,17 @@ +package at.favre.lib.armadillo; + +import at.favre.lib.bytes.Bytes; + +public class TestEncryptionFingerprint implements EncryptionFingerprint { + private byte[] fp = Bytes.random(16).array(); + + @Override + public byte[] getBytes() { + return fp; + } + + @Override + public void wipe() { + Bytes.wrap(fp).mutable().secureWipe(); + } +} diff --git a/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java b/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java new file mode 100644 index 0000000..26701a8 --- /dev/null +++ b/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java @@ -0,0 +1,55 @@ +package at.favre.lib.armadillo; + +import org.junit.Before; +import org.junit.Test; + +import java.security.SecureRandom; + +import at.favre.lib.bytes.Bytes; + +import static org.junit.Assert.assertArrayEquals; + +public class ByteArrayObfuscatorTest { + + @Before + public void setUp() { + } + + @Test + public void obfuscateRandom() { + for (int i = 0; i < 100; i++) { + testIntern(Bytes.random(32).array()); + } + } + + @Test + public void obfuscateVariousLengths() { + testIntern(Bytes.random(1).array()); + testIntern(Bytes.random(2).array()); + testIntern(Bytes.random(3).array()); + testIntern(Bytes.random(3).array()); + testIntern(Bytes.random(16).array()); + testIntern(Bytes.random(23).array()); + testIntern(Bytes.random(24).array()); + testIntern(Bytes.random(25).array()); + testIntern(Bytes.random(32).array()); + testIntern(Bytes.random(512).array()); + } + + @Test + public void obfuscateSimpleData() { + testIntern(Bytes.allocate(1).array()); + testIntern(Bytes.allocate(2).array()); + testIntern(Bytes.allocate(16).array()); + testIntern(Bytes.allocate(32).array()); + } + + private void testIntern(byte[] target) { + System.out.println("original: " + Bytes.wrap(target).encodeHex()); + ByteArrayRuntimeObfuscator obfuscator = new ByteArrayRuntimeObfuscator.Default(Bytes.wrap(target).copy().array(), new SecureRandom()); + byte[] unobfuscated = obfuscator.getBytes(); + System.out.println("deobfuscated: " + Bytes.wrap(unobfuscated).encodeHex()); + + assertArrayEquals(target, unobfuscated); + } +} diff --git a/armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java b/armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java new file mode 100644 index 0000000..f644572 --- /dev/null +++ b/armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java @@ -0,0 +1,29 @@ +package at.favre.lib.armadillo; + +import org.junit.Before; +import org.junit.Test; + +import java.security.SecureRandom; + +import static org.junit.Assert.assertArrayEquals; + +public class DefaultEncryptionProtocolTest { + + @Before + public void setUp() { + } + + @Test + public void obfuscatePassword() { + EncryptionProtocol protocol = new DefaultEncryptionProtocol + .Factory(0, new TestEncryptionFingerprint(), new HkdfMessageDigest(new byte[16], 20), new AesGcmEncryption(), + AuthenticatedEncryption.STRENGTH_HIGH, new FastKeyStretcher(), new HkdfXorObfuscator.Factory(), + new SecureRandom(), null).create(new byte[16]); + + char[] pw = "5187h_asd€éÀä'#\uD83E\uDD2F#_".toCharArray(); + ByteArrayRuntimeObfuscator o = protocol.obfuscatePassword(pw); + char[] pw2 = protocol.deobfuscatePassword(o); + + assertArrayEquals(pw, pw2); + } +} From c27216b3fbfed3be87c7b90f737a11170512436d Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Thu, 1 Nov 2018 15:49:52 +0100 Subject: [PATCH 02/12] Fix failing tests --- .../armadillo/ByteArrayRuntimeObfuscator.java | 21 ++++--------------- .../ASecureSharedPreferencesTest.java | 2 +- .../armadillo/TestEncryptionFingerprint.java | 12 +++++++++-- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java index cba1dfc..a1eceb5 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java @@ -28,30 +28,17 @@ public interface ByteArrayRuntimeObfuscator { void wipe(); final class Default implements ByteArrayRuntimeObfuscator { + private final byte[][] data; Default(byte[] array, SecureRandom secureRandom) { - int len = (int) (Math.abs(Bytes.random(8).toLong()) % 9) + 1; - this.data = new byte[len + 1][]; - for (int i = 0; i < data.length - 1; i++) { - byte[] key = Bytes.random(array.length, secureRandom).array(); - this.data[i] = key; - Bytes.wrap(array).mutable().xor(key); - } - this.data[data.length - 1] = array; + byte[] key = Bytes.random(array.length, secureRandom).array(); + this.data = new byte[][] {key, Bytes.wrap(array).mutable().xor(key).array()}; } @Override public byte[] getBytes() { - Bytes b = Bytes.empty(); - for (int i = data.length - 1; i >= 0; i--) { - if (b.isEmpty()) { - b = Bytes.wrap(data[i]).mutable(); - continue; - } - b.xor(data[i]); - } - return b.array(); + return Bytes.wrap(data[0]).xor(data[1]).array(); } @Override diff --git a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java index 37e3c1e..8976cc1 100644 --- a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java +++ b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java @@ -250,7 +250,7 @@ public void testWithDifferentFingerprint() { preferenceSmokeTest(create("fingerprint", null) .encryptionFingerprint(Bytes.random(16).array()).build()); preferenceSmokeTest(create("fingerprint2", null) - .encryptionFingerprint(new TestEncryptionFingerprint()).build()); + .encryptionFingerprint(new TestEncryptionFingerprint(new byte[16])).build()); } @Test diff --git a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java index f99379e..7f08895 100644 --- a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java +++ b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java @@ -3,11 +3,19 @@ import at.favre.lib.bytes.Bytes; public class TestEncryptionFingerprint implements EncryptionFingerprint { - private byte[] fp = Bytes.random(16).array(); + private final byte[] fp; + + public TestEncryptionFingerprint() { + this(Bytes.random(16).array()); + } + + public TestEncryptionFingerprint(byte[] fp) { + this.fp = fp; + } @Override public byte[] getBytes() { - return fp; + return Bytes.wrap(fp).copy().array(); } @Override From 47df60b5fcc7423085711c8575dbf2a7869e6cbf Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Thu, 1 Nov 2018 16:10:25 +0100 Subject: [PATCH 03/12] Improve ByteArrayRuntimeObfuscator --- .../armadillo/ByteArrayRuntimeObfuscator.java | 22 +++++++++++++++---- .../armadillo/ByteArrayObfuscatorTest.java | 18 ++++++++++----- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java index a1eceb5..1346ea0 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java @@ -28,17 +28,31 @@ public interface ByteArrayRuntimeObfuscator { void wipe(); final class Default implements ByteArrayRuntimeObfuscator { - private final byte[][] data; Default(byte[] array, SecureRandom secureRandom) { - byte[] key = Bytes.random(array.length, secureRandom).array(); - this.data = new byte[][] {key, Bytes.wrap(array).mutable().xor(key).array()}; + int len = (int) (Math.abs(Bytes.random(8).toLong()) % 9) + 1; + this.data = new byte[len + 1][]; + Bytes copy = Bytes.from(array).mutable(); + for (int i = 0; i < data.length - 1; i++) { + byte[] key = Bytes.random(array.length, secureRandom).array(); + this.data[i] = key; + copy.xor(key); + } + this.data[data.length - 1] = copy.array(); } @Override public byte[] getBytes() { - return Bytes.wrap(data[0]).xor(data[1]).array(); + Bytes b = Bytes.empty(); + for (int i = data.length - 1; i >= 0; i--) { + if (b.isEmpty()) { + b = Bytes.from(data[i]).mutable(); + continue; + } + b.xor(data[i]); + } + return b.array(); } @Override diff --git a/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java b/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java index 26701a8..ba5279d 100644 --- a/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java +++ b/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java @@ -1,20 +1,17 @@ package at.favre.lib.armadillo; -import org.junit.Before; import org.junit.Test; import java.security.SecureRandom; +import java.util.Arrays; import at.favre.lib.bytes.Bytes; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertFalse; public class ByteArrayObfuscatorTest { - @Before - public void setUp() { - } - @Test public void obfuscateRandom() { for (int i = 0; i < 100; i++) { @@ -47,9 +44,18 @@ public void obfuscateSimpleData() { private void testIntern(byte[] target) { System.out.println("original: " + Bytes.wrap(target).encodeHex()); ByteArrayRuntimeObfuscator obfuscator = new ByteArrayRuntimeObfuscator.Default(Bytes.wrap(target).copy().array(), new SecureRandom()); - byte[] unobfuscated = obfuscator.getBytes(); + byte[] unobfuscated = Bytes.wrap(obfuscator.getBytes()).copy().array(); + byte[] unobfuscated2 = Bytes.wrap(obfuscator.getBytes()).copy().array(); System.out.println("deobfuscated: " + Bytes.wrap(unobfuscated).encodeHex()); + System.out.println("deobfuscated: " + Bytes.wrap(unobfuscated2).encodeHex()); assertArrayEquals(target, unobfuscated); + assertArrayEquals(target, unobfuscated2); + + obfuscator.wipe(); + + assertFalse(Arrays.equals(obfuscator.getBytes(), target)); + assertFalse(Arrays.equals(obfuscator.getBytes(), unobfuscated)); + assertFalse(Arrays.equals(obfuscator.getBytes(), unobfuscated2)); } } From a9c49d2667e30ba96267a70cea46dc3120d603c6 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Thu, 1 Nov 2018 16:31:25 +0100 Subject: [PATCH 04/12] Improve ByteArrayRuntimeObfuscator #2 refs #32 --- .../armadillo/ByteArrayRuntimeObfuscator.java | 17 +++++++++++++---- .../lib/armadillo/ByteArrayObfuscatorTest.java | 7 +++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java index 1346ea0..ea6608f 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java @@ -29,13 +29,19 @@ public interface ByteArrayRuntimeObfuscator { final class Default implements ByteArrayRuntimeObfuscator { private final byte[][] data; + private final SecureRandom secureRandom; Default(byte[] array, SecureRandom secureRandom) { - int len = (int) (Math.abs(Bytes.random(8).toLong()) % 9) + 1; - this.data = new byte[len + 1][]; - Bytes copy = Bytes.from(array).mutable(); + this.secureRandom = secureRandom; + int keyCount = (int) (Math.abs(Bytes.random(8).toLong()) % 9) + 1; + this.data = new byte[keyCount + 1][]; + createAndEncrypt(secureRandom, Bytes.from(array), array.length); + } + + private void createAndEncrypt(SecureRandom secureRandom, Bytes from, int length) { + Bytes copy = from.mutable(); for (int i = 0; i < data.length - 1; i++) { - byte[] key = Bytes.random(array.length, secureRandom).array(); + byte[] key = Bytes.random(length, secureRandom).array(); this.data[i] = key; copy.xor(key); } @@ -52,6 +58,9 @@ public byte[] getBytes() { } b.xor(data[i]); } + + createAndEncrypt(this.secureRandom, Bytes.from(b), b.length()); + return b.array(); } diff --git a/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java b/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java index ba5279d..f8738d6 100644 --- a/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java +++ b/armadillo/src/test/java/at/favre/lib/armadillo/ByteArrayObfuscatorTest.java @@ -44,13 +44,16 @@ public void obfuscateSimpleData() { private void testIntern(byte[] target) { System.out.println("original: " + Bytes.wrap(target).encodeHex()); ByteArrayRuntimeObfuscator obfuscator = new ByteArrayRuntimeObfuscator.Default(Bytes.wrap(target).copy().array(), new SecureRandom()); + byte[] unobfuscated = Bytes.wrap(obfuscator.getBytes()).copy().array(); byte[] unobfuscated2 = Bytes.wrap(obfuscator.getBytes()).copy().array(); + byte[] unobfuscated3 = Bytes.wrap(obfuscator.getBytes()).copy().array(); + System.out.println("deobfuscated: " + Bytes.wrap(unobfuscated).encodeHex()); - System.out.println("deobfuscated: " + Bytes.wrap(unobfuscated2).encodeHex()); assertArrayEquals(target, unobfuscated); - assertArrayEquals(target, unobfuscated2); + assertArrayEquals(unobfuscated, unobfuscated2); + assertArrayEquals(unobfuscated2, unobfuscated3); obfuscator.wipe(); From 1bdeb7868600d491b2798cee38849070d6a5fe42 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Fri, 2 Nov 2018 12:49:24 +0100 Subject: [PATCH 05/12] Add javadoc to new obfuscate methods in EncryptionProtocol refs #32 --- .../at/favre/lib/armadillo/EncryptionProtocol.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java index 1e7396f..fe1b008 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java @@ -86,9 +86,21 @@ interface EncryptionProtocol { */ KeyStretchingFunction getKeyStretchingFunction(); + /** + * Take raw char password and obfuscate (for use in-memory) + * + * @param password to obfuscate + * @return obfuscated password + */ @Nullable ByteArrayRuntimeObfuscator obfuscatePassword(@Nullable char[] password); + /** + * Convert obfuscated obfuscated bytes to char array again + * + * @param obfuscated to deobfuscate + * @return char array + */ @Nullable char[] deobfuscatePassword(@Nullable ByteArrayRuntimeObfuscator obfuscated); From 60b13b0e9dbaa2d2cb1b04c815c0e4ba4c4059d5 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Fri, 2 Nov 2018 22:27:18 +0100 Subject: [PATCH 06/12] Add derived user password cache to speed up get calls refs #33 --- .../at/favre/lib/armadillo/Armadillo.java | 33 +++++- .../armadillo/ByteArrayRuntimeObfuscator.java | 4 +- .../armadillo/DefaultEncryptionProtocol.java | 29 ++++- .../lib/armadillo/DerivedPasswordCache.java | 103 ++++++++++++++++++ .../EncryptionFingerprintFactory.java | 2 +- .../lib/armadillo/EncryptionProtocol.java | 5 + .../ASecureSharedPreferencesTest.java | 22 +++- .../armadillo/TestEncryptionFingerprint.java | 6 +- .../DefaultEncryptionProtocolTest.java | 98 +++++++++++++++-- 9 files changed, 275 insertions(+), 27 deletions(-) create mode 100644 armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/Armadillo.java b/armadillo/src/main/java/at/favre/lib/armadillo/Armadillo.java index 67bbb0c..077ed87 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/Armadillo.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/Armadillo.java @@ -51,6 +51,7 @@ public static final class Builder { private boolean supportVerifyPassword = false; private Provider provider; private int cryptoProtocolVersion = 0; + private boolean enableDerivedPasswordCache = false; private Compressor compressor = new DisabledCompressor(); private Builder(SharedPreferences sharedPreferences) { @@ -134,7 +135,6 @@ public Builder contentKeyDigest(StringMessageDigest stringMessageDigest) { * @return builder */ public Builder encryptionKeyStrength(@AuthenticatedEncryption.KeyStrength int keyStrength) { - Objects.requireNonNull(keyStrength); this.keyStrength = keyStrength; return this; } @@ -281,6 +281,35 @@ public Builder cryptoProtocolVersion(int version) { return this; } + /** + * Per default every put and get operation, when using a user provided password, requires the + * full expensive key derivation function (KDF) to derive the key. This can add up to multiple + * seconds if you get/put multiple values consecutively. Default is false. + *

+ * To make get* calls faster, you can enable this cache, which caches the derived + * password. This will not speed up put* operations since every time a new salt will be created + * making it impossible to cache. The disadvantage is that the derived password stays in cache + * , therefor in memory for way longer, making it easier to read when the device is used with + * instrumentation tool like FRIDA (this is a more specific attack, since when the attacker has + * full access to the device, there is not much you can do). + *

+ * + * Summary + *

    + *
  • Improves performance of consecutive get calls when using expensive key stretching function and user password
    • + *
  • Slightly reduces security strenght, since the stretched bytes are kept in memory for longer
    • + *
+ *

+ * See {@link DerivedPasswordCache} for details of the implementation. + * + * @param enable caching + * @return builder + */ + public Builder enableDerivedPasswordCache(boolean enable) { + enableDerivedPasswordCache = enable; + return this; + } + /** * Compresses the content with Gzip before encrypting and writing it to shared preference. This only makes * sense if bigger structural data is persisted like long xml or json. @@ -316,7 +345,7 @@ public ArmadilloSharedPreferences build() { } EncryptionProtocol.Factory factory = new DefaultEncryptionProtocol.Factory(cryptoProtocolVersion, fingerprint, stringMessageDigest, authenticatedEncryption, keyStrength, - keyStretchingFunction, dataObfuscatorFactory, secureRandom, compressor); + keyStretchingFunction, dataObfuscatorFactory, secureRandom, enableDerivedPasswordCache, compressor); if (sharedPreferences != null) { return new SecureSharedPreferences(sharedPreferences, factory, recoveryPolicy, password, supportVerifyPassword); diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java index ea6608f..48c8126 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java @@ -1,6 +1,7 @@ package at.favre.lib.armadillo; import java.security.SecureRandom; +import java.util.Objects; import at.favre.lib.bytes.Bytes; @@ -32,7 +33,8 @@ final class Default implements ByteArrayRuntimeObfuscator { private final SecureRandom secureRandom; Default(byte[] array, SecureRandom secureRandom) { - this.secureRandom = secureRandom; + Objects.requireNonNull(array); + this.secureRandom = Objects.requireNonNull(secureRandom); int keyCount = (int) (Math.abs(Bytes.random(8).toLong()) % 9) + 1; this.data = new byte[keyCount + 1][]; createAndEncrypt(secureRandom, Bytes.from(array), array.length); diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java b/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java index 2000ae3..fc7acfa 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/DefaultEncryptionProtocol.java @@ -48,12 +48,14 @@ final class DefaultEncryptionProtocol implements EncryptionProtocol { private final SecureRandom secureRandom; private final int keyLengthBit; private final int protocolVersion; + private final DerivedPasswordCache derivedPasswordCache; private KeyStretchingFunction keyStretchingFunction; private DefaultEncryptionProtocol(int protocolVersion, byte[] preferenceSalt, EncryptionFingerprint fingerprint, StringMessageDigest stringMessageDigest, AuthenticatedEncryption authenticatedEncryption, @AuthenticatedEncryption.KeyStrength int keyStrength, KeyStretchingFunction keyStretchingFunction, - DataObfuscator.Factory dataObfuscatorFactory, SecureRandom secureRandom, Compressor compressor) { + DataObfuscator.Factory dataObfuscatorFactory, SecureRandom secureRandom, + boolean enableDerivedPasswordCaching, Compressor compressor) { this.protocolVersion = protocolVersion; this.preferenceSalt = preferenceSalt; this.authenticatedEncryption = authenticatedEncryption; @@ -64,6 +66,7 @@ private DefaultEncryptionProtocol(int protocolVersion, byte[] preferenceSalt, En this.keyLengthBit = authenticatedEncryption.byteSizeLength(keyStrength) * 8; this.dataObfuscatorFactory = dataObfuscatorFactory; this.secureRandom = secureRandom; + this.derivedPasswordCache = new DerivedPasswordCache.Default(enableDerivedPasswordCaching, secureRandom); } @Override @@ -164,8 +167,8 @@ public byte[] decrypt(@NonNull String contentKey, @Nullable char[] password, byt @Override public void setKeyStretchingFunction(@NonNull KeyStretchingFunction function) { - Objects.requireNonNull(function); - keyStretchingFunction = function; + keyStretchingFunction = Objects.requireNonNull(function); + derivedPasswordCache.wipe(); } @Override @@ -194,11 +197,21 @@ public char[] deobfuscatePassword(@Nullable ByteArrayRuntimeObfuscator obfuscate return charBuffer.array(); } + @Override + public void wipeDerivedPasswordCache() { + derivedPasswordCache.wipe(); + } + private byte[] keyDerivationFunction(String contentKey, byte[] fingerprint, byte[] contentSalt, byte[] preferenceSalt, @Nullable char[] password) { Bytes ikm = Bytes.from(fingerprint, contentSalt, Bytes.from(contentKey, Normalizer.Form.NFKD).array()); if (password != null) { - ikm = ikm.append(keyStretchingFunction.stretch(contentSalt, password, STRETCHED_PASSWORD_LENGTH_BYTES)); + byte[] stretched; + if ((stretched = derivedPasswordCache.get(contentSalt, password)) == null) { + stretched = keyStretchingFunction.stretch(contentSalt, password, STRETCHED_PASSWORD_LENGTH_BYTES); + derivedPasswordCache.put(contentSalt, password, stretched); + } + ikm = ikm.append(stretched); } return HKDF.fromHmacSha512().extractAndExpand(preferenceSalt, ikm.array(), "DefaultEncryptionProtocol".getBytes(), keyLengthBit / 8); @@ -215,12 +228,13 @@ public static final class Factory implements EncryptionProtocol.Factory { private final KeyStretchingFunction keyStretchingFunction; private final DataObfuscator.Factory dataObfuscatorFactory; private final SecureRandom secureRandom; + private final boolean enableDerivedPasswordCaching; private final Compressor compressor; Factory(int protocolVersion, EncryptionFingerprint fingerprint, StringMessageDigest stringMessageDigest, AuthenticatedEncryption authenticatedEncryption, @AuthenticatedEncryption.KeyStrength int keyStrength, KeyStretchingFunction keyStretchingFunction, DataObfuscator.Factory dataObfuscatorFactory, - SecureRandom secureRandom, @Nullable Compressor compressor) { + SecureRandom secureRandom, boolean enableDerivedPasswordCaching, @Nullable Compressor compressor) { this.protocolVersion = protocolVersion; this.fingerprint = fingerprint; this.stringMessageDigest = stringMessageDigest; @@ -229,12 +243,15 @@ public static final class Factory implements EncryptionProtocol.Factory { this.keyStretchingFunction = keyStretchingFunction; this.dataObfuscatorFactory = dataObfuscatorFactory; this.secureRandom = secureRandom; + this.enableDerivedPasswordCaching = enableDerivedPasswordCaching; this.compressor = compressor; } @Override public EncryptionProtocol create(byte[] preferenceSalt) { - return new DefaultEncryptionProtocol(protocolVersion, preferenceSalt, fingerprint, stringMessageDigest, authenticatedEncryption, keyStrength, keyStretchingFunction, dataObfuscatorFactory, secureRandom, compressor); + return new DefaultEncryptionProtocol(protocolVersion, preferenceSalt, fingerprint, stringMessageDigest, + authenticatedEncryption, keyStrength, keyStretchingFunction, dataObfuscatorFactory, + secureRandom, enableDerivedPasswordCaching, compressor); } @Override diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java b/armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java new file mode 100644 index 0000000..b04fcb6 --- /dev/null +++ b/armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java @@ -0,0 +1,103 @@ +package at.favre.lib.armadillo; + +import android.support.annotation.Nullable; +import android.util.LruCache; + +import java.security.SecureRandom; + +import at.favre.lib.bytes.Bytes; + +/** + * A simple caches, that helps caching derived passwords so that not every get operation requires + * to derive over the (expensive) key stretching function. + */ +public interface DerivedPasswordCache { + + /** + * Get the derived bytes from given salt and password. + * + * @param salt the used salt for the stretching function - key part 2 of the internal key-value storage + * @param pw the used password - key part 2 of the internal key-value storage + * @return the cached bytes or null of not found + */ + @Nullable + byte[] get(byte[] salt, char[] pw); + + /** + * Put a new stretched password with given salt in the cache + * + * @param salt the used salt for the stretching function - key part 2 of the internal key-value storage + * @param pw the used password - key part 2 of the internal key-value storage + * @param value the stretched bytes + */ + void put(byte[] salt, char[] pw, byte[] value); + + /** + * Overwrite and remove whole cache + */ + void wipe(); + + /** + * Standard implementation of {@link DerivedPasswordCache} + */ + class Default implements DerivedPasswordCache { + + private final boolean enabled; + private final SecureRandom secureRandom; + private final LruCache cache; + private long key; + + public Default(boolean enabled, SecureRandom secureRandom) { + this.enabled = enabled; + this.secureRandom = secureRandom; + this.cache = new LruCache<>(8); + } + + @Nullable + @Override + public byte[] get(byte[] salt, char[] rawData) { + if (!enabled) return null; + + if (key == getPwKey(rawData)) { + ByteArrayRuntimeObfuscator o = cache.get(getSaltKey(salt)); + return o != null ? o.getBytes() : null; + } + + wipe(); + return null; + } + + @Override + public void put(byte[] salt, char[] rawData, byte[] value) { + if (enabled) { + long pwKey = getPwKey(rawData); + if (pwKey != key) { + wipe(); + } + + key = pwKey; + long saltKey = getSaltKey(salt); + cache.put(saltKey, new ByteArrayRuntimeObfuscator.Default(value, secureRandom)); + } + } + + private long getPwKey(char[] rawData) { + return Bytes.from(rawData).hashSha256().longAt(0); + } + + private long getSaltKey(byte[] rawData) { + return Bytes.from(rawData).hashSha256().longAt(0); + } + + @Override + public void wipe() { + key = 0; + if (cache.snapshot() != null) { + for (ByteArrayRuntimeObfuscator obfuscator : cache.snapshot().values()) { + obfuscator.wipe(); + } + } + cache.evictAll(); + } + } +} diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprintFactory.java b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprintFactory.java index e4ed65c..8a33ebd 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprintFactory.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionFingerprintFactory.java @@ -50,7 +50,7 @@ public static EncryptionFingerprint create(Context context, @Nullable String add Bytes.from(getApplicationPackage(context)).array(), Bytes.from(getBuildDetails()).array(), BuildConfig.STATIC_RANDOM, - additionalData != null ? Bytes.from(additionalData).array() : Bytes.from("").array()).array()); + additionalData != null ? Bytes.from(additionalData).array() : Bytes.empty().array()).array()); } /** diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java index fe1b008..48309b6 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/EncryptionProtocol.java @@ -104,6 +104,11 @@ interface EncryptionProtocol { @Nullable char[] deobfuscatePassword(@Nullable ByteArrayRuntimeObfuscator obfuscated); + /** + * If enabled, wipes the internal derived password cache. + */ + void wipeDerivedPasswordCache(); + /** * Factory creating a new instance of {@link EncryptionProtocol} */ diff --git a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java index 8976cc1..82fcf35 100644 --- a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java +++ b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/ASecureSharedPreferencesTest.java @@ -334,30 +334,35 @@ void preferenceSmokeTest(SharedPreferences preferences) { @Test public void testChangePassword() { - testChangePassword("testChangePassword", "pw1".toCharArray(), "pw2".toCharArray()); + testChangePassword("testChangePassword", "pw1".toCharArray(), "pw2".toCharArray(), false); + } + + @Test + public void testChangePasswordWithEnabledDerivedPwCache() { + testChangePassword("testChangePassword", "pw1".toCharArray(), "pw2".toCharArray(), true); } @Test public void testChangePasswordFromNullPassword() { - testChangePassword("testChangePasswordFromNullPassword", null, "pw2".toCharArray()); + testChangePassword("testChangePasswordFromNullPassword", null, "pw2".toCharArray(), false); } @Test public void testChangePasswordToNullPassword() { - testChangePassword("testChangePasswordToNullPassword", "pw1".toCharArray(), null); + testChangePassword("testChangePasswordToNullPassword", "pw1".toCharArray(), null, false); } @Test public void testChangePasswordFromEmptyPassword() { - testChangePassword("testChangePasswordFromEmptyPassword", "".toCharArray(), "pw2".toCharArray()); + testChangePassword("testChangePasswordFromEmptyPassword", "".toCharArray(), "pw2".toCharArray(), false); } @Test public void testChangePasswordToEmptyPassword() { - testChangePassword("testChangePasswordToEmptyPassword", "pw1".toCharArray(), "".toCharArray()); + testChangePassword("testChangePasswordToEmptyPassword", "pw1".toCharArray(), "".toCharArray(), false); } - private void testChangePassword(String name, char[] currentPassword, char[] newPassword) { + private void testChangePassword(String name, char[] currentPassword, char[] newPassword, boolean enableCache) { Set testSet = new HashSet<>(); testSet.add("t1"); testSet.add("t2"); @@ -365,6 +370,7 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open new shared pref and add some data ArmadilloSharedPreferences pref = create(name, clonePassword(currentPassword)) + .enableDerivedPasswordCache(enableCache) .keyStretchingFunction(new FastKeyStretcher()).build(); pref.edit().putString("k1", "string1").putInt("k2", 2).putStringSet("set", testSet) .putBoolean("k3", true).commit(); @@ -372,6 +378,7 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open again and check if can be used pref = create(name, clonePassword(currentPassword)) + .enableDerivedPasswordCache(enableCache) .keyStretchingFunction(new FastKeyStretcher()).build(); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); @@ -381,6 +388,7 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open with old pw and change to new one, all the values should be accessible pref = create(name, clonePassword(currentPassword)) + .enableDerivedPasswordCache(enableCache) .keyStretchingFunction(new FastKeyStretcher()).build(); pref.changePassword(clonePassword(newPassword)); assertEquals("string1", pref.getString("k1", null)); @@ -391,6 +399,7 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open with new pw, should be accessible pref = create(name, clonePassword(newPassword)) + .enableDerivedPasswordCache(enableCache) .keyStretchingFunction(new FastKeyStretcher()).build(); assertEquals("string1", pref.getString("k1", null)); assertEquals(2, pref.getInt("k2", 0)); @@ -400,6 +409,7 @@ private void testChangePassword(String name, char[] currentPassword, char[] newP // open with old pw, should throw exception, since cannot decrypt pref = create(name, clonePassword(currentPassword)) + .enableDerivedPasswordCache(enableCache) .keyStretchingFunction(new FastKeyStretcher()).build(); try { pref.getString("k1", null); diff --git a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java index 7f08895..e8cfbe0 100644 --- a/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java +++ b/armadillo/src/sharedTest/java/at/favre/lib/armadillo/TestEncryptionFingerprint.java @@ -2,14 +2,14 @@ import at.favre.lib.bytes.Bytes; -public class TestEncryptionFingerprint implements EncryptionFingerprint { +class TestEncryptionFingerprint implements EncryptionFingerprint { private final byte[] fp; - public TestEncryptionFingerprint() { + TestEncryptionFingerprint() { this(Bytes.random(16).array()); } - public TestEncryptionFingerprint(byte[] fp) { + TestEncryptionFingerprint(byte[] fp) { this.fp = fp; } diff --git a/armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java b/armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java index f644572..4d25fa0 100644 --- a/armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java +++ b/armadillo/src/test/java/at/favre/lib/armadillo/DefaultEncryptionProtocolTest.java @@ -1,29 +1,111 @@ package at.favre.lib.armadillo; -import org.junit.Before; +import android.support.annotation.NonNull; + import org.junit.Test; import java.security.SecureRandom; +import at.favre.lib.bytes.Bytes; + import static org.junit.Assert.assertArrayEquals; public class DefaultEncryptionProtocolTest { - @Before - public void setUp() { + @Test + public void testEncryptDecrypt() throws Exception { + testEncryptDecrypt(Bytes.random(20).encodeHex(), Bytes.random(1).array(), null); + testEncryptDecrypt(Bytes.random(20).encodeHex(), Bytes.random(16).array(), null); + testEncryptDecrypt(Bytes.random(20).encodeHex(), Bytes.random(16).array(), "1234".toCharArray()); + testEncryptDecrypt(Bytes.random(20).encodeHex(), Bytes.random(354).array(), "乨ØDzDzɻaД\u058DAא\u08A1A2$ᶀṻὉ\u202A₸ꜽ!ö\uD83E\uDD2Fײַ".toCharArray()); + } + + private EncryptionProtocol testEncryptDecrypt(String contentKey, byte[] content, char[] pw) throws EncryptionProtocolException { + EncryptionProtocol protocol = createDefaultProtocol(new TestEncryptionFingerprint()); + byte[] encrypted = protocol.encrypt(contentKey, pw, content); + + // decrypt twice so state does not matter + assertArrayEquals(content, protocol.decrypt(contentKey, pw, encrypted)); + assertArrayEquals(content, protocol.decrypt(contentKey, pw, encrypted)); + return protocol; + } + + @Test + public void testAgainstHardcodedValuesToSpotMigrationIssues() throws Exception { + testDecrypt("969f3276e85b74af6154ce6ddf912c334cf311b3", + "122919b282023568695c04a863f32e3f", + null, + "0000000010ddd75ec912994a21bfe6e85e7264f77e0000002d7e9beac3c605c0fbecf5c4b3e119b35b40469683e0ca82285d51799926788630cdfe5d7ca27e77bddd4d8260b1"); + + testDecrypt("969f3276e85b74af6154ce6ddf912c334cf311b3", + "122919b282023568695c04a863f32e3f", + "1234", + "000000001034ccac29ba0ad1ed80475d991dce29ee0000002d7e1c39db5d17df6f84ebe6805115125e702b2ae6fe0ba49a16e8281fb08e0733e53310ed677814965491ffeb9c"); + + testDecrypt("969f3276e85b74af6154ce6ddf912c334cf311b3", + "122919b282023568695c04a863f32e3f", + "1234", + "0000000010e96ba92b343976ee8858f1d6143a42cc0000002d7e9d7318b1a420dd090ac5bd7d65ddc9e3e3c8f87547719b5ac20c6f03501235ad03909b4fdcd7864aae018e4a"); + + testDecrypt("df60a9c11db3fd443cd89def85d8ff046683ae27", + "b94325505418d8cf56fdca8c3e096a5f813a9ad84937ed99e28bce0a3f259bee83834e0bee24705720da13", + "乨ØDzDzɻaД\u058DAא\u08A1A2", + "0000000010b6b7daf84c988d3dc5034a52cddd91c50000004833aacb15329f192ff4542c5d010fcd1fc747306757d69b2869641c827115a4cd274b55b4f6c08277ef8cd565abc515a9bc03cf6c2b960588079ced91106dad418af12a16bd7069e8"); + } + + private void testDecrypt(String contentKey, String contentHex, String pw, String refContent) throws Exception { + byte[] fingerprint = Bytes.parseHex("f5ce0a3105f75006008608edf417b40d").array(); + byte[] preferencesSalt = Bytes.parseHex("9752f5ea8c8a35446d22d6a59d1d1b0544c5978d").array(); + EncryptionProtocol protocol = createDefaultProtocolFactory(new TestEncryptionFingerprint(fingerprint)).create(preferencesSalt); + //byte[] enc = protocol.encrypt(contentKey,null, Bytes.parseHex(contentHex).array()); + assertArrayEquals(Bytes.parseHex(contentHex).array(), protocol.decrypt(contentKey, pw != null ? pw.toCharArray() : null, Bytes.parseHex(refContent).array())); } @Test public void obfuscatePassword() { - EncryptionProtocol protocol = new DefaultEncryptionProtocol - .Factory(0, new TestEncryptionFingerprint(), new HkdfMessageDigest(new byte[16], 20), new AesGcmEncryption(), - AuthenticatedEncryption.STRENGTH_HIGH, new FastKeyStretcher(), new HkdfXorObfuscator.Factory(), - new SecureRandom(), null).create(new byte[16]); + testObfuscatePw("123456abcdefABCDEF_:;$%&/()=?\"}][{"); + testObfuscatePw("5187h_asd€éÀä'#\uD83E\uDD2F#_"); + testObfuscatePw("µ€ßüöäáé´Ààó"); + testObfuscatePw("乨ØDzDzɻД\u058Dא\u08A1ᶀṻὉ\u202A₸ꜽײַ"); + testObfuscatePw("乨ØDzDzɻaД\u058DAא\u08A1A2$ᶀṻὉ\u202A₸ꜽ!ö\uD83E\uDD2Fײַ"); + } - char[] pw = "5187h_asd€éÀä'#\uD83E\uDD2F#_".toCharArray(); + private void testObfuscatePw(String pwString) { + EncryptionProtocol protocol = createDefaultProtocol(new TestEncryptionFingerprint()); + + char[] pw = pwString.toCharArray(); ByteArrayRuntimeObfuscator o = protocol.obfuscatePassword(pw); char[] pw2 = protocol.deobfuscatePassword(o); assertArrayEquals(pw, pw2); } + + @Test + public void testWipeCache() throws Exception { + EncryptionProtocol protocol = createDefaultProtocol(new TestEncryptionFingerprint()); + String contentKey = Bytes.random(20).encodeHex(); + byte[] content = Bytes.random(354).array(); + char[] pw = "乨ØDzDzɻaД\u058DAא\u08A1A2$ᶀṻὉ\u202A₸ꜽ!ö\uD83E\uDD2Fײַ".toCharArray(); + byte[] encrypted = protocol.encrypt(contentKey, pw, content); + + protocol.decrypt(contentKey, pw, encrypted); + protocol.decrypt(contentKey, pw, encrypted); + protocol.decrypt(contentKey, pw, encrypted); + + protocol.wipeDerivedPasswordCache(); + + protocol.decrypt(contentKey, pw, encrypted); + } + + @NonNull + private EncryptionProtocol createDefaultProtocol(EncryptionFingerprint fingerprint) { + return createDefaultProtocolFactory(fingerprint).create(Bytes.random(16).array()); + } + + private EncryptionProtocol.Factory createDefaultProtocolFactory(EncryptionFingerprint fingerprint) { + return new DefaultEncryptionProtocol + .Factory(0, fingerprint, new HkdfMessageDigest(new byte[16], 20), new AesGcmEncryption(), + AuthenticatedEncryption.STRENGTH_HIGH, new ArmadilloBcryptKeyStretcher(4), new HkdfXorObfuscator.Factory(), + new SecureRandom(), false, new DisabledCompressor()); + } } From 6f0d3566a113f66875872d162c48e923224f4181 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Sat, 3 Nov 2018 13:11:07 +0100 Subject: [PATCH 07/12] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d72f629..3855d05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## v0.6.0 * [Security] Fix bcrypt implementation #16, #8 +* Improve derive user password memory protection #32 +* Add derived password cache to speed up consecutive .get() calls #32 * Add change password feature #13, #22 * Add change key stretching feature #16 From 3713f359fb8fb1de9cbf2701fb46179bd6a1556a Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Thu, 8 Nov 2018 14:29:53 +0100 Subject: [PATCH 08/12] Fix checkstyle issues --- .../lib/securesharedpreferences/ChangePasswordActivity.java | 2 +- .../java/at/favre/lib/securesharedpreferences/MainActivity.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/at/favre/lib/securesharedpreferences/ChangePasswordActivity.java b/app/src/main/java/at/favre/lib/securesharedpreferences/ChangePasswordActivity.java index b657a8f..7edcbe5 100644 --- a/app/src/main/java/at/favre/lib/securesharedpreferences/ChangePasswordActivity.java +++ b/app/src/main/java/at/favre/lib/securesharedpreferences/ChangePasswordActivity.java @@ -46,7 +46,7 @@ public void onChangePasswordClicked(View view) { .password(currentPassword) .supportVerifyPassword(true) .build(); - if(!armadillo.isValidPassword()) { + if (!armadillo.isValidPassword()) { binding.currentPasswordLayout.setError("Incorrect password!"); return; } diff --git a/app/src/main/java/at/favre/lib/securesharedpreferences/MainActivity.java b/app/src/main/java/at/favre/lib/securesharedpreferences/MainActivity.java index 620e71b..3c1a3d7 100644 --- a/app/src/main/java/at/favre/lib/securesharedpreferences/MainActivity.java +++ b/app/src/main/java/at/favre/lib/securesharedpreferences/MainActivity.java @@ -39,7 +39,7 @@ public void onInitClicked(View view) { builder.password(password); } encryptedPreferences = builder.build(); - if(encryptedPreferences.isValidPassword()) { + if (encryptedPreferences.isValidPassword()) { onArmadilloInitialised(); } else { onWrongPassword(); From 8ada9ff9d28098ed0096435ec4087924d732cae8 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Fri, 9 Nov 2018 11:27:34 +0100 Subject: [PATCH 09/12] Fix checkstyle issues #2 --- .../at/favre/lib/armadillo/SecureSharedPreferencesTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/armadillo/src/androidTest/java/at/favre/lib/armadillo/SecureSharedPreferencesTest.java b/armadillo/src/androidTest/java/at/favre/lib/armadillo/SecureSharedPreferencesTest.java index 251609a..ca69618 100644 --- a/armadillo/src/androidTest/java/at/favre/lib/armadillo/SecureSharedPreferencesTest.java +++ b/armadillo/src/androidTest/java/at/favre/lib/armadillo/SecureSharedPreferencesTest.java @@ -12,11 +12,11 @@ import java.nio.charset.StandardCharsets; import java.security.SecureRandom; -import java.security.Security; -import static junit.framework.TestCase.assertEquals; import at.favre.lib.bytes.Bytes; +import static junit.framework.TestCase.assertEquals; + /** * Instrumented test, which will execute on an Android device. * From 56a349e266107ccd0da0a3a7e18d29f97eb96642 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Mon, 12 Nov 2018 20:48:42 +0100 Subject: [PATCH 10/12] Add finalizer to ByteArrayRuntimeObfuscator.java --- .../at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java | 7 +++++++ config/checkstyle/checkstyle.xml | 1 + 2 files changed, 8 insertions(+) diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java index 48c8126..de81af2 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java @@ -72,5 +72,12 @@ public void wipe() { Bytes.wrap(arr).mutable().secureWipe(); } } + + @SuppressWarnings("checkstyle:nofinalizercheck") + @Override + protected void finalize() throws Throwable { + wipe(); + super.finalize(); + } } } diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml index ae7bd29..c410605 100644 --- a/config/checkstyle/checkstyle.xml +++ b/config/checkstyle/checkstyle.xml @@ -33,6 +33,7 @@ + From 31a34cddcf1c3a8a8adcf12eca50e5530eb74470 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Sun, 25 Nov 2018 20:28:34 +0100 Subject: [PATCH 11/12] Fix checkstyle issue --- .../java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java | 1 - config/checkstyle/checkstyle.xml | 1 - 2 files changed, 2 deletions(-) diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java index de81af2..4c5555d 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/ByteArrayRuntimeObfuscator.java @@ -73,7 +73,6 @@ public void wipe() { } } - @SuppressWarnings("checkstyle:nofinalizercheck") @Override protected void finalize() throws Throwable { wipe(); diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml index c410605..7f65407 100644 --- a/config/checkstyle/checkstyle.xml +++ b/config/checkstyle/checkstyle.xml @@ -86,7 +86,6 @@ - From 25d5cfd46dd6cbc46e8715ecf0f6aeb328946b61 Mon Sep 17 00:00:00 2001 From: Patrick Favre-Bulle Date: Sun, 25 Nov 2018 21:37:41 +0100 Subject: [PATCH 12/12] Add DerivedPasswordCacheTest --- .../armadillo/DerivedPasswordCacheTest.java | 44 +++++++++++++++++++ .../lib/armadillo/DerivedPasswordCache.java | 2 +- 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 armadillo/src/androidTest/java/at/favre/lib/armadillo/DerivedPasswordCacheTest.java diff --git a/armadillo/src/androidTest/java/at/favre/lib/armadillo/DerivedPasswordCacheTest.java b/armadillo/src/androidTest/java/at/favre/lib/armadillo/DerivedPasswordCacheTest.java new file mode 100644 index 0000000..c0636a9 --- /dev/null +++ b/armadillo/src/androidTest/java/at/favre/lib/armadillo/DerivedPasswordCacheTest.java @@ -0,0 +1,44 @@ +package at.favre.lib.armadillo; + +import org.junit.Before; +import org.junit.Test; + +import java.security.SecureRandom; + +import at.favre.lib.bytes.Bytes; + +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertNull; + +public class DerivedPasswordCacheTest { + + private DerivedPasswordCache cache; + private char[] pw; + private byte[] salt; + + @Before + public void setUp() { + cache = new DerivedPasswordCache.Default(true, new SecureRandom()); + pw = Bytes.random(12).encodeBase64Url().toCharArray(); + salt = Bytes.random(16).array(); + } + + @Test + public void get() { + Bytes val = Bytes.random(128); + cache.put(salt, pw, val.copy().array()); + assertEquals(val, Bytes.wrapNullSafe(cache.get(salt, pw))); + assertEquals(val, Bytes.wrapNullSafe(cache.get(salt, pw))); + + cache.wipe(); + + assertNull(cache.get(salt, pw)); + } + + @Test + public void getWhileDisabled() { + cache = new DerivedPasswordCache.Default(false, new SecureRandom()); + cache.put(salt, pw, Bytes.random(128).array()); + assertNull(cache.get(salt, pw)); + } +} diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java b/armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java index b04fcb6..6b8ed7f 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/DerivedPasswordCache.java @@ -40,7 +40,7 @@ public interface DerivedPasswordCache { /** * Standard implementation of {@link DerivedPasswordCache} */ - class Default implements DerivedPasswordCache { + final class Default implements DerivedPasswordCache { private final boolean enabled; private final SecureRandom secureRandom;