Various Fixes (#279)

* load password only if it's passed

* cancel fingerprint if password entered

* rework fingerprint code

* cleanup unused params

* new version code

app/build.gradle

@@ -7,8 +7,8 @@ android {
         applicationId "com.m2049r.xmrwallet"
         minSdkVersion 21
         targetSdkVersion 25
-        versionCode 92
+        versionCode 93
-        versionName "1.5.2 'CrAzY Nacho'"
+        versionName "1.5.3 'CrAzY Nacho'"
         testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"
         externalNativeBuild {
             cmake {

app/src/main/java/com/m2049r/xmrwallet/GenerateReviewFragment.java

@@ -53,7 +53,6 @@ import com.m2049r.xmrwallet.util.MoneroThreadPoolExecutor;
 import com.m2049r.xmrwallet.widget.Toolbar;
 
 import java.io.File;
-import java.security.KeyStoreException;
 
 import timber.log.Timber;
 
@@ -392,18 +391,18 @@ public class GenerateReviewFragment extends Fragment {
 
         @Override
         protected Boolean doInBackground(String... params) {
-            if (params.length != 4) return false;
+            if (params.length != 2) return false;
-            File walletFile = Helper.getWalletFile(getActivity(), params[0]);
+            final String userPassword = params[0];
-            String oldPassword = params[1];
+            final boolean fingerPassValid = Boolean.valueOf(params[1]);
-            String userPassword = params[2];
-            boolean fingerprintAuthAllowed = Boolean.valueOf(params[3]);
             newPassword = KeyStoreHelper.getCrazyPass(getActivity(), userPassword);
-            boolean success = changeWalletPassword(newPassword);
+            final boolean success = changeWalletPassword(newPassword);
             if (success) {
-                if (fingerprintAuthAllowed) {
+                Context ctx = getActivity();
-                    KeyStoreHelper.saveWalletUserPass(getActivity(), walletName, userPassword);
+                if (ctx != null)
+                    if (fingerPassValid) {
+                        KeyStoreHelper.saveWalletUserPass(ctx, walletName, userPassword);
                     } else {
-                    KeyStoreHelper.removeWalletUserPass(getActivity(), walletName);
+                        KeyStoreHelper.removeWalletUserPass(ctx, walletName);
                     }
             }
             return success;
@@ -412,7 +411,7 @@ public class GenerateReviewFragment extends Fragment {
         @Override
         protected void onPostExecute(Boolean result) {
             super.onPostExecute(result);
-            if (getActivity().isDestroyed()) {
+            if ((getActivity() == null) || getActivity().isDestroyed()) {
                 return;
             }
             if (progressCallback != null)
@@ -467,11 +466,7 @@ public class GenerateReviewFragment extends Fragment {
                 }
             });
 
-            try {
+            swFingerprintAllowed.setChecked(FingerprintHelper.isFingerPassValid(getActivity(), walletName));
-                swFingerprintAllowed.setChecked(FingerprintHelper.isFingerprintAuthAllowed(walletName));
-            } catch (KeyStoreException ex) {
-                ex.printStackTrace();
-            }
         }
 
         etPasswordA.getEditText().addTextChangedListener(new TextWatcher() {
@@ -547,7 +542,7 @@ public class GenerateReviewFragment extends Fragment {
                         } else if (!newPasswordA.equals(newPasswordB)) {
                             etPasswordB.setError(getString(R.string.generate_bad_passwordB));
                         } else if (newPasswordA.equals(newPasswordB)) {
-                            new AsyncChangePassword().execute(walletName, getPassword(), newPasswordA, Boolean.toString(swFingerprintAllowed.isChecked()));
+                            new AsyncChangePassword().execute(newPasswordA, Boolean.toString(swFingerprintAllowed.isChecked()));
                             Helper.hideKeyboardAlways(getActivity());
                             openDialog.dismiss();
                             openDialog = null;
@@ -569,7 +564,7 @@ public class GenerateReviewFragment extends Fragment {
                     } else if (!newPasswordA.equals(newPasswordB)) {
                         etPasswordB.setError(getString(R.string.generate_bad_passwordB));
                     } else if (newPasswordA.equals(newPasswordB)) {
-                        new AsyncChangePassword().execute(walletName, getPassword(), newPasswordA, Boolean.toString(swFingerprintAllowed.isChecked()));
+                        new AsyncChangePassword().execute(newPasswordA, Boolean.toString(swFingerprintAllowed.isChecked()));
                         Helper.hideKeyboardAlways(getActivity());
                         openDialog.dismiss();
                         openDialog = null;

app/src/main/java/com/m2049r/xmrwallet/LoginActivity.java

@@ -50,7 +50,6 @@ import com.m2049r.xmrwallet.model.NetworkType;
 import com.m2049r.xmrwallet.model.Wallet;
 import com.m2049r.xmrwallet.model.WalletManager;
 import com.m2049r.xmrwallet.service.WalletService;
-import com.m2049r.xmrwallet.util.FingerprintHelper;
 import com.m2049r.xmrwallet.util.Helper;
 import com.m2049r.xmrwallet.util.KeyStoreHelper;
 import com.m2049r.xmrwallet.util.MoneroThreadPoolExecutor;
@@ -63,7 +62,6 @@ import java.io.IOException;
 import java.net.Socket;
 import java.net.SocketAddress;
 import java.nio.channels.FileChannel;
-import java.security.KeyStoreException;
 import java.util.Date;
 
 import timber.log.Timber;
@@ -229,17 +227,20 @@ public class LoginActivity extends SecureActivity
         @Override
         protected Boolean doInBackground(String... params) {
             if (params.length != 2) return false;
-            File walletFile = Helper.getWalletFile(LoginActivity.this, params[0]);
+            String oldName = params[0];
             String newName = params[1];
+            File walletFile = Helper.getWalletFile(LoginActivity.this, oldName);
             boolean success = renameWallet(walletFile, newName);
             try {
-                if (success && FingerprintHelper.isFingerprintAuthAllowed(params[0])) {
+                if (success) {
-                    String savedPass = KeyStoreHelper.loadWalletUserPass(LoginActivity.this, params[0]);
+                    String savedPass = KeyStoreHelper.loadWalletUserPass(LoginActivity.this, oldName);
                     KeyStoreHelper.saveWalletUserPass(LoginActivity.this, newName, savedPass);
-                    KeyStoreHelper.removeWalletUserPass(LoginActivity.this, params[0]);
                 }
-            } catch (KeyStoreException ex) {
+            } catch (KeyStoreHelper.BrokenPasswordStoreException ex) {
-                ex.printStackTrace();
+                Timber.w(ex);
+            } finally {
+                // we have either set a new password or it is broken - kill the old one either way
+                KeyStoreHelper.removeWalletUserPass(LoginActivity.this, oldName);
             }
             return success;
         }

app/src/main/java/com/m2049r/xmrwallet/WalletActivity.java

@@ -133,6 +133,7 @@ public class WalletActivity extends SecureActivity implements WalletFragment.Lis
             acquireWakeLock();
             String walletId = extras.getString(REQUEST_ID);
             needVerifyIdentity = extras.getBoolean(REQUEST_FINGERPRINT_USED);
+            password = extras.getString(REQUEST_PW);
             connectWalletService(walletId, password);
         } else {
             finish();
@@ -259,8 +260,6 @@ public class WalletActivity extends SecureActivity implements WalletFragment.Lis
                 .add(R.id.fragment_container, walletFragment, WalletFragment.class.getName()).commit();
         Timber.d("fragment added");
 
-        password = getIntent().getExtras().getString(REQUEST_PW);
-
         startWalletService();
         Timber.d("onCreate() done.");
     }

app/src/main/java/com/m2049r/xmrwallet/util/FingerprintHelper.java

@@ -5,8 +5,7 @@ import android.content.Context;
 import android.support.v4.hardware.fingerprint.FingerprintManagerCompat;
 import android.support.v4.os.CancellationSignal;
 
-import java.security.KeyStore;
+import timber.log.Timber;
-import java.security.KeyStoreException;
 
 public class FingerprintHelper {
 
@@ -20,15 +19,14 @@ public class FingerprintHelper {
                 fingerprintManager.hasEnrolledFingerprints();
     }
 
-    public static boolean isFingerprintAuthAllowed(String wallet) throws KeyStoreException {
+    public static boolean isFingerPassValid(Context context, String wallet) {
-        KeyStore keyStore = KeyStore.getInstance(KeyStoreHelper.SecurityConstants.KEYSTORE_PROVIDER_ANDROID_KEYSTORE);
         try {
-            keyStore.load(null);
+            KeyStoreHelper.loadWalletUserPass(context, wallet);
-        } catch (Exception ex) {
+            return true;
-            throw new IllegalStateException("Could not load KeyStore", ex);
+        } catch (KeyStoreHelper.BrokenPasswordStoreException ex) {
+            Timber.w(ex);
+            return false;
         }
-
-        return keyStore.containsAlias(KeyStoreHelper.SecurityConstants.WALLET_PASS_KEY_PREFIX + wallet);
     }
 
     public static void authenticate(Context context, CancellationSignal cancelSignal,
@@ -36,5 +34,4 @@ public class FingerprintHelper {
         FingerprintManagerCompat manager = FingerprintManagerCompat.from(context);
         manager.authenticate(null, 0, cancelSignal, callback, null);
     }
-
 }

app/src/main/java/com/m2049r/xmrwallet/util/Helper.java

@@ -64,7 +64,6 @@ import java.math.BigInteger;
 import java.net.MalformedURLException;
 import java.net.SocketTimeoutException;
 import java.net.URL;
-import java.security.KeyStoreException;
 import java.util.Locale;
 
 import javax.net.ssl.HttpsURLConnection;
@@ -368,12 +367,7 @@ public class Helper {
         final TextInputLayout etPassword = (TextInputLayout) promptsView.findViewById(R.id.etPassword);
         etPassword.setHint(context.getString(R.string.prompt_password, wallet));
 
-        boolean fingerprintAuthCheck;
+        final boolean fingerprintAuthCheck = FingerprintHelper.isFingerPassValid(context, wallet);
-        try {
-            fingerprintAuthCheck = FingerprintHelper.isFingerprintAuthAllowed(wallet);
-        } catch (KeyStoreException ex) {
-            fingerprintAuthCheck = false;
-        }
 
         final boolean fingerprintAuthAllowed = !fingerprintDisabled && fingerprintAuthCheck;
         final CancellationSignal cancelSignal = new CancellationSignal();
@@ -425,6 +419,7 @@ public class Helper {
 
             @Override
             public void onAuthenticationSucceeded(FingerprintManagerCompat.AuthenticationResult result) {
+                try {
                     String userPass = KeyStoreHelper.loadWalletUserPass(context, wallet);
                     if (Helper.processPasswordEntry(context, wallet, userPass, true, action)) {
                         Helper.hideKeyboardAlways((Activity) context);
@@ -433,6 +428,10 @@ public class Helper {
                     } else {
                         etPassword.setError(context.getString(R.string.bad_password));
                     }
+                } catch (KeyStoreHelper.BrokenPasswordStoreException ex) {
+                    etPassword.setError(context.getString(R.string.bad_password));
+                    // TODO: better errror message here - what would it be?
+                }
             }
 
             @Override
@@ -455,6 +454,7 @@ public class Helper {
                         String pass = etPassword.getEditText().getText().toString();
                         if (processPasswordEntry(context, wallet, pass, false, action)) {
                             Helper.hideKeyboardAlways((Activity) context);
+                            cancelSignal.cancel();
                             openDialog.dismiss();
                             openDialog = null;
                         } else {
@@ -472,6 +472,7 @@ public class Helper {
                     String pass = etPassword.getEditText().getText().toString();
                     if (processPasswordEntry(context, wallet, pass, false, action)) {
                         Helper.hideKeyboardAlways((Activity) context);
+                        cancelSignal.cancel();
                         openDialog.dismiss();
                         openDialog = null;
                     } else {

app/src/main/java/com/m2049r/xmrwallet/util/KeyStoreHelper.java

@@ -23,8 +23,10 @@ import android.os.Build;
 import android.security.KeyPairGeneratorSpec;
 import android.security.keystore.KeyGenParameterSpec;
 import android.security.keystore.KeyProperties;
+import android.support.annotation.NonNull;
 import android.util.Base64;
 
+import java.io.IOException;
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.security.InvalidAlgorithmParameterException;
@@ -39,10 +41,15 @@ import java.security.PrivateKey;
 import java.security.PublicKey;
 import java.security.Signature;
 import java.security.SignatureException;
+import java.security.UnrecoverableEntryException;
+import java.security.cert.CertificateException;
 import java.util.Calendar;
 import java.util.GregorianCalendar;
 
+import javax.crypto.BadPaddingException;
 import javax.crypto.Cipher;
+import javax.crypto.IllegalBlockSizeException;
+import javax.crypto.NoSuchPaddingException;
 import javax.security.auth.x500.X500Principal;
 
 import timber.log.Timber;
@@ -58,43 +65,54 @@ public class KeyStoreHelper {
     static final private String RSA_ALIAS = "MonerujoRSA";
 
     public static String getCrazyPass(Context context, String password) {
-        // TODO we should check Locale.getDefault().getLanguage() here but for now we default to English
-        return getCrazyPass(context, password, "English");
-    }
-
-    public static String getCrazyPass(Context context, String password, String language) {
         byte[] data = password.getBytes(StandardCharsets.UTF_8);
         byte[] sig = null;
         try {
             KeyStoreHelper.createKeys(context, RSA_ALIAS);
             sig = KeyStoreHelper.signData(RSA_ALIAS, data);
-        } catch (Exception ex) {
+        } catch (NoSuchProviderException | NoSuchAlgorithmException |
+                InvalidAlgorithmParameterException | KeyStoreException |
+                InvalidKeyException | SignatureException ex) {
             throw new IllegalStateException(ex);
         }
         return CrazyPassEncoder.encode(cnSlowHash(sig));
     }
 
-    public static void saveWalletUserPass(Context context, String wallet, String password) {
+    public static boolean saveWalletUserPass(@NonNull Context context, String wallet, String password) {
         String walletKeyAlias = SecurityConstants.WALLET_PASS_KEY_PREFIX + wallet;
         byte[] data = password.getBytes(StandardCharsets.UTF_8);
         try {
             KeyStoreHelper.createKeys(context, walletKeyAlias);
             byte[] encrypted = KeyStoreHelper.encrypt(walletKeyAlias, data);
+            if (encrypted == null) return false;
             context.getSharedPreferences(SecurityConstants.WALLET_PASS_PREFS_NAME, Context.MODE_PRIVATE).edit()
                     .putString(wallet, Base64.encodeToString(encrypted, Base64.DEFAULT))
                     .apply();
-        } catch (Exception ex) {
+            return true;
-            throw new IllegalStateException(ex);
+        } catch (NoSuchProviderException | NoSuchAlgorithmException |
+                InvalidAlgorithmParameterException | KeyStoreException ex) {
+            Timber.w(ex);
+            return false;
         }
     }
 
-    public static String loadWalletUserPass(Context context, String wallet) {
+    static public class BrokenPasswordStoreException extends Exception {
+        BrokenPasswordStoreException() {
+            super();
+        }
+
+        BrokenPasswordStoreException(Throwable cause) {
+            super(cause);
+        }
+    }
+
+    public static String loadWalletUserPass(@NonNull Context context, String wallet) throws BrokenPasswordStoreException {
         String walletKeyAlias = SecurityConstants.WALLET_PASS_KEY_PREFIX + wallet;
         String encoded = context.getSharedPreferences(SecurityConstants.WALLET_PASS_PREFS_NAME, Context.MODE_PRIVATE)
                 .getString(wallet, "");
         byte[] data = Base64.decode(encoded, Base64.DEFAULT);
         byte[] decrypted = KeyStoreHelper.decrypt(walletKeyAlias, data);
-
+        if (decrypted == null) throw new BrokenPasswordStoreException();
         return new String(decrypted, StandardCharsets.UTF_8);
     }
 
@@ -102,23 +120,23 @@ public class KeyStoreHelper {
         String walletKeyAlias = SecurityConstants.WALLET_PASS_KEY_PREFIX + wallet;
         try {
             KeyStoreHelper.deleteKeys(walletKeyAlias);
+        } catch (KeyStoreException ex) {
+            Timber.w(ex);
+        }
         context.getSharedPreferences(SecurityConstants.WALLET_PASS_PREFS_NAME, Context.MODE_PRIVATE).edit()
                 .remove(wallet).apply();
-        } catch (Exception ex) {
-            throw new IllegalStateException(ex);
-        }
     }
 
     /**
      * Creates a public and private key and stores it using the Android Key
      * Store, so that only this application will be able to access the keys.
      */
-    public static void createKeys(Context context, String alias) throws NoSuchProviderException,
+    private static void createKeys(Context context, String alias) throws NoSuchProviderException,
             NoSuchAlgorithmException, InvalidAlgorithmParameterException, KeyStoreException {
         KeyStore keyStore = KeyStore.getInstance(SecurityConstants.KEYSTORE_PROVIDER_ANDROID_KEYSTORE);
         try {
             keyStore.load(null);
-        } catch (Exception ex) { // don't care why it failed
+        } catch (IOException | CertificateException ex) {
             throw new IllegalStateException("Could not load KeySotre", ex);
         }
         if (!keyStore.containsAlias(alias)) {
@@ -130,13 +148,25 @@ public class KeyStoreHelper {
         }
     }
 
-    public static void deleteKeys(String alias) throws KeyStoreException {
+    private static boolean deleteKeys(String alias) throws KeyStoreException {
         KeyStore keyStore = KeyStore.getInstance(SecurityConstants.KEYSTORE_PROVIDER_ANDROID_KEYSTORE);
         try {
             keyStore.load(null);
             keyStore.deleteEntry(alias);
-        } catch (Exception ex) { // don't care why it failed
+            return true;
-            throw new IllegalStateException("Could not load KeySotre", ex);
+        } catch (IOException | NoSuchAlgorithmException | CertificateException ex) {
+            Timber.w(ex);
+            return false;
+        }
+    }
+
+    public static boolean keyExists(String wallet) throws BrokenPasswordStoreException {
+        try {
+            KeyStore keyStore = KeyStore.getInstance(KeyStoreHelper.SecurityConstants.KEYSTORE_PROVIDER_ANDROID_KEYSTORE);
+            keyStore.load(null);
+            return keyStore.containsAlias(KeyStoreHelper.SecurityConstants.WALLET_PASS_KEY_PREFIX + wallet);
+        } catch (IOException | NoSuchAlgorithmException | CertificateException | KeyStoreException ex) {
+            throw new BrokenPasswordStoreException(ex);
         }
     }
 
@@ -164,8 +194,8 @@ public class KeyStoreHelper {
     }
 
     @TargetApi(Build.VERSION_CODES.M)
-    private static void createKeysM(String alias) {
+    private static void createKeysM(String alias) throws NoSuchProviderException,
-        try {
+            NoSuchAlgorithmException, InvalidAlgorithmParameterException {
         KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance(
                 KeyProperties.KEY_ALGORITHM_RSA, SecurityConstants.KEYSTORE_PROVIDER_ANDROID_KEYSTORE);
         keyPairGenerator.initialize(
@@ -177,9 +207,6 @@ public class KeyStoreHelper {
                         .build());
         KeyPair keyPair = keyPairGenerator.generateKeyPair();
         Timber.d("M Keys created");
-        } catch (NoSuchProviderException | NoSuchAlgorithmException | InvalidAlgorithmParameterException e) {
-            throw new RuntimeException(e);
-        }
     }
 
     private static KeyStore.PrivateKeyEntry getPrivateKeyEntry(String alias) {
@@ -199,33 +226,38 @@ public class KeyStoreHelper {
                 return null;
             }
             return (KeyStore.PrivateKeyEntry) entry;
-        } catch (Exception ex) {
+        } catch (IOException | NoSuchAlgorithmException | CertificateException
+                | UnrecoverableEntryException | KeyStoreException ex) {
             Timber.e(ex);
             return null;
         }
     }
 
-    public static byte[] encrypt(String alias, byte[] data) {
+    private static byte[] encrypt(String alias, byte[] data) {
         try {
             PublicKey publicKey = getPrivateKeyEntry(alias).getCertificate().getPublicKey();
             Cipher cipher = Cipher.getInstance(SecurityConstants.CIPHER_RSA_ECB_PKCS1);
 
             cipher.init(Cipher.ENCRYPT_MODE, publicKey);
             return cipher.doFinal(data);
-        } catch (Exception ex) {
+        } catch (InvalidKeyException | IllegalBlockSizeException | BadPaddingException
-            throw new IllegalStateException("Could not initialize RSA cipher", ex);
+                | NoSuchAlgorithmException | NoSuchPaddingException ex) {
+            Timber.e(ex);
+            return null;
         }
     }
 
-    public static byte[] decrypt(String alias, byte[] data) {
+    private static byte[] decrypt(String alias, byte[] data) {
         try {
             PrivateKey privateKey = getPrivateKeyEntry(alias).getPrivateKey();
             Cipher cipher = Cipher.getInstance(SecurityConstants.CIPHER_RSA_ECB_PKCS1);
 
             cipher.init(Cipher.DECRYPT_MODE, privateKey);
             return cipher.doFinal(data);
-        } catch (Exception ex) {
+        } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException |
-            throw new IllegalStateException("Could not initialize RSA cipher", ex);
+                IllegalBlockSizeException | BadPaddingException ex) {
+            Timber.e(ex);
+            return null;
         }
     }
 
@@ -236,7 +268,7 @@ public class KeyStoreHelper {
      *
      * @return The data signature generated
      */
-    public static byte[] signData(String alias, byte[] data) throws NoSuchAlgorithmException,
+    private static byte[] signData(String alias, byte[] data) throws NoSuchAlgorithmException,
             InvalidKeyException, SignatureException {
 
         PrivateKey privateKey = getPrivateKeyEntry(alias).getPrivateKey();
@@ -256,7 +288,7 @@ public class KeyStoreHelper {
      * @return A boolean value telling you whether the signature is valid or
      * not.
      */
-    public static boolean verifyData(String alias, byte[] data, byte[] signature)
+    private static boolean verifyData(String alias, byte[] data, byte[] signature)
             throws NoSuchAlgorithmException, InvalidKeyException, SignatureException {
 
         // Make sure the signature string exists