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

Commit fabfcb05 authored by Eric Sandness's avatar Eric Sandness
Browse files

Stop saving password metrics to disk 

Previously, DevicePolicyManager saved password stats (number of letters,
number of symbols, etc) to disk for FDE devices. This made it possible
for the isActivePasswordSufficient() API to return a result before the
password was entered for the first time after a reboot. Access to these
stats would significantly narrow the space of possible passwords an
attacker would need to explore.

Going forward, every time either the password or the password
requirements change, a flag will be persisted indicating whether the
current password meets the requirements. Before the password is entered
for the first time after a reboot, isActivePasswordSufficient() simply
returns the value of this flag. (After the password is entered for the
first time, isActivePasswordSufficient() uses password stats saved in
memory, as is the case today.)

This creates a window where isActivePasswordSufficient() may return an
incorrect value before the password is entered for the first time, if
the requirements are changed after startup so that the current password
no longer meets the requirements. This has been deemed an acceptable
compromise in order to avoid storing potentially sensitive data.

Test: runtest -c com.android.server.devicepolicy.DevicePolicyManagerTest
frameworks-services

Bug: 34218769
Change-Id: I5d3cd008a9ee2787bcb10ed5455bb61c6014ae00
parent bcee3cc3

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

+3 −19
Original line number Diff line number Diff line
@@ -16,16 +16,14 @@ 


package android.app.admin;



import org.junit.Test;

import org.junit.runner.RunWith;

import static org.junit.Assert.assertEquals;



import android.os.Parcel;

import android.support.test.filters.SmallTest;

import android.support.test.runner.AndroidJUnit4;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;

import org.junit.Test;

import org.junit.runner.RunWith;



/** Unit tests for {@link PasswordMetrics}. */

@RunWith(AndroidJUnit4.class)
@@ -43,20 +41,6 @@ public class PasswordMetricsTest { 
        assertEquals(0, metrics.numeric);

        assertEquals(0, metrics.symbols);

        assertEquals(0, metrics.nonLetter);

        assertTrue("default constructor does not produce default metrics", metrics.isDefault());

    }



    @Test

    public void testIsNotDefault() {

        final PasswordMetrics metrics = new PasswordMetrics(

                DevicePolicyManager.PASSWORD_QUALITY_NUMERIC, 12);

        assertFalse("non-default metrics are repoted as default", metrics.isDefault());

    }



    @Test

    public void testComputeForEmptyPassword() {

        final PasswordMetrics metrics = PasswordMetrics.computeForPassword("");

        assertTrue("empty password has default metrics", metrics.isDefault());

    }



    @Test

services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java

+53 −26
Original line number Diff line number Diff line
@@ -254,6 +254,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 


    private static final String TAG_PASSWORD_TOKEN_HANDLE = "password-token";



    private static final String TAG_PASSWORD_VALIDITY = "password-validity";



    private static final int REQUEST_EXPIRE_PASSWORD = 5571;



    private static final long MS_PER_DAY = TimeUnit.DAYS.toMillis(1);
