From 15c0a441c9a0078fd99c1f2fcebe3585e28be00e Mon Sep 17 00:00:00 2001 From: joshmccloskey Date: Fri, 18 Oct 2019 16:09:06 -0700 Subject: [PATCH] Add builder method to check DevicePolicyManager This is a hidden method and BiometricService will enforce that 3rd party applications cannot use it. Fixes: 142966163 Test: Verified disabling fingerprint will not allow the user to unlock work apps with fingerprint. (But can use fingeprint within apps.) Test: Verified disabling face and/or iris on a fingerprint device will continue to allow the user to unlock work apps with fingerprint. Test: Verified disabling face on a face authentication device will not allow the user to unlock work apps with face authentication. (But can use face authentication within apps.) Test: Verified disabling fingerprint and/or iris on a face authentication device will continue to allow the user to unlock work apps with face authentication. Change-Id: I21ae00d0993d78a641cb8c5d8af2dd02be0b4a21 --- core/java/android/app/KeyguardManager.java | 32 +++++++ .../hardware/biometrics/BiometricPrompt.java | 19 +++++ .../systemui/keyguard/WorkLockActivity.java | 10 +-- .../server/biometrics/BiometricService.java | 60 +++++++++++++- .../server/wm/ActivityStartInterceptor.java | 3 +- .../biometrics/BiometricServiceTest.java | 83 ++++++++++++++++++- .../wm/ActivityStartInterceptorTest.java | 5 +- 7 files changed, 196 insertions(+), 16 deletions(-) diff --git a/core/java/android/app/KeyguardManager.java b/core/java/android/app/KeyguardManager.java index b1565ab8a501b..f8929c45a5d69 100644 --- a/core/java/android/app/KeyguardManager.java +++ b/core/java/android/app/KeyguardManager.java @@ -117,6 +117,16 @@ public class KeyguardManager { */ public static final int RESULT_ALTERNATE = 1; + /** + * + * If this is set, check device policy for allowed biometrics when the user is authenticating. + * This should only be used in the context of managed profiles. + * + * @hide + */ + public static final String EXTRA_DISALLOW_BIOMETRICS_IF_POLICY_EXISTS = "check_dpm"; + + /** * Get an intent to prompt the user to confirm credentials (pin, pattern, password or biometrics * if enrolled) for the current user of the device. The caller is expected to launch this @@ -164,6 +174,28 @@ public class KeyguardManager { return intent; } + /** + * Get an intent to prompt the user to confirm credentials (pin, pattern or password) + * for the given user. The caller is expected to launch this activity using + * {@link android.app.Activity#startActivityForResult(Intent, int)} and check for + * {@link android.app.Activity#RESULT_OK} if the user successfully completes the challenge. + * + * @param disallowBiometricsIfPolicyExists If true check if the Device Policy Manager has + * disabled biometrics on the device. If biometrics are disabled, fall back to PIN/pattern/pass. + * + * @return the intent for launching the activity or null if no password is required. + * + * @hide + */ + public Intent createConfirmDeviceCredentialIntent( + CharSequence title, CharSequence description, int userId, + boolean disallowBiometricsIfPolicyExists) { + Intent intent = this.createConfirmDeviceCredentialIntent(title, description, userId); + intent.putExtra(EXTRA_DISALLOW_BIOMETRICS_IF_POLICY_EXISTS, + disallowBiometricsIfPolicyExists); + return intent; + } + /** * Get an intent to prompt the user to confirm credentials (pin, pattern or password) * for the previous owner of the device. The caller is expected to launch this activity using diff --git a/core/java/android/hardware/biometrics/BiometricPrompt.java b/core/java/android/hardware/biometrics/BiometricPrompt.java index 6f9c9e6db025c..cb8fc8b1cbb15 100644 --- a/core/java/android/hardware/biometrics/BiometricPrompt.java +++ b/core/java/android/hardware/biometrics/BiometricPrompt.java @@ -87,6 +87,11 @@ public class BiometricPrompt implements BiometricAuthenticator, BiometricConstan * @hide */ public static final String KEY_AUTHENTICATORS_ALLOWED = "authenticators_allowed"; + /** + * If this is set, check the Device Policy Manager for allowed biometrics. + * @hide + */ + public static final String EXTRA_DISALLOW_BIOMETRICS_IF_POLICY_EXISTS = "check_dpm"; /** * Error/help message will show for this amount of time. @@ -325,6 +330,20 @@ public class BiometricPrompt implements BiometricAuthenticator, BiometricConstan return this; } + /** + * If set check the Device Policy Manager for disabled biometrics. + * + * @param checkDevicePolicyManager + * @return This builder. + * @hide + */ + @NonNull + public Builder setDisallowBiometricsIfPolicyExists(boolean checkDevicePolicyManager) { + mBundle.putBoolean(EXTRA_DISALLOW_BIOMETRICS_IF_POLICY_EXISTS, + checkDevicePolicyManager); + return this; + } + /** * Creates a {@link BiometricPrompt}. * diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/WorkLockActivity.java b/packages/SystemUI/src/com/android/systemui/keyguard/WorkLockActivity.java index ca04633b316a2..24ad75d7dcc08 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/WorkLockActivity.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/WorkLockActivity.java @@ -51,12 +51,6 @@ import javax.inject.Inject; public class WorkLockActivity extends Activity { private static final String TAG = "WorkLockActivity"; - /** - * Add additional extra {@link com.android.settings.password.ConfirmDeviceCredentialActivity} to - * enable device policy management enforcement from systemui. - */ - public static final String EXTRA_FROM_WORK_LOCK_ACTIVITY = "from_work_lock_activity"; - /** * Contains a {@link TaskDescription} for the activity being covered. */ @@ -156,7 +150,8 @@ public class WorkLockActivity extends Activity { } final Intent credential = getKeyguardManager() - .createConfirmDeviceCredentialIntent(null, null, getTargetUserId()); + .createConfirmDeviceCredentialIntent(null, null, getTargetUserId(), + true /* disallowBiometricsIfPolicyExists */); if (credential == null) { return; } @@ -172,7 +167,6 @@ public class WorkLockActivity extends Activity { if (target != null) { credential.putExtra(Intent.EXTRA_INTENT, target.getIntentSender()); - credential.putExtra(EXTRA_FROM_WORK_LOCK_ACTIVITY, true); } final ActivityOptions launchOptions = ActivityOptions.makeBasic(); diff --git a/services/core/java/com/android/server/biometrics/BiometricService.java b/services/core/java/com/android/server/biometrics/BiometricService.java index 0f51e39d128a0..e1a9f3b97e9ac 100644 --- a/services/core/java/com/android/server/biometrics/BiometricService.java +++ b/services/core/java/com/android/server/biometrics/BiometricService.java @@ -28,6 +28,7 @@ import static android.hardware.biometrics.BiometricManager.Authenticators; import android.app.ActivityManager; import android.app.IActivityManager; import android.app.UserSwitchObserver; +import android.app.admin.DevicePolicyManager; import android.app.trust.ITrustManager; import android.content.ContentResolver; import android.content.Context; @@ -210,6 +211,7 @@ public class BiometricService extends SystemService { } private final Injector mInjector; + private final DevicePolicyManager mDevicePolicyManager; @VisibleForTesting final IBiometricService.Stub mImpl; @VisibleForTesting @@ -648,6 +650,10 @@ public class BiometricService extends SystemService { throw new SecurityException("Invalid authenticator configuration"); } + if (bundle.getBoolean(BiometricPrompt.EXTRA_DISALLOW_BIOMETRICS_IF_POLICY_EXISTS)) { + checkInternalPermission(); + } + Utils.combineAuthenticatorBundles(bundle); // Check the usage of this in system server. Need to remove this check if it becomes a @@ -712,8 +718,8 @@ public class BiometricService extends SystemService { int biometricConstantsResult = BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE; final long ident = Binder.clearCallingIdentity(); try { - biometricConstantsResult = - checkAndGetAuthenticators(userId, bundle, opPackageName).second; + biometricConstantsResult = checkAndGetAuthenticators(userId, bundle, opPackageName, + false /* checkDevicePolicyManager */).second; if (biometricConstantsResult != BiometricConstants.BIOMETRIC_SUCCESS && Utils.isDeviceCredentialAllowed(bundle)) { // If there's an issue with biometrics, but device credential is allowed and @@ -947,6 +953,8 @@ public class BiometricService extends SystemService { super(context); mInjector = injector; + mDevicePolicyManager = (DevicePolicyManager) context + .getSystemService(context.DEVICE_POLICY_SERVICE); mImpl = new BiometricServiceWrapper(); mEnabledOnKeyguardCallbacks = new ArrayList<>(); mSettingObserver = mInjector.getSettingObserver(context, mHandler, @@ -977,6 +985,42 @@ public class BiometricService extends SystemService { mBiometricStrengthController.startListening(); } + /** + * @param modality one of {@link BiometricAuthenticator#TYPE_FINGERPRINT}, + * {@link BiometricAuthenticator#TYPE_IRIS} or {@link BiometricAuthenticator#TYPE_FACE} + * @return + */ + private int mapModalityToDevicePolicyType(int modality) { + switch (modality) { + case TYPE_FINGERPRINT: + return DevicePolicyManager.KEYGUARD_DISABLE_FINGERPRINT; + case TYPE_IRIS: + return DevicePolicyManager.KEYGUARD_DISABLE_IRIS; + case TYPE_FACE: + return DevicePolicyManager.KEYGUARD_DISABLE_FACE; + default: + Slog.e(TAG, "Error modality=" + modality); + return DevicePolicyManager.KEYGUARD_DISABLE_FEATURES_NONE; + } + } + + // TODO(joshmccloskey): Update this to throw an error if a new modality is added and this + // logic is not updated. + private boolean isBiometricDisabledByDevicePolicy(int modality, int effectiveUserId) { + final int biometricToCheck = mapModalityToDevicePolicyType(modality); + if (biometricToCheck == DevicePolicyManager.KEYGUARD_DISABLE_FEATURES_NONE) { + Slog.e(TAG, "Allowing unknown modality " + modality + " to pass Device Policy check"); + return false; + } + final int devicePolicyDisabledFeatures = + mDevicePolicyManager.getKeyguardDisabledFeatures(null, effectiveUserId); + final boolean isBiometricDisabled = + (biometricToCheck & devicePolicyDisabledFeatures) != 0; + Slog.w(TAG, "isBiometricDisabledByDevicePolicy(" + modality + "," + effectiveUserId + + ")=" + isBiometricDisabled); + return isBiometricDisabled; + } + /** * Checks if there are any available biometrics, and returns the modality. This method also * returns errors through the callback (no biometric feature, hardware not detected, no @@ -996,7 +1040,7 @@ public class BiometricService extends SystemService { * TODO(kchyn): Update this to handle DEVICE_CREDENTIAL better, reduce duplicate code in callers */ private Pair checkAndGetAuthenticators(int userId, Bundle bundle, - String opPackageName) throws RemoteException { + String opPackageName, boolean checkDevicePolicyManager) throws RemoteException { if (!Utils.isBiometricAllowed(bundle) && Utils.isDeviceCredentialAllowed(bundle) && !mTrustManager.isDeviceSecure(userId)) { @@ -1033,6 +1077,11 @@ public class BiometricService extends SystemService { } if (authenticator.impl.hasEnrolledTemplates(userId, opPackageName)) { hasTemplatesEnrolled = true; + // If the device policy manager disables a specific biometric, skip it. + if (checkDevicePolicyManager && + isBiometricDisabledByDevicePolicy(modality, userId)) { + continue; + } if (isEnabledForApp(modality, userId)) { enabledForApps = true; break; @@ -1043,6 +1092,7 @@ public class BiometricService extends SystemService { } Slog.d(TAG, "checkAndGetAuthenticators: user=" + userId + + " checkDevicePolicyManager=" + checkDevicePolicyManager + " isHardwareDetected=" + isHardwareDetected + " hasTemplatesEnrolled=" + hasTemplatesEnrolled + " enabledForApps=" + enabledForApps); @@ -1502,8 +1552,10 @@ public class BiometricService extends SystemService { int result; try { + final boolean checkDevicePolicyManager = bundle.getBoolean( + BiometricPrompt.EXTRA_DISALLOW_BIOMETRICS_IF_POLICY_EXISTS, false); final Pair pair = checkAndGetAuthenticators(userId, bundle, - opPackageName); + opPackageName, checkDevicePolicyManager); modality = pair.first; result = pair.second; } catch (RemoteException e) { diff --git a/services/core/java/com/android/server/wm/ActivityStartInterceptor.java b/services/core/java/com/android/server/wm/ActivityStartInterceptor.java index b2fb93d1e98e1..be3a61383de14 100644 --- a/services/core/java/com/android/server/wm/ActivityStartInterceptor.java +++ b/services/core/java/com/android/server/wm/ActivityStartInterceptor.java @@ -301,7 +301,8 @@ class ActivityStartInterceptor { FLAG_CANCEL_CURRENT | FLAG_ONE_SHOT | FLAG_IMMUTABLE); final KeyguardManager km = (KeyguardManager) mServiceContext .getSystemService(KEYGUARD_SERVICE); - final Intent newIntent = km.createConfirmDeviceCredentialIntent(null, null, userId); + final Intent newIntent = km.createConfirmDeviceCredentialIntent(null, null, userId, + true /* disallowBiometricsIfPolicyExists */); if (newIntent == null) { return null; } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java index 211fc4db6211f..f96d9961d364b 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceTest.java @@ -36,6 +36,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.IActivityManager; +import android.app.admin.DevicePolicyManager; import android.app.trust.ITrustManager; import android.content.ContentResolver; import android.content.Context; @@ -104,6 +105,8 @@ public class BiometricServiceTest { IBiometricAuthenticator mFaceAuthenticator; @Mock ITrustManager mTrustManager; + @Mock + DevicePolicyManager mDevicePolicyManager; @Before public void setUp() { @@ -111,6 +114,8 @@ public class BiometricServiceTest { when(mContext.getContentResolver()).thenReturn(mContentResolver); when(mContext.getResources()).thenReturn(mResources); + when(mContext.getSystemService(Context.DEVICE_POLICY_SERVICE)) + .thenReturn(mDevicePolicyManager); when(mInjector.getActivityManagerService()).thenReturn(mock(IActivityManager.class)); when(mInjector.getStatusBarService()).thenReturn(mock(IStatusBarService.class)); @@ -1198,10 +1203,71 @@ public class BiometricServiceTest { for (String s : mInjector.getConfiguration(null)) { SensorConfig config = new SensorConfig(s); mBiometricService.mImpl.registerAuthenticator(config.mId, config.mModality, - config.mStrength, mFingerprintAuthenticator); + config.mStrength, mFingerprintAuthenticator); } } + @Test + public void testWorkAuthentication_fingerprintWorksIfNotDisabledByDevicePolicyManager() + throws Exception { + setupAuthForOnly(BiometricAuthenticator.TYPE_FINGERPRINT, Authenticators.BIOMETRIC_STRONG); + when(mDevicePolicyManager + .getKeyguardDisabledFeatures(any() /* admin */, anyInt() /* userHandle */)) + .thenReturn(~DevicePolicyManager.KEYGUARD_DISABLE_FINGERPRINT); + invokeAuthenticateForWorkApp(mBiometricService.mImpl, mReceiver1); + waitForIdle(); + assertEquals(mBiometricService.mPendingAuthSession.mState, + BiometricService.STATE_AUTH_CALLED); + startPendingAuthSession(mBiometricService); + waitForIdle(); + assertEquals(mBiometricService.mCurrentAuthSession.mState, + BiometricService.STATE_AUTH_STARTED); + } + + @Test + public void testWorkAuthentication_faceWorksIfNotDisabledByDevicePolicyManager() + throws Exception { + setupAuthForOnly(BiometricAuthenticator.TYPE_FACE, Authenticators.BIOMETRIC_STRONG); + when(mDevicePolicyManager + .getKeyguardDisabledFeatures(any() /* admin*/, anyInt() /* userHandle */)) + .thenReturn(~DevicePolicyManager.KEYGUARD_DISABLE_FACE); + invokeAuthenticateForWorkApp(mBiometricService.mImpl, mReceiver1); + waitForIdle(); + assertEquals(mBiometricService.mPendingAuthSession.mState, + BiometricService.STATE_AUTH_CALLED); + startPendingAuthSession(mBiometricService); + waitForIdle(); + assertEquals(mBiometricService.mCurrentAuthSession.mState, + BiometricService.STATE_AUTH_STARTED); + } + + @Test + public void testWorkAuthentication_fingerprintFailsIfDisabledByDevicePolicyManager() + throws Exception { + setupAuthForOnly(BiometricAuthenticator.TYPE_FINGERPRINT, Authenticators.BIOMETRIC_STRONG); + when(mDevicePolicyManager + .getKeyguardDisabledFeatures(any() /* admin */, anyInt() /* userHandle */)) + .thenReturn(DevicePolicyManager.KEYGUARD_DISABLE_FINGERPRINT); + invokeAuthenticateForWorkApp(mBiometricService.mImpl, mReceiver1); + waitForIdle(); + assertNotNull(mBiometricService.mCurrentAuthSession); + assertEquals(mBiometricService.mCurrentAuthSession.mState, + BiometricService.STATE_SHOWING_DEVICE_CREDENTIAL); + } + + @Test + public void testWorkAuthentication_faceFailsIfDisabledByDevicePolicyManager() throws Exception { + setupAuthForOnly(BiometricAuthenticator.TYPE_FACE, Authenticators.BIOMETRIC_STRONG); + when(mDevicePolicyManager + .getKeyguardDisabledFeatures(any() /* admin */, anyInt() /* userHandle */)) + .thenReturn(DevicePolicyManager.KEYGUARD_DISABLE_FACE); + invokeAuthenticateForWorkApp(mBiometricService.mImpl, mReceiver1); + waitForIdle(); + assertNotNull(mBiometricService.mCurrentAuthSession); + assertEquals(mBiometricService.mCurrentAuthSession.mState, + BiometricService.STATE_SHOWING_DEVICE_CREDENTIAL); + } + // Helper methods private int invokeCanAuthenticate(BiometricService service, int authenticators) @@ -1311,6 +1377,21 @@ public class BiometricServiceTest { createTestBiometricPromptBundle(requireConfirmation, authenticators)); } + private static void invokeAuthenticateForWorkApp(IBiometricService.Stub service, + IBiometricServiceReceiver receiver) throws Exception { + final Bundle bundle = new Bundle(); + bundle.putBoolean(BiometricPrompt.EXTRA_DISALLOW_BIOMETRICS_IF_POLICY_EXISTS, true); + bundle.putBoolean(BiometricPrompt.KEY_REQUIRE_CONFIRMATION, true); + bundle.putBoolean(BiometricPrompt.KEY_ALLOW_DEVICE_CREDENTIAL, true); + service.authenticate( + new Binder() /* token */, + 0 /* sessionId */, + 0 /* userId */, + receiver, + TEST_PACKAGE_NAME /* packageName */, + bundle); + } + private static Bundle createTestBiometricPromptBundle( boolean requireConfirmation, Integer authenticators) { diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStartInterceptorTest.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStartInterceptorTest.java index d78e3af85e963..42347208ea47a 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityStartInterceptorTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStartInterceptorTest.java @@ -24,6 +24,7 @@ import static com.android.server.pm.PackageManagerService.PLATFORM_PACKAGE_NAME; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; @@ -137,8 +138,8 @@ public class ActivityStartInterceptorTest { // Mock KeyguardManager when(mContext.getSystemService(Context.KEYGUARD_SERVICE)).thenReturn(mKeyguardManager); when(mKeyguardManager.createConfirmDeviceCredentialIntent( - nullable(CharSequence.class), nullable(CharSequence.class), eq(TEST_USER_ID))) - .thenReturn(CONFIRM_CREDENTIALS_INTENT); + nullable(CharSequence.class), nullable(CharSequence.class), eq(TEST_USER_ID), + eq(true))).thenReturn(CONFIRM_CREDENTIALS_INTENT); // Mock PackageManager when(mService.getPackageManager()).thenReturn(mPackageManager);