Android: Fix #554: Added PowerAuthAuthentication.destroy()

wultra · Sep 21, 2023 · b2fe769 · b2fe769
1 parent a9457b5
commit b2fe769
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 3 deletions.
diff --git a/...werAuthLibrary/src/main/java/io/getlime/security/powerauth/biometry/BiometricKeyData.java b/...werAuthLibrary/src/main/java/io/getlime/security/powerauth/biometry/BiometricKeyData.java
@@ -18,6 +18,8 @@
 
 import androidx.annotation.NonNull;
 
+import java.util.Arrays;
+
 /**
  * The {@code BiometricKeyData} class contains result from the biometric authentication in case that
  * authentication succeeded.
@@ -61,4 +63,12 @@ public BiometricKeyData(@NonNull byte[] dataToSave, @NonNull byte[] derivedData,
     public boolean isNewKey() {
         return newKey;
     }
+
+    /**
+     * Destroy potential sensitive content stored in this object.
+     */
+    public void destroy() {
+        Arrays.fill(dataToSave, (byte) 0xCD);
+        Arrays.fill(derivedData, (byte) 0xCD);
+    }
 }
diff --git a/...rAuthLibrary/src/main/java/io/getlime/security/powerauth/sdk/PowerAuthAuthentication.java b/...rAuthLibrary/src/main/java/io/getlime/security/powerauth/sdk/PowerAuthAuthentication.java
@@ -21,6 +21,8 @@
 import io.getlime.security.powerauth.core.Password;
 import io.getlime.security.powerauth.system.PowerAuthLog;
 
+import java.util.Arrays;
+
 /**
  * Class representing a multi-factor authentication object.
  */
@@ -48,9 +50,14 @@ public class PowerAuthAuthentication {
     /**
      * Construct object with desired combination of factors. Such authentication object can be used
      * either to persist activation and for the signature calculation.
-     *
+     * <p>
      * Note that you should prefer static construction functions instead of this constructor, unless
      * you have a special reason for it.
+     * <p>
+     * Note that the constructor internally makes a copy of provided byte arrays. This is because authentication object
+     * now supports {@link #destroy()} method that clears content of such arrays. We don't want to clear instance of
+     * array that is still in use somewhere else.
+     *
      *
      * @param persistActivation If true, then authentication can be used to persist activation.
      * @param password If set, then knowledge factor will be used to persist activation or signature calculation.
@@ -62,12 +69,31 @@ public class PowerAuthAuthentication {
             @Nullable Password password,
             @Nullable byte[] biometryFactorRelatedKey,
             @Nullable byte[] overriddenPossessionKey) {
-        this.useBiometry = biometryFactorRelatedKey;
+        this.useBiometry = safeArrayCopy(biometryFactorRelatedKey);
         this.password = password;
-        this.overriddenPossessionKey = overriddenPossessionKey;
+        this.overriddenPossessionKey = safeArrayCopy(overriddenPossessionKey);
         this.persistActivation = persistActivation;
     }
 
+    /**
+     * Make copy of provided byte array if non-null array is provided.
+     * @param original Array to safely copy.
+     * @return new array with copied content or null if input is null.
+     */
+    private static byte[] safeArrayCopy(byte[] original) {
+        if (original != null) {
+            return Arrays.copyOf(original, original.length);
+        }
+        return null;
+    }
+
+    /**
+     * Make sure that the sensitive data is always wiped out from the memory.
+     */
+    protected void finalize() {
+        destroy();
+    }
+
     // Persist activation
 
     /**
@@ -347,6 +373,22 @@ public byte[] getOverriddenPossessionKey() {
         return overriddenPossessionKey;
     }
 
+    /**
+     * Destroys any sensitive content, such as password stored in the authentication object.
+     * After this call, the object becomes unusable for authentication operations.
+     */
+    public void destroy() {
+        if (password != null) {
+            password.destroy();
+        }
+        if (useBiometry != null) {
+            Arrays.fill(useBiometry, (byte) 0xCD);  // This may help with the debugging. CD CD CD is more suspicious than 00 00 00
+        }
+        if (overriddenPossessionKey != null) {
+            Arrays.fill(overriddenPossessionKey, (byte) 0xCD);
+        }
+    }
+
     // Internal interfaces
 
     /**

diff --git a/...ndroid/PowerAuthLibrary/src/main/java/io/getlime/security/powerauth/sdk/PowerAuthSDK.java b/...ndroid/PowerAuthLibrary/src/main/java/io/getlime/security/powerauth/sdk/PowerAuthSDK.java
@@ -1045,6 +1045,7 @@ public void onBiometricDialogCancelled(boolean userCancel) {
             public void onBiometricDialogSuccess(@NonNull BiometricKeyData biometricKeyData) {
                 final PowerAuthAuthentication authentication = PowerAuthAuthentication.persistWithPasswordAndBiometry(password, biometricKeyData.getDerivedData());
                 final int errorCode = persistActivationWithAuthentication(context, authentication);
+                biometricKeyData.destroy();
                 if (errorCode == PowerAuthErrorCodes.SUCCEED) {
                     callback.onBiometricDialogSuccess();
                 } else {
@@ -2360,6 +2361,8 @@ public void onBiometricDialogCancelled(boolean userCancel) {
             @Override
             public void onBiometricDialogSuccess(@NonNull BiometricKeyData biometricKeyData) {
                 final PowerAuthAuthentication authentication = PowerAuthAuthentication.possessionWithBiometry(biometricKeyData.getDerivedData());
+                // TODO: This should be moved in the next release to some global point to make sure that we always clear this object.
+                biometricKeyData.destroy();
                 listener.onBiometricDialogSuccess(authentication);
             }