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

Commit 85c4d5c0 authored by Eric Biggers's avatar Eric Biggers
Browse files

Zeroize LockscreenCredential objects received by LockSettingsService 

LockscreenCredential objects that were unmarshalled from a Binder
transaction need to be explicitly zeroized.  Otherwise it may be
possible to find them in a ramdump taken afterwards.

There are six methods in ILockSettings that take one or more
LockscreenCredential arguments.  Update all of them to zeroize these
arguments at the end of their execution, provided that they were
actually constructed from a Parcel rather than directly.

Test: atest FrameworksServicesTests:com.android.server.locksettings
Test: Temporarily added log statement for the zeroization, and verified
      that it triggered when unlocking device with LSKF.
Bug: 320392352
Bug: 416768837
Flag: EXEMPT bugfix
Change-Id: I40e06d47b01b1d665122ba9bfd766b9d0df3b485
parent 90e7c178

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

+26 −10
Original line number Diff line number Diff line
@@ -82,6 +82,9 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
    // generates and manages transparently to the user. Implies mType == CREDENTIAL_TYPE_PASSWORD.

    private final boolean mIsUnifiedProfilePassword;



    // Whether the credential was unmarshalled from a Parcel, rather than constructed directly.

    private final boolean mIsFromParcel;



    /**

     * Private constructor, use static builder methods instead.

     *
@@ -93,7 +96,8 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
            int type,

            byte[] credential,

            boolean hasInvalidChars,

            boolean isUnifiedProfilePassword) {

            boolean isUnifiedProfilePassword,

            boolean isFromParcel) {

        Objects.requireNonNull(credential);

        if (type == CREDENTIAL_TYPE_NONE) {

            Preconditions.checkArgument(credential.length == 0);
@@ -115,6 +119,7 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
        mCredential = credential;

        mHasInvalidChars = hasInvalidChars;

        mIsUnifiedProfilePassword = isUnifiedProfilePassword;

        mIsFromParcel = isFromParcel;

    }



    private LockscreenCredential(int type, CharSequence credential) {
@@ -122,14 +127,15 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
                type,

                charsToBytesTruncating(credential),

                hasInvalidChars(credential),

                /* isUnifiedProfilePassword= */ false);

                /* isUnifiedProfilePassword= */ false,

                /* isFromParcel= */ false);

    }



    /**

     * Creates a LockscreenCredential object representing a none credential.

     */

    public static LockscreenCredential createNone() {

        return new LockscreenCredential(CREDENTIAL_TYPE_NONE, new byte[0], false, false);

        return new LockscreenCredential(CREDENTIAL_TYPE_NONE, new byte[0], false, false, false);

    }



    /**
@@ -140,7 +146,8 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
                CREDENTIAL_TYPE_PATTERN,

                LockPatternUtils.patternToByteArray(pattern),

                /* hasInvalidChars= */ false,

                /* isUnifiedProfilePassword= */ false);

                /* isUnifiedProfilePassword= */ false,

                /* isFromParcel= */ false);

    }



    /**
@@ -161,7 +168,8 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
                CREDENTIAL_TYPE_PASSWORD,

                copyOfArrayNonMovable(password),

                /* hasInvalidChars= */ false,

                /* isUnifiedProfilePassword= */ true);

                /* isUnifiedProfilePassword= */ true,

                /* isFromParcel= */ false);

    }



    /**
@@ -271,12 +279,12 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
                mType,

                mCredential != null ? copyOfArrayNonMovable(mCredential) : null,

                mHasInvalidChars,

                mIsUnifiedProfilePassword);

                mIsUnifiedProfilePassword,

                /* Any duplicate copy is not from a Parcel, so set isFromParcel=false */

                /* isFromParcel= */ false);

    }



    /**

     * Zeroize the credential bytes.

     */

    /** Zeroizes the credential bytes. */

    public void zeroize() {

        if (mCredential != null) {

            ArrayUtils.zeroize(mCredential);
@@ -284,6 +292,13 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
        }

    }



    /** Zeroizes the given LockscreenCredential if it is non-null and was created from a Parcel. */

    public static void zeroizeIfFromParcel(@Nullable LockscreenCredential credential) {

        if (credential != null && credential.mIsFromParcel) {

            credential.zeroize();

        }

    }



    /**

     * Copies the given array into a new non-movable array.

     */
@@ -412,7 +427,8 @@ public class LockscreenCredential implements Parcelable, AutoCloseable { 
                            source.readInt(),

                            source.createByteArray(),

                            source.readBoolean(),

                            source.readBoolean());

                            source.readBoolean(),

                            /* isFromParcel= */ true);

                }



                @Override

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

