From ef9b5256efbbfe74d6ca04b86d760b5e942d27e4 Mon Sep 17 00:00:00 2001 From: Pelle Stenild Coltau Date: Thu, 29 Jun 2017 09:49:35 +0200 Subject: [PATCH] Moving null checks from CipherStorageFacebookConceal and PrefsStorage to KeychainModule and introducing checks for empty service in CipherStorageKeystoreAESCBC. --- .../com/oblador/keychain/KeychainModule.java | 12 +++++++++++ .../com/oblador/keychain/PrefsStorage.java | 14 ++++--------- .../keychain/cipherStorage/CipherStorage.java | 8 +++++--- .../CipherStorageFacebookConceal.java | 13 +++++------- .../CipherStorageKeystoreAESCBC.java | 20 ++++++++++++------- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/android/src/main/java/com/oblador/keychain/KeychainModule.java b/android/src/main/java/com/oblador/keychain/KeychainModule.java index fb4ff96..edbac50 100644 --- a/android/src/main/java/com/oblador/keychain/KeychainModule.java +++ b/android/src/main/java/com/oblador/keychain/KeychainModule.java @@ -30,6 +30,7 @@ public class KeychainModule extends ReactContextBaseJavaModule { public static final String E_CRYPTO_FAILED = "E_CRYPTO_FAILED"; public static final String E_KEYSTORE_ACCESS_ERROR = "E_KEYSTORE_ACCESS_ERROR"; public static final String KEYCHAIN_MODULE = "RNKeychainManager"; + public static final String EMPTY_STRING = ""; private final Map cipherStorageMap = new HashMap<>(); private final PrefsStorage prefsStorage; @@ -57,6 +58,8 @@ public class KeychainModule extends ReactContextBaseJavaModule { if (username == null || username.isEmpty() || password == null || password.isEmpty()) { throw new EmptyParameterException("you passed empty or null username/password"); } + service = getDefaultServiceIfNull(service); + CipherStorage currentCipherStorage = getCipherStorageForCurrentAPILevel(); EncryptionResult result = currentCipherStorage.encrypt(service, username, password); @@ -75,6 +78,8 @@ public class KeychainModule extends ReactContextBaseJavaModule { @ReactMethod public void getGenericPasswordForOptions(String service, Promise promise) { try { + service = getDefaultServiceIfNull(service); + CipherStorage currentCipherStorage = getCipherStorageForCurrentAPILevel(); final DecryptionResult decryptionResult; @@ -121,6 +126,8 @@ public class KeychainModule extends ReactContextBaseJavaModule { @ReactMethod public void resetGenericPasswordForOptions(String service, Promise promise) { try { + service = getDefaultServiceIfNull(service); + // First we clean up the cipher storage (using the cipher storage that was used to store the entry) ResultSet resultSet = prefsStorage.getEncryptedEntry(service); if (resultSet != null) { @@ -176,4 +183,9 @@ public class KeychainModule extends ReactContextBaseJavaModule { private CipherStorage getCipherStorageByName(String cipherStorageName) { return cipherStorageMap.get(cipherStorageName); } + + @NonNull + private String getDefaultServiceIfNull(String service) { + return service == null ? EMPTY_STRING : service; + } } \ No newline at end of file diff --git a/android/src/main/java/com/oblador/keychain/PrefsStorage.java b/android/src/main/java/com/oblador/keychain/PrefsStorage.java index 14990a1..a9219b4 100644 --- a/android/src/main/java/com/oblador/keychain/PrefsStorage.java +++ b/android/src/main/java/com/oblador/keychain/PrefsStorage.java @@ -2,6 +2,7 @@ package com.oblador.keychain; import android.content.Context; import android.content.SharedPreferences; +import android.support.annotation.NonNull; import android.util.Base64; import com.facebook.react.bridge.ReactApplicationContext; @@ -10,7 +11,6 @@ import com.oblador.keychain.cipherStorage.CipherStorageFacebookConceal; public class PrefsStorage { public static final String KEYCHAIN_DATA = "RN_KEYCHAIN"; - public static final String EMPTY_STRING = ""; static public class ResultSet { public final String cipherStorageName; @@ -30,9 +30,7 @@ public class PrefsStorage { this.prefs = reactContext.getSharedPreferences(KEYCHAIN_DATA, Context.MODE_PRIVATE); } - public ResultSet getEncryptedEntry(String service) { - service = service == null ? EMPTY_STRING : service; - + public ResultSet getEncryptedEntry(@NonNull String service) { byte[] bytesForUsername = getBytesForUsername(service); byte[] bytesForPassword = getBytesForPassword(service); String cipherStorageName = getCipherStorageName(service); @@ -46,9 +44,7 @@ public class PrefsStorage { return null; } - public void removeEntry(String service) { - service = service == null ? EMPTY_STRING : service; - + public void removeEntry(@NonNull String service) { String keyForUsername = getKeyForUsername(service); String keyForPassword = getKeyForPassword(service); String keyForCipherStorage = getKeyForCipherStorage(service); @@ -59,9 +55,7 @@ public class PrefsStorage { .remove(keyForCipherStorage).apply(); } - public void storeEncryptedEntry(String service, EncryptionResult encryptionResult) { - service = service == null ? EMPTY_STRING : service; - + public void storeEncryptedEntry(@NonNull String service, @NonNull EncryptionResult encryptionResult) { String keyForUsername = getKeyForUsername(service); String keyForPassword = getKeyForPassword(service); String keyForCipherStorage = getKeyForCipherStorage(service); diff --git a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorage.java b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorage.java index 58a78ac..0cdaaf7 100644 --- a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorage.java +++ b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorage.java @@ -1,5 +1,7 @@ package com.oblador.keychain.cipherStorage; +import android.support.annotation.NonNull; + import com.oblador.keychain.exceptions.CryptoFailedException; import com.oblador.keychain.exceptions.KeyStoreAccessException; @@ -29,11 +31,11 @@ public interface CipherStorage { } } - EncryptionResult encrypt(String service, String username, String password) throws CryptoFailedException; + EncryptionResult encrypt(@NonNull String service, @NonNull String username, @NonNull String password) throws CryptoFailedException; - DecryptionResult decrypt(String service, byte[] username, byte[] password) throws CryptoFailedException; + DecryptionResult decrypt(@NonNull String service, @NonNull byte[] username, @NonNull byte[] password) throws CryptoFailedException; - void removeKey(String service) throws KeyStoreAccessException; + void removeKey(@NonNull String service) throws KeyStoreAccessException; String getCipherStorageName(); diff --git a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageFacebookConceal.java b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageFacebookConceal.java index 6cd6822..58f06f6 100644 --- a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageFacebookConceal.java +++ b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageFacebookConceal.java @@ -1,6 +1,7 @@ package com.oblador.keychain.cipherStorage; import android.os.Build; +import android.support.annotation.NonNull; import com.facebook.android.crypto.keychain.AndroidConceal; import com.facebook.android.crypto.keychain.SharedPrefsBackedKeyChain; @@ -16,7 +17,6 @@ import java.nio.charset.Charset; public class CipherStorageFacebookConceal implements CipherStorage { public static final String CIPHER_STORAGE_NAME = "FacebookConceal"; public static final String KEYCHAIN_DATA = "RN_KEYCHAIN"; - public static final String EMPTY_STRING = ""; private final Crypto crypto; public CipherStorageFacebookConceal(ReactApplicationContext reactContext) { @@ -35,12 +35,10 @@ public class CipherStorageFacebookConceal implements CipherStorage { } @Override - public EncryptionResult encrypt(String service, String username, String password) throws CryptoFailedException { + public EncryptionResult encrypt(@NonNull String service, @NonNull String username, @NonNull String password) throws CryptoFailedException { if (!crypto.isAvailable()) { throw new CryptoFailedException("Crypto is missing"); } - service = service == null ? EMPTY_STRING : service; - Entity usernameEntity = createUsernameEntity(service); Entity passwordEntity = createPasswordEntity(service); @@ -55,12 +53,10 @@ public class CipherStorageFacebookConceal implements CipherStorage { } @Override - public DecryptionResult decrypt(String service, byte[] username, byte[] password) throws CryptoFailedException { + public DecryptionResult decrypt(@NonNull String service, @NonNull byte[] username, @NonNull byte[] password) throws CryptoFailedException { if (!crypto.isAvailable()) { throw new CryptoFailedException("Crypto is missing"); } - service = service == null ? EMPTY_STRING : service; - Entity usernameEntity = createUsernameEntity(service); Entity passwordEntity = createPasswordEntity(service); @@ -77,9 +73,10 @@ public class CipherStorageFacebookConceal implements CipherStorage { } @Override - public void removeKey(String service) { + public void removeKey(@NonNull String service) { // Facebook Conceal stores only one key across all services, so we cannot delete the key (otherwise decryption will fail for encrypted data of other services). } + private Entity createUsernameEntity(String service) { String prefix = getEntityPrefix(service); return Entity.create(prefix + "user"); diff --git a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageKeystoreAESCBC.java b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageKeystoreAESCBC.java index 8ce398a..7cc45d2 100644 --- a/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageKeystoreAESCBC.java +++ b/android/src/main/java/com/oblador/keychain/cipherStorage/CipherStorageKeystoreAESCBC.java @@ -4,6 +4,7 @@ import android.annotation.TargetApi; import android.os.Build; import android.security.keystore.KeyGenParameterSpec; import android.security.keystore.KeyProperties; +import android.support.annotation.NonNull; import com.oblador.keychain.exceptions.CryptoFailedException; import com.oblador.keychain.exceptions.KeyStoreAccessException; @@ -30,7 +31,7 @@ import javax.crypto.spec.IvParameterSpec; public class CipherStorageKeystoreAESCBC implements CipherStorage { public static final String CIPHER_STORAGE_NAME = "KeystoreAESCBC"; - public static final String DEFAULT_ALIAS = "RN_KEYCHAIN_DEFAULT_ALIAS"; + public static final String DEFAULT_SERVICE = "RN_KEYCHAIN_DEFAULT_ALIAS"; public static final String KEYSTORE_TYPE = "AndroidKeyStore"; public static final String ENCRYPTION_ALGORITHM = KeyProperties.KEY_ALGORITHM_AES; public static final String ENCRYPTION_BLOCK_MODE = KeyProperties.BLOCK_MODE_CBC; @@ -53,8 +54,8 @@ public class CipherStorageKeystoreAESCBC implements CipherStorage { @TargetApi(Build.VERSION_CODES.M) @Override - public EncryptionResult encrypt(String service, String username, String password) throws CryptoFailedException { - service = service == null ? DEFAULT_ALIAS : service; + public EncryptionResult encrypt(@NonNull String service, @NonNull String username, @NonNull String password) throws CryptoFailedException { + service = getDefaultServiceIfEmpty(service); try { KeyStore keyStore = getKeyStoreAndLoad(); @@ -91,8 +92,8 @@ public class CipherStorageKeystoreAESCBC implements CipherStorage { } @Override - public DecryptionResult decrypt(String service, byte[] username, byte[] password) throws CryptoFailedException { - service = service == null ? DEFAULT_ALIAS : service; + public DecryptionResult decrypt(@NonNull String service, @NonNull byte[] username, @NonNull byte[] password) throws CryptoFailedException { + service = getDefaultServiceIfEmpty(service); try { KeyStore keyStore = getKeyStoreAndLoad(); @@ -111,8 +112,8 @@ public class CipherStorageKeystoreAESCBC implements CipherStorage { } @Override - public void removeKey(String service) throws KeyStoreAccessException { - service = service == null ? DEFAULT_ALIAS : service; + public void removeKey(@NonNull String service) throws KeyStoreAccessException { + service = getDefaultServiceIfEmpty(service); try { KeyStore keyStore = getKeyStoreAndLoad(); @@ -183,4 +184,9 @@ public class CipherStorageKeystoreAESCBC implements CipherStorage { throw new KeyStoreAccessException("Could not access Keystore", e); } } + + @NonNull + private String getDefaultServiceIfEmpty(@NonNull String service) { + return service.isEmpty() ? DEFAULT_SERVICE : service; + } }