@@ -478,6 +480,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
    public static class DevicePolicyData {

        @NonNull PasswordMetrics mActivePasswordMetrics = new PasswordMetrics();

        int mFailedPasswordAttempts = 0;

        boolean mPasswordStateHasBeenSetSinceBoot = false;

        boolean mPasswordValidAtLastCheckpoint = false;



        int mUserHandle;

        int mPasswordOwner = -1;
@@ -2574,19 +2578,14 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                out.endTag(null, "failed-password-attempts");

            }



            // Don't save metrics for FBE devices

            final PasswordMetrics metrics = policy.mActivePasswordMetrics;

            if (!mInjector.storageManagerIsFileBasedEncryptionEnabled() && !metrics.isDefault()) {

                out.startTag(null, "active-password");

                out.attribute(null, "quality", Integer.toString(metrics.quality));

                out.attribute(null, "length", Integer.toString(metrics.length));

                out.attribute(null, "uppercase", Integer.toString(metrics.upperCase));

                out.attribute(null, "lowercase", Integer.toString(metrics.lowerCase));

                out.attribute(null, "letters", Integer.toString(metrics.letters));

                out.attribute(null, "numeric", Integer.toString(metrics.numeric));

                out.attribute(null, "symbols", Integer.toString(metrics.symbols));

                out.attribute(null, "nonletter", Integer.toString(metrics.nonLetter));

                out.endTag(null, "active-password");

            // For FDE devices only, we save this flag so we can report on password sufficiency

            // before the user enters their password for the first time after a reboot.  For

            // security reasons, we don't want to store the full set of active password metrics.

            if (!mInjector.storageManagerIsFileBasedEncryptionEnabled()) {

                out.startTag(null, TAG_PASSWORD_VALIDITY);

                out.attribute(null, ATTR_VALUE,

                        Boolean.toString(policy.mPasswordValidAtLastCheckpoint));

                out.endTag(null, TAG_PASSWORD_VALIDITY);

            }



            for (int i = 0; i < policy.mAcceptedCaCertificates.size(); i++) {
@@ -2864,19 +2863,14 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                } else if (TAG_INITIALIZATION_BUNDLE.equals(tag)) {

                    policy.mInitBundle = PersistableBundle.restoreFromXml(parser);

                } else if ("active-password".equals(tag)) {

                    if (mInjector.storageManagerIsFileBasedEncryptionEnabled()) {

                        // Remove this from FBE devices

                    // Remove password metrics from saved settings, as we no longer wish to store

                    // these on disk

                    needsRewrite = true;

                    } else {

                        final PasswordMetrics m = policy.mActivePasswordMetrics;

                        m.quality = Integer.parseInt(parser.getAttributeValue(null, "quality"));

                        m.length = Integer.parseInt(parser.getAttributeValue(null, "length"));

                        m.upperCase = Integer.parseInt(parser.getAttributeValue(null, "uppercase"));

                        m.lowerCase = Integer.parseInt(parser.getAttributeValue(null, "lowercase"));

                        m.letters = Integer.parseInt(parser.getAttributeValue(null, "letters"));

                        m.numeric = Integer.parseInt(parser.getAttributeValue(null, "numeric"));

                        m.symbols = Integer.parseInt(parser.getAttributeValue(null, "symbols"));

                        m.nonLetter = Integer.parseInt(parser.getAttributeValue(null, "nonletter"));

                } else if (TAG_PASSWORD_VALIDITY.equals(tag)) {

                    if (!mInjector.storageManagerIsFileBasedEncryptionEnabled()) {

                        // This flag is only used for FDE devices

                        policy.mPasswordValidAtLastCheckpoint = Boolean.parseBoolean(

                                parser.getAttributeValue(null, ATTR_VALUE));

                    }

                } else if (TAG_PASSWORD_TOKEN_HANDLE.equals(tag)) {

                    policy.mPasswordTokenHandle = Long.parseLong(
@@ -3453,11 +3447,24 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.quality != quality) {

                ap.minimumPasswordMetrics.quality = quality;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }

    }



    /**

     * Updates flag in memory that tells us whether the user's password currently satisfies the

     * requirements set by all of the user's active admins.  This should be called before

     * {@link #saveSettingsLocked} whenever the password or the admin policies have changed.

     */

    @GuardedBy("DevicePolicyManagerService.this")

    private void updatePasswordValidityCheckpointLocked(int userHandle) {

        DevicePolicyData policy = getUserData(userHandle);

        policy.mPasswordValidAtLastCheckpoint = isActivePasswordSufficientForUserLocked(

                policy, policy.mUserHandle, false);

    }



    @Override

    public int getPasswordQuality(ComponentName who, int userHandle, boolean parent) {

        if (!mHasFeature) {
@@ -3540,6 +3547,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.length != length) {

                ap.minimumPasswordMetrics.length = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -3584,6 +3592,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.passwordHistoryLength != length) {

                ap.passwordHistoryLength = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -3796,6 +3805,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.upperCase != length) {

                ap.minimumPasswordMetrics.upperCase = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -3837,6 +3847,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.lowerCase != length) {

                ap.minimumPasswordMetrics.lowerCase = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -3881,6 +3892,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.letters != length) {

                ap.minimumPasswordMetrics.letters = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -3928,6 +3940,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.numeric != length) {

                ap.minimumPasswordMetrics.numeric = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -3975,6 +3988,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.symbols != length) {

                ap.minimumPasswordMetrics.symbols = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -4022,6 +4036,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                    who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);

            if (ap.minimumPasswordMetrics.nonLetter != length) {

                ap.minimumPasswordMetrics.nonLetter = length;

                updatePasswordValidityCheckpointLocked(mInjector.userHandleGetCallingUserId());

                saveSettingsLocked(mInjector.userHandleGetCallingUserId());

            }

        }
@@ -4093,6 +4108,16 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
            DevicePolicyData policy, int userHandle, boolean parent) {

        enforceUserUnlocked(userHandle, parent);



        if (!mInjector.storageManagerIsFileBasedEncryptionEnabled()

                && !policy.mPasswordStateHasBeenSetSinceBoot) {

            // Before user enters their password for the first time after a reboot, return the

            // value of this flag, which tells us whether the password was valid the last time

            // settings were saved.  If DPC changes password requirements on boot so that the

            // current password no longer meets the requirements, this value will be stale until

            // the next time the password is entered.

            return policy.mPasswordValidAtLastCheckpoint;

        }



        final int requiredPasswordQuality = getPasswordQuality(null, userHandle, parent);

        if (policy.mActivePasswordMetrics.quality < requiredPasswordQuality) {

            return false;
@@ -5436,6 +5461,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
        DevicePolicyData policy = getUserData(userHandle);

        synchronized (this) {

            policy.mActivePasswordMetrics = metrics;

            policy.mPasswordStateHasBeenSetSinceBoot = true;

        }

    }
@@ -5460,6 +5486,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
        try {

            synchronized (this) {

                policy.mFailedPasswordAttempts = 0;

                updatePasswordValidityCheckpointLocked(userId);

                saveSettingsLocked(userId);

                updatePasswordExpirationsLocked(userId);

                setExpirationAlarmCheckLocked(mContext, userId, /* parent */ false);

services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java

+75 −0
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import android.app.NotificationManager; 
import android.app.admin.DeviceAdminReceiver;

import android.app.admin.DevicePolicyManager;

import android.app.admin.DevicePolicyManagerInternal;

import android.app.admin.PasswordMetrics;

import android.content.BroadcastReceiver;

import android.content.ComponentName;

import android.content.Context;
@@ -3836,6 +3837,80 @@ public class DevicePolicyManagerTest extends DpmTestBase { 
        assertTrue(dpm.clearResetPasswordToken(admin1));

    }



    public void testIsActivePasswordSufficient() throws Exception {

        mContext.binder.callingUid = DpmMockContext.CALLER_SYSTEM_USER_UID;

        mContext.packageName = admin1.getPackageName();

        setupDeviceOwner();



        dpm.setPasswordQuality(admin1, DevicePolicyManager.PASSWORD_QUALITY_COMPLEX);

        dpm.setPasswordMinimumLength(admin1, 8);

        dpm.setPasswordMinimumLetters(admin1, 6);

        dpm.setPasswordMinimumLowerCase(admin1, 3);

        dpm.setPasswordMinimumUpperCase(admin1, 1);

        dpm.setPasswordMinimumNonLetter(admin1, 1);

        dpm.setPasswordMinimumNumeric(admin1, 1);

        dpm.setPasswordMinimumSymbols(admin1, 0);



        PasswordMetrics passwordMetricsNoSymbols = new PasswordMetrics(

                DevicePolicyManager.PASSWORD_QUALITY_COMPLEX, 9,

                8, 2,

                6, 1,

                0, 1);



        setActivePasswordState(passwordMetricsNoSymbols);

        assertTrue(dpm.isActivePasswordSufficient());



        initializeDpms();

        reset(mContext.spiedContext);

        assertTrue(dpm.isActivePasswordSufficient());



        // This call simulates the user entering the password for the first time after a reboot.

        // This causes password metrics to be reloaded into memory.  Until this happens,

        // dpm.isActivePasswordSufficient() will continue to return its last checkpointed value,

        // even if the DPC changes password requirements so that the password no longer meets the

        // requirements.  This is a known limitation of the current implementation of

        // isActivePasswordSufficient() - see b/34218769.

        setActivePasswordState(passwordMetricsNoSymbols);

        assertTrue(dpm.isActivePasswordSufficient());



        dpm.setPasswordMinimumSymbols(admin1, 1);

        // This assertion would fail if we had not called setActivePasswordState() again after

        // initializeDpms() - see previous comment.

        assertFalse(dpm.isActivePasswordSufficient());



        initializeDpms();

        reset(mContext.spiedContext);

        assertFalse(dpm.isActivePasswordSufficient());



        PasswordMetrics passwordMetricsWithSymbols = new PasswordMetrics(

                DevicePolicyManager.PASSWORD_QUALITY_COMPLEX, 9,

                7, 2,

                5, 1,

                1, 2);



        setActivePasswordState(passwordMetricsWithSymbols);

        assertTrue(dpm.isActivePasswordSufficient());

    }



    private void setActivePasswordState(PasswordMetrics passwordMetrics) {

        int userHandle = UserHandle.getUserId(mContext.binder.callingUid);

        final long ident = mContext.binder.clearCallingIdentity();

        try {

            dpm.setActivePasswordState(passwordMetrics, userHandle);

            dpm.reportPasswordChanged(userHandle);



            final Intent intent = new Intent(DeviceAdminReceiver.ACTION_PASSWORD_CHANGED);

            intent.setComponent(admin1);

            intent.putExtra(Intent.EXTRA_USER, UserHandle.of(mContext.binder.callingUid));



            verify(mContext.spiedContext, times(1)).sendBroadcastAsUser(

                    MockUtils.checkIntent(intent),

                    MockUtils.checkUserHandle(userHandle));

        } finally {

            mContext.binder.restoreCallingIdentity(ident);

        }

    }



    public void testIsCurrentInputMethodSetByOwnerForDeviceOwner() throws Exception {

        final String currentIme = Settings.Secure.DEFAULT_INPUT_METHOD;

        final Uri currentImeUri = Settings.Secure.getUriFor(currentIme);