From a778cd5661078a7e87a79dcb81e78037e355867f Mon Sep 17 00:00:00 2001
From: Alexander Bakker <ab@alexbakker.me>
Date: Tue, 13 Feb 2018 21:01:27 +0100
Subject: [PATCH] Don't try to zero out sensitive memory

Perhaps revisit this later
---
 app/src/main/java/me/impy/aegis/AuthActivity.java |  2 +-
 .../me/impy/aegis/CustomAuthenticatedSlide.java   |  3 +--
 .../main/java/me/impy/aegis/DerivationTask.java   |  1 -
 .../java/me/impy/aegis/EditProfileActivity.java   |  1 -
 .../me/impy/aegis/PasswordDialogFragment.java     |  4 +---
 .../java/me/impy/aegis/SlotCollectionTask.java    |  4 ----
 .../java/me/impy/aegis/crypto/CryptoUtils.java    | 14 +-------------
 .../java/me/impy/aegis/crypto/slots/Slot.java     |  5 +----
 .../main/java/me/impy/aegis/encoding/Base32.java  |  4 +---
 .../java/me/impy/aegis/helpers/AuthHelper.java    | 15 ++-------------
 10 files changed, 8 insertions(+), 45 deletions(-)

diff --git a/app/src/main/java/me/impy/aegis/AuthActivity.java b/app/src/main/java/me/impy/aegis/AuthActivity.java
index 1bde01ec..91210bde 100644
--- a/app/src/main/java/me/impy/aegis/AuthActivity.java
+++ b/app/src/main/java/me/impy/aegis/AuthActivity.java
@@ -93,7 +93,7 @@ public class AuthActivity extends AegisActivity implements FingerprintUiHelper.C
         button.setOnClickListener(new View.OnClickListener() {
             @Override
             public void onClick(View v) {
-                char[] password = AuthHelper.getPassword(_textPassword, true);
+                char[] password = AuthHelper.getEditTextChars(_textPassword);
                 trySlots(PasswordSlot.class, password);
             }
         });
