From 520da04fba6d7023182e959e6b8f236de9c7264b Mon Sep 17 00:00:00 2001 From: Derek Jedral Date: Wed, 14 Dec 2022 13:53:26 -0800 Subject: [PATCH] Add newlyUnlocked to onTrustChanged onTrustChanged doesn't notify listeners if the trust change was actually used to unlock the device, and so listeners cannot react to the change properly. We need to pass an additional boolean. Test: manual test Bug: 258001148 Change-Id: Ia205640fbdb5ddeb328ea81f1fe74c80ce25bca8 --- .../android/app/trust/ITrustListener.aidl | 4 ++-- core/java/android/app/trust/TrustManager.java | 22 +++++++++++++----- .../keyguard/KeyguardUpdateMonitor.java | 2 +- .../keyguard/KeyguardUpdateMonitorTest.java | 19 ++++++++++----- .../hidl/Fingerprint21UdfpsMock.java | 2 +- .../server/trust/TrustManagerService.java | 23 ++++++++++++------- .../trust/test/lib/LockStateTrackingRule.kt | 1 + 7 files changed, 49 insertions(+), 24 deletions(-) diff --git a/core/java/android/app/trust/ITrustListener.aidl b/core/java/android/app/trust/ITrustListener.aidl index 6b9d2c73450ec..e4ac01195bcbe 100644 --- a/core/java/android/app/trust/ITrustListener.aidl +++ b/core/java/android/app/trust/ITrustListener.aidl @@ -24,8 +24,8 @@ import java.util.List; * {@hide} */ oneway interface ITrustListener { - void onTrustChanged(boolean enabled, int userId, int flags, + void onTrustChanged(boolean enabled, boolean newlyUnlocked, int userId, int flags, in List trustGrantedMessages); void onTrustManagedChanged(boolean managed, int userId); void onTrustError(in CharSequence message); -} \ No newline at end of file +} diff --git a/core/java/android/app/trust/TrustManager.java b/core/java/android/app/trust/TrustManager.java index 9e825b7207e05..62f755d0268c0 100644 --- a/core/java/android/app/trust/TrustManager.java +++ b/core/java/android/app/trust/TrustManager.java @@ -22,6 +22,7 @@ import android.annotation.SystemService; import android.compat.annotation.UnsupportedAppUsage; import android.content.Context; import android.hardware.biometrics.BiometricSourceType; +import android.os.Bundle; import android.os.Handler; import android.os.IBinder; import android.os.Looper; @@ -45,6 +46,7 @@ public class TrustManager { private static final String TAG = "TrustManager"; private static final String DATA_FLAGS = "initiatedByUser"; + private static final String DATA_NEWLY_UNLOCKED = "newlyUnlocked"; private static final String DATA_MESSAGE = "message"; private static final String DATA_GRANTED_MESSAGES = "grantedMessages"; @@ -171,13 +173,14 @@ public class TrustManager { try { ITrustListener.Stub iTrustListener = new ITrustListener.Stub() { @Override - public void onTrustChanged(boolean enabled, int userId, int flags, - List trustGrantedMessages) { + public void onTrustChanged(boolean enabled, boolean newlyUnlocked, int userId, + int flags, List trustGrantedMessages) { Message m = mHandler.obtainMessage(MSG_TRUST_CHANGED, (enabled ? 1 : 0), userId, trustListener); if (flags != 0) { m.getData().putInt(DATA_FLAGS, flags); } + m.getData().putInt(DATA_NEWLY_UNLOCKED, newlyUnlocked ? 1 : 0); m.getData().putCharSequenceArrayList( DATA_GRANTED_MESSAGES, (ArrayList) trustGrantedMessages); m.sendToTarget(); @@ -265,9 +268,14 @@ public class TrustManager { public void handleMessage(Message msg) { switch(msg.what) { case MSG_TRUST_CHANGED: - int flags = msg.peekData() != null ? msg.peekData().getInt(DATA_FLAGS) : 0; - ((TrustListener) msg.obj).onTrustChanged(msg.arg1 != 0, msg.arg2, flags, - msg.getData().getStringArrayList(DATA_GRANTED_MESSAGES)); + Bundle data = msg.peekData(); + int flags = data != null ? data.getInt(DATA_FLAGS) : 0; + boolean enabled = msg.arg1 != 0; + int newlyUnlockedInt = + data != null ? data.getInt(DATA_NEWLY_UNLOCKED) : 0; + boolean newlyUnlocked = newlyUnlockedInt != 0; + ((TrustListener) msg.obj).onTrustChanged(enabled, newlyUnlocked, msg.arg2, + flags, msg.getData().getStringArrayList(DATA_GRANTED_MESSAGES)); break; case MSG_TRUST_MANAGED_CHANGED: ((TrustListener)msg.obj).onTrustManagedChanged(msg.arg1 != 0, msg.arg2); @@ -284,6 +292,8 @@ public class TrustManager { /** * Reports that the trust state has changed. * @param enabled If true, the system believes the environment to be trusted. + * @param newlyUnlocked If true, the system believes the device is newly unlocked due + * to the trust changing. * @param userId The user, for which the trust changed. * @param flags Flags specified by the trust agent when granting trust. See * {@link android.service.trust.TrustAgentService#grantTrust(CharSequence, long, int) @@ -291,7 +301,7 @@ public class TrustManager { * @param trustGrantedMessages Messages to display to the user when trust has been granted * by one or more trust agents. */ - void onTrustChanged(boolean enabled, int userId, int flags, + void onTrustChanged(boolean enabled, boolean newlyUnlocked, int userId, int flags, List trustGrantedMessages); /** diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java index 71d5bf57baf60..f88f5a66b7f76 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardUpdateMonitor.java @@ -480,7 +480,7 @@ public class KeyguardUpdateMonitor implements TrustManager.TrustListener, Dumpab } @Override - public void onTrustChanged(boolean enabled, int userId, int flags, + public void onTrustChanged(boolean enabled, boolean newlyUnlocked, int userId, int flags, List trustGrantedMessages) { Assert.isMainThread(); boolean wasTrusted = mUserHasTrust.get(userId, false); diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java index 40542d25689d7..632078390a3e4 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardUpdateMonitorTest.java @@ -823,7 +823,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { mKeyguardUpdateMonitor.dispatchStartedWakingUp(PowerManager.WAKE_REASON_POWER_BUTTON); mTestableLooper.processAllMessages(); lockscreenBypassIsAllowed(); - mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, + mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, true /* newlyUnlocked */, KeyguardUpdateMonitor.getCurrentUser(), 0 /* flags */, new ArrayList<>()); keyguardIsVisible(); @@ -834,7 +834,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { public void testIgnoresAuth_whenTrustAgentOnKeyguard_withoutBypass() { mKeyguardUpdateMonitor.dispatchStartedWakingUp(PowerManager.WAKE_REASON_POWER_BUTTON); mTestableLooper.processAllMessages(); - mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, + mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, true /* newlyUnlocked */, KeyguardUpdateMonitor.getCurrentUser(), 0 /* flags */, new ArrayList<>()); keyguardIsVisible(); verify(mFaceManager, never()).authenticate(any(), any(), any(), any(), anyInt(), @@ -1049,8 +1049,8 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testGetUserCanSkipBouncer_whenTrust() { int user = KeyguardUpdateMonitor.getCurrentUser(); - mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, user, 0 /* flags */, - new ArrayList<>()); + mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, true /* newlyUnlocked */, + user, 0 /* flags */, new ArrayList<>()); assertThat(mKeyguardUpdateMonitor.getUserCanSkipBouncer(user)).isTrue(); } @@ -1314,7 +1314,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { when(mStrongAuthTracker.hasUserAuthenticatedSinceBoot()).thenReturn(true); // WHEN trust is enabled (ie: via smartlock) - mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, + mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, true /* newlyUnlocked */, KeyguardUpdateMonitor.getCurrentUser(), 0 /* flags */, new ArrayList<>()); // THEN we shouldn't listen for udfps @@ -1418,7 +1418,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { @Test public void testShowTrustGrantedMessage_onTrustGranted() { // WHEN trust is enabled (ie: via some trust agent) with a trustGranted string - mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, + mKeyguardUpdateMonitor.onTrustChanged(true /* enabled */, true /* newlyUnlocked */, KeyguardUpdateMonitor.getCurrentUser(), 0 /* flags */, Arrays.asList("Unlocked by wearable")); @@ -1870,6 +1870,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { // WHEN onTrustChanged with TRUST_DISMISS_KEYGUARD flag mKeyguardUpdateMonitor.onTrustChanged( true /* enabled */, + true /* newlyUnlocked */, getCurrentUser() /* userId */, TrustAgentService.FLAG_GRANT_TRUST_DISMISS_KEYGUARD /* flags */, null /* trustGrantedMessages */); @@ -1893,6 +1894,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { // WHEN onTrustChanged with TRUST_DISMISS_KEYGUARD flag mKeyguardUpdateMonitor.onTrustChanged( true /* enabled */, + true /* newlyUnlocked */, getCurrentUser() /* userId */, TrustAgentService.FLAG_GRANT_TRUST_DISMISS_KEYGUARD /* flags */, null /* trustGrantedMessages */); @@ -1917,6 +1919,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { // WHEN onTrustChanged for a different user mKeyguardUpdateMonitor.onTrustChanged( true /* enabled */, + true /* newlyUnlocked */, 546 /* userId, not the current userId */, 0 /* flags */, null /* trustGrantedMessages */); @@ -1941,6 +1944,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { // flags (temporary & rewable is active unlock) mKeyguardUpdateMonitor.onTrustChanged( true /* enabled */, + true /* newlyUnlocked */, getCurrentUser() /* userId */, TrustAgentService.FLAG_GRANT_TRUST_DISMISS_KEYGUARD | TrustAgentService.FLAG_GRANT_TRUST_TEMPORARY_AND_RENEWABLE /* flags */, @@ -1968,6 +1972,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { // WHEN onTrustChanged with INITIATED_BY_USER flag mKeyguardUpdateMonitor.onTrustChanged( true /* enabled */, + true /* newlyUnlocked */, getCurrentUser() /* userId, not the current userId */, TrustAgentService.FLAG_GRANT_TRUST_INITIATED_BY_USER /* flags */, null /* trustGrantedMessages */); @@ -1992,6 +1997,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { // WHEN onTrustChanged with INITIATED_BY_USER flag mKeyguardUpdateMonitor.onTrustChanged( true /* enabled */, + true /* newlyUnlocked */, getCurrentUser() /* userId, not the current userId */, TrustAgentService.FLAG_GRANT_TRUST_INITIATED_BY_USER | TrustAgentService.FLAG_GRANT_TRUST_TEMPORARY_AND_RENEWABLE /* flags */, @@ -2216,6 +2222,7 @@ public class KeyguardUpdateMonitorTest extends SysuiTestCase { private void currentUserDoesNotHaveTrust() { mKeyguardUpdateMonitor.onTrustChanged( + false, false, KeyguardUpdateMonitor.getCurrentUser(), -1, diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java index bea0f4ffd45de..846c2d9f3df71 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/hidl/Fingerprint21UdfpsMock.java @@ -417,7 +417,7 @@ public class Fingerprint21UdfpsMock extends Fingerprint21 implements TrustManage } @Override - public void onTrustChanged(boolean enabled, int userId, int flags, + public void onTrustChanged(boolean enabled, boolean newlyUnlocked, int userId, int flags, List trustGrantedMessages) { mUserHasTrust.put(userId, enabled); } diff --git a/services/core/java/com/android/server/trust/TrustManagerService.java b/services/core/java/com/android/server/trust/TrustManagerService.java index 2888b9a2d3ccf..b005507540e89 100644 --- a/services/core/java/com/android/server/trust/TrustManagerService.java +++ b/services/core/java/com/android/server/trust/TrustManagerService.java @@ -563,7 +563,12 @@ public class TrustManagerService extends SystemService { changed = mUserIsTrusted.get(userId) != trusted; mUserIsTrusted.put(userId, trusted); } - dispatchOnTrustChanged(trusted, userId, flags, getTrustGrantedMessages(userId)); + dispatchOnTrustChanged( + trusted, + false /* newlyUnlocked */, + userId, + flags, + getTrustGrantedMessages(userId)); if (changed) { refreshDeviceLockedForUser(userId); if (!trusted) { @@ -628,7 +633,9 @@ public class TrustManagerService extends SystemService { if (DEBUG) Slog.d(TAG, "pendingTrustState: " + pendingTrustState); boolean isNowTrusted = pendingTrustState == TrustState.TRUSTED; - dispatchOnTrustChanged(isNowTrusted, userId, flags, getTrustGrantedMessages(userId)); + boolean newlyUnlocked = !alreadyUnlocked && isNowTrusted; + dispatchOnTrustChanged( + isNowTrusted, newlyUnlocked, userId, flags, getTrustGrantedMessages(userId)); if (isNowTrusted != wasTrusted) { refreshDeviceLockedForUser(userId); if (!isNowTrusted) { @@ -643,8 +650,7 @@ public class TrustManagerService extends SystemService { } } - boolean wasLocked = !alreadyUnlocked; - boolean shouldSendCallback = wasLocked && pendingTrustState == TrustState.TRUSTED; + boolean shouldSendCallback = newlyUnlocked; if (shouldSendCallback) { if (resultCallback != null) { if (DEBUG) Slog.d(TAG, "calling back with UNLOCKED_BY_GRANT"); @@ -1387,16 +1393,17 @@ public class TrustManagerService extends SystemService { } } - private void dispatchOnTrustChanged(boolean enabled, int userId, int flags, - @NonNull List trustGrantedMessages) { + private void dispatchOnTrustChanged(boolean enabled, boolean newlyUnlocked, int userId, + int flags, @NonNull List trustGrantedMessages) { if (DEBUG) { - Log.i(TAG, "onTrustChanged(" + enabled + ", " + userId + ", 0x" + Log.i(TAG, "onTrustChanged(" + enabled + ", " + newlyUnlocked + ", " + userId + ", 0x" + Integer.toHexString(flags) + ")"); } if (!enabled) flags = 0; for (int i = 0; i < mTrustListeners.size(); i++) { try { - mTrustListeners.get(i).onTrustChanged(enabled, userId, flags, trustGrantedMessages); + mTrustListeners.get(i).onTrustChanged( + enabled, newlyUnlocked, userId, flags, trustGrantedMessages); } catch (DeadObjectException e) { Slog.d(TAG, "Removing dead TrustListener."); mTrustListeners.remove(i); diff --git a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt index 2031af2cf0c99..1930a1c8bbb25 100644 --- a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt +++ b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt @@ -63,6 +63,7 @@ class LockStateTrackingRule : TestRule { inner class Listener : TrustListener { override fun onTrustChanged( enabled: Boolean, + newlyUnlocked: Boolean, userId: Int, flags: Int, trustGrantedMessages: MutableList