From 76769dc120ba5a0fc9cfeeba16cd712b6ac25dfb Mon Sep 17 00:00:00 2001 From: Thomas Stuart Date: Thu, 23 Jun 2022 14:27:43 -0700 Subject: [PATCH 01/10] switch TelecomManager List getters to ParceledListSlice It was shown that given a large phoneAccountHandles that are over 1 mb, a TransactionTooLarge exception can be silently thrown causing an empty list to be returned. In order to prevent this behavior, all Lists that return a PhoneAccountHandle or PhoneAccount have been switched to ParceledListSlice. bug: 236263294 Test: manual #1 - bug exists without fix, manual #2 - bug is fixed with patch, 4 new CTS tests in android.telecom.cts.PhoneAccountRegistrarTest Change-Id: I025245b2a6f8cfaca86f268851a9d8f0817e07dd Merged-In: I025245b2a6f8cfaca86f268851a9d8f0817e07dd (cherry picked from commit 76c4d2d0b11397875b8c094b54016a178750c73d) Merged-In: I025245b2a6f8cfaca86f268851a9d8f0817e07dd --- telecomm/java/android/telecom/TelecomManager.java | 14 +++++++------- .../android/internal/telecom/ITelecomService.aidl | 15 ++++++++------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/telecomm/java/android/telecom/TelecomManager.java b/telecomm/java/android/telecom/TelecomManager.java index e0f5b20951901..983d82b4f8600 100644 --- a/telecomm/java/android/telecom/TelecomManager.java +++ b/telecomm/java/android/telecom/TelecomManager.java @@ -1269,7 +1269,7 @@ public class TelecomManager { if (service != null) { try { return service.getPhoneAccountsSupportingScheme(uriScheme, - mContext.getOpPackageName()); + mContext.getOpPackageName()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getPhoneAccountsSupportingScheme", e); } @@ -1312,7 +1312,7 @@ public class TelecomManager { if (service != null) { try { return service.getSelfManagedPhoneAccounts(mContext.getOpPackageName(), - mContext.getAttributionTag()); + mContext.getAttributionTag()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getSelfManagedPhoneAccounts()", e); } @@ -1340,7 +1340,7 @@ public class TelecomManager { if (service != null) { try { return service.getOwnSelfManagedPhoneAccounts(mContext.getOpPackageName(), - mContext.getAttributionTag()); + mContext.getAttributionTag()).getList(); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -1366,7 +1366,7 @@ public class TelecomManager { if (service != null) { try { return service.getCallCapablePhoneAccounts(includeDisabledAccounts, - mContext.getOpPackageName(), mContext.getAttributionTag()); + mContext.getOpPackageName(), mContext.getAttributionTag()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getCallCapablePhoneAccounts(" + includeDisabledAccounts + ")", e); @@ -1390,7 +1390,7 @@ public class TelecomManager { ITelecomService service = getTelecomService(); if (service != null) { try { - return service.getPhoneAccountsForPackage(mContext.getPackageName()); + return service.getPhoneAccountsForPackage(mContext.getPackageName()).getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getPhoneAccountsForPackage", e); } @@ -1450,7 +1450,7 @@ public class TelecomManager { ITelecomService service = getTelecomService(); if (service != null) { try { - return service.getAllPhoneAccounts(); + return service.getAllPhoneAccounts().getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getAllPhoneAccounts", e); } @@ -1469,7 +1469,7 @@ public class TelecomManager { ITelecomService service = getTelecomService(); if (service != null) { try { - return service.getAllPhoneAccountHandles(); + return service.getAllPhoneAccountHandles().getList(); } catch (RemoteException e) { Log.e(TAG, "Error calling ITelecomService#getAllPhoneAccountHandles", e); } diff --git a/telecomm/java/com/android/internal/telecom/ITelecomService.aidl b/telecomm/java/com/android/internal/telecom/ITelecomService.aidl index 37403a806daf9..74b5545e75de9 100644 --- a/telecomm/java/com/android/internal/telecom/ITelecomService.aidl +++ b/telecomm/java/com/android/internal/telecom/ITelecomService.aidl @@ -24,6 +24,7 @@ import android.net.Uri; import android.os.Bundle; import android.os.UserHandle; import android.telecom.PhoneAccount; +import android.content.pm.ParceledListSlice; /** * Interface used to interact with Telecom. Mostly this is used by TelephonyManager for passing @@ -57,31 +58,31 @@ interface ITelecomService { /** * @see TelecomServiceImpl#getCallCapablePhoneAccounts */ - List getCallCapablePhoneAccounts( + ParceledListSlice getCallCapablePhoneAccounts( boolean includeDisabledAccounts, String callingPackage, String callingFeatureId); /** * @see TelecomServiceImpl#getSelfManagedPhoneAccounts */ - List getSelfManagedPhoneAccounts(String callingPackage, + ParceledListSlice getSelfManagedPhoneAccounts(String callingPackage, String callingFeatureId); /** * @see TelecomServiceImpl#getOwnSelfManagedPhoneAccounts */ - List getOwnSelfManagedPhoneAccounts(String callingPackage, + ParceledListSlice getOwnSelfManagedPhoneAccounts(String callingPackage, String callingFeatureId); /** * @see TelecomManager#getPhoneAccountsSupportingScheme */ - List getPhoneAccountsSupportingScheme(in String uriScheme, + ParceledListSlice getPhoneAccountsSupportingScheme(in String uriScheme, String callingPackage); /** * @see TelecomManager#getPhoneAccountsForPackage */ - List getPhoneAccountsForPackage(in String packageName); + ParceledListSlice getPhoneAccountsForPackage(in String packageName); /** * @see TelecomManager#getPhoneAccount @@ -96,12 +97,12 @@ interface ITelecomService { /** * @see TelecomManager#getAllPhoneAccounts */ - List getAllPhoneAccounts(); + ParceledListSlice getAllPhoneAccounts(); /** * @see TelecomManager#getAllPhoneAccountHandles */ - List getAllPhoneAccountHandles(); + ParceledListSlice getAllPhoneAccountHandles(); /** * @see TelecomServiceImpl#getSimCallManager From 54e57bbbd679cd7dd25c394d98ae399c8312a867 Mon Sep 17 00:00:00 2001 From: Louis Chang Date: Tue, 2 Aug 2022 03:33:39 +0000 Subject: [PATCH 02/10] Do not send new Intent to non-exported activity when navigateUpTo The new Intent was delivered to a non-exported activity while #navigateUpTo was called from an Activity of a different uid. Bug: 238605611 Test: atest StartActivityTests Change-Id: I854dd825bfd9a2c08851980d480d1f3a177af6cf Merged-In: I854dd825bfd9a2c08851980d480d1f3a177af6cf (cherry picked from commit 4c355690494f17c8ebdecbc8b1a1eaef21ffc0f3) Merged-In: I854dd825bfd9a2c08851980d480d1f3a177af6cf --- .../core/java/com/android/server/wm/Task.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index bf5246f2339a2..888dc3aee86a7 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -5457,7 +5457,23 @@ class Task extends TaskFragment { parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TASK || parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TOP || (destIntentFlags & Intent.FLAG_ACTIVITY_CLEAR_TOP) != 0) { - parent.deliverNewIntentLocked(callingUid, destIntent, destGrants, srec.packageName); + boolean abort; + try { + abort = !mTaskSupervisor.checkStartAnyActivityPermission(destIntent, + parent.info, null /* resultWho */, -1 /* requestCode */, srec.getPid(), + callingUid, srec.info.packageName, null /* callingFeatureId */, + false /* ignoreTargetSecurity */, false /* launchingInTask */, srec.app, + null /* resultRecord */, null /* resultRootTask */); + } catch (SecurityException e) { + abort = true; + } + if (abort) { + android.util.EventLog.writeEvent(0x534e4554, "238605611", callingUid, ""); + foundParentInTask = false; + } else { + parent.deliverNewIntentLocked(callingUid, destIntent, destGrants, + srec.packageName); + } } else { try { ActivityInfo aInfo = AppGlobals.getPackageManager().getActivityInfo( From 7b9ea7a75ed2de51e883f450b701c8d0d82e6e9c Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Fri, 12 Aug 2022 11:40:41 -0700 Subject: [PATCH 03/10] Do not send AccessibilityEvent if notification is for different user. Bug: 237540408 Test: BuzzBeepBlinkTest#testA11yCrossUserEventNotSent Change-Id: I62a875e26e214847ec72ce3c41b4f2fa8e597e07 (cherry picked from commit a367c0a16a9070ed6bee3028ac5bbc967773ee8f) Merged-In: I62a875e26e214847ec72ce3c41b4f2fa8e597e07 --- .../notification/NotificationManagerService.java | 3 ++- .../server/notification/BuzzBeepBlinkTest.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 88f543260871a..3dfbff1639d1d 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -7808,7 +7808,8 @@ public class NotificationManagerService extends SystemService { && (record.getSuppressedVisualEffects() & SUPPRESSED_EFFECT_STATUS_BAR) != 0; if (!record.isUpdate && record.getImportance() > IMPORTANCE_MIN - && !suppressedByDnd) { + && !suppressedByDnd + && isNotificationForCurrentUser(record)) { sendAccessibilityEvent(record); sentAccessibilityEvent = true; } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java index 911fb6a87e964..08c2c6e6f26ef 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java @@ -1300,6 +1300,21 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { verify(mAccessibilityService, times(1)).sendAccessibilityEvent(any(), anyInt()); } + @Test + public void testA11yCrossUserEventNotSent() throws Exception { + final Notification n = new Builder(getContext(), "test") + .setSmallIcon(android.R.drawable.sym_def_app_icon).build(); + int userId = mUser.getIdentifier() + 1; + StatusBarNotification sbn = new StatusBarNotification(mPkg, mPkg, 0, mTag, mUid, + mPid, n, UserHandle.of(userId), null, System.currentTimeMillis()); + NotificationRecord r = new NotificationRecord(getContext(), sbn, + new NotificationChannel("test", "test", IMPORTANCE_HIGH)); + + mService.buzzBeepBlinkLocked(r); + + verify(mAccessibilityService, never()).sendAccessibilityEvent(any(), anyInt()); + } + @Test public void testLightsScreenOn() { mService.mScreenOn = true; From 1aae720772a86e2db682d2e9ed77937334e475f3 Mon Sep 17 00:00:00 2001 From: Hani Kazmi Date: Mon, 8 Aug 2022 08:44:00 +0000 Subject: [PATCH 04/10] Update BaseBundle to not use LazyValues when using ReadWriteHelper This is a partial backport of ag/19532036. It fixes a bug where a LazyValue might have been pointing to a parcel which was already recycled. Bug: 240138318 Test: atest android.os.cts.ParcelTest android.os.cts.BundleTest android.os.BundleTest android.os.ParcelTest android.os.BundleRecylingTest Test: Running PoC app from linked bug Change-Id: I3acf7127fdf373ee8b38a55702ae315be499601c Merged-In: I8a878a6d0055b04b2611909b4b17977ba5677a4f (cherry picked from commit d2f9cc6342141cdb39f08a229d548d7b29cadd86) Merged-In: I3acf7127fdf373ee8b38a55702ae315be499601c --- core/java/android/os/BaseBundle.java | 6 +- .../coretests/src/android/os/BundleTest.java | 63 +++++++++++++++++++ 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/core/java/android/os/BaseBundle.java b/core/java/android/os/BaseBundle.java index 0418a4bb9f806..b599028ccb9b5 100644 --- a/core/java/android/os/BaseBundle.java +++ b/core/java/android/os/BaseBundle.java @@ -438,8 +438,11 @@ public class BaseBundle { map.ensureCapacity(count); } try { + // recycleParcel being false implies that we do not own the parcel. In this case, do + // not use lazy values to be safe, as the parcel could be recycled outside of our + // control. recycleParcel &= parcelledData.readArrayMap(map, count, !parcelledByNative, - /* lazy */ true, mClassLoader); + /* lazy */ recycleParcel, mClassLoader); } catch (BadParcelableException e) { if (sShouldDefuse) { Log.w(TAG, "Failed to parse Bundle, but defusing quietly", e); @@ -1845,7 +1848,6 @@ public class BaseBundle { // bundle immediately; neither of which is obvious. synchronized (this) { initializeFromParcelLocked(parcel, /*recycleParcel=*/ false, isNativeBundle); - unparcel(/* itemwise */ true); } return; } diff --git a/core/tests/coretests/src/android/os/BundleTest.java b/core/tests/coretests/src/android/os/BundleTest.java index a3bda8b23f30e..0fa5ec33749f3 100644 --- a/core/tests/coretests/src/android/os/BundleTest.java +++ b/core/tests/coretests/src/android/os/BundleTest.java @@ -408,6 +408,69 @@ public class BundleTest { assertThat(bundle.getParcelable("key")).isEqualTo(parcelable); } + @Test + public void readFromParcel_withLazyValues_copiesUnderlyingParcel() { + Bundle bundle = new Bundle(); + Parcelable parcelable = new CustomParcelable(13, "Tiramisu"); + bundle.putParcelable("key", parcelable); + bundle.putString("string", "value"); + Parcel parcelledBundle = getParcelledBundle(bundle); + + Bundle testBundle = new Bundle(); + testBundle.setClassLoader(getClass().getClassLoader()); + testBundle.readFromParcel(parcelledBundle); + // Recycle the parcel as it should have been copied + parcelledBundle.recycle(); + assertThat(testBundle.getString("string")).isEqualTo("value"); + assertThat(testBundle.getParcelable("key")).isEqualTo(parcelable); + } + + @Test + public void readFromParcelWithRwHelper_whenThrowingAndNotDefusing_throws() { + Bundle bundle = new Bundle(); + Parcelable parcelable = new CustomParcelable(13, "Tiramisu"); + bundle.putParcelable("key", parcelable); + bundle.putString("string", "value"); + Parcel parcelledBundle = getParcelledBundle(bundle); + parcelledBundle.setReadWriteHelper(new Parcel.ReadWriteHelper()); + + Bundle testBundle = new Bundle(); + assertThrows(BadParcelableException.class, + () -> testBundle.readFromParcel(parcelledBundle)); + } + + @Test + public void readFromParcelWithRwHelper_whenThrowingAndDefusing_returnsNull() { + Bundle bundle = new Bundle(); + Parcelable parcelable = new CustomParcelable(13, "Tiramisu"); + bundle.putParcelable("key", parcelable); + bundle.putString("string", "value"); + Parcel parcelledBundle = getParcelledBundle(bundle); + parcelledBundle.setReadWriteHelper(new Parcel.ReadWriteHelper()); + + Bundle.setShouldDefuse(true); + Bundle testBundle = new Bundle(); + testBundle.readFromParcel(parcelledBundle); + // Recycle the parcel as it should not be referenced + parcelledBundle.recycle(); + assertThat(testBundle.getString("string")).isNull(); + assertThat(testBundle.getParcelable("key")).isNull(); + } + + @Test + public void readFromParcelWithRwHelper_withoutLazyObject_returnsValue() { + Bundle bundle = new Bundle(); + bundle.putString("string", "value"); + Parcel parcelledBundle = getParcelledBundle(bundle); + parcelledBundle.setReadWriteHelper(new Parcel.ReadWriteHelper()); + + Bundle testBundle = new Bundle(); + testBundle.readFromParcel(parcelledBundle); + // Recycle the parcel as it should not be referenced + parcelledBundle.recycle(); + assertThat(testBundle.getString("string")).isEqualTo("value"); + } + @Test public void partialDeserialization_whenNotDefusing_throws() throws Exception { Bundle.setShouldDefuse(false); From 83183e08c1ca3f21b2389df786cd5fb257081243 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Thu, 25 Aug 2022 16:23:12 -0400 Subject: [PATCH 05/10] Check rule package name in ZenModeHelper.addAutomaticRule instead of checking that of the configuration activity, which is potentially spoofable. The package name is verified to be the same app as the caller by NMS. This change removes isSystemRule (called only once) in favor of checking the provided package name directly. Bug: 242537431 Test: ZenModeHelperTest, manual by verifying via provided exploit apk Change-Id: Ic7f350618c26a613df455a4128c9195f4b424a4d Merged-In: Ic7f350618c26a613df455a4128c9195f4b424a4d (cherry picked from commit a826f9bd15d149305814a835fb5d1a3921a085f5) Merged-In: Ic7f350618c26a613df455a4128c9195f4b424a4d --- .../server/notification/ZenModeHelper.java | 7 +---- .../notification/ZenModeHelperTest.java | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/notification/ZenModeHelper.java b/services/core/java/com/android/server/notification/ZenModeHelper.java index 2b00ad7c5cd76..85c47a02bb905 100644 --- a/services/core/java/com/android/server/notification/ZenModeHelper.java +++ b/services/core/java/com/android/server/notification/ZenModeHelper.java @@ -314,7 +314,7 @@ public class ZenModeHelper { public String addAutomaticZenRule(String pkg, AutomaticZenRule automaticZenRule, String reason) { - if (!isSystemRule(automaticZenRule)) { + if (!ZenModeConfig.SYSTEM_AUTHORITY.equals(pkg)) { PackageItemInfo component = getServiceInfo(automaticZenRule.getOwner()); if (component == null) { component = getActivityInfo(automaticZenRule.getConfigurationActivity()); @@ -570,11 +570,6 @@ public class ZenModeHelper { } } - private boolean isSystemRule(AutomaticZenRule rule) { - return rule.getOwner() != null - && ZenModeConfig.SYSTEM_AUTHORITY.equals(rule.getOwner().getPackageName()); - } - private ServiceInfo getServiceInfo(ComponentName owner) { Intent queryIntent = new Intent(); queryIntent.setComponent(owner); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java index 4550b56f6fd0a..2ccdcaace8bf3 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ZenModeHelperTest.java @@ -1671,6 +1671,36 @@ public class ZenModeHelperTest extends UiServiceTestCase { } } + @Test + public void testAddAutomaticZenRule_claimedSystemOwner() { + // Make sure anything that claims to have a "system" owner but not actually part of the + // system package still gets limited on number of rules + for (int i = 0; i < RULE_LIMIT_PER_PACKAGE; i++) { + ScheduleInfo si = new ScheduleInfo(); + si.startHour = i; + AutomaticZenRule zenRule = new AutomaticZenRule("name" + i, + new ComponentName("android", "ScheduleConditionProvider" + i), + null, // configuration activity + ZenModeConfig.toScheduleConditionId(si), + new ZenPolicy.Builder().build(), + NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); + assertNotNull(id); + } + try { + AutomaticZenRule zenRule = new AutomaticZenRule("name", + new ComponentName("android", "ScheduleConditionProviderFinal"), + null, // configuration activity + ZenModeConfig.toScheduleConditionId(new ScheduleInfo()), + new ZenPolicy.Builder().build(), + NotificationManager.INTERRUPTION_FILTER_PRIORITY, true); + String id = mZenModeHelperSpy.addAutomaticZenRule("pkgname", zenRule, "test"); + fail("allowed too many rules to be created"); + } catch (IllegalArgumentException e) { + // yay + } + } + @Test public void testAddAutomaticZenRule_CA() { AutomaticZenRule zenRule = new AutomaticZenRule("name", From 0873a30210d6425ffb28021000ef3fe40a8da711 Mon Sep 17 00:00:00 2001 From: Ganesh Olekar Date: Tue, 2 Aug 2022 18:44:30 +0000 Subject: [PATCH 06/10] Fix auto-grant of AR runtime permission if device is upgrading from pre-Q Test: Manually install app apks targeting Q and verifying that AR permission is not auto-granted Test: atest ActivityRecognitionPermissionTest Bug: 210065877 Change-Id: I0d4a8d01db7b60f7c82642b875cdee3f600e7548 (cherry picked from commit 41dc761e081d8c6de48ddc3b960e12e4fd24c8b7) Merged-In: I0d4a8d01db7b60f7c82642b875cdee3f600e7548 --- .../PermissionManagerServiceImpl.java | 41 +------------------ 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java index d34682df34130..1e13333c1ce09 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java @@ -2664,7 +2664,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt final Permission bp = mRegistry.getPermission(permName); final boolean appSupportsRuntimePermissions = pkg.getTargetSdkVersion() >= Build.VERSION_CODES.M; - String legacyActivityRecognitionPermission = null; if (DEBUG_INSTALL && bp != null) { Log.i(TAG, "Package " + friendlyName @@ -2688,47 +2687,12 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt // Cache newImplicitPermissions before modifing permissionsState as for the // shared uids the original and new state are the same object if (!origState.hasPermissionState(permName) - && (pkg.getImplicitPermissions().contains(permName) - || (permName.equals(Manifest.permission.ACTIVITY_RECOGNITION)))) { - if (pkg.getImplicitPermissions().contains(permName)) { + && (pkg.getImplicitPermissions().contains(permName))) { // If permName is an implicit permission, try to auto-grant newImplicitPermissions.add(permName); - if (DEBUG_PERMISSIONS) { Slog.i(TAG, permName + " is newly added for " + friendlyName); } - } else { - // Special case for Activity Recognition permission. Even if AR - // permission is not an implicit permission we want to add it to the - // list (try to auto-grant it) if the app was installed on a device - // before AR permission was split, regardless of if the app now requests - // the new AR permission or has updated its target SDK and AR is no - // longer implicit to it. This is a compatibility workaround for apps - // when AR permission was split in Q. - // TODO(zhanghai): This calls into SystemConfig, which generally - // shouldn't cause deadlock, but maybe we should keep a cache of the - // split permission list and just eliminate the possibility. - final List permissionList = - getSplitPermissionInfos(); - int numSplitPerms = permissionList.size(); - for (int splitPermNum = 0; splitPermNum < numSplitPerms; - splitPermNum++) { - PermissionManager.SplitPermissionInfo sp = permissionList.get( - splitPermNum); - String splitPermName = sp.getSplitPermission(); - if (sp.getNewPermissions().contains(permName) - && origState.isPermissionGranted(splitPermName)) { - legacyActivityRecognitionPermission = splitPermName; - newImplicitPermissions.add(permName); - - if (DEBUG_PERMISSIONS) { - Slog.i(TAG, permName + " is newly added for " - + friendlyName); - } - break; - } - } - } } // TODO(b/140256621): The package instant app method has been removed @@ -2862,8 +2826,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt // Hard restricted permissions cannot be held. } else if (!permissionPolicyInitialized || (!hardRestricted || restrictionExempt)) { - if ((origPermState != null && origPermState.isGranted()) - || legacyActivityRecognitionPermission != null) { + if ((origPermState != null && origPermState.isGranted())) { if (!uidState.grantPermission(bp)) { wasChanged = true; } From e533796b4545161232e4248897ddedffd533f3bd Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Mon, 29 Aug 2022 17:40:14 -0400 Subject: [PATCH 07/10] Trim any long string inputs that come in to AutomaticZenRule This change both prevents any rules from being unable to be written to disk and also avoids risk of running out of memory while handling all the zen rules. Bug: 242703460 Bug: 242703505 Bug: 242703780 Bug: 242704043 Bug: 243794204 Test: cts AutomaticZenRuleTest; atest android.app.AutomaticZenRuleTest; manually confirmed each exploit example either saves the rule successfully with a truncated string (in the case of name & conditionId) or may fail to save the rule at all (if the owner/configactivity is invalid). Additionally ran the memory-exhausting PoC without device crashes. Change-Id: I110172a43f28528dd274b3b346eb29c3796ff2c6 Merged-In: I110172a43f28528dd274b3b346eb29c3796ff2c6 (cherry picked from commit de172ba0d434c940be9e2aad8685719731ab7da2) (cherry picked from commit d4b5212eb6d6f9ecb967d8403d1d8dd63cf69afb) Merged-In: I110172a43f28528dd274b3b346eb29c3796ff2c6 --- core/java/android/app/AutomaticZenRule.java | 62 +++++-- .../src/android/app/AutomaticZenRuleTest.java | 153 ++++++++++++++++++ 2 files changed, 204 insertions(+), 11 deletions(-) create mode 100644 core/tests/coretests/src/android/app/AutomaticZenRuleTest.java diff --git a/core/java/android/app/AutomaticZenRule.java b/core/java/android/app/AutomaticZenRule.java index c0aebeed596a6..7bfb1b5c1ba69 100644 --- a/core/java/android/app/AutomaticZenRule.java +++ b/core/java/android/app/AutomaticZenRule.java @@ -47,6 +47,13 @@ public final class AutomaticZenRule implements Parcelable { private boolean mModified = false; private String mPkg; + /** + * The maximum string length for any string contained in this automatic zen rule. This pertains + * both to fields in the rule itself (such as its name) and items with sub-fields. + * @hide + */ + public static final int MAX_STRING_LENGTH = 1000; + /** * Creates an automatic zen rule. * @@ -93,10 +100,10 @@ public final class AutomaticZenRule implements Parcelable { public AutomaticZenRule(@NonNull String name, @Nullable ComponentName owner, @Nullable ComponentName configurationActivity, @NonNull Uri conditionId, @Nullable ZenPolicy policy, int interruptionFilter, boolean enabled) { - this.name = name; - this.owner = owner; - this.configurationActivity = configurationActivity; - this.conditionId = conditionId; + this.name = getTrimmedString(name); + this.owner = getTrimmedComponentName(owner); + this.configurationActivity = getTrimmedComponentName(configurationActivity); + this.conditionId = getTrimmedUri(conditionId); this.interruptionFilter = interruptionFilter; this.enabled = enabled; this.mZenPolicy = policy; @@ -115,12 +122,14 @@ public final class AutomaticZenRule implements Parcelable { public AutomaticZenRule(Parcel source) { enabled = source.readInt() == ENABLED; if (source.readInt() == ENABLED) { - name = source.readString(); + name = getTrimmedString(source.readString()); } interruptionFilter = source.readInt(); - conditionId = source.readParcelable(null, android.net.Uri.class); - owner = source.readParcelable(null, android.content.ComponentName.class); - configurationActivity = source.readParcelable(null, android.content.ComponentName.class); + conditionId = getTrimmedUri(source.readParcelable(null, android.net.Uri.class)); + owner = getTrimmedComponentName( + source.readParcelable(null, android.content.ComponentName.class)); + configurationActivity = getTrimmedComponentName( + source.readParcelable(null, android.content.ComponentName.class)); creationTime = source.readLong(); mZenPolicy = source.readParcelable(null, android.service.notification.ZenPolicy.class); mModified = source.readInt() == ENABLED; @@ -196,7 +205,7 @@ public final class AutomaticZenRule implements Parcelable { * Sets the representation of the state that causes this rule to become active. */ public void setConditionId(Uri conditionId) { - this.conditionId = conditionId; + this.conditionId = getTrimmedUri(conditionId); } /** @@ -211,7 +220,7 @@ public final class AutomaticZenRule implements Parcelable { * Sets the name of this rule. */ public void setName(String name) { - this.name = name; + this.name = getTrimmedString(name); } /** @@ -243,7 +252,7 @@ public final class AutomaticZenRule implements Parcelable { * that are not backed by {@link android.service.notification.ConditionProviderService}. */ public void setConfigurationActivity(@Nullable ComponentName componentName) { - this.configurationActivity = componentName; + this.configurationActivity = getTrimmedComponentName(componentName); } /** @@ -333,4 +342,35 @@ public final class AutomaticZenRule implements Parcelable { return new AutomaticZenRule[size]; } }; + + /** + * If the package or class name of the provided ComponentName are longer than MAX_STRING_LENGTH, + * return a trimmed version that truncates each of the package and class name at the max length. + */ + private static ComponentName getTrimmedComponentName(ComponentName cn) { + if (cn == null) return null; + return new ComponentName(getTrimmedString(cn.getPackageName()), + getTrimmedString(cn.getClassName())); + } + + /** + * Returns a truncated copy of the string if the string is longer than MAX_STRING_LENGTH. + */ + private static String getTrimmedString(String input) { + if (input != null && input.length() > MAX_STRING_LENGTH) { + return input.substring(0, MAX_STRING_LENGTH); + } + return input; + } + + /** + * Returns a truncated copy of the Uri by trimming the string representation to the maximum + * string length. + */ + private static Uri getTrimmedUri(Uri input) { + if (input != null && input.toString().length() > MAX_STRING_LENGTH) { + return Uri.parse(getTrimmedString(input.toString())); + } + return input; + } } diff --git a/core/tests/coretests/src/android/app/AutomaticZenRuleTest.java b/core/tests/coretests/src/android/app/AutomaticZenRuleTest.java new file mode 100644 index 0000000000000..282fdad294ebc --- /dev/null +++ b/core/tests/coretests/src/android/app/AutomaticZenRuleTest.java @@ -0,0 +1,153 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.app; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.fail; + +import android.content.ComponentName; +import android.net.Uri; +import android.os.Parcel; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import com.google.common.base.Strings; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.lang.reflect.Field; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class AutomaticZenRuleTest { + private static final String CLASS = "android.app.AutomaticZenRule"; + + @Test + public void testLongFields_inConstructor() { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + + // test both variants where there's an owner, and where there's a configuration activity + AutomaticZenRule rule1 = new AutomaticZenRule( + longString, // name + new ComponentName("pkg", longString), // owner + null, // configuration activity + longUri, // conditionId + null, // zen policy + 0, // interruption filter + true); // enabled + + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule1.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule1.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule1.getOwner().getClassName().length()); + + AutomaticZenRule rule2 = new AutomaticZenRule( + longString, // name + null, // owner + new ComponentName(longString, "SomeClassName"), // configuration activity + longUri, // conditionId + null, // zen policy + 0, // interruption filter + false); // enabled + + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule2.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule2.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule2.getConfigurationActivity().getPackageName().length()); + } + + @Test + public void testLongFields_inSetters() { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + + AutomaticZenRule rule = new AutomaticZenRule( + "sensible name", + new ComponentName("pkg", "ShortClass"), + null, + Uri.parse("uri://short"), + null, 0, true); + + rule.setName(longString); + rule.setConditionId(longUri); + rule.setConfigurationActivity(new ComponentName(longString, longString)); + + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, rule.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule.getConfigurationActivity().getPackageName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + rule.getConfigurationActivity().getClassName().length()); + } + + @Test + public void testLongInputsFromParcel() { + // Create a rule with long fields, set directly via reflection so that we can confirm that + // a rule with too-long fields that comes in via a parcel has its fields truncated directly. + AutomaticZenRule rule = new AutomaticZenRule( + "placeholder", + new ComponentName("place", "holder"), + null, + Uri.parse("uri://placeholder"), + null, 0, true); + + try { + String longString = Strings.repeat("A", 65536); + Uri longUri = Uri.parse("uri://" + Strings.repeat("A", 65530)); + Field name = Class.forName(CLASS).getDeclaredField("name"); + name.setAccessible(true); + name.set(rule, longString); + Field conditionId = Class.forName(CLASS).getDeclaredField("conditionId"); + conditionId.setAccessible(true); + conditionId.set(rule, longUri); + Field owner = Class.forName(CLASS).getDeclaredField("owner"); + owner.setAccessible(true); + owner.set(rule, new ComponentName(longString, longString)); + Field configActivity = Class.forName(CLASS).getDeclaredField("configurationActivity"); + configActivity.setAccessible(true); + configActivity.set(rule, new ComponentName(longString, longString)); + } catch (NoSuchFieldException e) { + fail(e.toString()); + } catch (ClassNotFoundException e) { + fail(e.toString()); + } catch (IllegalAccessException e) { + fail(e.toString()); + } + + Parcel parcel = Parcel.obtain(); + rule.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + + AutomaticZenRule fromParcel = new AutomaticZenRule(parcel); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, fromParcel.getName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getConditionId().toString().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getConfigurationActivity().getPackageName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getConfigurationActivity().getClassName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getOwner().getPackageName().length()); + assertEquals(AutomaticZenRule.MAX_STRING_LENGTH, + fromParcel.getOwner().getClassName().length()); + } +} From 80c15ed6591620034386a190d833cee829e4aace Mon Sep 17 00:00:00 2001 From: Abhijeet Kaur Date: Tue, 16 Aug 2022 15:22:22 +0100 Subject: [PATCH 08/10] Remove legacy WRITE_EXTERNAL_STORAGE permission check for Installers Bug: 239495492 Bug: 243924784 Test: atest ScopedStorageHostTest#testCheckInstallerAppAccessToObbDirs Change-Id: Id96c854c0b31e94d367dccd125ad5c14523cdc79 Merged-In: Id96c854c0b31e94d367dccd125ad5c14523cdc79 (cherry picked from commit 33e2c5008b925c6e4349788be1aba19a6d19aa99) (cherry picked from commit 3a3cc4d91994ffa1da76ca8ff36c0eff929d545a) Merged-In: Id96c854c0b31e94d367dccd125ad5c14523cdc79 --- .../java/com/android/server/StorageManagerService.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 5eec6e58e9250..e54665eb2593b 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -19,12 +19,10 @@ package com.android.server; import static android.Manifest.permission.ACCESS_MTP; import static android.Manifest.permission.INSTALL_PACKAGES; import static android.Manifest.permission.MANAGE_EXTERNAL_STORAGE; -import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; import static android.app.AppOpsManager.MODE_ALLOWED; import static android.app.AppOpsManager.OP_LEGACY_STORAGE; import static android.app.AppOpsManager.OP_MANAGE_EXTERNAL_STORAGE; import static android.app.AppOpsManager.OP_REQUEST_INSTALL_PACKAGES; -import static android.app.AppOpsManager.OP_WRITE_EXTERNAL_STORAGE; import static android.app.PendingIntent.FLAG_CANCEL_CURRENT; import static android.app.PendingIntent.FLAG_IMMUTABLE; import static android.app.PendingIntent.FLAG_ONE_SHOT; @@ -4512,11 +4510,7 @@ class StorageManagerService extends IStorageManager.Stub } } - // Determine if caller is holding runtime permission - final boolean hasWrite = StorageManager.checkPermissionAndCheckOp(mContext, false, 0, - uid, packageName, WRITE_EXTERNAL_STORAGE, OP_WRITE_EXTERNAL_STORAGE); - - // We're only willing to give out installer access if they also hold + // We're only willing to give out installer access if they hold // runtime permission; this is a firm CDD requirement final boolean hasInstall = mIPackageManager.checkUidPermission(INSTALL_PACKAGES, uid) == PERMISSION_GRANTED; @@ -4532,7 +4526,7 @@ class StorageManagerService extends IStorageManager.Stub break; } } - if ((hasInstall || hasInstallOp) && hasWrite) { + if (hasInstall || hasInstallOp) { return StorageManager.MOUNT_MODE_EXTERNAL_INSTALLER; } return StorageManager.MOUNT_MODE_EXTERNAL_DEFAULT; From 23e3c741225cc97dd3da6569af9abf2d6eed6877 Mon Sep 17 00:00:00 2001 From: Yuri Lin Date: Tue, 6 Sep 2022 16:14:16 -0400 Subject: [PATCH 09/10] Fix system zen rules by using owner package name if caller is system Previously were unable to add new zen rules because rules added via the settings pages were getting registered under package "com.android.settings", which then were not considered "system rules". These rules should have package android, so when we can trust the caller (via checking that the caller is system) we should be taking the package name from the owner of the rule. Bug: 245236706 Bug: 242537431 Test: NMSTest; manual Change-Id: Id69b671592396ac3304862dadbe73de328a8e27a Merged-In: Id69b671592396ac3304862dadbe73de328a8e27a (cherry picked from commit b8c4819ec24a0ab8af72c10055ef02d5fa10a194) Merged-In: Id69b671592396ac3304862dadbe73de328a8e27a --- .../NotificationManagerService.java | 11 +++++- .../NotificationManagerServiceTest.java | 37 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 3dfbff1639d1d..a8647c624bdcf 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -4927,7 +4927,16 @@ public class NotificationManagerService extends SystemService { } enforcePolicyAccess(Binder.getCallingUid(), "addAutomaticZenRule"); - return mZenModeHelper.addAutomaticZenRule(pkg, automaticZenRule, + // If the caller is system, take the package name from the rule's owner rather than + // from the caller's package. + String rulePkg = pkg; + if (isCallingUidSystem()) { + if (automaticZenRule.getOwner() != null) { + rulePkg = automaticZenRule.getOwner().getPackageName(); + } + } + + return mZenModeHelper.addAutomaticZenRule(rulePkg, automaticZenRule, "addAutomaticZenRule"); } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index b1b323b734bd2..98adefa1178aa 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -7452,6 +7452,43 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { mBinderService.addAutomaticZenRule(rule, mContext.getPackageName()); } + @Test + public void testAddAutomaticZenRule_systemCallTakesPackageFromOwner() throws Exception { + mService.isSystemUid = true; + ZenModeHelper mockZenModeHelper = mock(ZenModeHelper.class); + when(mConditionProviders.isPackageOrComponentAllowed(anyString(), anyInt())) + .thenReturn(true); + mService.setZenHelper(mockZenModeHelper); + ComponentName owner = new ComponentName("android", "ProviderName"); + ZenPolicy zenPolicy = new ZenPolicy.Builder().allowAlarms(true).build(); + boolean isEnabled = true; + AutomaticZenRule rule = new AutomaticZenRule("test", owner, owner, mock(Uri.class), + zenPolicy, NotificationManager.INTERRUPTION_FILTER_PRIORITY, isEnabled); + mBinderService.addAutomaticZenRule(rule, "com.android.settings"); + + // verify that zen mode helper gets passed in a package name of "android" + verify(mockZenModeHelper).addAutomaticZenRule(eq("android"), eq(rule), anyString()); + } + + @Test + public void testAddAutomaticZenRule_nonSystemCallTakesPackageFromArg() throws Exception { + mService.isSystemUid = false; + ZenModeHelper mockZenModeHelper = mock(ZenModeHelper.class); + when(mConditionProviders.isPackageOrComponentAllowed(anyString(), anyInt())) + .thenReturn(true); + mService.setZenHelper(mockZenModeHelper); + ComponentName owner = new ComponentName("android", "ProviderName"); + ZenPolicy zenPolicy = new ZenPolicy.Builder().allowAlarms(true).build(); + boolean isEnabled = true; + AutomaticZenRule rule = new AutomaticZenRule("test", owner, owner, mock(Uri.class), + zenPolicy, NotificationManager.INTERRUPTION_FILTER_PRIORITY, isEnabled); + mBinderService.addAutomaticZenRule(rule, "another.package"); + + // verify that zen mode helper gets passed in the package name from the arg, not the owner + verify(mockZenModeHelper).addAutomaticZenRule( + eq("another.package"), eq(rule), anyString()); + } + @Test public void testAreNotificationsEnabledForPackage() throws Exception { mBinderService.areNotificationsEnabledForPackage(mContext.getPackageName(), From ecbed81c3a331f2f0458923cc7e744c85ece96da Mon Sep 17 00:00:00 2001 From: Matt Pietal Date: Thu, 4 Aug 2022 13:00:39 +0000 Subject: [PATCH 10/10] Do not dismiss keyguard after SIM PUK unlock After PUK unlock, multiple calls to KeyguardSecurityContainerController#dismiss() were being called from the KeyguardSimPukViewController, which begins the transition to the next security screen, if any. At the same time, other parts of the system, also listening to SIM events, recognize the PUK unlock and call KeyguardSecurityContainer#showSecurityScreen, which updates which security method comes next. After boot, this should be one of PIN, Password, Pattern, assuming they have a security method. If one of the first dismiss() calls comes AFTER the security method changes, this is incorrectly recognized by the code as a successful PIN/pattern/password unlock. This causes the keyguard to be marked as done, causing screen flickers and incorrect system state. The solution: every call to dismiss() should include a new parameter for the security method used. If there is a difference between this parameter and the current value in KeyguardSecurityContainerCallback, ignore the request, as the system state has changed. Fixes: 238804980 Bug: 218500036 Test: atest KeyguardSecurityContainerTest AdminSecondaryLockScreenControllerTest KeyguardHostViewControllerTest KeyguardSecurityContainerControllerTest Change-Id: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243 Merged-In: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243 (cherry picked from commit 37aeb26b0ae28a48c1ed40f008d5808d8a84be23) (cherry picked from commit 3d89cc5df6729cab8d98c967d73e03e23048d52b) Merged-In: I7c8714a177bc85fbce92f6e8fe911f74ca2ac243 --- .../AdminSecondaryLockScreenController.java | 3 +- .../KeyguardAbsKeyInputViewController.java | 2 +- .../keyguard/KeyguardHostViewController.java | 15 +++---- .../keyguard/KeyguardInputViewController.java | 5 ++- .../KeyguardPatternViewController.java | 2 +- .../keyguard/KeyguardSecurityCallback.java | 9 +++- .../keyguard/KeyguardSecurityContainer.java | 7 +++- .../KeyguardSecurityContainerController.java | 31 ++++++++++---- .../KeyguardSimPinViewController.java | 3 +- .../KeyguardSimPukViewController.java | 6 ++- ...dminSecondaryLockScreenControllerTest.java | 3 +- ...yguardSecurityContainerControllerTest.java | 42 +++++++++++++++++++ 12 files changed, 102 insertions(+), 26 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java b/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java index 23195af8bdea2..00f1c0108d0b5 100644 --- a/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java +++ b/packages/SystemUI/src/com/android/keyguard/AdminSecondaryLockScreenController.java @@ -33,6 +33,7 @@ import android.view.SurfaceView; import android.view.ViewGroup; import com.android.internal.annotations.VisibleForTesting; +import com.android.keyguard.KeyguardSecurityModel.SecurityMode; import com.android.keyguard.dagger.KeyguardBouncerScope; import com.android.systemui.dagger.qualifiers.Main; @@ -208,7 +209,7 @@ public class AdminSecondaryLockScreenController { hide(); if (mKeyguardCallback != null) { mKeyguardCallback.dismiss(/* securityVerified= */ true, userId, - /* bypassSecondaryLockScreen= */true); + /* bypassSecondaryLockScreen= */true, SecurityMode.Invalid); } } } diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java index eb418ff3b142a..b8fcb103402d6 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardAbsKeyInputViewController.java @@ -179,7 +179,7 @@ public abstract class KeyguardAbsKeyInputViewController Log.i(TAG, "TrustAgent dismissed Keyguard."); } mSecurityCallback.dismiss(false /* authenticated */, userId, - /* bypassSecondaryLockScreen */ false); + /* bypassSecondaryLockScreen */ false, SecurityMode.Invalid); } else { mViewMediatorCallback.playTrustedSound(); } @@ -102,9 +102,9 @@ public class KeyguardHostViewController extends ViewController @Override public boolean dismiss(boolean authenticated, int targetUserId, - boolean bypassSecondaryLockScreen) { + boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) { return mKeyguardSecurityContainerController.showNextSecurityScreenOrFinish( - authenticated, targetUserId, bypassSecondaryLockScreen); + authenticated, targetUserId, bypassSecondaryLockScreen, expectedSecurityMode); } @Override @@ -212,7 +212,8 @@ public class KeyguardHostViewController extends ViewController * @return True if the keyguard is done. */ public boolean dismiss(int targetUserId) { - return mSecurityCallback.dismiss(false, targetUserId, false); + return mSecurityCallback.dismiss(false, targetUserId, false, + getCurrentSecurityMode()); } /** @@ -355,10 +356,10 @@ public class KeyguardHostViewController extends ViewController } public boolean handleBackKey() { - if (mKeyguardSecurityContainerController.getCurrentSecurityMode() - != SecurityMode.None) { + SecurityMode securityMode = mKeyguardSecurityContainerController.getCurrentSecurityMode(); + if (securityMode != SecurityMode.None) { mKeyguardSecurityContainerController.dismiss( - false, KeyguardUpdateMonitor.getCurrentUser()); + false, KeyguardUpdateMonitor.getCurrentUser(), securityMode); return true; } return false; diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java index 98ac640bf7037..87300c3f05049 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardInputViewController.java @@ -59,10 +59,11 @@ public abstract class KeyguardInputViewController return false; } @Override - public void dismiss(boolean securityVerified, int targetUserId) { } + public void dismiss(boolean securityVerified, int targetUserId, + SecurityMode expectedSecurityMode) { } @Override public void dismiss(boolean authenticated, int targetId, - boolean bypassSecondaryLockScreen) { } + boolean bypassSecondaryLockScreen, SecurityMode expectedSecurityMode) { } @Override public void onUserInput() { } @Override diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java index 39c3949811937..1a59b820c1bdb 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardPatternViewController.java @@ -171,7 +171,7 @@ public class KeyguardPatternViewController if (dismissKeyguard) { mLockPatternView.setDisplayMode(LockPatternView.DisplayMode.Correct); mLatencyTracker.onActionStart(LatencyTracker.ACTION_LOCKSCREEN_UNLOCK); - getKeyguardSecurityCallback().dismiss(true, userId); + getKeyguardSecurityCallback().dismiss(true, userId, SecurityMode.Pattern); } } else { mLockPatternView.setDisplayMode(LockPatternView.DisplayMode.Wrong); diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java index e384727452349..bc72f7979a746 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityCallback.java @@ -15,14 +15,17 @@ */ package com.android.keyguard; +import com.android.keyguard.KeyguardSecurityModel.SecurityMode; + public interface KeyguardSecurityCallback { /** * Dismiss the given security screen. * @param securityVerified true if the user correctly entered credentials for the given screen. * @param targetUserId a user that needs to be the foreground user at the dismissal completion. + * @param expectedSecurityMode The security mode that is invoking this dismiss. */ - void dismiss(boolean securityVerified, int targetUserId); + void dismiss(boolean securityVerified, int targetUserId, SecurityMode expectedSecurityMode); /** * Dismiss the given security screen. @@ -30,8 +33,10 @@ public interface KeyguardSecurityCallback { * @param targetUserId a user that needs to be the foreground user at the dismissal completion. * @param bypassSecondaryLockScreen true if the user can bypass the secondary lock screen, * if any, during this dismissal. + * @param expectedSecurityMode The security mode that is invoking this dismiss. */ - void dismiss(boolean securityVerified, int targetUserId, boolean bypassSecondaryLockScreen); + void dismiss(boolean securityVerified, int targetUserId, boolean bypassSecondaryLockScreen, + SecurityMode expectedSecurityMode); /** * Manually report user activity to keep the device awake. diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java index cce516d981a51..12bb47b81d7fc 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainer.java @@ -233,7 +233,12 @@ public class KeyguardSecurityContainer extends FrameLayout { // Used to notify the container when something interesting happens. public interface SecurityCallback { - boolean dismiss(boolean authenticated, int targetUserId, boolean bypassSecondaryLockScreen); + /** + * Potentially dismiss the current security screen, after validating that all device + * security has been unlocked. Otherwise show the next screen. + */ + boolean dismiss(boolean authenticated, int targetUserId, boolean bypassSecondaryLockScreen, + SecurityMode expectedSecurityMode); void userActivity(); diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java index 19a2d9ed5b7b3..2b9553d3eda26 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java @@ -153,14 +153,17 @@ public class KeyguardSecurityContainerController extends ViewController