diff --git a/app/src/main/java/me/impy/aegis/CustomAuthenticatedSlide.java b/app/src/main/java/me/impy/aegis/CustomAuthenticatedSlide.java
index e9ea2f73..f99b6946 100644
--- a/app/src/main/java/me/impy/aegis/CustomAuthenticatedSlide.java
+++ b/app/src/main/java/me/impy/aegis/CustomAuthenticatedSlide.java
@@ -67,8 +67,7 @@ public class CustomAuthenticatedSlide extends Fragment implements FingerprintUiH
     }
 
     public char[] getPassword() {
-        AuthHelper.clearPassword(_textPasswordConfirm);
-        return AuthHelper.getPassword(_textPassword, true);
+        return AuthHelper.getEditTextChars(_textPassword);
     }
 
     public Cipher getFingerCipher() {
diff --git a/app/src/main/java/me/impy/aegis/DerivationTask.java b/app/src/main/java/me/impy/aegis/DerivationTask.java
index cc446920..9eae2bfa 100644
--- a/app/src/main/java/me/impy/aegis/DerivationTask.java
+++ b/app/src/main/java/me/impy/aegis/DerivationTask.java
@@ -24,7 +24,6 @@ public class DerivationTask extends ProgressDialogTask<DerivationTask.Params, Se
         try {
             byte[] salt = CryptoUtils.generateSalt();
             SecretKey key = params.Slot.deriveKey(params.Password, salt, CryptoUtils.CRYPTO_SCRYPT_N, CryptoUtils.CRYPTO_SCRYPT_r, CryptoUtils.CRYPTO_SCRYPT_p);
-            CryptoUtils.zero(params.Password);
             return key;
         } catch (Exception e) {
             return null;
diff --git a/app/src/main/java/me/impy/aegis/EditProfileActivity.java b/app/src/main/java/me/impy/aegis/EditProfileActivity.java
index cc2921d1..f96538ca 100644
--- a/app/src/main/java/me/impy/aegis/EditProfileActivity.java
+++ b/app/src/main/java/me/impy/aegis/EditProfileActivity.java
@@ -219,7 +219,6 @@ public class EditProfileActivity extends AegisActivity {
         try {
             char[] secret = AuthHelper.getEditTextChars(_textSecret);
             info.setSecret(secret);
-            CryptoUtils.zero(secret);
             info.setIssuer(_textIssuer.getText().toString());
             info.setPeriod(period);
             info.setDigits(digits);
diff --git a/app/src/main/java/me/impy/aegis/PasswordDialogFragment.java b/app/src/main/java/me/impy/aegis/PasswordDialogFragment.java
index 9cfdf59b..7cf2ad5d 100644
--- a/app/src/main/java/me/impy/aegis/PasswordDialogFragment.java
+++ b/app/src/main/java/me/impy/aegis/PasswordDialogFragment.java
@@ -43,9 +43,7 @@ public class PasswordDialogFragment extends SlotDialogFragment {
                     return;
                 }
 
-                AuthHelper.clearPassword(textPasswordConfirm);
-                char[] password = AuthHelper.getPassword(textPassword, true);
-
+                char[] password = AuthHelper.getEditTextChars(textPassword);
                 PasswordSlot slot = new PasswordSlot();
                 DerivationTask task = new DerivationTask(getContext(), key -> {
                     Cipher cipher;
diff --git a/app/src/main/java/me/impy/aegis/SlotCollectionTask.java b/app/src/main/java/me/impy/aegis/SlotCollectionTask.java
index 30bc70ee..4de65b93 100644
--- a/app/src/main/java/me/impy/aegis/SlotCollectionTask.java
+++ b/app/src/main/java/me/impy/aegis/SlotCollectionTask.java
@@ -62,10 +62,6 @@ public class SlotCollectionTask<T extends Slot> extends ProgressDialogTask<SlotC
             return null;
         } catch (Exception e) {
             throw new UndeclaredThrowableException(e);
-        } finally {
-            if (params.Obj instanceof char[]) {
-                CryptoUtils.zero((char[]) params.Obj);
-            }
         }
     }
 
diff --git a/app/src/main/java/me/impy/aegis/crypto/CryptoUtils.java b/app/src/main/java/me/impy/aegis/crypto/CryptoUtils.java
index 642350b8..6b336dca 100644
--- a/app/src/main/java/me/impy/aegis/crypto/CryptoUtils.java
+++ b/app/src/main/java/me/impy/aegis/crypto/CryptoUtils.java
@@ -44,10 +44,7 @@ public class CryptoUtils {
     public static SecretKey deriveKey(char[] password, byte[] salt, int n, int r, int p) throws NoSuchAlgorithmException, InvalidKeySpecException {
         byte[] bytes = toBytes(password);
         byte[] keyBytes = SCrypt.generate(bytes, salt, n, r, p, CRYPTO_KEY_SIZE);
-        zero(bytes);
-        SecretKey key = new SecretKeySpec(keyBytes, 0, keyBytes.length, "AES");
-        zero(keyBytes);
-        return key;
+        return new SecretKeySpec(keyBytes, 0, keyBytes.length, "AES");
     }
 
     public static Cipher createCipher(SecretKey key, int opmode) throws NoSuchPaddingException, NoSuchAlgorithmException, InvalidAlgorithmParameterException, InvalidKeyException {
@@ -102,7 +99,6 @@ public class CryptoUtils {
 
         byte[] bytes = key.getEncoded();
         hash.update(bytes);
-        CryptoUtils.zero(bytes);
         return hash.digest();
     }
 
@@ -127,14 +123,6 @@ public class CryptoUtils {
         return data;
     }
 
-    public static void zero(char[] data) {
-        Arrays.fill(data, '\0');
-    }
-
-    public static void zero(byte[] data) {
-        Arrays.fill(data, (byte) 0);
-    }
-
     private static byte[] toBytes(char[] chars) {
         CharBuffer charBuf = CharBuffer.wrap(chars);
         ByteBuffer byteBuf = Charset.forName("UTF-8").encode(charBuf);
diff --git a/app/src/main/java/me/impy/aegis/crypto/slots/Slot.java b/app/src/main/java/me/impy/aegis/crypto/slots/Slot.java
index 6ccf3f82..7c1247b2 100644
--- a/app/src/main/java/me/impy/aegis/crypto/slots/Slot.java
+++ b/app/src/main/java/me/impy/aegis/crypto/slots/Slot.java
@@ -33,16 +33,13 @@ public abstract class Slot implements Serializable {
     // getKey decrypts the encrypted master key in this slot with the given key and returns it.
     public SecretKey getKey(Cipher cipher) throws BadPaddingException, IllegalBlockSizeException {
         byte[] decryptedKeyBytes = cipher.doFinal(_encryptedMasterKey);
-        SecretKey decryptedKey = new SecretKeySpec(decryptedKeyBytes, CryptoUtils.CRYPTO_CIPHER_AEAD);
-        CryptoUtils.zero(decryptedKeyBytes);
-        return decryptedKey;
+        return new SecretKeySpec(decryptedKeyBytes, CryptoUtils.CRYPTO_CIPHER_AEAD);
     }
 
     // setKey encrypts the given master key with the given key and stores the result in this slot.
     public void setKey(MasterKey masterKey, Cipher cipher) throws BadPaddingException, IllegalBlockSizeException {
         byte[] masterKeyBytes = masterKey.getBytes();
         _encryptedMasterKey = cipher.doFinal(masterKeyBytes);
-        CryptoUtils.zero(masterKeyBytes);
     }
 
     // suppress the AES ECB warning
diff --git a/app/src/main/java/me/impy/aegis/encoding/Base32.java b/app/src/main/java/me/impy/aegis/encoding/Base32.java
index 1a07905c..a217701d 100644
--- a/app/src/main/java/me/impy/aegis/encoding/Base32.java
+++ b/app/src/main/java/me/impy/aegis/encoding/Base32.java
@@ -87,9 +87,7 @@ public class Base32 {
             base32[j++] = base32Chars.charAt(digit);
         }
 
-        char[] res = Arrays.copyOf(base32, j);
-        CryptoUtils.zero(base32);
-        return res;
+        return Arrays.copyOf(base32, j);
     }
 
     /**
diff --git a/app/src/main/java/me/impy/aegis/helpers/AuthHelper.java b/app/src/main/java/me/impy/aegis/helpers/AuthHelper.java
index 675e1f82..be804ebe 100644
--- a/app/src/main/java/me/impy/aegis/helpers/AuthHelper.java
+++ b/app/src/main/java/me/impy/aegis/helpers/AuthHelper.java
@@ -11,15 +11,7 @@ public class AuthHelper {
     private AuthHelper() {
     }
 
-    public static char[] getPassword(EditText text, boolean clear) {
-        char[] password = getEditTextChars(text);
-        if (clear) {
-            clearPassword(text);
-        }
-        return password;
-    }
-
-    public static void clearPassword(EditText text) {
+    public static void clearEditText(EditText text) {
         text.getText().clear();
     }
 
@@ -33,9 +25,6 @@ public class AuthHelper {
     public static boolean arePasswordsEqual(EditText text1, EditText text2) {
         char[] password = getEditTextChars(text1);
         char[] passwordConfirm = getEditTextChars(text2);
-        boolean equal = password.length != 0 && Arrays.equals(password, passwordConfirm);
-        CryptoUtils.zero(password);
-        CryptoUtils.zero(passwordConfirm);
-        return equal;
+        return password.length != 0 && Arrays.equals(password, passwordConfirm);
     }
 }