+42 −16
Original line number Diff line number Diff line
@@ -1328,6 +1328,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
    public void setSeparateProfileChallengeEnabled(int userId, boolean enabled,

            LockscreenCredential profileUserPassword) {

        checkWritePermission();

        try {

            if (!mHasSecureLockScreen

                    && profileUserPassword != null

                    && profileUserPassword.getType() != CREDENTIAL_TYPE_NONE) {
@@ -1335,10 +1336,17 @@ public class LockSettingsService extends ILockSettings.Stub { 
                        "This operation requires secure lock screen feature.");

            }

            synchronized (mSeparateChallengeLock) {

            setSeparateProfileChallengeEnabledLocked(userId, enabled, profileUserPassword != null

                    ? profileUserPassword : LockscreenCredential.createNone());

                setSeparateProfileChallengeEnabledLocked(

                        userId,

                        enabled,

                        profileUserPassword != null

                                ? profileUserPassword

                                : LockscreenCredential.createNone());

            }

            notifySeparateProfileChallengeChanged(userId);

        } finally {

            LockscreenCredential.zeroizeIfFromParcel(profileUserPassword);

        }

    }



    @GuardedBy("mSeparateChallengeLock")
@@ -1841,10 +1849,11 @@ public class LockSettingsService extends ILockSettings.Stub { 
                                + "ACCESS_KEYGUARD_SECURE_STORAGE");

            }

        }

        credential.validateBasicRequirements();



        final long identity = Binder.clearCallingIdentity();

        try {

            credential.validateBasicRequirements();



            enforceFrpNotActive();

            // When changing credential for profiles with unified challenge, some callers

            // will pass in empty credential while others will pass in the credential of
@@ -1885,6 +1894,8 @@ public class LockSettingsService extends ILockSettings.Stub { 
            return true;

        } finally {

            Binder.restoreCallingIdentity(identity);

            LockscreenCredential.zeroizeIfFromParcel(credential);

            LockscreenCredential.zeroizeIfFromParcel(savedCredential);

        }

    }
@@ -2348,6 +2359,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
            return doVerifyCredential(credential, userId, progressCallback, 0 /* flags */);

        } finally {

            Binder.restoreCallingIdentity(identity);

            LockscreenCredential.zeroizeIfFromParcel(credential);

            scheduleGc();

        }

    }
@@ -2367,6 +2379,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
            return doVerifyCredential(credential, userId, null /* progressCallback */, flags);

        } finally {

            Binder.restoreCallingIdentity(identity);

            LockscreenCredential.zeroizeIfFromParcel(credential);

            scheduleGc();

        }

    }
@@ -2540,10 +2553,8 @@ public class LockSettingsService extends ILockSettings.Stub { 
        }

    }



    @Override

    public VerifyCredentialResponse verifyTiedProfileChallenge(LockscreenCredential credential,

            int userId, @LockPatternUtils.VerifyFlag int flags) {

        checkPasswordReadPermission();

    private VerifyCredentialResponse doVerifyTiedProfileChallenge(

            LockscreenCredential credential, int userId, @LockPatternUtils.VerifyFlag int flags) {

        Slogf.i(TAG, "Verifying tied profile challenge for user %d", userId);



        if (!isProfileWithUnifiedLock(userId)) {
@@ -2576,6 +2587,17 @@ public class LockSettingsService extends ILockSettings.Stub { 
        }

    }



    @Override

    public VerifyCredentialResponse verifyTiedProfileChallenge(

            LockscreenCredential credential, int userId, @LockPatternUtils.VerifyFlag int flags) {

        checkPasswordReadPermission();

        try {

            return doVerifyTiedProfileChallenge(credential, userId, flags);

        } finally {

            LockscreenCredential.zeroizeIfFromParcel(credential);

        }

    }



    /**

     * Keep track of the given user's latest password metric. This should be called

     * when the user is authenticating or when a new password is being set. In comparison,
@@ -3340,7 +3362,11 @@ public class LockSettingsService extends ILockSettings.Stub { 
    @Override

    public byte[] getHashFactor(LockscreenCredential currentCredential, int userId) {

        checkPasswordReadPermission();

        try {

            return getHashFactorInternal(currentCredential, userId);

        } finally {

            LockscreenCredential.zeroizeIfFromParcel(currentCredential);

        }

    }



    private long addEscrowToken(@NonNull byte[] token, @TokenType int type, int userId,