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

Commit 7cf4509c authored by Rubin Xu's avatar Rubin Xu
Browse files

Fix resetPasswordWithToken before user unlock 

1. Fix system server crash when resetPasswordWithToken is called before use
   unlock, due to DPMS enforces user is unlocked when calculating password
   sufficiency.
2. Propogate new password metric from LockSettingsService to DPMS after a
   password reset with token, and fix a bug where stale quality was used.

Bug: 64923343
Bug: 64928518
Bug: 65286643
Test: cts-tradefed run cts-dev -m CtsDevicePolicyManagerTestCases -t com.android.cts.devicepolicy.ManagedProfileTest#testResetPasswordWithTokenBeforeUnlock
Test: cts-tradefed run cts-dev -m CtsDevicePolicyManagerTestCases -t com.android.cts.devicepolicy.MixedManagedProfileOwnerTest#testResetPasswordWithToken
Test: runtest frameworks-services -p com.android.server.locksettings
Test: cts-tradefed run cts-dev -m CtsDevicePolicyManagerTestCases -t com.android.cts.devicepolicy.DeviceAdminHostSideTestApi24#testRunDeviceOwnerPasswordTest
Test: runtest frameworks-core -c android.app.admin.PasswordMetricsTest
Test: runtest frameworks-services -c com.android.server.devicepolicy.DevicePolicyManagerTest
Change-Id: Ibb3736547b3b36da4a8a67af711e08a38427aa56
parent 2dac2cce

core/java/android/app/admin/DevicePolicyManager.java

