From 78a4dfb81fc28581facaea7445602385725f4ca3 Mon Sep 17 00:00:00 2001 From: pfavre Date: Sun, 28 Oct 2018 14:31:59 +0100 Subject: [PATCH] Fix AAD in cbc and and add kitkat support mode in Armadillo builder refs #6 --- .../lib/armadillo/BcryptMicroBenchmark.java | 2 + .../SecureSharedPreferencesTest.java | 6 +-- .../favre/lib/armadillo/AesCbcEncryption.java | 3 -- .../at/favre/lib/armadillo/Armadillo.java | 21 ++++++-- .../AuthenticatedEncryptionTest.java | 49 +++++++++++++++++-- 5 files changed, 67 insertions(+), 14 deletions(-) diff --git a/armadillo/src/androidTest/java/at/favre/lib/armadillo/BcryptMicroBenchmark.java b/armadillo/src/androidTest/java/at/favre/lib/armadillo/BcryptMicroBenchmark.java index 23356b9..ec3266e 100644 --- a/armadillo/src/androidTest/java/at/favre/lib/armadillo/BcryptMicroBenchmark.java +++ b/armadillo/src/androidTest/java/at/favre/lib/armadillo/BcryptMicroBenchmark.java @@ -1,5 +1,6 @@ package at.favre.lib.armadillo; +import org.junit.Ignore; import org.junit.Test; import java.nio.charset.StandardCharsets; @@ -15,6 +16,7 @@ import at.favre.lib.bytes.Bytes; import at.favre.lib.crypto.bcrypt.BCrypt; +@Ignore public class BcryptMicroBenchmark { private final Random rnd = new Random(); 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 44944ef..9d64f16 100644 --- a/armadillo/src/androidTest/java/at/favre/lib/armadillo/SecureSharedPreferencesTest.java +++ b/armadillo/src/androidTest/java/at/favre/lib/armadillo/SecureSharedPreferencesTest.java @@ -26,7 +26,7 @@ protected Armadillo.Builder create(String name, char[] pw) { } @Test - public void quickStartTest() throws Exception { + public void quickStartTest() { Context context = InstrumentationRegistry.getTargetContext(); SharedPreferences preferences = Armadillo.create(context, "myPrefs") .encryptionFingerprint(context) @@ -37,13 +37,13 @@ public void quickStartTest() throws Exception { } @Test - public void testWithDifferentKeyStrength() throws Exception { + public void testWithDifferentKeyStrength() { preferenceSmokeTest(create("fingerprint", null) .encryptionKeyStrength(AuthenticatedEncryption.STRENGTH_VERY_HIGH).build()); } @Test - public void advancedTest() throws Exception { + public void advancedTest() { Context context = InstrumentationRegistry.getTargetContext(); String userId = "1234"; SharedPreferences preferences = Armadillo.create(context, "myCustomPreferences") diff --git a/armadillo/src/main/java/at/favre/lib/armadillo/AesCbcEncryption.java b/armadillo/src/main/java/at/favre/lib/armadillo/AesCbcEncryption.java index bfc38d7..29fccc2 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/AesCbcEncryption.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/AesCbcEncryption.java @@ -146,9 +146,6 @@ public byte[] decrypt(byte[] rawEncryptionKey, byte[] encryptedData, @Nullable b final Cipher cipherDec = getCipher(); cipherDec.init(Cipher.DECRYPT_MODE, new SecretKeySpec(rawEncryptionKey, "AES"), new IvParameterSpec(iv)); - if (associatedData != null) { - cipherDec.updateAAD(associatedData); - } return cipherDec.doFinal(encrypted); } catch (Exception e) { throw new AuthenticatedEncryptionException("could not decrypt", e); 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 661b9fe..cfec940 100644 --- a/armadillo/src/main/java/at/favre/lib/armadillo/Armadillo.java +++ b/armadillo/src/main/java/at/favre/lib/armadillo/Armadillo.java @@ -2,6 +2,7 @@ import android.content.Context; import android.content.SharedPreferences; +import android.os.Build; import android.support.annotation.Nullable; import java.security.Provider; @@ -278,22 +279,36 @@ public Builder compress(Compressor compressor) { return this; } + public ArmadilloSharedPreferences buildWithKitkatSupport() { + return build(true); + } + /** * Build a {@link SharedPreferences} instance * * @return shared preference with given properties */ public ArmadilloSharedPreferences build() { + return build(false); + } + + public ArmadilloSharedPreferences build(boolean enableKitKatSupport) { if (fingerprint == null) { throw new IllegalArgumentException("No encryption fingerprint is set - see encryptionFingerprint() methods"); } if (authenticatedEncryption == null) { - authenticatedEncryption = new AesGcmEncryption(secureRandom, provider); + if(enableKitKatSupport && Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP) { + authenticatedEncryption = new AesCbcEncryption(secureRandom, provider); + cryptoProtocolVersion = -19; + } else { + authenticatedEncryption = new AesGcmEncryption(secureRandom, provider); + } } - EncryptionProtocol.Factory factory = new DefaultEncryptionProtocol.Factory(cryptoProtocolVersion, fingerprint, stringMessageDigest, authenticatedEncryption, keyStrength, - keyStretchingFunction, dataObfuscatorFactory, secureRandom, compressor); + EncryptionProtocol.Factory factory = new DefaultEncryptionProtocol.Factory(cryptoProtocolVersion, + fingerprint, stringMessageDigest, authenticatedEncryption, keyStrength, + keyStretchingFunction, dataObfuscatorFactory, secureRandom, compressor); if (sharedPreferences != null) { return new SecureSharedPreferences(sharedPreferences, factory, recoveryPolicy, password); diff --git a/armadillo/src/test/java/at/favre/lib/armadillo/AuthenticatedEncryptionTest.java b/armadillo/src/test/java/at/favre/lib/armadillo/AuthenticatedEncryptionTest.java index 59edcee..3a79df3 100644 --- a/armadillo/src/test/java/at/favre/lib/armadillo/AuthenticatedEncryptionTest.java +++ b/armadillo/src/test/java/at/favre/lib/armadillo/AuthenticatedEncryptionTest.java @@ -15,6 +15,7 @@ @RunWith(Parameterized.class) public class AuthenticatedEncryptionTest { + private static final int TEST_LOOP_COUNT = 10; private AuthenticatedEncryption authenticatedEncryption; @Parameterized.Parameters @@ -41,8 +42,8 @@ public void encryptNullArrays() throws Exception { } @Test - public void encryptMultiple() throws Exception { - for (int j = 0; j < 20; j++) { + public void encryptMultiple128BitKey() throws Exception { + for (int j = 0; j < TEST_LOOP_COUNT ; j++) { testEncryptDecrypt(Bytes.random(1).array(), Bytes.random(16).array(), null); testEncryptDecrypt(Bytes.random(2).array(), Bytes.random(16).array(), null); testEncryptDecrypt(Bytes.random(16).array(), Bytes.random(16).array(), null); @@ -50,6 +51,7 @@ public void encryptMultiple() throws Exception { testEncryptDecrypt(Bytes.random(32).array(), Bytes.random(16).array(), null); testEncryptDecrypt(Bytes.random(64).array(), Bytes.random(16).array(), null); testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(16).array(), null); + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(16).array(), Bytes.random(1).array()); testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(16).array(), Bytes.random(4).array()); testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(16).array(), Bytes.random(16).array()); @@ -58,16 +60,53 @@ public void encryptMultiple() throws Exception { } } + @Test + public void encryptMultiple256BitKey() throws Exception { + for (int j = 0; j < TEST_LOOP_COUNT; j++) { + testEncryptDecrypt(Bytes.random(1).array(), Bytes.random(32).array(), null); + testEncryptDecrypt(Bytes.random(2).array(), Bytes.random(32).array(), null); + testEncryptDecrypt(Bytes.random(16).array(), Bytes.random(32).array(), null); + testEncryptDecrypt(Bytes.random(24).array(), Bytes.random(32).array(), null); + testEncryptDecrypt(Bytes.random(32).array(), Bytes.random(32).array(), null); + testEncryptDecrypt(Bytes.random(64).array(), Bytes.random(32).array(), null); + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(32).array(), null); + } + } + + @Test + public void encryptMultipleWithAAD() throws Exception { + for (int j = 0; j < TEST_LOOP_COUNT; j++) { + testEncryptDecrypt(Bytes.random(1).array(), Bytes.random(16).array(), Bytes.random(1).array()); + testEncryptDecrypt(Bytes.random(1).array(), Bytes.random(16).array(), Bytes.random(4).array()); + testEncryptDecrypt(Bytes.random(1).array(), Bytes.random(16).array(), Bytes.random(16).array()); + testEncryptDecrypt(Bytes.random(1).array(), Bytes.random(16).array(), Bytes.random(32).array()); + testEncryptDecrypt(Bytes.random(1).array(), Bytes.random(16).array(), Bytes.random(128).array()); + + testEncryptDecrypt(Bytes.random(16).array(), Bytes.random(16).array(), Bytes.random(1).array()); + testEncryptDecrypt(Bytes.random(16).array(), Bytes.random(16).array(), Bytes.random(4).array()); + testEncryptDecrypt(Bytes.random(16).array(), Bytes.random(16).array(), Bytes.random(16).array()); + testEncryptDecrypt(Bytes.random(16).array(), Bytes.random(16).array(), Bytes.random(32).array()); + testEncryptDecrypt(Bytes.random(16).array(), Bytes.random(16).array(), Bytes.random(128).array()); + + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(16).array(), Bytes.random(16).array()); + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(16).array(), Bytes.random(32).array()); + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(16).array(), Bytes.random(128).array()); + + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(32).array(), Bytes.random(16).array()); + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(32).array(), Bytes.random(32).array()); + testEncryptDecrypt(Bytes.random(128).array(), Bytes.random(32).array(), Bytes.random(128).array()); + } + } + private void testEncryptDecrypt(byte[] content, byte[] key, byte[] aad) throws AuthenticatedEncryptionException { - byte[] encrypted = authenticatedEncryption.encrypt(key, content, null); + byte[] encrypted = authenticatedEncryption.encrypt(key, content, aad); assertTrue(encrypted.length >= content.length); assertFalse(Bytes.wrap(encrypted).equals(content)); System.out.println("content: " + Bytes.wrap(content).encodeHex()); System.out.println("encrypted: " + Bytes.wrap(encrypted).encodeHex()); - byte[] decrypt = authenticatedEncryption.decrypt(key, encrypted, null); + byte[] decrypt = authenticatedEncryption.decrypt(key, encrypted, aad); assertTrue(Bytes.wrap(decrypt).equals(content)); } - }