scottyab / secure-preferences

Android Shared preference wrapper than encrypts the values of Shared Preferences. It's not bullet proof security but rather a quick win for incrementally making your android app more secure.
1.53k stars 235 forks source link

handlePasswordChange(...) also try to decrypt auto-generated key #49

Open ShinJJang opened 8 years ago

ShinJJang commented 8 years ago

When already use default constructor not passing password or keys, auto-generated key is saved in default shared preference.

But handlePasswordChange(...) also try to change that auto-generated key. During decrypting for getting plain string before encrypt with new password, error occurred.

java.lang.IllegalArgumentException: Cannot parse iv:ciphertext:mac
at com.tozny.crypto.android.AesCbcWithIntegrity$CipherTextIvMac.<init>(AesCbcWithIntegrity.java:568)
at com.securepreferences.SecurePreferences.decrypt(SecurePreferences.java:328)
at com.securepreferences.SecurePreferences.handlePasswordChange(SecurePreferences.java:509)

There is need to be some flag to check if pass or not. If not, always that call generateAesKeyName to get key of auto-generated key. It will be slow.

macdonaldj commented 7 years ago

it seems to be crashing decrypting the key, so i just did this to get around it

public void handlePasswordChange(String newPassword, Context context) throws GeneralSecurityException {

        final byte[] salt = getDeviceSerialNumber(context).getBytes();
        AesCbcWithIntegrity.SecretKeys newKey= AesCbcWithIntegrity.generateKeyFromPassword(newPassword,salt);

        Map<String, ?> allOfThePrefs = sharedPreferences.getAll();
        Map<String, String> unencryptedPrefs = new HashMap<String, String>(allOfThePrefs.size());
        Iterator<String> keys = allOfThePrefs.keySet().iterator();
        //iterate through the current prefs unencrypting each one

        String keyPrefKey = "";
        while(keys.hasNext()) {
            String prefKey = keys.next();
            Object prefValue = allOfThePrefs.get(prefKey);
            if(prefValue instanceof String){
                //all the encrypted values will be Strings

                final String prefValueString = (String)prefValue;
                String keysString = this.keys.toString();
                if (prefValueString.equalsIgnoreCase(keysString)) {
                    keyPrefKey = prefKey;
                    continue;
                }
                final String plainTextPrefValue = decrypt(prefValueString);
                unencryptedPrefs.put(prefKey, plainTextPrefValue);
            }
        }

        //destroy and clear the current pref file
        destroyKeys();

        SharedPreferences.Editor editor = sharedPreferences.edit();
        editor.clear();
        editor.apply();

        //refresh the sharedPreferences object ref: I found it was retaining old ref/values
        sharedPreferences = null;
        sharedPreferences = getSharedPreferenceFile(context, sharedPrefFilename);

        //assign new key
        this.keys = newKey;

        SharedPreferences.Editor updatedEditor = sharedPreferences.edit();

        //iterate through the unencryptedPrefs encrypting each one with new key
        Iterator<String> unencryptedPrefsKeys = unencryptedPrefs.keySet().iterator();
        while (unencryptedPrefsKeys.hasNext()) {
            String prefKey = unencryptedPrefsKeys.next();
            String prefPlainText = unencryptedPrefs.get(prefKey);
            updatedEditor.putString(prefKey, encrypt(prefPlainText));
        }
        updatedEditor.commit();

        //save new key
        updatedEditor.putString(keyPrefKey, this.keys.toString()).commit();

    }

also one issue i found was in following method after doing handlePasswoordChange

private SharedPreferences getSharedPreferenceFile(Context context, String prefFilename) {
        this.sharedPrefFilename = sharedPrefFilename ;

this caused the filename to be null and created new pref file. needs to be

private SharedPreferences getSharedPreferenceFile(Context context, String prefFilename) {
        this.sharedPrefFilename = prefFilename;