Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 231799e6 authored by Eric Biggers's avatar Eric Biggers
Browse files

locksettings: use ArrayUtils.zeroize() 

Since direct uses of Arrays.fill() can potentially be optimized out by
the compiler and also do not immediately clean the data cache, use
ArrayUtils.zeroize() instead which is more reliable.

Flag this fix, to comply with trunk-stable policies.  As usual for
flags, the flag is temporary and will be removed soon.

This CL is just a straightforward replacement of existing zeroization
code.  Later CLs will add missing zeroization in some places and
allocate arrays as non-movable as needed.

Bug: 320392352
Test: atest FrameworksServicesTests:com.android.server.locksettings
Flag: android.security.secure_array_zeroization
Change-Id: I9b9830ada97bc0a8cb0b661fd9bea0e54f925d2d
parent 81036dbf

core/java/android/app/KeyguardManager.java

+1 −2
Original line number Diff line number Diff line
@@ -70,7 +70,6 @@ import com.android.internal.widget.VerifyCredentialResponse; 
import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.nio.charset.Charset;

import java.util.Arrays;

import java.util.List;

import java.util.Objects;

import java.util.concurrent.Executor;
@@ -1064,7 +1063,7 @@ public class KeyguardManager { 
            Log.e(TAG, "Save lock exception", e);

            success = false;

        } finally {

            Arrays.fill(password, (byte) 0);

            LockPatternUtils.zeroize(password);

        }

        return success;

    }

core/java/android/security/flags.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -41,6 +41,16 @@ flag { 
    bug: "325129836"

}



flag {

    name: "secure_array_zeroization"

    namespace: "platform_security"

    description: "Enable secure array zeroization"

    bug: "320392352"

    metadata {

      purpose: PURPOSE_BUGFIX

    }

}



flag {

    name: "deprecate_fsv_sig"

    namespace: "hardware_backed_security"

core/java/com/android/internal/widget/LockPatternUtils.java

+52 −0
Original line number Diff line number Diff line
@@ -61,6 +61,7 @@ import android.util.SparseIntArray; 
import android.util.SparseLongArray;



import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.ArrayUtils;

import com.android.server.LocalServices;



import com.google.android.collect.Lists;
@@ -71,6 +72,7 @@ import java.nio.charset.StandardCharsets; 
import java.security.NoSuchAlgorithmException;

import java.security.SecureRandom;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Collection;

import java.util.HashMap;

import java.util.List;
@@ -288,6 +290,56 @@ public class LockPatternUtils { 


    }



    /**

     * This exists temporarily due to trunk-stable policies.

     * Please use ArrayUtils directly if you can.

     */

    public static byte[] newNonMovableByteArray(int length) {

        if (!android.security.Flags.secureArrayZeroization()) {

            return new byte[length];

        }

        return ArrayUtils.newNonMovableByteArray(length);

    }



    /**

     * This exists temporarily due to trunk-stable policies.

     * Please use ArrayUtils directly if you can.

     */

    public static char[] newNonMovableCharArray(int length) {

        if (!android.security.Flags.secureArrayZeroization()) {

            return new char[length];

        }

        return ArrayUtils.newNonMovableCharArray(length);

    }



    /**

     * This exists temporarily due to trunk-stable policies.

     * Please use ArrayUtils directly if you can.

     */

    public static void zeroize(byte[] array) {

        if (!android.security.Flags.secureArrayZeroization()) {

            if (array != null) {

                Arrays.fill(array, (byte) 0);

            }

            return;

        }

        ArrayUtils.zeroize(array);

    }



    /**

     * This exists temporarily due to trunk-stable policies.

     * Please use ArrayUtils directly if you can.

     */

    public static void zeroize(char[] array) {

        if (!android.security.Flags.secureArrayZeroization()) {

            if (array != null) {

                Arrays.fill(array, (char) 0);

            }

            return;

        }

        ArrayUtils.zeroize(array);

    }



    @UnsupportedAppUsage

    public DevicePolicyManager getDevicePolicyManager() {

        if (mDevicePolicyManager == null) {

core/java/com/android/internal/widget/LockscreenCredential.java

+2 −2
Original line number Diff line number Diff line
@@ -246,7 +246,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
     */

    public void zeroize() {

        if (mCredential != null) {

            Arrays.fill(mCredential, (byte) 0);

            LockPatternUtils.zeroize(mCredential);

            mCredential = null;

        }

    }
@@ -346,7 +346,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
            byte[] sha1 = MessageDigest.getInstance("SHA-1").digest(saltedPassword);

            byte[] md5 = MessageDigest.getInstance("MD5").digest(saltedPassword);



            Arrays.fill(saltedPassword, (byte) 0);

            LockPatternUtils.zeroize(saltedPassword);

            return HexEncoding.encodeToString(ArrayUtils.concat(sha1, md5));

        } catch (NoSuchAlgorithmException e) {

            throw new AssertionError("Missing digest algorithm: ", e);

services/core/java/com/android/server/locksettings/LockSettingsService.java

+6 −6
Original line number Diff line number Diff line
@@ -438,9 +438,9 @@ public class LockSettingsService extends ILockSettings.Stub { 
        }

        LockscreenCredential credential =

                LockscreenCredential.createUnifiedProfilePassword(newPassword);

        Arrays.fill(newPasswordChars, '\u0000');

        Arrays.fill(newPassword, (byte) 0);

        Arrays.fill(randomLockSeed, (byte) 0);

        LockPatternUtils.zeroize(newPasswordChars);

        LockPatternUtils.zeroize(newPassword);

        LockPatternUtils.zeroize(randomLockSeed);

        return credential;

    }
@@ -1537,7 +1537,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
                        + userId);

            }

        } finally {

            Arrays.fill(password, (byte) 0);

            LockPatternUtils.zeroize(password);

        }

    }
@@ -1570,7 +1570,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
        decryptionResult = cipher.doFinal(encryptedPassword);

        LockscreenCredential credential = LockscreenCredential.createUnifiedProfilePassword(

                decryptionResult);

        Arrays.fill(decryptionResult, (byte) 0);

        LockPatternUtils.zeroize(decryptionResult);

        try {

            long parentSid = getGateKeeperService().getSecureUserId(

                    mUserManager.getProfileParent(userId).id);
@@ -2263,7 +2263,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
        } catch (RemoteException e) {

            Slogf.wtf(TAG, e, "Failed to unlock CE storage for %s user %d", userType, userId);

        } finally {

            Arrays.fill(secret, (byte) 0);

            LockPatternUtils.zeroize(secret);

        }

    }