+1 −1
Original line number Diff line number Diff line
@@ -2531,7 +2531,7 @@ public class DevicePolicyManager { 
     * @return Returns true if the password meets the current requirements, else false.

     * @throws SecurityException if the calling application does not own an active administrator

     *             that uses {@link DeviceAdminInfo#USES_POLICY_LIMIT_PASSWORD}

     * @throws InvalidStateException if the user is not unlocked.

     * @throws IllegalStateException if the user is not unlocked.

     */

    public boolean isActivePasswordSufficient() {

        if (mService != null) {

core/java/android/app/admin/PasswordMetrics.java

+17 −3
Original line number Diff line number Diff line
@@ -18,13 +18,11 @@ package android.app.admin; 


import android.annotation.IntDef;

import android.annotation.NonNull;

import android.app.admin.DevicePolicyManager;

import android.os.Parcelable;

import android.os.Parcel;

import android.os.Parcelable;



import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.io.IOException;



/**

 * A class that represents the metrics of a password that are used to decide whether or not a
@@ -159,6 +157,22 @@ public class PasswordMetrics implements Parcelable { 
                quality, length, letters, upperCase, lowerCase, numeric, symbols, nonLetter);

    }



    @Override

    public boolean equals(Object other) {

        if (!(other instanceof PasswordMetrics)) {

            return false;

        }

        PasswordMetrics o = (PasswordMetrics) other;

        return this.quality == o.quality

                && this.length == o.length

                && this.letters == o.letters

                && this.upperCase == o.upperCase

                && this.lowerCase == o.lowerCase

                && this.numeric == o.numeric

                && this.symbols == o.symbols

                && this.nonLetter == o.nonLetter;

    }



    /*

     * Returns the maximum length of a sequential characters. A sequence is defined as

     * monotonically increasing characters with a constant interval or the same character repeated.

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

+39 −10
Original line number Diff line number Diff line
@@ -803,12 +803,14 @@ public class LockPatternUtils { 
                        + "of length " + MIN_LOCK_PASSWORD_SIZE);

            }



            final int computedQuality = PasswordMetrics.computeForPassword(password).quality;

            setLong(PASSWORD_TYPE_KEY, Math.max(requestedQuality, computedQuality), userHandle);

            setLong(PASSWORD_TYPE_KEY,

                    computePasswordQuality(CREDENTIAL_TYPE_PASSWORD, password, requestedQuality),

                    userHandle);

            getLockSettings().setLockCredential(password, CREDENTIAL_TYPE_PASSWORD, savedPassword,

                    requestedQuality, userHandle);



            updateEncryptionPasswordIfNeeded(password, computedQuality, userHandle);

            updateEncryptionPasswordIfNeeded(password,

                    PasswordMetrics.computeForPassword(password).quality, userHandle);

            updatePasswordHistory(password, userHandle);

        } catch (RemoteException re) {

            // Cant do much
@@ -897,6 +899,24 @@ public class LockPatternUtils { 
                DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED, userHandle);

    }



    /**

     * Returns the password quality of the given credential, promoting it to a higher level

     * if DevicePolicyManager has a stronger quality requirement. This value will be written

     * to PASSWORD_TYPE_KEY.

     */

    private int computePasswordQuality(int type, String credential, int requestedQuality) {

        final int quality;

        if (type == CREDENTIAL_TYPE_PASSWORD) {

            int computedQuality = PasswordMetrics.computeForPassword(credential).quality;

            quality = Math.max(requestedQuality, computedQuality);

        } else if (type == CREDENTIAL_TYPE_PATTERN)  {

            quality = DevicePolicyManager.PASSWORD_QUALITY_SOMETHING;

        } else /* if (type == CREDENTIAL_TYPE_NONE) */ {

            quality = DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED;

        }

        return quality;

    }



    /**

     * Enables/disables the Separate Profile Challenge for this {@param userHandle}. This is a no-op

     * for user handles that do not belong to a managed profile.
@@ -1505,25 +1525,34 @@ public class LockPatternUtils { 
        }

    }



    public boolean setLockCredentialWithToken(String credential, int type, long tokenHandle,

            byte[] token, int userId) {

    /**

     * Change a user's lock credential with a pre-configured escrow token.

     *

     * @param credential The new credential to be set

     * @param type Credential type: password / pattern / none.

     * @param requestedQuality the requested password quality by DevicePolicyManager.

     *        See {@link DevicePolicyManager#getPasswordQuality(android.content.ComponentName)}

     * @param tokenHandle Handle of the escrow token

     * @param token Escrow token

     * @param userId The user who's lock credential to be changed

     * @return {@code true} if the operation is successful.

     */

    public boolean setLockCredentialWithToken(String credential, int type, int requestedQuality,

            long tokenHandle, byte[] token, int userId) {

        try {

            if (type != CREDENTIAL_TYPE_NONE) {

                if (TextUtils.isEmpty(credential) || credential.length() < MIN_LOCK_PASSWORD_SIZE) {

                    throw new IllegalArgumentException("password must not be null and at least "

                            + "of length " + MIN_LOCK_PASSWORD_SIZE);

                }



                final int computedQuality = PasswordMetrics.computeForPassword(credential).quality;

                int quality = Math.max(DevicePolicyManager.PASSWORD_QUALITY_NUMERIC,

                        computedQuality);

                final int quality = computePasswordQuality(type, credential, requestedQuality);

                if (!getLockSettings().setLockCredentialWithToken(credential, type, tokenHandle,

                        token, quality, userId)) {

                    return false;

                }

                setLong(PASSWORD_TYPE_KEY, quality, userId);



                updateEncryptionPasswordIfNeeded(credential, computedQuality, userId);

                updateEncryptionPasswordIfNeeded(credential, quality, userId);

                updatePasswordHistory(credential, userId);

            } else {

                if (!TextUtils.isEmpty(credential)) {

core/tests/coretests/src/android/app/admin/PasswordMetricsTest.java

+45 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package android.app.admin;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertNotEquals;



import android.os.Parcel;

import android.support.test.filters.SmallTest;
@@ -119,4 +120,48 @@ public class PasswordMetricsTest { 
        // ordered, but not composed of alphas or digits

        assertEquals(1, PasswordMetrics.maxLengthSequence(":;<=>"));

    }



    @Test

    public void testEquals() {

        PasswordMetrics metrics0 = new PasswordMetrics();

        PasswordMetrics metrics1 = new PasswordMetrics();

        assertNotEquals(metrics0, null);

        assertNotEquals(metrics0, new Object());

        assertEquals(metrics0, metrics0);

        assertEquals(metrics0, metrics1);



        assertEquals(new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_SOMETHING, 4),

                new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_SOMETHING, 4));



        assertNotEquals(new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_SOMETHING, 4),

                new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_SOMETHING, 5));



        assertNotEquals(new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_SOMETHING, 4),

                new PasswordMetrics(DevicePolicyManager.PASSWORD_QUALITY_COMPLEX, 4));



        metrics0 = PasswordMetrics.computeForPassword("1234abcd,./");

        metrics1 = PasswordMetrics.computeForPassword("1234abcd,./");

        assertEquals(metrics0, metrics1);

        metrics1.letters++;

        assertNotEquals(metrics0, metrics1);

        metrics1.letters--;

        metrics1.upperCase++;

        assertNotEquals(metrics0, metrics1);

        metrics1.upperCase--;

        metrics1.lowerCase++;

        assertNotEquals(metrics0, metrics1);

        metrics1.lowerCase--;

        metrics1.numeric++;

        assertNotEquals(metrics0, metrics1);

        metrics1.numeric--;

        metrics1.symbols++;

        assertNotEquals(metrics0, metrics1);

        metrics1.symbols--;

        metrics1.nonLetter++;

        assertNotEquals(metrics0, metrics1);

        metrics1.nonLetter--;

        assertEquals(metrics0, metrics1);





    }

}

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

+14 −3
Original line number Diff line number Diff line
@@ -154,7 +154,8 @@ public class LockSettingsService extends ILockSettings.Stub { 


    private final Injector mInjector;

    private final Context mContext;

    private final Handler mHandler;

    @VisibleForTesting

    protected final Handler mHandler;

    @VisibleForTesting

    protected final LockSettingsStorage mStorage;

    private final LockSettingsStrongAuth mStrongAuth;
@@ -1736,6 +1737,10 @@ public class LockSettingsService extends ILockSettings.Stub { 
        return response;

    }



    /**

     * Call this method to notify DPMS regarding the latest password metric. This should be called

     * when the user is authenticating or when a new password is being set.

     */

    private void notifyActivePasswordMetricsAvailable(String password, @UserIdInt int userId) {

        final PasswordMetrics metrics;

        if (password == null) {
@@ -2197,6 +2202,8 @@ public class LockSettingsService extends ILockSettings.Stub { 
        }

        setLong(SYNTHETIC_PASSWORD_HANDLE_KEY, newHandle, userId);

        synchronizeUnifiedWorkChallengeForProfiles(userId, profilePasswords);



        notifyActivePasswordMetricsAvailable(credential, userId);

        return newHandle;

    }
@@ -2246,13 +2253,13 @@ public class LockSettingsService extends ILockSettings.Stub { 
                    userId);

            synchronizeUnifiedWorkChallengeForProfiles(userId, null);

            mSpManager.destroyPasswordBasedSyntheticPassword(handle, userId);



            notifyActivePasswordMetricsAvailable(credential, userId);

        } else /* response == null || responseCode == VerifyCredentialResponse.RESPONSE_RETRY */ {

            Slog.w(TAG, "spBasedSetLockCredentialInternalLocked: " +

                    (response != null ? "rate limit exceeded" : "failed"));

            return;

        }

        notifyActivePasswordMetricsAvailable(credential, userId);



    }



    @Override
@@ -2358,6 +2365,10 @@ public class LockSettingsService extends ILockSettings.Stub { 
                Slog.w(TAG, "Invalid escrow token supplied");

                return false;

            }

            // Update PASSWORD_TYPE_KEY since it's needed by notifyActivePasswordMetricsAvailable()

            // called by setLockCredentialWithAuthTokenLocked().

            // TODO: refactor usage of PASSWORD_TYPE_KEY b/65239740

            setLong(LockPatternUtils.PASSWORD_TYPE_KEY, requestedQuality, userId);

            long oldHandle = getSyntheticPasswordHandleLocked(userId);

            setLockCredentialWithAuthTokenLocked(credential, type, result.authToken,

                    requestedQuality, userId);