Merge pull request #34 from patrickfav/feat-32-password-obfuscate

Improve user password memory protection and add derived password cache

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## v0.7.0
 
+* Improve derive user password memory protection #32
+* Add derived password cache to speed up consecutive .get() calls #32
 * Improve RecoveryPolicy #35
 
 > [Full changelog](https://github.com/patrickfav/armadillo/compare/v0.6.0...v0.7.0)

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;
         }

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();

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));
+    }
+}

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.
  *
@@ -61,7 +61,6 @@ public void advancedTest() {
         String userId = "1234";
         SharedPreferences preferences = Armadillo.create(context, "myCustomPreferences")
             .password("mySuperSecretPassword".toCharArray()) //use user based password
-            .securityProvider(Security.getProvider("BC")) //use bouncy-castle security provider
             .keyStretchingFunction(new PBKDF2KeyStretcher()) //use PBKDF2 as user password kdf
             .contentKeyDigest(Bytes.from(getAndroidId(context)).array()) //use custom content key digest salt
             .secureRandom(new SecureRandom()) //provide your own secure random for salt/iv generation

armadillo/src/main/java/at/favre/lib/armadillo/Armadillo.java

@@ -81,6 +81,7 @@ public static final class Builder {
         private char[] password;
         private boolean supportVerifyPassword = false;
         private Provider provider;
+        private boolean enableDerivedPasswordCache = false;
         private boolean enableKitKatSupport = false;
 
         private Builder(SharedPreferences sharedPreferences) {
@@ -427,6 +428,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.
+         * <p>
+         * To make get* calls faster, you can enable this cache, which caches the <em>derived</em>
+         * 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).
+         * <p>
+         *
+         * <strong>Summary</strong>
+         * <ul>
+         * <li>Improves performance of consecutive get calls when using expensive key stretching function and user password</li>
+         * <li>Slightly reduces security strenght, since the stretched bytes are kept in memory for longer</li>
+         * </ul>
+         * <p>
+         * 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.
@@ -536,7 +566,7 @@ public ArmadilloSharedPreferences build() {
             }
 
             EncryptionProtocol.Factory factory = new DefaultEncryptionProtocol.Factory(config,
-                    fingerprint, stringMessageDigest, secureRandom, Collections.unmodifiableList(additionalDecryptionConfigs));
+                    fingerprint, stringMessageDigest, secureRandom, enableDerivedPasswordCache, Collections.unmodifiableList(additionalDecryptionConfigs));
 
             checkKitKatSupport(config.authenticatedEncryption);
 

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;
-    }
 }

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;
 
@@ -22,18 +23,60 @@ 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;
+        private final SecureRandom secureRandom;
 
         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()};
+            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);
+        }
+
+        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(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]);
+            }
+
+            createAndEncrypt(this.secureRandom, Bytes.from(b), b.length());
+
+            return b.array();
+        }
+
+        @Override
+        public void wipe() {
+            for (byte[] arr : data) {
+                Bytes.wrap(arr).mutable().secureWipe();
+            }
+        }
+
+        @Override
+        protected void finalize() throws Throwable {
+            wipe();
+            super.finalize();
         }
     }
 }

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.List;
@@ -46,16 +48,19 @@ final class DefaultEncryptionProtocol implements EncryptionProtocol {
     private final StringMessageDigest stringMessageDigest;
     private final SecureRandom secureRandom;
     private final int keyLengthBit;
+    private final DerivedPasswordCache derivedPasswordCache;
 
     private DefaultEncryptionProtocol(EncryptionProtocolConfig defaultConfig, byte[] preferenceSalt,
                                       EncryptionFingerprint fingerprint, StringMessageDigest stringMessageDigest,
-                                      SecureRandom secureRandom, List<EncryptionProtocolConfig> additionalDecryptionConfigs) {
+                                      SecureRandom secureRandom, boolean enableDerivedPasswordCaching,
+                                      List<EncryptionProtocolConfig> additionalDecryptionConfigs) {
         this.defaultConfig = defaultConfig;
         this.preferenceSalt = preferenceSalt;
         this.fingerprint = fingerprint;
         this.stringMessageDigest = stringMessageDigest;
         this.keyLengthBit = defaultConfig.authenticatedEncryption.byteSizeLength(defaultConfig.keyStrength) * 8;
         this.secureRandom = secureRandom;
+        this.derivedPasswordCache = new DerivedPasswordCache.Default(enableDerivedPasswordCaching, secureRandom);
         this.additionalDecryptionConfigs = additionalDecryptionConfigs;
     }
 
@@ -173,18 +178,50 @@ public void setKeyStretchingFunction(@NonNull KeyStretchingFunction function) {
         defaultConfig = EncryptionProtocolConfig.newBuilder(defaultConfig)
                 .keyStretchingFunction(Objects.requireNonNull(function))
                 .build();
+        derivedPasswordCache.wipe();
     }
 
     @Override
     public KeyStretchingFunction getKeyStretchingFunction() {
         return defaultConfig.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();
+    }
+
+    @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(defaultConfig.keyStretchingFunction.stretch(contentSalt, password, STRETCHED_PASSWORD_LENGTH_BYTES));
+            byte[] stretched;
+            if ((stretched = derivedPasswordCache.get(contentSalt, password)) == null) {
+                stretched = defaultConfig.keyStretchingFunction.stretch(contentSalt, password, STRETCHED_PASSWORD_LENGTH_BYTES);
+                derivedPasswordCache.put(contentSalt, password, stretched);
+            }
+            ikm = ikm.append(stretched);
         }
 
         return HKDF.fromHmacSha512().extractAndExpand(preferenceSalt, ikm.array(), Bytes.from("DefaultEncryptionProtocol").array(), keyLengthBit / 8);
@@ -195,23 +232,26 @@ public static final class Factory implements EncryptionProtocol.Factory {
         private final EncryptionFingerprint fingerprint;
         private final StringMessageDigest stringMessageDigest;
         private final SecureRandom secureRandom;
+        private final boolean enableDerivedPasswordCaching;
         private EncryptionProtocolConfig defaultConfig;
         private final List<EncryptionProtocolConfig> additionalDecryptionConfigs;
 
         Factory(EncryptionProtocolConfig defaultConfig, EncryptionFingerprint fingerprint,
                 StringMessageDigest stringMessageDigest, SecureRandom secureRandom,
+                boolean enableDerivedPasswordCaching,
                 List<EncryptionProtocolConfig> additionalDecryptionConfigs) {
             this.defaultConfig = defaultConfig;
             this.fingerprint = fingerprint;
             this.stringMessageDigest = stringMessageDigest;
             this.secureRandom = secureRandom;
+            this.enableDerivedPasswordCaching = enableDerivedPasswordCaching;
             this.additionalDecryptionConfigs = additionalDecryptionConfigs;
         }
 
         @Override
         public EncryptionProtocol create(byte[] preferenceSalt) {
             return new DefaultEncryptionProtocol(defaultConfig, preferenceSalt, fingerprint,
-                    stringMessageDigest, secureRandom, additionalDecryptionConfigs);
+                    stringMessageDigest, secureRandom, enableDerivedPasswordCaching, additionalDecryptionConfigs);
         }
 
         @Override
@@ -228,5 +268,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);
     }
 }