From f3385d197075d10659b388982b3d59c73adaa967 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 3 Aug 2023 06:09:51 +0000 Subject: Strips spans from AssistStructure text When a new view becomes visible on the screen, the view notifies AutofillManager about its visibility. AutofillManager then requests the activity to compile an AssistStructure object which will contain the view hierarchy of the activity as well as texts contained inside all the views. If there are Span text fields that contain custom implementation of ClickableSpan, these spans are also copied over during the construction of the AssistStructure. By keeping the references to the ClickableSpan, it causes memory leak when the AssistStructure object outlives the activity. Test: atest CtsAutoFillServiceTestCases, atest CtsAssistTestCases, atest android.widget.cts.TextViewTest Bug: 146100180 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e42d05e3d01f4c3b8aa7bce29086cd968970b4ec) Merged-In: I1fd97d9d6fdc569d14529347fcb85da409c3c1ff Change-Id: I1fd97d9d6fdc569d14529347fcb85da409c3c1ff --- core/java/android/app/assist/AssistStructure.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/core/java/android/app/assist/AssistStructure.java b/core/java/android/app/assist/AssistStructure.java index 6e49e956fe7e..27b8239edd4d 100644 --- a/core/java/android/app/assist/AssistStructure.java +++ b/core/java/android/app/assist/AssistStructure.java @@ -22,6 +22,7 @@ import android.os.PooledStringWriter; import android.os.RemoteException; import android.os.SystemClock; import android.service.autofill.FillRequest; +import android.text.Spanned; import android.text.TextUtils; import android.util.Log; import android.util.Pair; @@ -1556,6 +1557,10 @@ public class AssistStructure implements Parcelable { /** * Returns any text associated with the node that is displayed to the user, or null * if there is none. + * + *

The text will be stripped of any spans that could potentially contain reference to + * the activity context, to avoid memory leak. If the text contained a span, a plain + * string version of the text will be returned. */ @Nullable public CharSequence getText() { @@ -1995,14 +2000,16 @@ public class AssistStructure implements Parcelable { @Override public void setText(CharSequence text) { ViewNodeText t = getNodeText(); - t.mText = TextUtils.trimNoCopySpans(text); + // Strip spans from the text to avoid memory leak + t.mText = TextUtils.trimToParcelableSize(stripAllSpansFromText(text)); t.mTextSelectionStart = t.mTextSelectionEnd = -1; } @Override public void setText(CharSequence text, int selectionStart, int selectionEnd) { ViewNodeText t = getNodeText(); - t.mText = TextUtils.trimNoCopySpans(text); + // Strip spans from the text to avoid memory leak + t.mText = stripAllSpansFromText(text); t.mTextSelectionStart = selectionStart; t.mTextSelectionEnd = selectionEnd; } @@ -2221,6 +2228,13 @@ public class AssistStructure implements Parcelable { public void setHtmlInfo(@NonNull HtmlInfo htmlInfo) { mNode.mHtmlInfo = htmlInfo; } + + private CharSequence stripAllSpansFromText(CharSequence text) { + if (text instanceof Spanned) { + return text.toString(); + } + return text; + } } private static final class HtmlInfoNode extends HtmlInfo implements Parcelable { -- cgit v1.2.3 From d92a19573e04be8bb7188dc313ab880ae075d57d Mon Sep 17 00:00:00 2001 From: Lifu Tang Date: Wed, 5 Jul 2023 13:03:00 -0700 Subject: Fix bypass BAL via `requestGeofence` Bug: 273729172 Test: manually (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:7f9be7c3c859dc82d37452570d9878b58f6437a9) Merged-In: Ia8094244f908b20d42711b6ea8f58f9b3345b563 Change-Id: Ia8094244f908b20d42711b6ea8f58f9b3345b563 --- services/core/java/com/android/server/PendingIntentUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/services/core/java/com/android/server/PendingIntentUtils.java b/services/core/java/com/android/server/PendingIntentUtils.java index 1600101b20f4..a72a4d254a2a 100644 --- a/services/core/java/com/android/server/PendingIntentUtils.java +++ b/services/core/java/com/android/server/PendingIntentUtils.java @@ -34,6 +34,7 @@ public class PendingIntentUtils { public static Bundle createDontSendToRestrictedAppsBundle(@Nullable Bundle bundle) { final BroadcastOptions options = BroadcastOptions.makeBasic(); options.setDontSendToRestrictedApps(true); + options.setPendingIntentBackgroundActivityLaunchAllowed(false); if (bundle == null) { return options.toBundle(); } -- cgit v1.2.3 From 2dc9a5ca503f4dd7fd15566c2e4ea42dd7852bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Hern=C3=A1ndez?= Date: Fri, 11 Aug 2023 18:27:57 +0200 Subject: Visit Uris related to Notification style extras Even if the corresponding styles themselves were not applied to the Notification.Builder. (Also unify all Bundle.getParcelable/getParcelableArray() calls in visitUris to use the type-safe version). Test: atest NotificationManagerServiceTest Bug: 287640400 Bug: 281549104 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3c2ebb81ff064cdf1fbe58c15920f44d343e9391) Merged-In: I25acab19be7dd486aabede8c91dbad5a1a217abf Change-Id: I25acab19be7dd486aabede8c91dbad5a1a217abf --- core/java/android/app/Notification.java | 32 ++++++++-------- .../NotificationManagerServiceTest.java | 43 ++++++++++++++++++++++ 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 330ab87cdd53..c60c66614a8b 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -2850,18 +2850,14 @@ public class Notification implements Parcelable visitor.accept(Uri.parse(extras.getString(EXTRA_BACKGROUND_IMAGE_URI))); } - ArrayList people = extras.getParcelableArrayList(EXTRA_PEOPLE_LIST); + ArrayList people = extras.getParcelableArrayList(EXTRA_PEOPLE_LIST, + Person.class); if (people != null && !people.isEmpty()) { for (Person p : people) { visitor.accept(p.getIconUri()); } } - final Person person = extras.getParcelable(EXTRA_MESSAGING_PERSON, Person.class); - if (person != null) { - visitor.accept(person.getIconUri()); - } - final RemoteInputHistoryItem[] history = extras.getParcelableArray( Notification.EXTRA_REMOTE_INPUT_HISTORY_ITEMS, RemoteInputHistoryItem.class); @@ -2873,10 +2869,16 @@ public class Notification implements Parcelable } } } - } - if (isStyle(MessagingStyle.class) && extras != null) { - final Parcelable[] messages = extras.getParcelableArray(EXTRA_MESSAGES); + // Extras for MessagingStyle. We visit them even if not isStyle(MessagingStyle), since + // Notification Listeners might use directly (without the isStyle check). + final Person person = extras.getParcelable(EXTRA_MESSAGING_PERSON, Person.class); + if (person != null) { + visitor.accept(person.getIconUri()); + } + + final Parcelable[] messages = extras.getParcelableArray(EXTRA_MESSAGES, + Parcelable.class); if (!ArrayUtils.isEmpty(messages)) { for (MessagingStyle.Message message : MessagingStyle.Message .getMessagesFromBundleArray(messages)) { @@ -2889,7 +2891,8 @@ public class Notification implements Parcelable } } - final Parcelable[] historic = extras.getParcelableArray(EXTRA_HISTORIC_MESSAGES); + final Parcelable[] historic = extras.getParcelableArray(EXTRA_HISTORIC_MESSAGES, + Parcelable.class); if (!ArrayUtils.isEmpty(historic)) { for (MessagingStyle.Message message : MessagingStyle.Message .getMessagesFromBundleArray(historic)) { @@ -2902,15 +2905,14 @@ public class Notification implements Parcelable } } - visitIconUri(visitor, extras.getParcelable(EXTRA_CONVERSATION_ICON)); - } + visitIconUri(visitor, extras.getParcelable(EXTRA_CONVERSATION_ICON, Icon.class)); - if (isStyle(CallStyle.class) & extras != null) { - Person callPerson = extras.getParcelable(EXTRA_CALL_PERSON); + // Extras for CallStyle (same reason for visiting without checking isStyle). + Person callPerson = extras.getParcelable(EXTRA_CALL_PERSON, Person.class); if (callPerson != null) { visitor.accept(callPerson.getIconUri()); } - visitIconUri(visitor, extras.getParcelable(EXTRA_VERIFICATION_ICON)); + visitIconUri(visitor, extras.getParcelable(EXTRA_VERIFICATION_ICON, Icon.class)); } if (mBubbleMetadata != null) { 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 39b215d9a2ec..cb217fef33a7 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -5564,6 +5564,49 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { verify(visitor, times(1)).accept(eq(verificationIcon.getUri())); } + @Test + public void testVisitUris_styleExtrasWithoutStyle() { + Notification notification = new Notification.Builder(mContext, "a") + .setSmallIcon(android.R.drawable.sym_def_app_icon) + .build(); + + Notification.MessagingStyle messagingStyle = new Notification.MessagingStyle( + personWithIcon("content://user")) + .addHistoricMessage(new Notification.MessagingStyle.Message("Heyhey!", + System.currentTimeMillis(), + personWithIcon("content://historicalMessenger"))) + .addMessage(new Notification.MessagingStyle.Message("Are you there", + System.currentTimeMillis(), + personWithIcon("content://messenger"))) + .setShortcutIcon( + Icon.createWithContentUri("content://conversationShortcut")); + messagingStyle.addExtras(notification.extras); // Instead of Builder.setStyle(style). + + Notification.CallStyle callStyle = Notification.CallStyle.forOngoingCall( + personWithIcon("content://caller"), + PendingIntent.getActivity(mContext, 0, new Intent(), + PendingIntent.FLAG_IMMUTABLE)) + .setVerificationIcon(Icon.createWithContentUri("content://callVerification")); + callStyle.addExtras(notification.extras); // Same. + + Consumer visitor = (Consumer) spy(Consumer.class); + notification.visitUris(visitor); + + verify(visitor).accept(eq(Uri.parse("content://user"))); + verify(visitor).accept(eq(Uri.parse("content://historicalMessenger"))); + verify(visitor).accept(eq(Uri.parse("content://messenger"))); + verify(visitor).accept(eq(Uri.parse("content://conversationShortcut"))); + verify(visitor).accept(eq(Uri.parse("content://caller"))); + verify(visitor).accept(eq(Uri.parse("content://callVerification"))); + } + + private static Person personWithIcon(String iconUri) { + return new Person.Builder() + .setName("Mr " + iconUri) + .setIcon(Icon.createWithContentUri(iconUri)) + .build(); + } + @Test public void testVisitUris_wearableExtender() { Icon actionIcon = Icon.createWithContentUri("content://media/action"); -- cgit v1.2.3 From ecdb457437cad91a8c0a1c0727a099dd053035ba Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Fri, 23 Sep 2022 21:06:53 +0000 Subject: Drop invalid data. Drop invalid data when writing or reading from XML. PersistableBundle does lazy unparcelling, so checking the values during unparcelling would remove the benefit of the lazy unparcelling. Checking the validity when writing to or reading from XML seems like the best alternative. Bug: 246542285 Bug: 247513680 Test: install test app with invalid job config, start app to schedule job, then check logcat and jobscheduler persisted file (cherry picked from commit 666e8ac60a31e2cc52b335b41004263f28a8db06) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a5182ce159d82005f87d88e2aee5e10c19c996f6) Merged-In: Ie817aa0993e9046cb313a750d2323cadc8c1ef15 Change-Id: Ie817aa0993e9046cb313a750d2323cadc8c1ef15 --- core/java/android/os/PersistableBundle.java | 42 +++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java index f4edcb1317f4..acfd15cb005e 100644 --- a/core/java/android/os/PersistableBundle.java +++ b/core/java/android/os/PersistableBundle.java @@ -21,6 +21,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import android.annotation.NonNull; import android.annotation.Nullable; import android.util.ArrayMap; +import android.util.Slog; import android.util.TypedXmlPullParser; import android.util.TypedXmlSerializer; import android.util.Xml; @@ -50,6 +51,8 @@ import java.util.ArrayList; */ public final class PersistableBundle extends BaseBundle implements Cloneable, Parcelable, XmlUtils.WriteMapCallback { + private static final String TAG = "PersistableBundle"; + private static final String TAG_PERSISTABLEMAP = "pbundle_as_map"; /** An unmodifiable {@code PersistableBundle} that is always {@link #isEmpty() empty}. */ @@ -118,7 +121,11 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa * @hide */ public PersistableBundle(Bundle b) { - this(b.getItemwiseMap()); + this(b, true); + } + + private PersistableBundle(Bundle b, boolean throwException) { + this(b.getItemwiseMap(), throwException); } /** @@ -127,7 +134,7 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa * @param map a Map containing only those items that can be persisted. * @throws IllegalArgumentException if any element of #map cannot be persisted. */ - private PersistableBundle(ArrayMap map) { + private PersistableBundle(ArrayMap map, boolean throwException) { super(); mFlags = FLAG_DEFUSABLE; @@ -136,16 +143,23 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa // Now verify each item throwing an exception if there is a violation. final int N = mMap.size(); - for (int i=0; i= 0; --i) { Object value = mMap.valueAt(i); if (value instanceof ArrayMap) { // Fix up any Maps by replacing them with PersistableBundles. - mMap.setValueAt(i, new PersistableBundle((ArrayMap) value)); + mMap.setValueAt(i, + new PersistableBundle((ArrayMap) value, throwException)); } else if (value instanceof Bundle) { - mMap.setValueAt(i, new PersistableBundle(((Bundle) value))); + mMap.setValueAt(i, new PersistableBundle((Bundle) value, throwException)); } else if (!isValidType(value)) { - throw new IllegalArgumentException("Bad value in PersistableBundle key=" - + mMap.keyAt(i) + " value=" + value); + final String errorMsg = "Bad value in PersistableBundle key=" + + mMap.keyAt(i) + " value=" + value; + if (throwException) { + throw new IllegalArgumentException(errorMsg); + } else { + Slog.wtfStack(TAG, errorMsg); + mMap.removeAt(i); + } } } } @@ -268,6 +282,15 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa /** @hide */ public void saveToXml(TypedXmlSerializer out) throws IOException, XmlPullParserException { unparcel(); + // Explicitly drop invalid types an attacker may have added before persisting. + for (int i = mMap.size() - 1; i >= 0; --i) { + final Object value = mMap.valueAt(i); + if (!isValidType(value)) { + Slog.e(TAG, "Dropping bad data before persisting: " + + mMap.keyAt(i) + "=" + value); + mMap.removeAt(i); + } + } XmlUtils.writeMapXml(mMap, out, this); } @@ -322,9 +345,12 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa while (((event = in.next()) != XmlPullParser.END_DOCUMENT) && (event != XmlPullParser.END_TAG || in.getDepth() < outerDepth)) { if (event == XmlPullParser.START_TAG) { + // Don't throw an exception when restoring from XML since an attacker could try to + // input invalid data in the persisted file. return new PersistableBundle((ArrayMap) XmlUtils.readThisArrayMapXml(in, startTag, tagName, - new MyReadMapCallback())); + new MyReadMapCallback()), + /* throwException */ false); } } return new PersistableBundle(); // An empty mutable PersistableBundle -- cgit v1.2.3 From 5a642aed787f3fd00dc7b99e00b5baa71ad4b3a5 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Wed, 12 Jul 2023 21:38:36 +0000 Subject: Validate URI-based shortcut icon at creation time. Bug: 288113797 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3d41fb7620ffb9c81b23977c8367c323e4721e65) Merged-In: I392f8e923923bf40827a2b6207c4eaa262694fbc Change-Id: I392f8e923923bf40827a2b6207c4eaa262694fbc --- .../com/android/server/pm/ShortcutService.java | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index a2b2983a8f35..d5788295c3c9 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -34,6 +34,7 @@ import android.app.usage.UsageStatsManagerInternal; import android.appwidget.AppWidgetProviderInfo; import android.content.BroadcastReceiver; import android.content.ComponentName; +import android.content.ContentProvider; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; @@ -1914,11 +1915,32 @@ public class ShortcutService extends IShortcutService.Stub { } if (shortcut.getIcon() != null) { ShortcutInfo.validateIcon(shortcut.getIcon()); + validateIconURI(shortcut); } shortcut.replaceFlags(shortcut.getFlags() & ShortcutInfo.FLAG_LONG_LIVED); } + // Validates the calling process has permission to access shortcut icon's image uri + private void validateIconURI(@NonNull final ShortcutInfo si) { + final int callingUid = injectBinderCallingUid(); + final Icon icon = si.getIcon(); + if (icon == null) { + // There's no icon in this shortcut, nothing to validate here. + return; + } + int iconType = icon.getType(); + if (iconType != Icon.TYPE_URI && iconType != Icon.TYPE_URI_ADAPTIVE_BITMAP) { + // The icon is not URI-based, nothing to validate. + return; + } + final Uri uri = icon.getUri(); + mUriGrantsManagerInternal.checkGrantUriPermission(callingUid, si.getPackage(), + ContentProvider.getUriWithoutUserId(uri), + Intent.FLAG_GRANT_READ_URI_PERMISSION, + ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(callingUid))); + } + private void fixUpIncomingShortcutInfo(@NonNull ShortcutInfo shortcut, boolean forUpdate) { fixUpIncomingShortcutInfo(shortcut, forUpdate, /*forPinRequest=*/ false); } -- cgit v1.2.3 From ec5fb59d894ab07625c355b842fd0bf97d5d058d Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Fri, 4 Aug 2023 13:29:29 -0400 Subject: Disable priority conversation widget for secondary users Test: NotificationConversationInfoTest.java Test: make a conversation priority on the primary user Test: make a conversation priority on a secondary user Bug: 288896269 (cherry picked from commit adf620316dcfaf19d7d4a73e2c63322b4a3a4d3a) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e7ea4378a7a07b53c1f0147623be90f5045e9fc2) Merged-In: I3f3991d2cb7fb9970cc8ada39ceae9a7ff2fcb31 Change-Id: I3f3991d2cb7fb9970cc8ada39ceae9a7ff2fcb31 --- .../row/NotificationConversationInfo.java | 9 ++- .../notification/row/NotificationGutsManager.java | 7 +++ .../row/NotificationConversationInfoTest.java | 71 ++++++++++++++++++++++ .../row/NotificationGutsManagerTest.java | 5 +- 4 files changed, 90 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfo.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfo.java index f21db0bde59a..783650bd8399 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfo.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfo.java @@ -48,6 +48,7 @@ import android.os.Bundle; import android.os.Handler; import android.os.RemoteException; import android.os.UserHandle; +import android.os.UserManager; import android.service.notification.StatusBarNotification; import android.text.TextUtils; import android.transition.ChangeBounds; @@ -118,6 +119,8 @@ public class NotificationConversationInfo extends LinearLayout implements private NotificationGuts mGutsContainer; private OnConversationSettingsClickListener mOnConversationSettingsClickListener; + private UserManager mUm; + @VisibleForTesting boolean mSkipPost = false; private int mActualHeight; @@ -155,7 +158,9 @@ public class NotificationConversationInfo extends LinearLayout implements // People Tile add request. if (mSelectedAction == ACTION_FAVORITE && getPriority() != mSelectedAction) { mShadeController.animateCollapseShade(); - mPeopleSpaceWidgetManager.requestPinAppWidget(mShortcutInfo, new Bundle()); + if (mUm.isSameProfileGroup(UserHandle.USER_SYSTEM, mSbn.getNormalizedUserId())) { + mPeopleSpaceWidgetManager.requestPinAppWidget(mShortcutInfo, new Bundle()); + } } mGutsContainer.closeControls(v, /* save= */ true); }; @@ -188,6 +193,7 @@ public class NotificationConversationInfo extends LinearLayout implements public void bindNotification( ShortcutManager shortcutManager, PackageManager pm, + UserManager um, PeopleSpaceWidgetManager peopleSpaceWidgetManager, INotificationManager iNotificationManager, OnUserInteractionCallback onUserInteractionCallback, @@ -211,6 +217,7 @@ public class NotificationConversationInfo extends LinearLayout implements mEntry = entry; mSbn = entry.getSbn(); mPm = pm; + mUm = um; mAppName = mPackageName; mOnSettingsClickListener = onSettingsClick; mNotificationChannel = notificationChannel; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java index 46f1bb5ebd6f..b5354130e1c7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java @@ -30,6 +30,7 @@ import android.net.Uri; import android.os.Bundle; import android.os.Handler; import android.os.UserHandle; +import android.os.UserManager; import android.provider.Settings; import android.service.notification.StatusBarNotification; import android.util.ArraySet; @@ -113,6 +114,9 @@ public class NotificationGutsManager implements NotifGutsViewManager { private Runnable mOpenRunnable; private final INotificationManager mNotificationManager; private final PeopleSpaceWidgetManager mPeopleSpaceWidgetManager; + + private final UserManager mUserManager; + private final LauncherApps mLauncherApps; private final ShortcutManager mShortcutManager; private final UserContextProvider mContextTracker; @@ -128,6 +132,7 @@ public class NotificationGutsManager implements NotifGutsViewManager { AccessibilityManager accessibilityManager, HighPriorityProvider highPriorityProvider, INotificationManager notificationManager, + UserManager userManager, PeopleSpaceWidgetManager peopleSpaceWidgetManager, LauncherApps launcherApps, ShortcutManager shortcutManager, @@ -149,6 +154,7 @@ public class NotificationGutsManager implements NotifGutsViewManager { mAccessibilityManager = accessibilityManager; mHighPriorityProvider = highPriorityProvider; mNotificationManager = notificationManager; + mUserManager = userManager; mPeopleSpaceWidgetManager = peopleSpaceWidgetManager; mLauncherApps = launcherApps; mShortcutManager = shortcutManager; @@ -468,6 +474,7 @@ public class NotificationGutsManager implements NotifGutsViewManager { notificationInfoView.bindNotification( mShortcutManager, pmUser, + mUserManager, mPeopleSpaceWidgetManager, mNotificationManager, mOnUserInteractionCallback, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfoTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfoTest.java index 90adabfadd5d..596e9a2613d7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfoTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationConversationInfoTest.java @@ -62,6 +62,7 @@ import android.graphics.drawable.Drawable; import android.graphics.drawable.Icon; import android.os.Handler; import android.os.UserHandle; +import android.os.UserManager; import android.service.notification.StatusBarNotification; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; @@ -132,6 +133,8 @@ public class NotificationConversationInfoTest extends SysuiTestCase { @Mock private PackageManager mMockPackageManager; @Mock + private UserManager mUserManager; + @Mock private OnUserInteractionCallback mOnUserInteractionCallback; @Mock private BubblesManager mBubblesManager; @@ -238,6 +241,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -262,6 +266,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -314,6 +319,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -339,6 +345,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -363,6 +370,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -398,6 +406,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -423,6 +432,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -452,6 +462,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -476,6 +487,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -504,6 +516,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -532,6 +545,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -563,6 +577,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -600,6 +615,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -628,6 +644,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -663,6 +680,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -691,6 +709,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -735,6 +754,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -778,6 +798,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -822,6 +843,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -860,6 +882,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -896,6 +919,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -936,6 +960,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -967,6 +992,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -996,6 +1022,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1033,6 +1060,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1069,6 +1097,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1104,6 +1133,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1143,6 +1173,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1173,6 +1204,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1198,6 +1230,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1219,11 +1252,13 @@ public class NotificationConversationInfoTest extends SysuiTestCase { @Test public void testSelectPriorityRequestsPinPeopleTile() { + when(mUserManager.isSameProfileGroup(anyInt(), anyInt())).thenReturn(true); //WHEN channel is default importance mNotificationChannel.setImportantConversation(false); mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1249,11 +1284,46 @@ public class NotificationConversationInfoTest extends SysuiTestCase { verify(mPeopleSpaceWidgetManager, times(1)).requestPinAppWidget(any(), any()); } + @Test + public void testSelectPriorityRequestsPinPeopleTile_noMultiuser() { + when(mUserManager.isSameProfileGroup(anyInt(), anyInt())).thenReturn(false); + //WHEN channel is default importance + mNotificationChannel.setImportantConversation(false); + mNotificationInfo.bindNotification( + mShortcutManager, + mMockPackageManager, + mUserManager, + mPeopleSpaceWidgetManager, + mMockINotificationManager, + mOnUserInteractionCallback, + TEST_PACKAGE_NAME, + mNotificationChannel, + mEntry, + mBubbleMetadata, + null, + mIconFactory, + mContext, + true, + mTestHandler, + mTestHandler, null, Optional.of(mBubblesManager), + mShadeController); + + // WHEN user clicks "priority" + mNotificationInfo.setSelectedAction(NotificationConversationInfo.ACTION_FAVORITE); + + // and then done + mNotificationInfo.findViewById(R.id.done).performClick(); + + // No widget prompt; on a secondary user + verify(mPeopleSpaceWidgetManager, never()).requestPinAppWidget(any(), any()); + } + @Test public void testSelectDefaultDoesNotRequestPinPeopleTile() { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, @@ -1288,6 +1358,7 @@ public class NotificationConversationInfoTest extends SysuiTestCase { mNotificationInfo.bindNotification( mShortcutManager, mMockPackageManager, + mUserManager, mPeopleSpaceWidgetManager, mMockINotificationManager, mOnUserInteractionCallback, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java index 3d8a74466a5c..2507146a6200 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java @@ -52,6 +52,7 @@ import android.content.pm.ShortcutManager; import android.graphics.Color; import android.os.Binder; import android.os.Handler; +import android.os.UserManager; import android.provider.Settings; import android.service.notification.StatusBarNotification; import android.testing.AndroidTestingRunner; @@ -135,6 +136,8 @@ public class NotificationGutsManagerTest extends SysuiTestCase { @Mock private NotificationLockscreenUserManager mNotificationLockscreenUserManager; @Mock private StatusBarStateController mStatusBarStateController; + @Mock private UserManager mUserManager; + @Before public void setUp() { mTestableLooper = TestableLooper.get(this); @@ -145,7 +148,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { mGutsManager = new NotificationGutsManager(mContext, () -> Optional.of(mCentralSurfaces), mHandler, mHandler, mAccessibilityManager, - mHighPriorityProvider, mINotificationManager, + mHighPriorityProvider, mINotificationManager, mUserManager, mPeopleSpaceWidgetManager, mLauncherApps, mShortcutManager, mChannelEditorDialogController, mContextTracker, mAssistantFeedbackController, Optional.of(mBubblesManager), new UiEventLoggerFake(), mOnUserInteractionCallback, -- cgit v1.2.3 From 820a36e2b3df12042fb10e6f799ef6689bc8206c Mon Sep 17 00:00:00 2001 From: Marzia Favaro Date: Mon, 31 Jul 2023 15:10:34 +0000 Subject: Require permission to unlock keyguard Bug: 288896339 Test: Manual, verify that the app which can be found on the bug can no longer call keyguardGoingAway successfully Require permission to unlock keyguard Bug: 288896339 Test: Manual, verify that the app which can be found on the bug can no longer call keyguardGoingAway successfully (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:bd2aa5d309c5bf8e73161975bd5aba7945b25e84) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ade83209d9d1635bf894b71103e4ac52c51411f4) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:55b8322714f876cc1d36d5774c1c1f0c1101581b) Merged-In: I7ba7e56f954c8e6f1f734311f735215918975bc6 Change-Id: I7ba7e56f954c8e6f1f734311f735215918975bc6 --- core/java/android/app/IActivityTaskManager.aidl | 1 + .../core/java/com/android/server/wm/ActivityTaskManagerService.java | 2 ++ 2 files changed, 3 insertions(+) diff --git a/core/java/android/app/IActivityTaskManager.aidl b/core/java/android/app/IActivityTaskManager.aidl index 52732d3eba78..d7367f900205 100644 --- a/core/java/android/app/IActivityTaskManager.aidl +++ b/core/java/android/app/IActivityTaskManager.aidl @@ -241,6 +241,7 @@ interface IActivityTaskManager { * {@link android.view.WindowManagerPolicyConstants#KEYGUARD_GOING_AWAY_FLAG_TO_SHADE} * etc. */ + @JavaPassthrough(annotation="@android.annotation.RequiresPermission(android.Manifest.permission.CONTROL_KEYGUARD)") void keyguardGoingAway(int flags); void suppressResizeConfigChanges(boolean suppress); diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index f6e92a642900..704ab31ef8a9 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -18,6 +18,7 @@ package com.android.server.wm; import static android.Manifest.permission.BIND_VOICE_INTERACTION; import static android.Manifest.permission.CHANGE_CONFIGURATION; +import static android.Manifest.permission.CONTROL_KEYGUARD; import static android.Manifest.permission.CONTROL_REMOTE_APP_TRANSITION_ANIMATIONS; import static android.Manifest.permission.INTERACT_ACROSS_USERS; import static android.Manifest.permission.INTERACT_ACROSS_USERS_FULL; @@ -3472,6 +3473,7 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { @Override public void keyguardGoingAway(int flags) { + mAmInternal.enforceCallingPermission(CONTROL_KEYGUARD, "unlock keyguard"); enforceNotIsolatedCaller("keyguardGoingAway"); final long token = Binder.clearCallingIdentity(); try { -- cgit v1.2.3 From 1dfeabd868ed623d029309042cfaab2337b37003 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 8 Jun 2023 14:38:19 -0700 Subject: Restrict number of shortcuts can be added through addDynamicShortcuts This CL fixes the issue where, when an app have multiple main activities, the total number of shortcuts can grow indefinitely if they were published through addDynamicShortcuts. Bug: 281061287 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3215e73e36aa0463429226b5743ce24badf31227) Merged-In: Ib3eecefee34517b670c59dd5b8526fe9eb24f463 Change-Id: Ib3eecefee34517b670c59dd5b8526fe9eb24f463 --- services/core/java/com/android/server/pm/ShortcutPackage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index 921233132931..5a662d9f3139 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -376,6 +376,7 @@ class ShortcutPackage extends ShortcutPackageItem { // Extract Icon and update the icon res ID and the bitmap path. s.saveIconAndFixUpShortcutLocked(this, newShortcut); s.fixUpShortcutResourceNamesAndValues(newShortcut); + ensureShortcutCountBeforePush(); saveShortcut(newShortcut); } @@ -430,7 +431,6 @@ class ShortcutPackage extends ShortcutPackageItem { @NonNull List changedShortcuts) { Preconditions.checkArgument(newShortcut.isEnabled(), "pushDynamicShortcuts() cannot publish disabled shortcuts"); - ensureShortcutCountBeforePush(); newShortcut.addFlags(ShortcutInfo.FLAG_DYNAMIC); -- cgit v1.2.3 From 22a0b623d869fb470a2c97bbfff36610198161c5 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Mon, 24 Jul 2023 14:58:56 -0700 Subject: Validate userId when publishing shortcuts Bug: 288110451 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:01bfd04ff445db6290ae430d44ea1bf1a115fe3c) Merged-In: Idbde676f871db83825155730e3714f3727e25762 Change-Id: Idbde676f871db83825155730e3714f3727e25762 --- services/core/java/com/android/server/pm/ShortcutService.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index d5788295c3c9..7fd32d2a6b1b 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -1730,6 +1730,10 @@ public class ShortcutService extends IShortcutService.Stub { android.util.EventLog.writeEvent(0x534e4554, "109824443", -1, ""); throw new SecurityException("Shortcut package name mismatch"); } + final int callingUid = injectBinderCallingUid(); + if (UserHandle.getUserId(callingUid) != si.getUserId()) { + throw new SecurityException("User-ID in shortcut doesn't match the caller"); + } } private void verifyShortcutInfoPackages( -- cgit v1.2.3 From 8cc78a0169323fb196195302648dd6e3fc873e76 Mon Sep 17 00:00:00 2001 From: Beverly Tai Date: Thu, 14 Sep 2023 20:51:27 +0000 Subject: Revert "On device lockdown, always show the keyguard" This reverts commit 100ae42365d7fc8ba7d241e8c9a7ef6aa0cdb961. Reason for revert: b/300463732 regression Bug: 300463732 Bug: 218495634 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:996896e672f28aa96a3d8158192de3cd4a105bc3) Merged-In: Ida8f2ab5e4ac79b38c21ba7bacfe9c374c9cc600 Change-Id: Ida8f2ab5e4ac79b38c21ba7bacfe9c374c9cc600 --- .../com/android/systemui/keyguard/KeyguardViewMediator.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index 82189763def6..f177d2da8def 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -720,13 +720,6 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } } } - - @Override - public void onStrongAuthStateChanged(int userId) { - if (mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { - doKeyguardLocked(null); - } - } }; ViewMediatorCallback mViewMediatorCallback = new ViewMediatorCallback() { @@ -1933,8 +1926,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, */ private void doKeyguardLocked(Bundle options) { // if another app is disabling us, don't show - if (!mExternallyEnabled - && !mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { + if (!mExternallyEnabled) { if (DEBUG) Log.d(TAG, "doKeyguard: not showing because externally disabled"); mNeedToReshowWhenReenabled = true; -- cgit v1.2.3 From cc3966a0f3bc1b0e54577f5131cee6c0122dc0ec Mon Sep 17 00:00:00 2001 From: Kunal Malhotra Date: Thu, 2 Feb 2023 23:48:27 +0000 Subject: Adding in verification of calling UID in onShellCommand Test: manual testing on device Bug: b/261709193 (cherry picked from commit b651d295b44eb82d664861b77f33dbde1bce9453) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3ef3f18ba3094c4cc4f954ba23d1da421f9ca8b0) Merged-In: I68903ebd6d3d85f4bc820b745e3233a448b62273 Change-Id: I68903ebd6d3d85f4bc820b745e3233a448b62273 --- .../core/java/com/android/server/am/ActivityManagerService.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 13953d873c0f..46f62b83d669 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -9262,6 +9262,13 @@ public class ActivityManagerService extends IActivityManager.Stub public void onShellCommand(FileDescriptor in, FileDescriptor out, FileDescriptor err, String[] args, ShellCallback callback, ResultReceiver resultReceiver) { + final int callingUid = Binder.getCallingUid(); + if (callingUid != ROOT_UID && callingUid != Process.SHELL_UID) { + if (resultReceiver != null) { + resultReceiver.send(-1, null); + } + throw new SecurityException("Shell commands are only callable by root or shell"); + } (new ActivityManagerShellCommand(this, false)).exec( this, in, out, err, args, callback, resultReceiver); } -- cgit v1.2.3 From 0e0b4eadfb3e8bec4508f10d8914358d51398b31 Mon Sep 17 00:00:00 2001 From: Evan Severson Date: Tue, 4 Apr 2023 14:49:26 -0700 Subject: Move startWatchingModeWithFlags to the native supported binder calls Bug: 247768581 Test: Add logging to verify invocation (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:b1f82ee37403e40513ef3b9e2657feb3871c4e71) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3dddd6bc6e7ac57de69df8f1e66e03fff822d3b9) Merged-In: I54eefca5f0aa4f924debc1817b04a103b6e8e2e6 Change-Id: I54eefca5f0aa4f924debc1817b04a103b6e8e2e6 --- core/java/com/android/internal/app/IAppOpsService.aidl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/java/com/android/internal/app/IAppOpsService.aidl b/core/java/com/android/internal/app/IAppOpsService.aidl index 88447daf7338..ff3c015cf66f 100644 --- a/core/java/com/android/internal/app/IAppOpsService.aidl +++ b/core/java/com/android/internal/app/IAppOpsService.aidl @@ -52,6 +52,8 @@ interface IAppOpsService { int checkAudioOperation(int code, int usage, int uid, String packageName); boolean shouldCollectNotes(int opCode); void setCameraAudioRestriction(int mode); + void startWatchingModeWithFlags(int op, String packageName, int flags, + IAppOpsCallback callback); // End of methods also called by native code. // Any new method exposed to native must be added after the last one, do not reorder @@ -110,8 +112,6 @@ interface IAppOpsService { void startWatchingStarted(in int[] ops, IAppOpsStartedCallback callback); void stopWatchingStarted(IAppOpsStartedCallback callback); - void startWatchingModeWithFlags(int op, String packageName, int flags, IAppOpsCallback callback); - void startWatchingNoted(in int[] ops, IAppOpsNotedCallback callback); void stopWatchingNoted(IAppOpsNotedCallback callback); -- cgit v1.2.3 From 1100b33a95fbc0588491473a366e864c78eabc41 Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Wed, 4 Oct 2023 15:02:03 -0700 Subject: Stop using deprecated getParcelable method in AccountManagerService. Test: N/A Bug: 299930871 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:7a24a283a360e816d2bfb2aa124c4eb2efd1be61) Merged-In: I5a55acffe2e9ef2842c383a352b0bac66c6f1a55 Change-Id: I5a55acffe2e9ef2842c383a352b0bac66c6f1a55 --- .../core/java/com/android/server/accounts/AccountManagerService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index 7a51f5155a98..ba6cfabb9593 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -4932,7 +4932,7 @@ public class AccountManagerService p.setDataPosition(0); Bundle simulateBundle = p.readBundle(); p.recycle(); - Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT); + Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT, Intent.class); if (intent != null && intent.getClass() != Intent.class) { return false; } -- cgit v1.2.3 From 0fecd31ed1f7bcede691e0d88ec6764acbe5c809 Mon Sep 17 00:00:00 2001 From: Pawan Wagh Date: Tue, 13 Jun 2023 17:37:26 +0000 Subject: Use readUniqueFileDescriptor in incidentd service readFileDescriptor doesn't provide ownership of the fds. fdopen needs ownership of the fds. Fds read from parcel should be duped in this scenario and readUniqueFileDescriptor dups fds internally. Test: m incidentd_service_fuzzer && adb sync data && adb shell /data/fuzz/x86_64/incidentd_service_fuzzer/incidentd_service_fuzzer Test: atest incidentd_test Bug: 286931110 Bug: 283699145 (cherry picked from commit ba78ef276951269f7b024baebdf1b8fa40bedb23) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3904fb61fcbda61925ca0ab33000df359a79f8a8) Merged-In: Ibe03a17dee91ac5bf25d123d4fd9c0bdd3c7d80e Change-Id: Ibe03a17dee91ac5bf25d123d4fd9c0bdd3c7d80e --- cmds/incidentd/src/IncidentService.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/cmds/incidentd/src/IncidentService.cpp b/cmds/incidentd/src/IncidentService.cpp index fbb99d2aea89..8e461afd9db0 100644 --- a/cmds/incidentd/src/IncidentService.cpp +++ b/cmds/incidentd/src/IncidentService.cpp @@ -500,9 +500,13 @@ status_t IncidentService::onTransact(uint32_t code, const Parcel& data, Parcel* switch (code) { case SHELL_COMMAND_TRANSACTION: { - int in = data.readFileDescriptor(); - int out = data.readFileDescriptor(); - int err = data.readFileDescriptor(); + unique_fd in, out, err; + if (status_t status = data.readUniqueFileDescriptor(&in); status != OK) return status; + + if (status_t status = data.readUniqueFileDescriptor(&out); status != OK) return status; + + if (status_t status = data.readUniqueFileDescriptor(&err); status != OK) return status; + int argc = data.readInt32(); Vector args; for (int i = 0; i < argc && data.dataAvail() > 0; i++) { @@ -512,15 +516,15 @@ status_t IncidentService::onTransact(uint32_t code, const Parcel& data, Parcel* sp resultReceiver = IResultReceiver::asInterface(data.readStrongBinder()); - FILE* fin = fdopen(in, "r"); - FILE* fout = fdopen(out, "w"); - FILE* ferr = fdopen(err, "w"); + FILE* fin = fdopen(in.release(), "r"); + FILE* fout = fdopen(out.release(), "w"); + FILE* ferr = fdopen(err.release(), "w"); if (fin == NULL || fout == NULL || ferr == NULL) { resultReceiver->send(NO_MEMORY); } else { - err = command(fin, fout, ferr, args); - resultReceiver->send(err); + status_t result = command(fin, fout, ferr, args); + resultReceiver->send(result); } if (fin != NULL) { -- cgit v1.2.3 From ee846de5d994580e4c4d6ef3f213c4718bae4062 Mon Sep 17 00:00:00 2001 From: Beth Thibodeau Date: Tue, 8 Aug 2023 16:19:48 -0500 Subject: Check URI permissions for resumable media artwork When resumable media is added that has artwork set via URI, check the permissions for the URI before attempting to load it Test: atest MediaDataManagerTest Test: manual with test app Bug: 284297452 (cherry picked from commit 470f62bc8954e45018796f87f56b78f41dad45d6) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:820fe895b24ad0ce53faa09f9aeda677bf2fa8e1) Merged-In: Ie79915d3d1712f08dc2e8dfbd5bc7fd32bb308a3 Change-Id: Ie79915d3d1712f08dc2e8dfbd5bc7fd32bb308a3 --- core/java/android/app/IUriGrantsManager.aidl | 3 + .../media/controls/pipeline/MediaDataManager.kt | 33 ++++++++- .../controls/pipeline/MediaDataManagerTest.kt | 79 ++++++++++++++++++++++ .../server/uri/UriGrantsManagerService.java | 42 ++++++++++++ 4 files changed, 154 insertions(+), 3 deletions(-) diff --git a/core/java/android/app/IUriGrantsManager.aidl b/core/java/android/app/IUriGrantsManager.aidl index 9e7f2fecfea0..b630d034dca9 100644 --- a/core/java/android/app/IUriGrantsManager.aidl +++ b/core/java/android/app/IUriGrantsManager.aidl @@ -39,4 +39,7 @@ interface IUriGrantsManager { void clearGrantedUriPermissions(in String packageName, int userId); ParceledListSlice getUriPermissions(in String packageName, boolean incoming, boolean persistedOnly); + + int checkGrantUriPermission_ignoreNonSystem( + int sourceUid, String targetPkg, in Uri uri, int modeFlags, int userId); } diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt b/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt index 4e20a24e9add..cae8eb994d95 100644 --- a/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt +++ b/packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt @@ -19,11 +19,13 @@ package com.android.systemui.media.controls.pipeline import android.app.Notification import android.app.Notification.EXTRA_SUBSTITUTE_APP_NAME import android.app.PendingIntent +import android.app.UriGrantsManager import android.app.smartspace.SmartspaceConfig import android.app.smartspace.SmartspaceManager import android.app.smartspace.SmartspaceSession import android.app.smartspace.SmartspaceTarget import android.content.BroadcastReceiver +import android.content.ContentProvider import android.content.ContentResolver import android.content.Context import android.content.Intent @@ -677,10 +679,13 @@ class MediaDataManager( Log.d(TAG, "adding track for $userId from browser: $desc") } + val currentEntry = mediaEntries.get(packageName) + val appUid = currentEntry?.appUid ?: Process.INVALID_UID + // Album art var artworkBitmap = desc.iconBitmap if (artworkBitmap == null && desc.iconUri != null) { - artworkBitmap = loadBitmapFromUri(desc.iconUri!!) + artworkBitmap = loadBitmapFromUriForUser(desc.iconUri!!, userId, appUid, packageName) } val artworkIcon = if (artworkBitmap != null) { @@ -689,9 +694,7 @@ class MediaDataManager( null } - val currentEntry = mediaEntries.get(packageName) val instanceId = currentEntry?.instanceId ?: logger.getNewInstanceId() - val appUid = currentEntry?.appUid ?: Process.INVALID_UID val isExplicit = desc.extras?.getLong(MediaConstants.METADATA_KEY_IS_EXPLICIT) == MediaConstants.METADATA_VALUE_ATTRIBUTE_PRESENT && @@ -1231,6 +1234,30 @@ class MediaDataManager( false } } + + /** Returns a bitmap if the user can access the given URI, else null */ + private fun loadBitmapFromUriForUser( + uri: Uri, + userId: Int, + appUid: Int, + packageName: String, + ): Bitmap? { + try { + val ugm = UriGrantsManager.getService() + ugm.checkGrantUriPermission_ignoreNonSystem( + appUid, + packageName, + ContentProvider.getUriWithoutUserId(uri), + Intent.FLAG_GRANT_READ_URI_PERMISSION, + ContentProvider.getUserIdFromUri(uri, userId) + ) + return loadBitmapFromUri(uri) + } catch (e: SecurityException) { + Log.e(TAG, "Failed to get URI permission: $e") + } + return null + } + /** * Load a bitmap from a URI * diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt index 9ced057d3ad8..255eb3d167b1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt @@ -16,10 +16,12 @@ package com.android.systemui.media.controls.pipeline +import android.app.IUriGrantsManager import android.app.Notification import android.app.Notification.FLAG_NO_CLEAR import android.app.Notification.MediaStyle import android.app.PendingIntent +import android.app.UriGrantsManager import android.app.smartspace.SmartspaceAction import android.app.smartspace.SmartspaceConfig import android.app.smartspace.SmartspaceManager @@ -27,12 +29,14 @@ import android.app.smartspace.SmartspaceTarget import android.content.Intent import android.content.pm.PackageManager import android.graphics.Bitmap +import android.graphics.ImageDecoder import android.graphics.drawable.Icon import android.media.MediaDescription import android.media.MediaMetadata import android.media.session.MediaController import android.media.session.MediaSession import android.media.session.PlaybackState +import android.net.Uri import android.os.Bundle import android.provider.Settings import android.service.notification.StatusBarNotification @@ -40,6 +44,7 @@ import android.testing.AndroidTestingRunner import android.testing.TestableLooper.RunWithLooper import androidx.media.utils.MediaConstants import androidx.test.filters.SmallTest +import com.android.dx.mockito.inline.extended.ExtendedMockito import com.android.internal.logging.InstanceId import com.android.keyguard.KeyguardUpdateMonitor import com.android.systemui.InstanceIdSequenceFake @@ -83,7 +88,9 @@ import org.mockito.Mockito.reset import org.mockito.Mockito.verify import org.mockito.Mockito.verifyNoMoreInteractions import org.mockito.Mockito.`when` as whenever +import org.mockito.MockitoSession import org.mockito.junit.MockitoJUnit +import org.mockito.quality.Strictness private const val KEY = "KEY" private const val KEY_2 = "KEY_2" @@ -149,6 +156,8 @@ class MediaDataManagerTest : SysuiTestCase() { @Captor lateinit var stateCallbackCaptor: ArgumentCaptor<(String, PlaybackState) -> Unit> @Captor lateinit var sessionCallbackCaptor: ArgumentCaptor<(String) -> Unit> @Captor lateinit var smartSpaceConfigBuilderCaptor: ArgumentCaptor + @Mock private lateinit var ugm: IUriGrantsManager + @Mock private lateinit var imageSource: ImageDecoder.Source private val instanceIdSequence = InstanceIdSequenceFake(1 shl 20) @@ -159,8 +168,17 @@ class MediaDataManagerTest : SysuiTestCase() { 1 ) + private lateinit var staticMockSession: MockitoSession + @Before fun setup() { + staticMockSession = + ExtendedMockito.mockitoSession() + .mockStatic(UriGrantsManager::class.java) + .mockStatic(ImageDecoder::class.java) + .strictness(Strictness.LENIENT) + .startMocking() + whenever(UriGrantsManager.getService()).thenReturn(ugm) foregroundExecutor = FakeExecutor(clock) backgroundExecutor = FakeExecutor(clock) uiExecutor = FakeExecutor(clock) @@ -271,6 +289,7 @@ class MediaDataManagerTest : SysuiTestCase() { @After fun tearDown() { + staticMockSession.finishMocking() session.release() mediaDataManager.destroy() Settings.Secure.putInt( @@ -2202,6 +2221,66 @@ class MediaDataManagerTest : SysuiTestCase() { verify(listener).onMediaDataRemoved(eq(KEY)) } + @Test + fun testResumeMediaLoaded_hasArtPermission_artLoaded() { + // When resume media is loaded and user/app has permission to access the art URI, + whenever( + ugm.checkGrantUriPermission_ignoreNonSystem( + anyInt(), + any(), + any(), + anyInt(), + anyInt() + ) + ) + .thenReturn(1) + val artwork = Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) + val uri = Uri.parse("content://example") + whenever(ImageDecoder.createSource(any(), eq(uri))).thenReturn(imageSource) + whenever(ImageDecoder.decodeBitmap(any(), any())).thenReturn(artwork) + + val desc = + MediaDescription.Builder().run { + setTitle(SESSION_TITLE) + setIconUri(uri) + build() + } + addResumeControlAndLoad(desc) + + // Then the artwork is loaded + assertThat(mediaDataCaptor.value.artwork).isNotNull() + } + + @Test + fun testResumeMediaLoaded_noArtPermission_noArtLoaded() { + // When resume media is loaded and user/app does not have permission to access the art URI + whenever( + ugm.checkGrantUriPermission_ignoreNonSystem( + anyInt(), + any(), + any(), + anyInt(), + anyInt() + ) + ) + .thenThrow(SecurityException("Test no permission")) + val artwork = Bitmap.createBitmap(1, 1, Bitmap.Config.ARGB_8888) + val uri = Uri.parse("content://example") + whenever(ImageDecoder.createSource(any(), eq(uri))).thenReturn(imageSource) + whenever(ImageDecoder.decodeBitmap(any(), any())).thenReturn(artwork) + + val desc = + MediaDescription.Builder().run { + setTitle(SESSION_TITLE) + setIconUri(uri) + build() + } + addResumeControlAndLoad(desc) + + // Then the artwork is not loaded + assertThat(mediaDataCaptor.value.artwork).isNull() + } + /** Helper function to add a basic media notification and capture the resulting MediaData */ private fun addNotificationAndLoad() { addNotificationAndLoad(mediaNotification) diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerService.java b/services/core/java/com/android/server/uri/UriGrantsManagerService.java index 6aa06e8ee068..2deeaed46064 100644 --- a/services/core/java/com/android/server/uri/UriGrantsManagerService.java +++ b/services/core/java/com/android/server/uri/UriGrantsManagerService.java @@ -41,6 +41,7 @@ import static org.xmlpull.v1.XmlPullParser.START_TAG; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.RequiresPermission; import android.app.ActivityManager; import android.app.ActivityManagerInternal; import android.app.AppGlobals; @@ -62,6 +63,7 @@ import android.os.Handler; import android.os.IBinder; import android.os.Looper; import android.os.Message; +import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; import android.os.UserHandle; @@ -1304,6 +1306,46 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub implements return false; } + /** + * Check if the targetPkg can be granted permission to access uri by + * the callingUid using the given modeFlags. See {@link #checkGrantUriPermissionUnlocked}. + * + * @param callingUid The uid of the grantor app that has permissions to the uri. + * @param targetPkg The package name of the granted app that needs permissions to the uri. + * @param uri The uri for which permissions should be granted. + * @param modeFlags The modes to grant. See {@link Intent#FLAG_GRANT_READ_URI_PERMISSION}, etc. + * @param userId The userId in which the uri is to be resolved. + * @return uid of the target or -1 if permission grant not required. Returns -1 if the caller + * does not hold INTERACT_ACROSS_USERS_FULL + * @throws SecurityException if the grant is not allowed. + */ + @Override + @RequiresPermission(android.Manifest.permission.INTERACT_ACROSS_USERS_FULL) + public int checkGrantUriPermission_ignoreNonSystem(int callingUid, String targetPkg, Uri uri, + int modeFlags, int userId) { + if (!isCallerIsSystemOrPrivileged()) { + return Process.INVALID_UID; + } + final long origId = Binder.clearCallingIdentity(); + try { + return checkGrantUriPermissionUnlocked(callingUid, targetPkg, uri, modeFlags, + userId); + } finally { + Binder.restoreCallingIdentity(origId); + } + } + + private boolean isCallerIsSystemOrPrivileged() { + final int uid = Binder.getCallingUid(); + if (uid == Process.SYSTEM_UID || uid == Process.ROOT_UID) { + return true; + } + return ActivityManager.checkComponentPermission( + android.Manifest.permission.INTERACT_ACROSS_USERS_FULL, + uid, /* owningUid = */-1, /* exported = */ true) + == PackageManager.PERMISSION_GRANTED; + } + @Override public ArrayList providePersistentUriGrants() { final ArrayList result = new ArrayList<>(); -- cgit v1.2.3 From ef80550c52610b1ab6e14bd0f62eba77d7653ced Mon Sep 17 00:00:00 2001 From: Beverly Tai Date: Tue, 19 Sep 2023 20:44:04 +0000 Subject: Updated: always show the keyguard on device lockdown Additionally, don't hide keyguard when it's disabled if the user has locked down the device. Manual test steps: 1. Enable app pinning and disable "Ask for PIN before unpinning" setting 2. Pin an app (ie: Settings) 3. Lockdown from the power menu 4. Observe: user is brought to the keyguard, primary auth is required to enter the device. => After entering correct credential, the device is still in app pinning mode. => After entering an incorrect credential, the keyguard remains showing and the user can attempt again up to the limit Bug: 300463732 Bug: 218495634 Test: atest KeyguardViewMediatorTest Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:faaf58d3b910c388b0a7c51dc370a7ae18e7cec2) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:21754dda9152b8b9004b7dc394767ac9ad9c07dd) Merged-In: I70fdae80f717712b3dfc9df54b9649959b4bb8f0 Change-Id: I70fdae80f717712b3dfc9df54b9649959b4bb8f0 --- .../android/systemui/keyguard/KeyguardViewMediator.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index f177d2da8def..2fa6b9be7866 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -720,6 +720,13 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } } } + + @Override + public void onStrongAuthStateChanged(int userId) { + if (mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { + doKeyguardLocked(null); + } + } }; ViewMediatorCallback mViewMediatorCallback = new ViewMediatorCallback() { @@ -1717,6 +1724,10 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, mExternallyEnabled = enabled; if (!enabled && mShowing) { + if (mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { + Log.d(TAG, "keyguardEnabled(false) overridden by user lockdown"); + return; + } // hiding keyguard that is showing, remember to reshow later if (DEBUG) Log.d(TAG, "remembering to reshow, hiding keyguard, " + "disabling status bar expansion"); @@ -1925,8 +1936,9 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, * Enable the keyguard if the settings are appropriate. */ private void doKeyguardLocked(Bundle options) { - // if another app is disabling us, don't show - if (!mExternallyEnabled) { + // if another app is disabling us, don't show unless we're in lockdown mode + if (!mExternallyEnabled + && !mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { if (DEBUG) Log.d(TAG, "doKeyguard: not showing because externally disabled"); mNeedToReshowWhenReenabled = true; -- cgit v1.2.3 From 427efc812d41fa673df60c6561d27075455e2923 Mon Sep 17 00:00:00 2001 From: Aaron Liu Date: Fri, 11 Aug 2023 11:02:33 -0700 Subject: DO NOT MERGE Ensure finish lockscreen when usersetup incomplete Ensure that when the usersetup for the user is not complete, we do not want to go to lockscreen, even if lockscreen is not disabled. Bug: 222446076 Test: add Unit test, Test: Wipe device, auth sim pin in setup, observe that lockscreen is not there. (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:cc1a7bbc05ab9be745c1a811abc7915d9c7d6d3f) Merged-In: I8e33db8eb6e2c917966cab3d6a4f982670473040 Change-Id: I8e33db8eb6e2c917966cab3d6a4f982670473040 --- .../keyguard/KeyguardSecurityContainerController.java | 14 +++++++++++--- .../keyguard/KeyguardSecurityContainerControllerTest.java | 4 +++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java index 16299c7aff7b..4ebc0ad9267d 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardSecurityContainerController.java @@ -78,6 +78,7 @@ import com.android.systemui.plugins.ActivityStarter; import com.android.systemui.plugins.FalsingManager; import com.android.systemui.shared.system.SysUiStatsLog; import com.android.systemui.statusbar.policy.ConfigurationController; +import com.android.systemui.statusbar.policy.DeviceProvisionedController; import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.systemui.statusbar.policy.UserSwitcherController; import com.android.systemui.util.ViewController; @@ -365,6 +366,8 @@ public class KeyguardSecurityContainerController extends ViewController Date: Thu, 24 Aug 2023 16:27:30 +0000 Subject: Truncate user data to a limit of 500 characters Fix vulnerability that allows creating users with no restrictions. This is done by creating an intent to create a user and putting extras that are too long to be serialized. It causes IOException and the restrictions are not written in the file. By truncating the string values when writing them to the file, we ensure that the exception does not happen and it can be recorded correctly. Bug: 293602317 Test: install app provided in the bug, open app and click add. Check logcat to see there is no more IOException. Reboot the device by either opening User details page or running adb shell dumpsys user | grep -A12 heen and see that the restrictions are in place. (cherry picked from commit 59042a32c7e192d160c295ecb6477a09bb5da0bb) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e5e5d53c31615b23a7c20ec0b900b7a6d690222c) Merged-In: I633dc10974a64ef2abd07e67ff2d209847129989 Change-Id: I633dc10974a64ef2abd07e67ff2d209847129989 --- .../com/android/server/pm/UserManagerService.java | 29 ++++++++++++++++------ 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 88aeb17dc2b4..edc3923b2998 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -258,6 +258,8 @@ public class UserManagerService extends IUserManager.Stub { private static final int USER_VERSION = 9; + private static final int MAX_USER_STRING_LENGTH = 500; + private static final long EPOCH_PLUS_30_YEARS = 30L * 365 * 24 * 60 * 60 * 1000L; // ms static final int WRITE_USER_MSG = 1; @@ -3514,15 +3516,17 @@ public class UserManagerService extends IUserManager.Stub { // Write seed data if (userData.persistSeedData) { if (userData.seedAccountName != null) { - serializer.attribute(null, ATTR_SEED_ACCOUNT_NAME, userData.seedAccountName); + serializer.attribute(null, ATTR_SEED_ACCOUNT_NAME, + truncateString(userData.seedAccountName)); } if (userData.seedAccountType != null) { - serializer.attribute(null, ATTR_SEED_ACCOUNT_TYPE, userData.seedAccountType); + serializer.attribute(null, ATTR_SEED_ACCOUNT_TYPE, + truncateString(userData.seedAccountType)); } } if (userInfo.name != null) { serializer.startTag(null, TAG_NAME); - serializer.text(userInfo.name); + serializer.text(truncateString(userInfo.name)); serializer.endTag(null, TAG_NAME); } synchronized (mRestrictionsLock) { @@ -3562,6 +3566,13 @@ public class UserManagerService extends IUserManager.Stub { serializer.endDocument(); } + private String truncateString(String original) { + if (original == null || original.length() <= MAX_USER_STRING_LENGTH) { + return original; + } + return original.substring(0, MAX_USER_STRING_LENGTH); + } + /* * Writes the user list file in this format: * @@ -3967,6 +3978,8 @@ public class UserManagerService extends IUserManager.Stub { boolean preCreate, @Nullable String[] disallowedPackages, @NonNull TimingsTraceAndSlog t, @Nullable Object token) throws UserManager.CheckedUserOperationException { + + String truncatedName = truncateString(name); final UserTypeDetails userTypeDetails = mUserTypes.get(userType); if (userTypeDetails == null) { Slog.e(LOG_TAG, "Cannot create user of invalid user type: " + userType); @@ -3998,8 +4011,8 @@ public class UserManagerService extends IUserManager.Stub { // Try to use a pre-created user (if available). if (!preCreate && parentId < 0 && isUserTypeEligibleForPreCreation(userTypeDetails)) { - final UserInfo preCreatedUser = convertPreCreatedUserIfPossible(userType, flags, name, - token); + final UserInfo preCreatedUser = convertPreCreatedUserIfPossible(userType, flags, + truncatedName, token); if (preCreatedUser != null) { return preCreatedUser; } @@ -4099,7 +4112,7 @@ public class UserManagerService extends IUserManager.Stub { flags |= UserInfo.FLAG_EPHEMERAL_ON_CREATE; } - userInfo = new UserInfo(userId, name, null, flags, userType); + userInfo = new UserInfo(userId, truncatedName, null, flags, userType); userInfo.serialNumber = mNextSerialNumber++; userInfo.creationTime = getCreationTime(); userInfo.partial = true; @@ -5531,8 +5544,8 @@ public class UserManagerService extends IUserManager.Stub { Slog.e(LOG_TAG, "No such user for settings seed data u=" + userId); return; } - userData.seedAccountName = accountName; - userData.seedAccountType = accountType; + userData.seedAccountName = truncateString(accountName); + userData.seedAccountType = truncateString(accountType); userData.seedAccountOptions = accountOptions; userData.persistSeedData = persist; } -- cgit v1.2.3 From 97fcf4bb0e5e2b83e8e28c718c98e00147ec3243 Mon Sep 17 00:00:00 2001 From: Raphael Kim Date: Mon, 18 Sep 2023 14:07:23 -0700 Subject: [CDM] Validate component name length before requesting notification access. Bug: 295335110 Test: Test app with long component name (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:254e53f796934aa873427b79f7cb41605006664f) Merged-In: I7ea5d5c1f78858db9865f3310d1e0aff9c8b5579 Change-Id: I7ea5d5c1f78858db9865f3310d1e0aff9c8b5579 --- .../com/android/server/companion/CompanionDeviceManagerService.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index a94e4b9b492d..f5ce00cb31c9 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -132,6 +132,7 @@ public class CompanionDeviceManagerService extends SystemService { "debug.cdm.cdmservice.removal_time_window"; private static final long ASSOCIATION_REMOVAL_TIME_WINDOW_DEFAULT = DAYS.toMillis(90); + private static final int MAX_CN_LENGTH = 500; private final ActivityManager mActivityManager; private final OnPackageVisibilityChangeListener mOnPackageVisibilityChangeListener; @@ -621,6 +622,9 @@ public class CompanionDeviceManagerService extends SystemService { String callingPackage = component.getPackageName(); checkCanCallNotificationApi(callingPackage); // TODO: check userId. + if (component.flattenToString().length() > MAX_CN_LENGTH) { + throw new IllegalArgumentException("Component name is too long."); + } final long identity = Binder.clearCallingIdentity(); try { return PendingIntent.getActivityAsUser(getContext(), -- cgit v1.2.3 From ea9711d1576af2d9381c726042a66c2811c328f3 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Mon, 28 Aug 2023 21:34:43 +0000 Subject: [SB][Privacy] Fetch current active appops on startup. This also updates SysUI's chip animation scheduler to ignore an `isTooEarly` check if the chip animation is forced to be visible (which is true for privacy events). Bug: 294104969 Test: start recording, then kill systemui via adb-> verify privacy chip reappears after restart. Pull down shade and verify chip is correctly attributed. Stop recording and verify chip/dot disappears. Test: open camera, then kill systemui via adb -> verify privacy chip reappears after restart. Pull down shade and verify chip is correctly attributed. Close camera and verify chip/dot disappears. Test: smoke test of privacy chip and dot Test: atest AppOpsControllerTest SystemStatusAnimationSchedulerImplTest (cherry picked from commit 084a7afb4bb41e0cdfdbe67bdd60728d940b4331) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:12d0064ef788844afbb85ac7e65f8d4b1d37bc5c) Merged-In: I664bb3003a2f6871113406e3257b7118bbdf2ab5 Change-Id: I664bb3003a2f6871113406e3257b7118bbdf2ab5 --- .../systemui/appops/AppOpsControllerImpl.java | 28 +++ .../events/SystemStatusAnimationSchedulerImpl.kt | 5 +- .../SystemStatusAnimationSchedulerLegacyImpl.kt | 5 +- .../systemui/appops/AppOpsControllerTest.java | 217 +++++++++++++++++++++ .../SystemStatusAnimationSchedulerImplTest.kt | 28 ++- 5 files changed, 275 insertions(+), 8 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java index 6b859763eb6f..c6c08585640f 100644 --- a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java @@ -51,6 +51,7 @@ import com.android.systemui.util.time.SystemClock; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Set; import javax.inject.Inject; @@ -144,6 +145,10 @@ public class AppOpsControllerImpl extends BroadcastReceiver implements AppOpsCon protected void setListening(boolean listening) { mListening = listening; if (listening) { + // System UI could be restarted while ops are active, so fetch the currently active ops + // once System UI starts listening again. + fetchCurrentActiveOps(); + mAppOps.startWatchingActive(OPS, this); mAppOps.startWatchingNoted(OPS, this); mAudioManager.registerAudioRecordingCallback(mAudioRecordingCallback, mBGHandler); @@ -176,6 +181,29 @@ public class AppOpsControllerImpl extends BroadcastReceiver implements AppOpsCon } } + private void fetchCurrentActiveOps() { + List packageOps = mAppOps.getPackagesForOps(OPS); + for (AppOpsManager.PackageOps op : packageOps) { + for (AppOpsManager.OpEntry entry : op.getOps()) { + for (Map.Entry attributedOpEntry : + entry.getAttributedOpEntries().entrySet()) { + if (attributedOpEntry.getValue().isRunning()) { + onOpActiveChanged( + entry.getOpStr(), + op.getUid(), + op.getPackageName(), + /* attributionTag= */ attributedOpEntry.getKey(), + /* active= */ true, + // AppOpsManager doesn't have a way to fetch attribution flags or + // chain ID given an op entry, so default them to none. + AppOpsManager.ATTRIBUTION_FLAGS_NONE, + AppOpsManager.ATTRIBUTION_CHAIN_ID_NONE); + } + } + } + } + } + /** * Adds a callback that will get notifified when an AppOp of the type the controller tracks * changes diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt index f7a4feafee25..997020155cdc 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImpl.kt @@ -128,8 +128,9 @@ constructor( override fun onStatusEvent(event: StatusEvent) { Assert.isMainThread() - // Ignore any updates until the system is up and running - if (isTooEarly() || !isImmersiveIndicatorEnabled()) { + // Ignore any updates until the system is up and running. However, for important events that + // request to be force visible (like privacy), ignore whether it's too early. + if ((isTooEarly() && !event.forceVisible) || !isImmersiveIndicatorEnabled()) { return } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerLegacyImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerLegacyImpl.kt index 5fa83ef5d454..6b5a548b0afe 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerLegacyImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerLegacyImpl.kt @@ -93,8 +93,9 @@ constructor( @SystemAnimationState override fun getAnimationState() = animationState override fun onStatusEvent(event: StatusEvent) { - // Ignore any updates until the system is up and running - if (isTooEarly() || !isImmersiveIndicatorEnabled()) { + // Ignore any updates until the system is up and running. However, for important events that + // request to be force visible (like privacy), ignore whether it's too early. + if ((isTooEarly() && !event.forceVisible) || !isImmersiveIndicatorEnabled()) { return } diff --git a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java index 61a651234e0c..e6c36c18342c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java @@ -19,6 +19,8 @@ package com.android.systemui.appops; import static android.hardware.SensorPrivacyManager.Sensors.CAMERA; import static android.hardware.SensorPrivacyManager.Sensors.MICROPHONE; +import static com.google.common.truth.Truth.assertThat; + import static junit.framework.TestCase.assertFalse; import static org.junit.Assert.assertEquals; @@ -66,6 +68,7 @@ import org.mockito.MockitoAnnotations; import java.util.Collections; import java.util.List; +import java.util.Map; @SmallTest @RunWith(AndroidTestingRunner.class) @@ -157,6 +160,204 @@ public class AppOpsControllerTest extends SysuiTestCase { verify(mSensorPrivacyController, times(1)).removeCallback(mController); } + @Test + public void startListening_fetchesCurrentActive_none() { + when(mAppOpsManager.getPackagesForOps(AppOpsControllerImpl.OPS)) + .thenReturn(List.of()); + + mController.setListening(true); + + assertThat(mController.getActiveAppOps()).isEmpty(); + } + + /** Regression test for b/294104969. */ + @Test + public void startListening_fetchesCurrentActive_oneActive() { + AppOpsManager.PackageOps packageOps = createPackageOp( + "package.test", + /* packageUid= */ 2, + AppOpsManager.OPSTR_FINE_LOCATION, + /* isRunning= */ true); + when(mAppOpsManager.getPackagesForOps(AppOpsControllerImpl.OPS)) + .thenReturn(List.of(packageOps)); + + // WHEN we start listening + mController.setListening(true); + + // THEN the active list has the op + List list = mController.getActiveAppOps(); + assertEquals(1, list.size()); + AppOpItem first = list.get(0); + assertThat(first.getPackageName()).isEqualTo("package.test"); + assertThat(first.getUid()).isEqualTo(2); + assertThat(first.getCode()).isEqualTo(AppOpsManager.OP_FINE_LOCATION); + } + + @Test + public void startListening_fetchesCurrentActive_multiplePackages() { + AppOpsManager.PackageOps packageOps1 = createPackageOp( + "package.one", + /* packageUid= */ 1, + AppOpsManager.OPSTR_FINE_LOCATION, + /* isRunning= */ true); + AppOpsManager.PackageOps packageOps2 = createPackageOp( + "package.two", + /* packageUid= */ 2, + AppOpsManager.OPSTR_FINE_LOCATION, + /* isRunning= */ false); + AppOpsManager.PackageOps packageOps3 = createPackageOp( + "package.three", + /* packageUid= */ 3, + AppOpsManager.OPSTR_FINE_LOCATION, + /* isRunning= */ true); + when(mAppOpsManager.getPackagesForOps(AppOpsControllerImpl.OPS)) + .thenReturn(List.of(packageOps1, packageOps2, packageOps3)); + + // WHEN we start listening + mController.setListening(true); + + // THEN the active list has the ops + List list = mController.getActiveAppOps(); + assertEquals(2, list.size()); + + AppOpItem item0 = list.get(0); + assertThat(item0.getPackageName()).isEqualTo("package.one"); + assertThat(item0.getUid()).isEqualTo(1); + assertThat(item0.getCode()).isEqualTo(AppOpsManager.OP_FINE_LOCATION); + + AppOpItem item1 = list.get(1); + assertThat(item1.getPackageName()).isEqualTo("package.three"); + assertThat(item1.getUid()).isEqualTo(3); + assertThat(item1.getCode()).isEqualTo(AppOpsManager.OP_FINE_LOCATION); + } + + @Test + public void startListening_fetchesCurrentActive_multipleEntries() { + AppOpsManager.PackageOps packageOps = mock(AppOpsManager.PackageOps.class); + when(packageOps.getUid()).thenReturn(1); + when(packageOps.getPackageName()).thenReturn("package.one"); + + // Entry 1 + AppOpsManager.OpEntry entry1 = mock(AppOpsManager.OpEntry.class); + when(entry1.getOpStr()).thenReturn(AppOpsManager.OPSTR_PHONE_CALL_MICROPHONE); + AppOpsManager.AttributedOpEntry attributed1 = mock(AppOpsManager.AttributedOpEntry.class); + when(attributed1.isRunning()).thenReturn(true); + when(entry1.getAttributedOpEntries()).thenReturn(Map.of("tag", attributed1)); + // Entry 2 + AppOpsManager.OpEntry entry2 = mock(AppOpsManager.OpEntry.class); + when(entry2.getOpStr()).thenReturn(AppOpsManager.OPSTR_CAMERA); + AppOpsManager.AttributedOpEntry attributed2 = mock(AppOpsManager.AttributedOpEntry.class); + when(attributed2.isRunning()).thenReturn(true); + when(entry2.getAttributedOpEntries()).thenReturn(Map.of("tag", attributed2)); + // Entry 3 + AppOpsManager.OpEntry entry3 = mock(AppOpsManager.OpEntry.class); + when(entry3.getOpStr()).thenReturn(AppOpsManager.OPSTR_FINE_LOCATION); + AppOpsManager.AttributedOpEntry attributed3 = mock(AppOpsManager.AttributedOpEntry.class); + when(attributed3.isRunning()).thenReturn(false); + when(entry3.getAttributedOpEntries()).thenReturn(Map.of("tag", attributed3)); + + when(packageOps.getOps()).thenReturn(List.of(entry1, entry2, entry3)); + when(mAppOpsManager.getPackagesForOps(AppOpsControllerImpl.OPS)) + .thenReturn(List.of(packageOps)); + + // WHEN we start listening + mController.setListening(true); + + // THEN the active list has the ops + List list = mController.getActiveAppOps(); + assertEquals(2, list.size()); + + AppOpItem first = list.get(0); + assertThat(first.getPackageName()).isEqualTo("package.one"); + assertThat(first.getUid()).isEqualTo(1); + assertThat(first.getCode()).isEqualTo(AppOpsManager.OP_PHONE_CALL_MICROPHONE); + + AppOpItem second = list.get(1); + assertThat(second.getPackageName()).isEqualTo("package.one"); + assertThat(second.getUid()).isEqualTo(1); + assertThat(second.getCode()).isEqualTo(AppOpsManager.OP_CAMERA); + } + + @Test + public void startListening_fetchesCurrentActive_multipleAttributes() { + AppOpsManager.PackageOps packageOps = mock(AppOpsManager.PackageOps.class); + when(packageOps.getUid()).thenReturn(1); + when(packageOps.getPackageName()).thenReturn("package.one"); + AppOpsManager.OpEntry entry = mock(AppOpsManager.OpEntry.class); + when(entry.getOpStr()).thenReturn(AppOpsManager.OPSTR_RECORD_AUDIO); + + AppOpsManager.AttributedOpEntry attributed1 = mock(AppOpsManager.AttributedOpEntry.class); + when(attributed1.isRunning()).thenReturn(false); + AppOpsManager.AttributedOpEntry attributed2 = mock(AppOpsManager.AttributedOpEntry.class); + when(attributed2.isRunning()).thenReturn(true); + AppOpsManager.AttributedOpEntry attributed3 = mock(AppOpsManager.AttributedOpEntry.class); + when(attributed3.isRunning()).thenReturn(true); + when(entry.getAttributedOpEntries()).thenReturn( + Map.of("attr1", attributed1, "attr2", attributed2, "attr3", attributed3)); + + when(packageOps.getOps()).thenReturn(List.of(entry)); + when(mAppOpsManager.getPackagesForOps(AppOpsControllerImpl.OPS)) + .thenReturn(List.of(packageOps)); + + // WHEN we start listening + mController.setListening(true); + + // THEN the active list has the ops + List list = mController.getActiveAppOps(); + // Multiple attributes get merged into one entry in the active ops + assertEquals(1, list.size()); + + AppOpItem first = list.get(0); + assertThat(first.getPackageName()).isEqualTo("package.one"); + assertThat(first.getUid()).isEqualTo(1); + assertThat(first.getCode()).isEqualTo(AppOpsManager.OP_RECORD_AUDIO); + } + + /** Regression test for b/294104969. */ + @Test + public void addCallback_existingCallbacksNotifiedOfCurrentActive() { + AppOpsManager.PackageOps packageOps1 = createPackageOp( + "package.one", + /* packageUid= */ 1, + AppOpsManager.OPSTR_FINE_LOCATION, + /* isRunning= */ true); + AppOpsManager.PackageOps packageOps2 = createPackageOp( + "package.two", + /* packageUid= */ 2, + AppOpsManager.OPSTR_RECORD_AUDIO, + /* isRunning= */ true); + AppOpsManager.PackageOps packageOps3 = createPackageOp( + "package.three", + /* packageUid= */ 3, + AppOpsManager.OPSTR_PHONE_CALL_MICROPHONE, + /* isRunning= */ true); + when(mAppOpsManager.getPackagesForOps(AppOpsControllerImpl.OPS)) + .thenReturn(List.of(packageOps1, packageOps2, packageOps3)); + + // WHEN we start listening + mController.addCallback( + new int[]{AppOpsManager.OP_RECORD_AUDIO, AppOpsManager.OP_FINE_LOCATION}, + mCallback); + mTestableLooper.processAllMessages(); + + // THEN the callback is notified of the current active ops it cares about + verify(mCallback).onActiveStateChanged( + AppOpsManager.OP_FINE_LOCATION, + /* uid= */ 1, + "package.one", + true); + verify(mCallback).onActiveStateChanged( + AppOpsManager.OP_RECORD_AUDIO, + /* uid= */ 2, + "package.two", + true); + verify(mCallback, never()).onActiveStateChanged( + AppOpsManager.OP_PHONE_CALL_MICROPHONE, + /* uid= */ 3, + "package.three", + true); + } + @Test public void addCallback_includedCode() { mController.addCallback( @@ -673,6 +874,22 @@ public class AppOpsControllerTest extends SysuiTestCase { assertEquals(AppOpsManager.OP_PHONE_CALL_CAMERA, list.get(cameraIdx).getCode()); } + private AppOpsManager.PackageOps createPackageOp( + String packageName, int packageUid, String opStr, boolean isRunning) { + AppOpsManager.PackageOps packageOps = mock(AppOpsManager.PackageOps.class); + when(packageOps.getPackageName()).thenReturn(packageName); + when(packageOps.getUid()).thenReturn(packageUid); + AppOpsManager.OpEntry entry = mock(AppOpsManager.OpEntry.class); + when(entry.getOpStr()).thenReturn(opStr); + AppOpsManager.AttributedOpEntry attributed = mock(AppOpsManager.AttributedOpEntry.class); + when(attributed.isRunning()).thenReturn(isRunning); + + when(packageOps.getOps()).thenReturn(Collections.singletonList(entry)); + when(entry.getAttributedOpEntries()).thenReturn(Map.of("tag", attributed)); + + return packageOps; + } + private class TestHandler extends AppOpsControllerImpl.H { TestHandler(Looper looper) { mController.super(looper); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt index 7b59cc284181..f0de14eaf2bc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/events/SystemStatusAnimationSchedulerImplTest.kt @@ -87,9 +87,6 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() { fakeFeatureFlags ) - // ensure that isTooEarly() check in SystemStatusAnimationScheduler does not return true - systemClock.advanceTime(Process.getStartUptimeMillis() + MIN_UPTIME) - // StatusBarContentInsetProvider is mocked. Ensure that it returns some mocked values. whenever(statusBarContentInsetProvider.getStatusBarContentInsetsForCurrentRotation()) .thenReturn(android.util.Pair(10, 10)) @@ -150,6 +147,21 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() { assertEquals(0f, batteryChip.view.alpha) } + /** Regression test for b/294104969. */ + @Test + fun testPrivacyStatusEvent_beforeSystemUptime_stillDisplayed() = runTest { + initializeSystemStatusAnimationScheduler(testScope = this, advancePastMinUptime = false) + + // WHEN the uptime hasn't quite passed the minimum required uptime... + systemClock.setUptimeMillis(Process.getStartUptimeMillis() + MIN_UPTIME / 2) + + // BUT the event is a privacy event + createAndScheduleFakePrivacyEvent() + + // THEN the privacy event still happens + assertEquals(ANIMATION_QUEUED, systemStatusAnimationScheduler.getAnimationState()) + } + @Test fun testPrivacyStatusEvent_standardAnimationLifecycle() = runTest { // Instantiate class under test with TestScope from runTest @@ -454,7 +466,10 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() { return batteryChip } - private fun initializeSystemStatusAnimationScheduler(testScope: TestScope) { + private fun initializeSystemStatusAnimationScheduler( + testScope: TestScope, + advancePastMinUptime: Boolean = true, + ) { systemStatusAnimationScheduler = SystemStatusAnimationSchedulerImpl( systemEventCoordinator, @@ -466,5 +481,10 @@ class SystemStatusAnimationSchedulerImplTest : SysuiTestCase() { ) // add a mock listener systemStatusAnimationScheduler.addCallback(listener) + + if (advancePastMinUptime) { + // ensure that isTooEarly() check in SystemStatusAnimationScheduler does not return true + systemClock.advanceTime(Process.getStartUptimeMillis() + MIN_UPTIME) + } } } -- cgit v1.2.3 From 677200f5e252e198986246c5ee85d02405808bd2 Mon Sep 17 00:00:00 2001 From: Nan Wu Date: Fri, 25 Aug 2023 15:02:28 +0000 Subject: RESTRICT AUTOMERGE Log to detect usage of whitelistToken when sending non-PI target Log ActivityManagerService.sendIntentSender if the target is not a PendingIntent and a non-null whitelistToken is sent to the client. This is simply to detect if there are real cases this would happen before we decide simply remove whitelistToken in that case. Do not pass whitelistToken when sending non-PI target In ActivityManagerService.sendIntentSender, if the target is not a PendingIntent, do not send whitelistToken to the client. Bug: 279428283 Test: Manual test (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e5069813ecf230b9fe9a3302a2a59c91d1aa6498) Merged-In: I017486354a1ab2f14d0472c355583d53c27c4810 Change-Id: I017486354a1ab2f14d0472c355583d53c27c4810 --- .../com/android/server/am/ActivityManagerService.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 46f62b83d669..70bef605b567 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -5434,7 +5434,20 @@ public class ActivityManagerService extends IActivityManager.Stub intent = new Intent(Intent.ACTION_MAIN); } try { - target.send(code, intent, resolvedType, allowlistToken, null, + if (allowlistToken != null) { + final int callingUid = Binder.getCallingUid(); + final String packageName; + final long token = Binder.clearCallingIdentity(); + try { + packageName = AppGlobals.getPackageManager().getNameForUid(callingUid); + } finally { + Binder.restoreCallingIdentity(token); + } + Slog.wtf(TAG, "Send a non-null allowlistToken to a non-PI target." + + " Calling package: " + packageName + "; intent: " + intent + + "; options: " + options); + } + target.send(code, intent, resolvedType, null, null, requiredPermission, options); } catch (RemoteException e) { } -- cgit v1.2.3 From 71700a87b71e0da7dc45fd0f9e7657291ea535ab Mon Sep 17 00:00:00 2001 From: Evan Chen Date: Tue, 17 Oct 2023 21:31:23 +0000 Subject: Do not allow setting notification access across users. For mutil user case, make sure the calling userid matching the passing userid Test: test it on sample app Bug: 298635078 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ee88e15751ff0bc3da6b7c43dc7d94cebf1fc1a2) Merged-In: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a Change-Id: I6c478ebcc1d981faf2d125a4b41909c3b6a30a2a --- .../android/server/companion/CompanionDeviceManagerService.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java index f5ce00cb31c9..41546d2bdc38 100644 --- a/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java +++ b/services/companion/java/com/android/server/companion/CompanionDeviceManagerService.java @@ -620,8 +620,7 @@ public class CompanionDeviceManagerService extends SystemService { public PendingIntent requestNotificationAccess(ComponentName component, int userId) throws RemoteException { String callingPackage = component.getPackageName(); - checkCanCallNotificationApi(callingPackage); - // TODO: check userId. + checkCanCallNotificationApi(callingPackage, userId); if (component.flattenToString().length() > MAX_CN_LENGTH) { throw new IllegalArgumentException("Component name is too long."); } @@ -647,7 +646,7 @@ public class CompanionDeviceManagerService extends SystemService { @Deprecated @Override public boolean hasNotificationAccess(ComponentName component) throws RemoteException { - checkCanCallNotificationApi(component.getPackageName()); + checkCanCallNotificationApi(component.getPackageName(), getCallingUserId()); NotificationManager nm = getContext().getSystemService(NotificationManager.class); return nm.isNotificationListenerAccessGranted(component); } @@ -804,8 +803,7 @@ public class CompanionDeviceManagerService extends SystemService { legacyCreateAssociation(userId, macAddress, packageName, null); } - private void checkCanCallNotificationApi(String callingPackage) { - final int userId = getCallingUserId(); + private void checkCanCallNotificationApi(String callingPackage, int userId) { enforceCallerIsSystemOr(userId, callingPackage); if (getCallingUid() == SYSTEM_UID) return; -- cgit v1.2.3 From e93790b67f694dcf5f4cbf2f29f45978f3f8341f Mon Sep 17 00:00:00 2001 From: Santos Cordon Date: Thu, 19 Aug 2021 01:11:07 +0100 Subject: WaitForNegativeProximity stopped with power button. If the device is in a state where it is waiting for negative proximity event to turn the screen back on (as with phone calls), this CL makes it so that power button will also turn the screen back on, even if proximity is still positive. Test: make phone call, cover prox, end call. Previously power button had no effect in this state, now it turns the screen back on. Bug: 190459207 Change-Id: I3ce33fe81e5ee0c0652517a8ce46632519ba73c6 (cherry picked from commit 325cf315e64e7698bee012737b64b3fc53473d23) --- .../core/java/com/android/server/display/DisplayPowerController.java | 1 - 1 file changed, 1 deletion(-) diff --git a/services/core/java/com/android/server/display/DisplayPowerController.java b/services/core/java/com/android/server/display/DisplayPowerController.java index 5e8f379d85aa..e027223931c5 100644 --- a/services/core/java/com/android/server/display/DisplayPowerController.java +++ b/services/core/java/com/android/server/display/DisplayPowerController.java @@ -2655,7 +2655,6 @@ final class DisplayPowerController implements AutomaticBrightnessController.Call private void ignoreProximitySensorUntilChangedInternal() { if (!mIgnoreProximityUntilChanged - && mPowerRequest.useProximitySensor && mProximity == PROXIMITY_POSITIVE) { // Only ignore if it is still reporting positive (near) mIgnoreProximityUntilChanged = true; -- cgit v1.2.3 From 4812b564d43410443839e7a756f0caf12d29dd90 Mon Sep 17 00:00:00 2001 From: Nan Wu Date: Fri, 2 Dec 2022 19:08:54 +0000 Subject: DO NOT MERGE Disallow Wallpaper service to launch activity from background. Add a flag so that when a foreground client binds to a service, disallow the bound service to launch activity from background. Modify the WallpaperManagerService to take advantage of the new flag. Test: atest BackgroundActivityLaunchTest WallpaperManagerServiceTests Bug: 298094386 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:8efec2c0958ef8316cd24f7c79de818a7ca4cde9) Merged-In: Id4e4cb6144597cf3638f2aaa34ea455a239fa1a7 Change-Id: Id4e4cb6144597cf3638f2aaa34ea455a239fa1a7 --- core/java/android/content/Context.java | 9 +++++ .../android/server/activitymanagerservice.proto | 1 + .../com/android/server/am/ConnectionRecord.java | 5 +++ .../android/server/am/ProcessServiceRecord.java | 23 +++++++----- .../java/com/android/server/am/ServiceRecord.java | 2 +- .../server/wallpaper/WallpaperManagerService.java | 3 +- .../wm/BackgroundLaunchProcessController.java | 42 ++++++++++++---------- .../android/server/wm/WindowProcessController.java | 14 ++++++-- 8 files changed, 69 insertions(+), 30 deletions(-) diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index 77ca48a8ed1d..225c1081bd1f 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -272,6 +272,7 @@ public abstract class Context { BIND_IMPORTANT, BIND_ADJUST_WITH_ACTIVITY, BIND_NOT_PERCEPTIBLE, + BIND_DENY_ACTIVITY_STARTS_PRE_34, BIND_INCLUDE_CAPABILITIES }) @Retention(RetentionPolicy.SOURCE) @@ -392,6 +393,14 @@ public abstract class Context { /*********** Public flags above this line ***********/ /*********** Hidden flags below this line ***********/ + /** + * Flag for {@link #bindService}: If binding from an app that is visible, the bound service is + * allowed to start an activity from background. Add a flag so that this behavior can be opted + * out. + * @hide + */ + public static final int BIND_DENY_ACTIVITY_STARTS_PRE_34 = 0X000004000; + /** * Flag for {@link #bindService}: This flag is only intended to be used by the system to * indicate that a service binding is not considered as real package component usage and should diff --git a/core/proto/android/server/activitymanagerservice.proto b/core/proto/android/server/activitymanagerservice.proto index 7205dd817eb5..305b7661aadc 100644 --- a/core/proto/android/server/activitymanagerservice.proto +++ b/core/proto/android/server/activitymanagerservice.proto @@ -524,6 +524,7 @@ message ConnectionRecordProto { DEAD = 15; NOT_PERCEPTIBLE = 16; INCLUDE_CAPABILITIES = 17; + DENY_ACTIVITY_STARTS_PRE_34 = 18; } repeated Flag flags = 3; optional string service_name = 4; diff --git a/services/core/java/com/android/server/am/ConnectionRecord.java b/services/core/java/com/android/server/am/ConnectionRecord.java index 17fff919ba20..89ada49d7561 100644 --- a/services/core/java/com/android/server/am/ConnectionRecord.java +++ b/services/core/java/com/android/server/am/ConnectionRecord.java @@ -77,6 +77,7 @@ final class ConnectionRecord { Context.BIND_NOT_VISIBLE, Context.BIND_NOT_PERCEPTIBLE, Context.BIND_INCLUDE_CAPABILITIES, + Context.BIND_DENY_ACTIVITY_STARTS_PRE_34, }; private static final int[] BIND_PROTO_ENUMS = new int[] { ConnectionRecordProto.AUTO_CREATE, @@ -96,6 +97,7 @@ final class ConnectionRecord { ConnectionRecordProto.NOT_VISIBLE, ConnectionRecordProto.NOT_PERCEPTIBLE, ConnectionRecordProto.INCLUDE_CAPABILITIES, + ConnectionRecordProto.DENY_ACTIVITY_STARTS_PRE_34, }; void dump(PrintWriter pw, String prefix) { @@ -237,6 +239,9 @@ final class ConnectionRecord { if ((flags & Context.BIND_NOT_PERCEPTIBLE) != 0) { sb.append("!PRCP "); } + if ((flags & Context.BIND_DENY_ACTIVITY_STARTS_PRE_34) != 0) { + sb.append("BALFD "); + } if ((flags & Context.BIND_INCLUDE_CAPABILITIES) != 0) { sb.append("CAPS "); } diff --git a/services/core/java/com/android/server/am/ProcessServiceRecord.java b/services/core/java/com/android/server/am/ProcessServiceRecord.java index 67eb675503ad..510a4bac6c83 100644 --- a/services/core/java/com/android/server/am/ProcessServiceRecord.java +++ b/services/core/java/com/android/server/am/ProcessServiceRecord.java @@ -27,6 +27,7 @@ import android.util.ArrayMap; import android.util.ArraySet; import com.android.internal.annotations.GuardedBy; +import com.android.server.wm.WindowProcessController; import java.io.PrintWriter; import java.util.ArrayList; @@ -424,19 +425,21 @@ final class ProcessServiceRecord { return mConnections.size(); } - void addBoundClientUid(int clientUid) { + void addBoundClientUid(int clientUid, String clientPackageName, int bindFlags) { mBoundClientUids.add(clientUid); - mApp.getWindowProcessController().setBoundClientUids(mBoundClientUids); + mApp.getWindowProcessController() + .addBoundClientUid(clientUid, clientPackageName, bindFlags); } void updateBoundClientUids() { + clearBoundClientUids(); if (mServices.isEmpty()) { - clearBoundClientUids(); return; } // grab a set of clientUids of all mConnections of all services final ArraySet boundClientUids = new ArraySet<>(); final int serviceCount = mServices.size(); + WindowProcessController controller = mApp.getWindowProcessController(); for (int j = 0; j < serviceCount; j++) { final ArrayMap> conns = mServices.valueAt(j).getConnections(); @@ -444,12 +447,13 @@ final class ProcessServiceRecord { for (int conni = 0; conni < size; conni++) { ArrayList c = conns.valueAt(conni); for (int i = 0; i < c.size(); i++) { - boundClientUids.add(c.get(i).clientUid); + ConnectionRecord cr = c.get(i); + boundClientUids.add(cr.clientUid); + controller.addBoundClientUid(cr.clientUid, cr.clientPackageName, cr.flags); } } } mBoundClientUids = boundClientUids; - mApp.getWindowProcessController().setBoundClientUids(mBoundClientUids); } void addBoundClientUidsOfNewService(ServiceRecord sr) { @@ -460,15 +464,18 @@ final class ProcessServiceRecord { for (int conni = conns.size() - 1; conni >= 0; conni--) { ArrayList c = conns.valueAt(conni); for (int i = 0; i < c.size(); i++) { - mBoundClientUids.add(c.get(i).clientUid); + ConnectionRecord cr = c.get(i); + mBoundClientUids.add(cr.clientUid); + mApp.getWindowProcessController() + .addBoundClientUid(cr.clientUid, cr.clientPackageName, cr.flags); + } } - mApp.getWindowProcessController().setBoundClientUids(mBoundClientUids); } void clearBoundClientUids() { mBoundClientUids.clear(); - mApp.getWindowProcessController().setBoundClientUids(mBoundClientUids); + mApp.getWindowProcessController().clearBoundClientUids(); } @GuardedBy("mService") diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java index 4b82ad863c8e..0a79b1547558 100644 --- a/services/core/java/com/android/server/am/ServiceRecord.java +++ b/services/core/java/com/android/server/am/ServiceRecord.java @@ -738,7 +738,7 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN // if we have a process attached, add bound client uid of this connection to it if (app != null) { - app.mServices.addBoundClientUid(c.clientUid); + app.mServices.addBoundClientUid(c.clientUid, c.clientPackageName, c.flags); app.mProfile.addHostingComponentType(HOSTING_COMPONENT_TYPE_BOUND_SERVICE); } } diff --git a/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java b/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java index 1574ff554fb1..57299eaa1020 100644 --- a/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java +++ b/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java @@ -3228,7 +3228,8 @@ public class WallpaperManagerService extends IWallpaperManager.Stub if (!mContext.bindServiceAsUser(intent, newConn, Context.BIND_AUTO_CREATE | Context.BIND_SHOWING_UI | Context.BIND_FOREGROUND_SERVICE_WHILE_AWAKE - | Context.BIND_INCLUDE_CAPABILITIES, + | Context.BIND_INCLUDE_CAPABILITIES + | Context.BIND_DENY_ACTIVITY_STARTS_PRE_34, new UserHandle(serviceUserId))) { String msg = "Unable to bind service: " + componentName; diff --git a/services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java b/services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java index cc47528f9950..f8f4b8a8d3e5 100644 --- a/services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java +++ b/services/core/java/com/android/server/wm/BackgroundLaunchProcessController.java @@ -29,11 +29,11 @@ import static com.android.server.wm.BackgroundActivityStartController.BAL_BLOCK; import android.annotation.NonNull; import android.annotation.Nullable; +import android.content.Context; import android.os.Binder; import android.os.IBinder; import android.os.SystemClock; import android.util.ArrayMap; -import android.util.ArraySet; import android.util.IntArray; import android.util.Slog; @@ -65,9 +65,11 @@ class BackgroundLaunchProcessController { @GuardedBy("this") private @Nullable ArrayMap mBackgroundActivityStartTokens; - /** Set of UIDs of clients currently bound to this process. */ + /** Set of UIDs of clients currently bound to this process and opt in to allow this process to + * launch background activity. + */ @GuardedBy("this") - private @Nullable IntArray mBoundClientUids; + private @Nullable IntArray mBalOptInBoundClientUids; BackgroundLaunchProcessController(@NonNull IntPredicate uidHasActiveVisibleWindowPredicate, @Nullable BackgroundActivityStartCallback callback) { @@ -164,9 +166,9 @@ class BackgroundLaunchProcessController { private boolean isBoundByForegroundUid() { synchronized (this) { - if (mBoundClientUids != null) { - for (int i = mBoundClientUids.size() - 1; i >= 0; i--) { - if (mUidHasActiveVisibleWindowPredicate.test(mBoundClientUids.get(i))) { + if (mBalOptInBoundClientUids != null) { + for (int i = mBalOptInBoundClientUids.size() - 1; i >= 0; i--) { + if (mUidHasActiveVisibleWindowPredicate.test(mBalOptInBoundClientUids.get(i))) { return true; } } @@ -175,19 +177,23 @@ class BackgroundLaunchProcessController { return false; } - void setBoundClientUids(ArraySet boundClientUids) { + void clearBalOptInBoundClientUids() { synchronized (this) { - if (boundClientUids == null || boundClientUids.isEmpty()) { - mBoundClientUids = null; - return; - } - if (mBoundClientUids == null) { - mBoundClientUids = new IntArray(); + if (mBalOptInBoundClientUids == null) { + mBalOptInBoundClientUids = new IntArray(); } else { - mBoundClientUids.clear(); + mBalOptInBoundClientUids.clear(); + } + } + } + + void addBoundClientUid(int clientUid, String clientPackageName, int bindFlags) { + if ((bindFlags & Context.BIND_DENY_ACTIVITY_STARTS_PRE_34) == 0) { + if (mBalOptInBoundClientUids == null) { + mBalOptInBoundClientUids = new IntArray(); } - for (int i = boundClientUids.size() - 1; i >= 0; i--) { - mBoundClientUids.add(boundClientUids.valueAt(i)); + if (mBalOptInBoundClientUids.indexOf(clientUid) == -1) { + mBalOptInBoundClientUids.add(clientUid); } } } @@ -253,10 +259,10 @@ class BackgroundLaunchProcessController { pw.println(mBackgroundActivityStartTokens.valueAt(i)); } } - if (mBoundClientUids != null && mBoundClientUids.size() > 0) { + if (mBalOptInBoundClientUids != null && mBalOptInBoundClientUids.size() > 0) { pw.print(prefix); pw.print("BoundClientUids:"); - pw.println(Arrays.toString(mBoundClientUids.toArray())); + pw.println(Arrays.toString(mBalOptInBoundClientUids.toArray())); } } } diff --git a/services/core/java/com/android/server/wm/WindowProcessController.java b/services/core/java/com/android/server/wm/WindowProcessController.java index 04799204322d..49e2de1e33a2 100644 --- a/services/core/java/com/android/server/wm/WindowProcessController.java +++ b/services/core/java/com/android/server/wm/WindowProcessController.java @@ -572,8 +572,18 @@ public class WindowProcessController extends ConfigurationContainer boundClientUids) { - mBgLaunchController.setBoundClientUids(boundClientUids); + /** + * Clear all bound client Uids. + */ + public void clearBoundClientUids() { + mBgLaunchController.clearBalOptInBoundClientUids(); + } + + /** + * Add bound client Uid. + */ + public void addBoundClientUid(int clientUid, String clientPackageName, int bindFlags) { + mBgLaunchController.addBoundClientUid(clientUid, clientPackageName, bindFlags); } /** -- cgit v1.2.3 From 681a4ff4f482d8ab044dc25344243c85b422abed Mon Sep 17 00:00:00 2001 From: Sergey Nikolaienkov Date: Sat, 1 Jul 2023 16:03:56 +0200 Subject: DO NOT MERGE: "Hide" /Android/data|obb|sanbox/ on shared storage Implement shouldHideDocument() in the ExternalStorageProvider so that it resitcts access to 'Android/data/', 'Android/obb/' and 'Android/sandbox' on the integrated shared storage along with all their content and subdirectories. Clean up the abstract FileSystemProvider, specifically all variants of queryChildDocuments(). Bug: 200034476 Bug: 220066255 Bug: 283962634 Test: make & flash systemimage, run manually Test: atest ExternalStorageProviderTests (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:131e73e1b3839111463aae26bae1b3db6782eb38) Merged-In: I48c2ce7ff2d7fc067961ea2af0ea63818316f086 Change-Id: I48c2ce7ff2d7fc067961ea2af0ea63818316f086 --- .../internal/content/FileSystemProvider.java | 166 ++++++++++---------- .../externalstorage/ExternalStorageProvider.java | 173 +++++++++++++-------- 2 files changed, 198 insertions(+), 141 deletions(-) diff --git a/core/java/com/android/internal/content/FileSystemProvider.java b/core/java/com/android/internal/content/FileSystemProvider.java index 563cf0414ab9..be944d6410e8 100644 --- a/core/java/com/android/internal/content/FileSystemProvider.java +++ b/core/java/com/android/internal/content/FileSystemProvider.java @@ -62,14 +62,14 @@ import java.nio.file.FileVisitor; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayDeque; import java.util.Arrays; import java.util.LinkedList; import java.util.List; import java.util.Locale; +import java.util.Queue; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.function.Predicate; -import java.util.regex.Pattern; /** * A helper class for {@link android.provider.DocumentsProvider} to perform file operations on local @@ -87,6 +87,8 @@ public abstract class FileSystemProvider extends DocumentsProvider { DocumentsContract.QUERY_ARG_LAST_MODIFIED_AFTER, DocumentsContract.QUERY_ARG_MIME_TYPES); + private static final int MAX_RESULTS_NUMBER = 23; + private static String joinNewline(String... args) { return TextUtils.join("\n", args); } @@ -373,62 +375,53 @@ public abstract class FileSystemProvider extends DocumentsProvider { } /** - * This method is similar to - * {@link DocumentsProvider#queryChildDocuments(String, String[], String)}. This method returns - * all children documents including hidden directories/files. - * - *

- * In a scoped storage world, access to "Android/data" style directories are hidden for privacy - * reasons. This method may show privacy sensitive data, so its usage should only be in - * restricted modes. - * - * @param parentDocumentId the directory to return children for. - * @param projection list of {@link Document} columns to put into the - * cursor. If {@code null} all supported columns should be - * included. - * @param sortOrder how to order the rows, formatted as an SQL - * {@code ORDER BY} clause (excluding the ORDER BY itself). - * Passing {@code null} will use the default sort order, which - * may be unordered. This ordering is a hint that can be used to - * prioritize how data is fetched from the network, but UI may - * always enforce a specific ordering - * @throws FileNotFoundException when parent document doesn't exist or query fails + * WARNING: this method should really be {@code final}, but for the backward compatibility it's + * not; new classes that extend {@link FileSystemProvider} should override + * {@link #queryChildDocuments(String, String[], String, boolean)}, not this method. */ - protected Cursor queryChildDocumentsShowAll( - String parentDocumentId, String[] projection, String sortOrder) + @Override + public Cursor queryChildDocuments(String documentId, String[] projection, String sortOrder) throws FileNotFoundException { - return queryChildDocuments(parentDocumentId, projection, sortOrder, File -> true); + return queryChildDocuments(documentId, projection, sortOrder, /* includeHidden */ false); } + /** + * This method is similar to {@link #queryChildDocuments(String, String[], String)}, however, it + * could return all content of the directory, including restricted (hidden) + * directories and files. + *

+ * In the scoped storage world, some directories and files (e.g. {@code Android/data/} and + * {@code Android/obb/} on the external storage) are hidden for privacy reasons. + * Hence, this method may reveal privacy-sensitive data, thus should be used with extra care. + */ @Override - public Cursor queryChildDocuments( - String parentDocumentId, String[] projection, String sortOrder) - throws FileNotFoundException { - // Access to some directories is hidden for privacy reasons. - return queryChildDocuments(parentDocumentId, projection, sortOrder, this::shouldShow); + public final Cursor queryChildDocumentsForManage(String documentId, String[] projection, + String sortOrder) throws FileNotFoundException { + return queryChildDocuments(documentId, projection, sortOrder, /* includeHidden */ true); } - private Cursor queryChildDocuments( - String parentDocumentId, String[] projection, String sortOrder, - @NonNull Predicate filter) throws FileNotFoundException { - final File parent = getFileForDocId(parentDocumentId); + protected Cursor queryChildDocuments(String documentId, String[] projection, String sortOrder, + boolean includeHidden) throws FileNotFoundException { + final File parent = getFileForDocId(documentId); final MatrixCursor result = new DirectoryCursor( - resolveProjection(projection), parentDocumentId, parent); + resolveProjection(projection), documentId, parent); + + if (!parent.isDirectory()) { + Log.w(TAG, '"' + documentId + "\" is not a directory"); + return result; + } - if (!filter.test(parent)) { - Log.w(TAG, "No permission to access parentDocumentId: " + parentDocumentId); + if (!includeHidden && shouldHideDocument(documentId)) { + Log.w(TAG, "Queried directory \"" + documentId + "\" is hidden"); return result; } - if (parent.isDirectory()) { - for (File file : FileUtils.listFilesOrEmpty(parent)) { - if (filter.test(file)) { - includeFile(result, null, file); - } - } - } else { - Log.w(TAG, "parentDocumentId '" + parentDocumentId + "' is not Directory"); + for (File file : FileUtils.listFilesOrEmpty(parent)) { + if (!includeHidden && shouldHideDocument(file)) continue; + + includeFile(result, null, file); } + return result; } @@ -450,23 +443,29 @@ public abstract class FileSystemProvider extends DocumentsProvider { * * @see ContentResolver#EXTRA_HONORED_ARGS */ - protected final Cursor querySearchDocuments( - File folder, String[] projection, Set exclusion, Bundle queryArgs) - throws FileNotFoundException { + protected final Cursor querySearchDocuments(File folder, String[] projection, + Set exclusion, Bundle queryArgs) throws FileNotFoundException { final MatrixCursor result = new MatrixCursor(resolveProjection(projection)); - final LinkedList pending = new LinkedList<>(); - pending.add(folder); - while (!pending.isEmpty() && result.getCount() < 24) { - final File file = pending.removeFirst(); - if (shouldHide(file)) continue; + + // We'll be a running a BFS here. + final Queue pending = new ArrayDeque<>(); + pending.offer(folder); + + while (!pending.isEmpty() && result.getCount() < MAX_RESULTS_NUMBER) { + final File file = pending.poll(); + + // Skip hidden documents (both files and directories) + if (shouldHideDocument(file)) continue; if (file.isDirectory()) { for (File child : FileUtils.listFilesOrEmpty(file)) { - pending.add(child); + pending.offer(child); } } - if (!exclusion.contains(file.getAbsolutePath()) && matchSearchQueryArguments(file, - queryArgs)) { + + if (exclusion.contains(file.getAbsolutePath())) continue; + + if (matchSearchQueryArguments(file, queryArgs)) { includeFile(result, null, file); } } @@ -610,26 +609,23 @@ public abstract class FileSystemProvider extends DocumentsProvider { final int flagIndex = ArrayUtils.indexOf(columns, Document.COLUMN_FLAGS); if (flagIndex != -1) { + final boolean isDir = mimeType.equals(Document.MIME_TYPE_DIR); int flags = 0; if (file.canWrite()) { - if (mimeType.equals(Document.MIME_TYPE_DIR)) { + flags |= Document.FLAG_SUPPORTS_DELETE; + flags |= Document.FLAG_SUPPORTS_RENAME; + flags |= Document.FLAG_SUPPORTS_MOVE; + if (isDir) { flags |= Document.FLAG_DIR_SUPPORTS_CREATE; - flags |= Document.FLAG_SUPPORTS_DELETE; - flags |= Document.FLAG_SUPPORTS_RENAME; - flags |= Document.FLAG_SUPPORTS_MOVE; - - if (shouldBlockFromTree(docId)) { - flags |= Document.FLAG_DIR_BLOCKS_OPEN_DOCUMENT_TREE; - } - } else { flags |= Document.FLAG_SUPPORTS_WRITE; - flags |= Document.FLAG_SUPPORTS_DELETE; - flags |= Document.FLAG_SUPPORTS_RENAME; - flags |= Document.FLAG_SUPPORTS_MOVE; } } + if (isDir && shouldBlockDirectoryFromTree(docId)) { + flags |= Document.FLAG_DIR_BLOCKS_OPEN_DOCUMENT_TREE; + } + if (mimeType.startsWith("image/")) { flags |= Document.FLAG_SUPPORTS_THUMBNAIL; } @@ -662,22 +658,36 @@ public abstract class FileSystemProvider extends DocumentsProvider { return row; } - private static final Pattern PATTERN_HIDDEN_PATH = Pattern.compile( - "(?i)^/storage/[^/]+/(?:[0-9]+/)?Android/(?:data|obb|sandbox)$"); - /** - * In a scoped storage world, access to "Android/data" style directories are - * hidden for privacy reasons. + * Some providers may want to restrict access to certain directories and files, + * e.g. "Android/data" and "Android/obb" on the shared storage for + * privacy reasons. + * Such providers should override this method. */ - protected boolean shouldHide(@NonNull File file) { - return (PATTERN_HIDDEN_PATH.matcher(file.getAbsolutePath()).matches()); + protected boolean shouldHideDocument(@NonNull String documentId) + throws FileNotFoundException { + return false; } - private boolean shouldShow(@NonNull File file) { - return !shouldHide(file); + /** + * A variant of the {@link #shouldHideDocument(String)} that takes a {@link File} instead of + * a {@link String} {@code documentId}. + * + * @see #shouldHideDocument(String) + */ + protected final boolean shouldHideDocument(@NonNull File document) + throws FileNotFoundException { + return shouldHideDocument(getDocIdForFile(document)); } - protected boolean shouldBlockFromTree(@NonNull String docId) { + /** + * @return if the directory that should be blocked from being selected when the user launches + * an {@link Intent#ACTION_OPEN_DOCUMENT_TREE} intent. + * + * @see Document#FLAG_DIR_BLOCKS_OPEN_DOCUMENT_TREE + */ + protected boolean shouldBlockDirectoryFromTree(@NonNull String documentId) + throws FileNotFoundException { return false; } diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java index 4c313b22f71e..3409c29d3c2c 100644 --- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java +++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java @@ -16,6 +16,8 @@ package com.android.externalstorage; +import static java.util.regex.Pattern.CASE_INSENSITIVE; + import android.annotation.NonNull; import android.annotation.Nullable; import android.app.usage.StorageStatsManager; @@ -64,7 +66,19 @@ import java.util.List; import java.util.Locale; import java.util.Objects; import java.util.UUID; - +import java.util.regex.Pattern; + +/** + * Presents content of the shared (a.k.a. "external") storage. + *

+ * Starting with Android 11 (R), restricts access to the certain sections of the shared storage: + * {@code Android/data/}, {@code Android/obb/} and {@code Android/sandbox/}, that will be hidden in + * the DocumentsUI by default. + * See + * Storage updates in Android 11. + *

+ * Documents ID format: {@code root:path/to/file}. + */ public class ExternalStorageProvider extends FileSystemProvider { private static final String TAG = "ExternalStorage"; @@ -75,7 +89,12 @@ public class ExternalStorageProvider extends FileSystemProvider { private static final Uri BASE_URI = new Uri.Builder().scheme(ContentResolver.SCHEME_CONTENT).authority(AUTHORITY).build(); - // docId format: root:path/to/file + /** + * Regex for detecting {@code /Android/data/}, {@code /Android/obb/} and + * {@code /Android/sandbox/} along with all their subdirectories and content. + */ + private static final Pattern PATTERN_RESTRICTED_ANDROID_SUBTREES = + Pattern.compile("^Android/(?:data|obb|sandbox)(?:/.+)?", CASE_INSENSITIVE); private static final String[] DEFAULT_ROOT_PROJECTION = new String[] { Root.COLUMN_ROOT_ID, Root.COLUMN_FLAGS, Root.COLUMN_ICON, Root.COLUMN_TITLE, @@ -278,76 +297,91 @@ public class ExternalStorageProvider extends FileSystemProvider { return projection != null ? projection : DEFAULT_ROOT_PROJECTION; } + /** + * Mark {@code Android/data/}, {@code Android/obb/} and {@code Android/sandbox/} on the + * integrated shared ("external") storage along with all their content and subdirectories as + * hidden. + */ @Override - public Cursor queryChildDocumentsForManage( - String parentDocId, String[] projection, String sortOrder) - throws FileNotFoundException { - return queryChildDocumentsShowAll(parentDocId, projection, sortOrder); + protected boolean shouldHideDocument(@NonNull String documentId) { + // Don't need to hide anything on USB drives. + if (isOnRemovableUsbStorage(documentId)) { + return false; + } + + final String path = getPathFromDocId(documentId); + return PATTERN_RESTRICTED_ANDROID_SUBTREES.matcher(path).matches(); } /** * Check that the directory is the root of storage or blocked file from tree. + *

+ * Note, that this is different from hidden documents: blocked documents WILL appear + * the UI, but the user WILL NOT be able to select them. * - * @param docId the docId of the directory to be checked + * @param documentId the docId of the directory to be checked * @return true, should be blocked from tree. Otherwise, false. + * + * @see Document#FLAG_DIR_BLOCKS_OPEN_DOCUMENT_TREE */ @Override - protected boolean shouldBlockFromTree(@NonNull String docId) { - try { - final File dir = getFileForDocId(docId, false /* visible */); - - // the file is null or it is not a directory - if (dir == null || !dir.isDirectory()) { - return false; - } + protected boolean shouldBlockDirectoryFromTree(@NonNull String documentId) + throws FileNotFoundException { + final File dir = getFileForDocId(documentId, false); + // The file is null or it is not a directory + if (dir == null || !dir.isDirectory()) { + return false; + } - // Allow all directories on USB, including the root. - try { - RootInfo rootInfo = getRootFromDocId(docId); - if ((rootInfo.flags & Root.FLAG_REMOVABLE_USB) == Root.FLAG_REMOVABLE_USB) { - return false; - } - } catch (FileNotFoundException e) { - Log.e(TAG, "Failed to determine rootInfo for docId"); - } + // Allow all directories on USB, including the root. + if (isOnRemovableUsbStorage(documentId)) { + return false; + } - final String path = getPathFromDocId(docId); + // Get canonical(!) path. Note that this path will have neither leading nor training "/". + // This the root's path will be just an empty string. + final String path = getPathFromDocId(documentId); - // Block the root of the storage - if (path.isEmpty()) { - return true; - } + // Block the root of the storage + if (path.isEmpty()) { + return true; + } - // Block Download folder from tree - if (TextUtils.equals(Environment.DIRECTORY_DOWNLOADS.toLowerCase(Locale.ROOT), - path.toLowerCase(Locale.ROOT))) { - return true; - } + // Block /Download/ and /Android/ folders from the tree. + if (equalIgnoringCase(path, Environment.DIRECTORY_DOWNLOADS) || + equalIgnoringCase(path, Environment.DIRECTORY_ANDROID)) { + return true; + } - // Block /Android - if (TextUtils.equals(Environment.DIRECTORY_ANDROID.toLowerCase(Locale.ROOT), - path.toLowerCase(Locale.ROOT))) { - return true; - } + // This shouldn't really make a difference, but just in case - let's block hidden + // directories as well. + if (shouldHideDocument(documentId)) { + return true; + } - // Block /Android/data, /Android/obb, /Android/sandbox and sub dirs - if (shouldHide(dir)) { - return true; - } + return false; + } + private boolean isOnRemovableUsbStorage(@NonNull String documentId) { + final RootInfo rootInfo; + try { + rootInfo = getRootFromDocId(documentId); + } catch (FileNotFoundException e) { + Log.e(TAG, "Failed to determine rootInfo for docId\"" + documentId + '"'); return false; - } catch (IOException e) { - throw new IllegalArgumentException( - "Failed to determine if " + docId + " should block from tree " + ": " + e); } + + return (rootInfo.flags & Root.FLAG_REMOVABLE_USB) != 0; } + @NonNull @Override - protected String getDocIdForFile(File file) throws FileNotFoundException { + protected String getDocIdForFile(@NonNull File file) throws FileNotFoundException { return getDocIdForFileMaybeCreate(file, false); } - private String getDocIdForFileMaybeCreate(File file, boolean createNewDir) + @NonNull + private String getDocIdForFileMaybeCreate(@NonNull File file, boolean createNewDir) throws FileNotFoundException { String path = file.getAbsolutePath(); @@ -417,31 +451,33 @@ public class ExternalStorageProvider extends FileSystemProvider { private File getFileForDocId(String docId, boolean visible, boolean mustExist) throws FileNotFoundException { RootInfo root = getRootFromDocId(docId); - return buildFile(root, docId, visible, mustExist); + return buildFile(root, docId, mustExist); } - private Pair resolveDocId(String docId, boolean visible) - throws FileNotFoundException { + private Pair resolveDocId(String docId) throws FileNotFoundException { RootInfo root = getRootFromDocId(docId); - return Pair.create(root, buildFile(root, docId, visible, true)); + return Pair.create(root, buildFile(root, docId, /* mustExist */ true)); } @VisibleForTesting - static String getPathFromDocId(String docId) throws IOException { + static String getPathFromDocId(String docId) { final int splitIndex = docId.indexOf(':', 1); final String docIdPath = docId.substring(splitIndex + 1); - // Get CanonicalPath and remove the first "/" - final String canonicalPath = new File(docIdPath).getCanonicalPath().substring(1); - if (canonicalPath.isEmpty()) { - return canonicalPath; + // Canonicalize path and strip the leading "/" + final String path; + try { + path = new File(docIdPath).getCanonicalPath().substring(1); + } catch (IOException e) { + Log.w(TAG, "Could not canonicalize \"" + docIdPath + '"'); + return ""; } - // remove trailing "/" - if (canonicalPath.charAt(canonicalPath.length() - 1) == '/') { - return canonicalPath.substring(0, canonicalPath.length() - 1); + // Remove the trailing "/" as well. + if (!path.isEmpty() && path.charAt(path.length() - 1) == '/') { + return path.substring(0, path.length() - 1); } else { - return canonicalPath; + return path; } } @@ -460,7 +496,7 @@ public class ExternalStorageProvider extends FileSystemProvider { return root; } - private File buildFile(RootInfo root, String docId, boolean visible, boolean mustExist) + private File buildFile(RootInfo root, String docId, boolean mustExist) throws FileNotFoundException { final int splitIndex = docId.indexOf(':', 1); final String path = docId.substring(splitIndex + 1); @@ -544,7 +580,7 @@ public class ExternalStorageProvider extends FileSystemProvider { @Override public Path findDocumentPath(@Nullable String parentDocId, String childDocId) throws FileNotFoundException { - final Pair resolvedDocId = resolveDocId(childDocId, false); + final Pair resolvedDocId = resolveDocId(childDocId); final RootInfo root = resolvedDocId.first; File child = resolvedDocId.second; @@ -648,6 +684,13 @@ public class ExternalStorageProvider extends FileSystemProvider { } } + /** + * Print the state into the given stream. + * Gets invoked when you run: + * 

+     * adb shell dumpsys activity provider com.android.externalstorage/.ExternalStorageProvider
+     *
+ */ @Override public void dump(FileDescriptor fd, PrintWriter writer, String[] args) { final IndentingPrintWriter pw = new IndentingPrintWriter(writer, " ", 160); @@ -731,4 +774,8 @@ public class ExternalStorageProvider extends FileSystemProvider { } return bundle; } + + private static boolean equalIgnoringCase(@NonNull String a, @NonNull String b) { + return TextUtils.equals(a.toLowerCase(Locale.ROOT), b.toLowerCase(Locale.ROOT)); + } } -- cgit v1.2.3 From c116526ea280eeaf0d4c042a458a78839b93676d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabi=C3=A1n=20Kozynski?= Date: Fri, 13 Oct 2023 16:19:27 -0400 Subject: Unbind TileService onNullBinding Test: atest TileLifecycleManagerTest Test: manual: adb shell dumpsys activity service Test: sts test Bug: 300903792 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3b93880c0b0052fe03c781a9768b81b098a353c0) Merged-In: Ia8126ac65432b124683960e3ebf47301ba6172a1 Change-Id: Ia8126ac65432b124683960e3ebf47301ba6172a1 --- .../systemui/qs/external/TileLifecycleManager.java | 5 +++++ .../qs/external/TileLifecycleManagerTest.java | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java b/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java index d39368012487..35d4c23af879 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java +++ b/packages/SystemUI/src/com/android/systemui/qs/external/TileLifecycleManager.java @@ -251,6 +251,11 @@ public class TileLifecycleManager extends BroadcastReceiver implements handlePendingMessages(); } + @Override + public void onNullBinding(ComponentName name) { + setBindService(false); + } + @Override public void onServiceDisconnected(ComponentName name) { if (DEBUG) Log.d(TAG, "onServiceDisconnected " + name); diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java index 04b50d8d98c1..09f612fff16b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/external/TileLifecycleManagerTest.java @@ -290,6 +290,27 @@ public class TileLifecycleManagerTest extends SysuiTestCase { verify(falseContext).unbindService(captor.getValue()); } + @Test + public void testNullBindingCallsUnbind() { + Context mockContext = mock(Context.class); + // Binding has to succeed + when(mockContext.bindServiceAsUser(any(), any(), anyInt(), any())).thenReturn(true); + TileLifecycleManager manager = new TileLifecycleManager(mHandler, mockContext, + mock(IQSService.class), + mMockPackageManagerAdapter, + mMockBroadcastDispatcher, + mTileServiceIntent, + mUser); + + manager.setBindService(true); + + ArgumentCaptor captor = ArgumentCaptor.forClass(ServiceConnection.class); + verify(mockContext).bindServiceAsUser(any(), captor.capture(), anyInt(), any()); + + captor.getValue().onNullBinding(mTileServiceComponentName); + verify(mockContext).unbindService(captor.getValue()); + } + private static class TestContextWrapper extends ContextWrapper { private IntentFilter mLastIntentFilter; private int mLastFlag; -- cgit v1.2.3 From bb031761317d08f5ca44de39ad5188aed983f9cd Mon Sep 17 00:00:00 2001 From: Christophe Pinelli Date: Tue, 16 May 2023 17:43:49 +0000 Subject: Restrict activity launch when caller is running in the background Test: test on device + atest-src BackgroundActivityLaunchTest#testBackgroundActivityBlockedInStartNextMatchingActivity Bug: 230492947 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3e9da3ec4705b072dbe8a10e8ffc841f4928381c) Merged-In: Ie774b142a7fab12d596ccd64872b781e3825e9ba Change-Id: Ie774b142a7fab12d596ccd64872b781e3825e9ba --- .../server/wm/ActivityTaskManagerService.java | 53 +++++++++++++--------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index 704ab31ef8a9..4d0ff32d3b15 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -1416,29 +1416,38 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { final long origId = Binder.clearCallingIdentity(); // TODO(b/64750076): Check if calling pid should really be -1. - final int res = getActivityStartController() - .obtainStarter(intent, "startNextMatchingActivity") - .setCaller(r.app.getThread()) - .setResolvedType(r.resolvedType) - .setActivityInfo(aInfo) - .setResultTo(resultTo != null ? resultTo.token : null) - .setResultWho(resultWho) - .setRequestCode(requestCode) - .setCallingPid(-1) - .setCallingUid(r.launchedFromUid) - .setCallingPackage(r.launchedFromPackage) - .setCallingFeatureId(r.launchedFromFeatureId) - .setRealCallingPid(-1) - .setRealCallingUid(r.launchedFromUid) - .setActivityOptions(options) - .execute(); - Binder.restoreCallingIdentity(origId); - - r.finishing = wasFinishing; - if (res != ActivityManager.START_SUCCESS) { - return false; + try { + if (options == null) { + options = new SafeActivityOptions(ActivityOptions.makeBasic()); + } + // Fixes b/230492947 + // Prevents background activity launch through #startNextMatchingActivity + // An activity going into the background could still go back to the foreground + // if the intent used matches both: + // - the activity in the background + // - a second activity. + options.getOptions(r).setAvoidMoveToFront(); + final int res = getActivityStartController() + .obtainStarter(intent, "startNextMatchingActivity") + .setCaller(r.app.getThread()) + .setResolvedType(r.resolvedType) + .setActivityInfo(aInfo) + .setResultTo(resultTo != null ? resultTo.token : null) + .setResultWho(resultWho) + .setRequestCode(requestCode) + .setCallingPid(-1) + .setCallingUid(r.launchedFromUid) + .setCallingPackage(r.launchedFromPackage) + .setCallingFeatureId(r.launchedFromFeatureId) + .setRealCallingPid(-1) + .setRealCallingUid(r.launchedFromUid) + .setActivityOptions(options) + .execute(); + r.finishing = wasFinishing; + return res == ActivityManager.START_SUCCESS; + } finally { + Binder.restoreCallingIdentity(origId); } - return true; } } -- cgit v1.2.3 From 910fb9cb06e1b218681f8a1ffa84e24c1b592079 Mon Sep 17 00:00:00 2001 From: Tim Yu Date: Tue, 12 Sep 2023 21:11:31 +0000 Subject: [RESTRICT AUTOMERGE] Check permission of Autofill icon URIs * SaveUI's template * Inline Suggestions slices Fixes: b/286235483 Fixes: b/292104015 Test: atest CtsAutoFillServiceTestCases (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0698b8fe72c06686fb020b2608f5746f76869e1a) Merged-In: I48879174664b70ced90492bb0991dc91cbf89b79 Change-Id: I48879174664b70ced90492bb0991dc91cbf89b79 --- .../java/com/android/server/autofill/Helper.java | 48 +++++++++++++++++++++- .../ui/RemoteInlineSuggestionViewConnector.java | 18 ++++---- .../com/android/server/autofill/ui/SaveUi.java | 3 +- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/services/autofill/java/com/android/server/autofill/Helper.java b/services/autofill/java/com/android/server/autofill/Helper.java index 48113a81cca5..5f36496a2284 100644 --- a/services/autofill/java/com/android/server/autofill/Helper.java +++ b/services/autofill/java/com/android/server/autofill/Helper.java @@ -23,7 +23,10 @@ import android.app.ActivityManager; import android.app.assist.AssistStructure; import android.app.assist.AssistStructure.ViewNode; import android.app.assist.AssistStructure.WindowNode; +import android.app.slice.Slice; +import android.app.slice.SliceItem; import android.content.ComponentName; +import android.graphics.drawable.Icon; import android.metrics.LogMaker; import android.service.autofill.Dataset; import android.service.autofill.InternalSanitizer; @@ -47,7 +50,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.concurrent.atomic.AtomicBoolean; - public final class Helper { private static final String TAG = "AutofillHelper"; @@ -85,7 +87,7 @@ public final class Helper { final AtomicBoolean permissionsOk = new AtomicBoolean(true); rView.visitUris(uri -> { - int uriOwnerId = android.content.ContentProvider.getUserIdFromUri(uri); + int uriOwnerId = android.content.ContentProvider.getUserIdFromUri(uri, userId); boolean allowed = uriOwnerId == userId; permissionsOk.set(allowed && permissionsOk.get()); }); @@ -117,6 +119,48 @@ public final class Helper { return (ok ? rView : null); } + /** + * Checks the URI permissions of the icon in the slice, to see if the current userId is able to + * access it. + * + *

Returns null if slice contains user inaccessible icons + * + *

TODO: instead of returning a null Slice when the current userId cannot access an icon, + * return a reconstructed Slice without the icons. This is currently non-trivial since there are + * no public methods to generically add SliceItems to Slices + */ + public static @Nullable Slice sanitizeSlice(Slice slice) { + if (slice == null) { + return null; + } + + int userId = ActivityManager.getCurrentUser(); + + // Recontruct the Slice, filtering out bad icons + for (SliceItem sliceItem : slice.getItems()) { + if (!sliceItem.getFormat().equals(SliceItem.FORMAT_IMAGE)) { + // Not an image slice + continue; + } + + Icon icon = sliceItem.getIcon(); + if (icon.getType() != Icon.TYPE_URI + && icon.getType() != Icon.TYPE_URI_ADAPTIVE_BITMAP) { + // No URIs to sanitize + continue; + } + + int iconUriId = android.content.ContentProvider.getUserIdFromUri(icon.getUri(), userId); + + if (iconUriId != userId) { + Slog.w(TAG, "sanitizeSlice() user: " + userId + " cannot access icons in Slice"); + return null; + } + } + + return slice; + } + @Nullable static AutofillId[] toArray(@Nullable ArraySet set) { diff --git a/services/autofill/java/com/android/server/autofill/ui/RemoteInlineSuggestionViewConnector.java b/services/autofill/java/com/android/server/autofill/ui/RemoteInlineSuggestionViewConnector.java index 46d435d8811d..83caf743a39a 100644 --- a/services/autofill/java/com/android/server/autofill/ui/RemoteInlineSuggestionViewConnector.java +++ b/services/autofill/java/com/android/server/autofill/ui/RemoteInlineSuggestionViewConnector.java @@ -27,6 +27,7 @@ import android.service.autofill.InlinePresentation; import android.util.Slog; import com.android.server.LocalServices; +import com.android.server.autofill.Helper; import com.android.server.autofill.RemoteInlineSuggestionRenderService; import com.android.server.inputmethod.InputMethodManagerInternal; @@ -39,12 +40,9 @@ import java.util.function.Consumer; final class RemoteInlineSuggestionViewConnector { private static final String TAG = RemoteInlineSuggestionViewConnector.class.getSimpleName(); - @Nullable - private final RemoteInlineSuggestionRenderService mRemoteRenderService; - @NonNull - private final InlinePresentation mInlinePresentation; - @Nullable - private final IBinder mHostInputToken; + @Nullable private final RemoteInlineSuggestionRenderService mRemoteRenderService; + @NonNull private final InlinePresentation mInlinePresentation; + @Nullable private final IBinder mHostInputToken; private final int mDisplayId; private final int mUserId; private final int mSessionId; @@ -78,8 +76,12 @@ final class RemoteInlineSuggestionViewConnector { * * @return true if the call is made to the remote renderer service, false otherwise. */ - public boolean renderSuggestion(int width, int height, - @NonNull IInlineSuggestionUiCallback callback) { + public boolean renderSuggestion( + int width, int height, @NonNull IInlineSuggestionUiCallback callback) { + if (Helper.sanitizeSlice(mInlinePresentation.getSlice()) == null) { + if (sDebug) Slog.d(TAG, "Skipped rendering inline suggestion."); + return false; + } if (mRemoteRenderService != null) { if (sDebug) Slog.d(TAG, "Request to recreate the UI"); mRemoteRenderService.renderSuggestion(callback, mInlinePresentation, width, height, diff --git a/services/autofill/java/com/android/server/autofill/ui/SaveUi.java b/services/autofill/java/com/android/server/autofill/ui/SaveUi.java index 533a7b69a650..47ac0ce704a7 100644 --- a/services/autofill/java/com/android/server/autofill/ui/SaveUi.java +++ b/services/autofill/java/com/android/server/autofill/ui/SaveUi.java @@ -427,7 +427,8 @@ final class SaveUi { } final BatchUpdates batchUpdates = pair.second; // First apply the updates... - final RemoteViews templateUpdates = batchUpdates.getUpdates(); + final RemoteViews templateUpdates = + Helper.sanitizeRemoteView(batchUpdates.getUpdates()); if (templateUpdates != null) { if (sDebug) Slog.d(TAG, "Applying template updates for batch update #" + i); templateUpdates.reapply(context, customSubtitleView); -- cgit v1.2.3 From af56cc691d21c7dea75d8781fd3938ac5077aaa9 Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Wed, 8 Nov 2023 11:01:32 +0100 Subject: Enforce persisted snoozed notifications limits Prevent DoS attack that causes boot-looping by serializing a huge amount of snoozed notifications: - Check snooze limits for persisted notifications - Remove persisted group summary notification when in-memory counterpart is removed - Prevent unpriviledged API calls that allow 3P apps to snooze notifications with context/criterion Test: atest SnoozeHelperTest Test: atest NotificationManagerServiceTest Bug: 307948424 Bug: 308414141 (cherry picked from commit 965ff2d3c5487f72a77f6153ed8542cb2621d93c) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ade22bfdf6698cb97b4edc303e8952d6cc1a2f73) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:98a9a914bdc00091c15688a04df094cf2669d28c) Merged-In: I3571fa9207b778def652130d3ca840183a9a8414 Change-Id: I3571fa9207b778def652130d3ca840183a9a8414 --- .../android/server/notification/SnoozeHelper.java | 23 ++++- .../server/notification/SnoozeHelperTest.java | 104 ++++++++++++++++++++- 2 files changed, 124 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index 12c16fc43bd3..73d4b147f90a 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -118,13 +118,29 @@ public final class SnoozeHelper { protected boolean canSnooze(int numberToSnooze) { synchronized (mLock) { - if ((mSnoozedNotifications.size() + numberToSnooze) > CONCURRENT_SNOOZE_LIMIT) { + if ((mSnoozedNotifications.size() + numberToSnooze) > CONCURRENT_SNOOZE_LIMIT + || (countPersistedNotificationsLocked() + numberToSnooze) + > CONCURRENT_SNOOZE_LIMIT) { return false; } } return true; } + private int countPersistedNotificationsLocked() { + int numNotifications = 0; + for (ArrayMap persistedWithContext : + mPersistedSnoozedNotificationsWithContext.values()) { + numNotifications += persistedWithContext.size(); + } + for (ArrayMap persistedWithDuration : + mPersistedSnoozedNotifications.values()) { + numNotifications += persistedWithDuration.size(); + } + return numNotifications; + } + + @NonNull protected Long getSnoozeTimeForUnpostedNotification(int userId, String pkg, String key) { Long time = null; @@ -344,6 +360,11 @@ public final class SnoozeHelper { if (groupSummaryKey != null) { NotificationRecord record = mSnoozedNotifications.remove(groupSummaryKey); + final String trimmedKey = getTrimmedString(groupSummaryKey); + removeRecordLocked(pkg, trimmedKey, userId, mPersistedSnoozedNotifications); + removeRecordLocked(pkg, trimmedKey, userId, + mPersistedSnoozedNotificationsWithContext); + if (record != null && !record.isCanceled) { Runnable runnable = () -> { MetricsLogger.action(record.getLogMaker() diff --git a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java index 9c3d5a7fb6d9..12590ac0d66a 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java @@ -18,6 +18,8 @@ package com.android.server.notification; import static com.android.server.notification.SnoozeHelper.CONCURRENT_SNOOZE_LIMIT; import static com.android.server.notification.SnoozeHelper.EXTRA_KEY; +import static com.google.common.truth.Truth.assertThat; + import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotNull; @@ -72,6 +74,16 @@ import java.io.IOException; public class SnoozeHelperTest extends UiServiceTestCase { private static final String TEST_CHANNEL_ID = "test_channel_id"; + private static final String XML_TAG_NAME = "snoozed-notifications"; + private static final String XML_SNOOZED_NOTIFICATION = "notification"; + private static final String XML_SNOOZED_NOTIFICATION_CONTEXT = "context"; + private static final String XML_SNOOZED_NOTIFICATION_KEY = "key"; + private static final String XML_SNOOZED_NOTIFICATION_TIME = "time"; + private static final String XML_SNOOZED_NOTIFICATION_CONTEXT_ID = "id"; + private static final String XML_SNOOZED_NOTIFICATION_VERSION_LABEL = "version"; + private static final String XML_SNOOZED_NOTIFICATION_PKG = "pkg"; + private static final String XML_SNOOZED_NOTIFICATION_USER_ID = "user-id"; + @Mock SnoozeHelper.Callback mCallback; @Mock AlarmManager mAm; @Mock ManagedServices.UserProfiles mUserProfiles; @@ -314,6 +326,56 @@ public class SnoozeHelperTest extends UiServiceTestCase { assertFalse(mSnoozeHelper.canSnooze(1)); } + @Test + public void testSnoozeLimit_maximumPersisted() throws XmlPullParserException, IOException { + final long snoozeTimeout = 1234; + final String snoozeContext = "ctx"; + // Serialize & deserialize notifications so that only persisted lists are used + TypedXmlSerializer serializer = Xml.newFastSerializer(); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + serializer.setOutput(new BufferedOutputStream(baos), "utf-8"); + serializer.startDocument(null, true); + serializer.startTag(null, XML_TAG_NAME); + // Serialize maximum number of timed + context snoozed notifications, half of each + for (int i = 0; i < CONCURRENT_SNOOZE_LIMIT; i++) { + final boolean timedNotification = i % 2 == 0; + if (timedNotification) { + serializer.startTag(null, XML_SNOOZED_NOTIFICATION); + } else { + serializer.startTag(null, XML_SNOOZED_NOTIFICATION_CONTEXT); + } + serializer.attribute(null, XML_SNOOZED_NOTIFICATION_PKG, "pkg"); + serializer.attributeInt(null, XML_SNOOZED_NOTIFICATION_USER_ID, + UserHandle.USER_SYSTEM); + serializer.attributeInt(null, XML_SNOOZED_NOTIFICATION_VERSION_LABEL, 1); + serializer.attribute(null, XML_SNOOZED_NOTIFICATION_KEY, "key" + i); + if (timedNotification) { + serializer.attributeLong(null, XML_SNOOZED_NOTIFICATION_TIME, snoozeTimeout); + serializer.endTag(null, XML_SNOOZED_NOTIFICATION); + } else { + serializer.attribute(null, XML_SNOOZED_NOTIFICATION_CONTEXT_ID, snoozeContext); + serializer.endTag(null, XML_SNOOZED_NOTIFICATION_CONTEXT); + } + } + serializer.endTag(null, XML_TAG_NAME); + serializer.endDocument(); + serializer.flush(); + + TypedXmlPullParser parser = Xml.newFastPullParser(); + parser.setInput(new BufferedInputStream( + new ByteArrayInputStream(baos.toByteArray())), "utf-8"); + mSnoozeHelper.readXml(parser, 1); + // Verify that we can't snooze any more notifications + // and that the limit is caused by persisted notifications + assertThat(mSnoozeHelper.canSnooze(1)).isFalse(); + assertThat(mSnoozeHelper.isSnoozed(UserHandle.USER_SYSTEM, "pkg", "key0")).isFalse(); + assertThat(mSnoozeHelper.getSnoozeTimeForUnpostedNotification(UserHandle.USER_SYSTEM, + "pkg", "key0")).isEqualTo(snoozeTimeout); + assertThat( + mSnoozeHelper.getSnoozeContextForUnpostedNotification(UserHandle.USER_SYSTEM, "pkg", + "key1")).isEqualTo(snoozeContext); + } + @Test public void testCancelByApp() throws Exception { NotificationRecord r = getNotificationRecord("pkg", 1, "one", UserHandle.SYSTEM); @@ -587,6 +649,7 @@ public class SnoozeHelperTest extends UiServiceTestCase { @Test public void repostGroupSummary_repostsSummary() throws Exception { + final int snoozeDuration = 1000; IntArray profileIds = new IntArray(); profileIds.add(UserHandle.USER_SYSTEM); when(mUserProfiles.getCurrentProfileIds()).thenReturn(profileIds); @@ -594,10 +657,44 @@ public class SnoozeHelperTest extends UiServiceTestCase { "pkg", 1, "one", UserHandle.SYSTEM, "group1", true); NotificationRecord r2 = getNotificationRecord( "pkg", 2, "two", UserHandle.SYSTEM, "group1", false); - mSnoozeHelper.snooze(r, 1000); - mSnoozeHelper.snooze(r2, 1000); + final long snoozeTime = System.currentTimeMillis() + snoozeDuration; + mSnoozeHelper.snooze(r, snoozeDuration); + mSnoozeHelper.snooze(r2, snoozeDuration); + assertEquals(2, mSnoozeHelper.getSnoozed().size()); + assertEquals(2, mSnoozeHelper.getSnoozed(UserHandle.USER_SYSTEM, "pkg").size()); + // Verify that summary notification was added to the persisted list + assertThat(mSnoozeHelper.getSnoozeTimeForUnpostedNotification(UserHandle.USER_SYSTEM, "pkg", + r.getKey())).isAtLeast(snoozeTime); + + mSnoozeHelper.repostGroupSummary("pkg", UserHandle.USER_SYSTEM, r.getGroupKey()); + + verify(mCallback, times(1)).repost(UserHandle.USER_SYSTEM, r, false); + verify(mCallback, never()).repost(UserHandle.USER_SYSTEM, r2, false); + + assertEquals(1, mSnoozeHelper.getSnoozed().size()); + assertEquals(1, mSnoozeHelper.getSnoozed(UserHandle.USER_SYSTEM, "pkg").size()); + // Verify that summary notification was removed from the persisted list + assertThat(mSnoozeHelper.getSnoozeTimeForUnpostedNotification(UserHandle.USER_SYSTEM, "pkg", + r.getKey())).isEqualTo(0); + } + + @Test + public void snoozeWithContext_repostGroupSummary_removesPersisted() throws Exception { + final String snoozeContext = "zzzzz"; + IntArray profileIds = new IntArray(); + profileIds.add(UserHandle.USER_SYSTEM); + when(mUserProfiles.getCurrentProfileIds()).thenReturn(profileIds); + NotificationRecord r = getNotificationRecord( + "pkg", 1, "one", UserHandle.SYSTEM, "group1", true); + NotificationRecord r2 = getNotificationRecord( + "pkg", 2, "two", UserHandle.SYSTEM, "group1", false); + mSnoozeHelper.snooze(r, snoozeContext); + mSnoozeHelper.snooze(r2, snoozeContext); assertEquals(2, mSnoozeHelper.getSnoozed().size()); assertEquals(2, mSnoozeHelper.getSnoozed(UserHandle.USER_SYSTEM, "pkg").size()); + // Verify that summary notification was added to the persisted list + assertThat(mSnoozeHelper.getSnoozeContextForUnpostedNotification(UserHandle.USER_SYSTEM, + "pkg", r.getKey())).isEqualTo(snoozeContext); mSnoozeHelper.repostGroupSummary("pkg", UserHandle.USER_SYSTEM, r.getGroupKey()); @@ -606,6 +703,9 @@ public class SnoozeHelperTest extends UiServiceTestCase { assertEquals(1, mSnoozeHelper.getSnoozed().size()); assertEquals(1, mSnoozeHelper.getSnoozed(UserHandle.USER_SYSTEM, "pkg").size()); + // Verify that summary notification was removed from the persisted list + assertThat(mSnoozeHelper.getSnoozeContextForUnpostedNotification(UserHandle.USER_SYSTEM, + "pkg", r.getKey())).isNull(); } @Test -- cgit v1.2.3 From f45668b56ffd2d0f5f0ea16bf604076425eee038 Mon Sep 17 00:00:00 2001 From: Vlad Popa Date: Mon, 26 Jun 2023 18:52:59 -0700 Subject: Refactor the SADeviceState to AdiDeviceState The idea is to have a device state catalog for all the known devices. Also refactored the name of the Settings.Secure key entry for persistence. The current code will check the legacy key first, erase it and update the new key. Test: atest SpatializerHelperTest & AudioDeviceBrokerTest Bug: 278265907 Bug: 285588444 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ada3f6041281b4441f63efe4a76e7a5c83e56dc7) Merged-In: Idabcc84cb0f5f6f88ba5aebc435511ab95016ef3 Change-Id: Idabcc84cb0f5f6f88ba5aebc435511ab95016ef3 --- core/java/android/provider/Settings.java | 7 + media/java/android/media/AudioSystem.java | 63 ++++ .../src/android/provider/SettingsBackupTest.java | 1 + .../com/android/server/audio/AdiDeviceState.java | 204 ++++++++++++ .../android/server/audio/AudioDeviceBroker.java | 104 +++++- .../android/server/audio/AudioDeviceInventory.java | 84 +++++ .../com/android/server/audio/AudioService.java | 56 +--- .../android/server/audio/SpatializerHelper.java | 355 +++++++-------------- .../server/audio/AudioDeviceBrokerTest.java | 39 ++- .../server/audio/SpatializerHelperTest.java | 86 ++--- 10 files changed, 635 insertions(+), 364 deletions(-) create mode 100644 services/core/java/com/android/server/audio/AdiDeviceState.java diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 8d8379831e87..f0324348c9ea 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -9525,6 +9525,13 @@ public final class Settings { */ public static final String SPATIAL_AUDIO_ENABLED = "spatial_audio_enabled"; + /** + * Internal collection of audio device inventory items + * The device item stored are {@link com.android.server.audio.AdiDeviceState} + * @hide + */ + public static final String AUDIO_DEVICE_INVENTORY = "audio_device_inventory"; + /** * Indicates whether notification display on the lock screen is enabled. *

diff --git a/media/java/android/media/AudioSystem.java b/media/java/android/media/AudioSystem.java index 7ccbe51bbd49..0134b2e23815 100644 --- a/media/java/android/media/AudioSystem.java +++ b/media/java/android/media/AudioSystem.java @@ -1217,6 +1217,9 @@ public class AudioSystem public static final Set DEVICE_IN_ALL_SCO_SET; /** @hide */ public static final Set DEVICE_IN_ALL_USB_SET; + /** @hide */ + public static final Set DEVICE_IN_ALL_BLE_SET; + static { DEVICE_IN_ALL_SET = new HashSet<>(); DEVICE_IN_ALL_SET.add(DEVICE_IN_COMMUNICATION); @@ -1256,6 +1259,66 @@ public class AudioSystem DEVICE_IN_ALL_USB_SET.add(DEVICE_IN_USB_ACCESSORY); DEVICE_IN_ALL_USB_SET.add(DEVICE_IN_USB_DEVICE); DEVICE_IN_ALL_USB_SET.add(DEVICE_IN_USB_HEADSET); + + DEVICE_IN_ALL_BLE_SET = new HashSet<>(); + DEVICE_IN_ALL_BLE_SET.add(DEVICE_IN_BLE_HEADSET); + } + + /** @hide */ + public static boolean isBluetoothDevice(int deviceType) { + return isBluetoothA2dpOutDevice(deviceType) + || isBluetoothScoDevice(deviceType) + || isBluetoothLeDevice(deviceType); + } + + /** @hide */ + public static boolean isBluetoothOutDevice(int deviceType) { + return isBluetoothA2dpOutDevice(deviceType) + || isBluetoothScoOutDevice(deviceType) + || isBluetoothLeOutDevice(deviceType); + } + + /** @hide */ + public static boolean isBluetoothInDevice(int deviceType) { + return isBluetoothScoInDevice(deviceType) + || isBluetoothLeInDevice(deviceType); + } + + /** @hide */ + public static boolean isBluetoothA2dpOutDevice(int deviceType) { + return DEVICE_OUT_ALL_A2DP_SET.contains(deviceType); + } + + /** @hide */ + public static boolean isBluetoothScoOutDevice(int deviceType) { + return DEVICE_OUT_ALL_SCO_SET.contains(deviceType); + } + + /** @hide */ + public static boolean isBluetoothScoInDevice(int deviceType) { + return DEVICE_IN_ALL_SCO_SET.contains(deviceType); + } + + /** @hide */ + public static boolean isBluetoothScoDevice(int deviceType) { + return isBluetoothScoOutDevice(deviceType) + || isBluetoothScoInDevice(deviceType); + } + + /** @hide */ + public static boolean isBluetoothLeOutDevice(int deviceType) { + return DEVICE_OUT_ALL_BLE_SET.contains(deviceType); + } + + /** @hide */ + public static boolean isBluetoothLeInDevice(int deviceType) { + return DEVICE_IN_ALL_BLE_SET.contains(deviceType); + } + + /** @hide */ + public static boolean isBluetoothLeDevice(int deviceType) { + return isBluetoothLeOutDevice(deviceType) + || isBluetoothLeInDevice(deviceType); } /** @hide */ diff --git a/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java b/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java index 3c2aefdaa8fb..5e5ea68717a3 100644 --- a/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java +++ b/packages/SettingsProvider/test/src/android/provider/SettingsBackupTest.java @@ -690,6 +690,7 @@ public class SettingsBackupTest { Settings.Secure.ASSIST_SCREENSHOT_ENABLED, Settings.Secure.ASSIST_STRUCTURE_ENABLED, Settings.Secure.ATTENTIVE_TIMEOUT, + Settings.Secure.AUDIO_DEVICE_INVENTORY, // setting not controllable by user Settings.Secure.AUTOFILL_FEATURE_FIELD_CLASSIFICATION, Settings.Secure.AUTOFILL_USER_DATA_MAX_CATEGORY_COUNT, Settings.Secure.AUTOFILL_USER_DATA_MAX_FIELD_CLASSIFICATION_IDS_SIZE, diff --git a/services/core/java/com/android/server/audio/AdiDeviceState.java b/services/core/java/com/android/server/audio/AdiDeviceState.java new file mode 100644 index 000000000000..683b3eb7a92e --- /dev/null +++ b/services/core/java/com/android/server/audio/AdiDeviceState.java @@ -0,0 +1,204 @@ +/* + * Copyright (C) 2023 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 com.android.server.audio; + +import static android.media.AudioSystem.DEVICE_NONE; +import static android.media.AudioSystem.isBluetoothDevice; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.media.AudioDeviceAttributes; +import android.media.AudioDeviceInfo; +import android.text.TextUtils; +import android.util.Log; + +import java.util.Objects; + +/** + * Class representing all devices that were previously or are currently connected. Data is + * persisted in {@link android.provider.Settings.Secure} + */ +/*package*/ final class AdiDeviceState { + private static final String TAG = "AS.AdiDeviceState"; + + private static final String SETTING_FIELD_SEPARATOR = ","; + + @AudioDeviceInfo.AudioDeviceType + private final int mDeviceType; + + private final int mInternalDeviceType; + @NonNull + private final String mDeviceAddress; + private boolean mSAEnabled; + private boolean mHasHeadTracker = false; + private boolean mHeadTrackerEnabled; + + /** + * Constructor + * + * @param deviceType external audio device type + * @param internalDeviceType if not set pass {@link DEVICE_NONE}, in this case the + * default conversion of the external type will be used + * @param address must be non-null for wireless devices + * @throws NullPointerException if a null address is passed for a wireless device + */ + AdiDeviceState(@AudioDeviceInfo.AudioDeviceType int deviceType, + int internalDeviceType, + @Nullable String address) { + mDeviceType = deviceType; + if (internalDeviceType != DEVICE_NONE) { + mInternalDeviceType = internalDeviceType; + } else { + mInternalDeviceType = AudioDeviceInfo.convertDeviceTypeToInternalDevice(deviceType); + + } + mDeviceAddress = isBluetoothDevice(mInternalDeviceType) ? Objects.requireNonNull( + address) : ""; + } + + @AudioDeviceInfo.AudioDeviceType + public int getDeviceType() { + return mDeviceType; + } + + public int getInternalDeviceType() { + return mInternalDeviceType; + } + + @NonNull + public String getDeviceAddress() { + return mDeviceAddress; + } + + public void setSAEnabled(boolean sAEnabled) { + mSAEnabled = sAEnabled; + } + + public boolean isSAEnabled() { + return mSAEnabled; + } + + public void setHeadTrackerEnabled(boolean headTrackerEnabled) { + mHeadTrackerEnabled = headTrackerEnabled; + } + + public boolean isHeadTrackerEnabled() { + return mHeadTrackerEnabled; + } + + public void setHasHeadTracker(boolean hasHeadTracker) { + mHasHeadTracker = hasHeadTracker; + } + + + public boolean hasHeadTracker() { + return mHasHeadTracker; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + // type check and cast + if (getClass() != obj.getClass()) { + return false; + } + final AdiDeviceState sads = (AdiDeviceState) obj; + return mDeviceType == sads.mDeviceType + && mInternalDeviceType == sads.mInternalDeviceType + && mDeviceAddress.equals(sads.mDeviceAddress) // NonNull + && mSAEnabled == sads.mSAEnabled + && mHasHeadTracker == sads.mHasHeadTracker + && mHeadTrackerEnabled == sads.mHeadTrackerEnabled; + } + + @Override + public int hashCode() { + return Objects.hash(mDeviceType, mInternalDeviceType, mDeviceAddress, mSAEnabled, + mHasHeadTracker, mHeadTrackerEnabled); + } + + @Override + public String toString() { + return "type: " + mDeviceType + "internal type: " + mInternalDeviceType + + " addr: " + mDeviceAddress + " enabled: " + mSAEnabled + + " HT: " + mHasHeadTracker + " HTenabled: " + mHeadTrackerEnabled; + } + + public String toPersistableString() { + return (new StringBuilder().append(mDeviceType) + .append(SETTING_FIELD_SEPARATOR).append(mDeviceAddress) + .append(SETTING_FIELD_SEPARATOR).append(mSAEnabled ? "1" : "0") + .append(SETTING_FIELD_SEPARATOR).append(mHasHeadTracker ? "1" : "0") + .append(SETTING_FIELD_SEPARATOR).append(mHeadTrackerEnabled ? "1" : "0") + .append(SETTING_FIELD_SEPARATOR).append(mInternalDeviceType) + .toString()); + } + + /** + * Gets the max size (including separators) when persisting the elements with + * {@link AdiDeviceState#toPersistableString()}. + */ + public static int getPeristedMaxSize() { + return 36; /* (mDeviceType)2 + (mDeviceAddresss)17 + (mInternalDeviceType)9 + (mSAEnabled)1 + + (mHasHeadTracker)1 + (mHasHeadTrackerEnabled)1 + + (SETTINGS_FIELD_SEPARATOR)5 */ + } + + @Nullable + public static AdiDeviceState fromPersistedString(@Nullable String persistedString) { + if (persistedString == null) { + return null; + } + if (persistedString.isEmpty()) { + return null; + } + String[] fields = TextUtils.split(persistedString, SETTING_FIELD_SEPARATOR); + // we may have 5 fields for the legacy AdiDeviceState and 6 containing the internal + // device type + if (fields.length != 5 && fields.length != 6) { + // expecting all fields, fewer may mean corruption, ignore those settings + return null; + } + try { + final int deviceType = Integer.parseInt(fields[0]); + int internalDeviceType = -1; + if (fields.length == 6) { + internalDeviceType = Integer.parseInt(fields[5]); + } + final AdiDeviceState deviceState = new AdiDeviceState(deviceType, + internalDeviceType, fields[1]); + deviceState.setHasHeadTracker(Integer.parseInt(fields[2]) == 1); + deviceState.setHasHeadTracker(Integer.parseInt(fields[3]) == 1); + deviceState.setHeadTrackerEnabled(Integer.parseInt(fields[4]) == 1); + return deviceState; + } catch (NumberFormatException e) { + Log.e(TAG, "unable to parse setting for AdiDeviceState: " + persistedString, e); + return null; + } + } + + public AudioDeviceAttributes getAudioDeviceAttributes() { + return new AudioDeviceAttributes(AudioDeviceAttributes.ROLE_OUTPUT, + mDeviceType, mDeviceAddress); + } + +} diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index 6410142278b5..ece6ff6e3ab5 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -49,6 +49,7 @@ import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.SystemClock; import android.os.UserHandle; +import android.provider.Settings; import android.text.TextUtils; import android.util.Log; import android.util.PrintWriterPrinter; @@ -67,8 +68,11 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; -/** @hide */ -/*package*/ final class AudioDeviceBroker { +/** + * @hide + * (non final for mocking/spying) + */ +public class AudioDeviceBroker { private static final String TAG = "AS.AudioDeviceBroker"; @@ -1588,6 +1592,9 @@ import java.util.concurrent.atomic.AtomicBoolean; final int capturePreset = msg.arg1; mDeviceInventory.onSaveClearPreferredDevicesForCapturePreset(capturePreset); } break; + case MSG_PERSIST_AUDIO_DEVICE_SETTINGS: + onPersistAudioDeviceSettings(); + break; default: Log.wtf(TAG, "Invalid message " + msg.what); } @@ -1662,6 +1669,8 @@ import java.util.concurrent.atomic.AtomicBoolean; private static final int MSG_IL_BTLEAUDIO_TIMEOUT = 49; + private static final int MSG_PERSIST_AUDIO_DEVICE_SETTINGS = 54; + private static boolean isMessageHandledUnderWakelock(int msgId) { switch(msgId) { case MSG_L_SET_WIRED_DEVICE_CONNECTION_STATE: @@ -2081,4 +2090,95 @@ import java.util.concurrent.atomic.AtomicBoolean; return mDeviceInventory.getDeviceSensorUuid(device); } } + + /** + * post a message to persist the audio device settings. + * Message is delayed by 1s on purpose in case of successive changes in quick succession (at + * init time for instance) + * Note this method is made public to work around a Mockito bug where it needs to be public + * in order to be mocked by a test a the same package + * (see https://code.google.com/archive/p/mockito/issues/127) + */ + public void persistAudioDeviceSettings() { + sendMsg(MSG_PERSIST_AUDIO_DEVICE_SETTINGS, SENDMSG_REPLACE, /*delay*/ 1000); + } + + void onPersistAudioDeviceSettings() { + final String deviceSettings = mDeviceInventory.getDeviceSettings(); + Log.v(TAG, "saving audio device settings: " + deviceSettings); + final SettingsAdapter settings = mAudioService.getSettings(); + boolean res = settings.putSecureStringForUser(mAudioService.getContentResolver(), + Settings.Secure.AUDIO_DEVICE_INVENTORY, + deviceSettings, UserHandle.USER_CURRENT); + if (!res) { + Log.e(TAG, "error saving audio device settings: " + deviceSettings); + } + } + + void onReadAudioDeviceSettings() { + final SettingsAdapter settingsAdapter = mAudioService.getSettings(); + final ContentResolver contentResolver = mAudioService.getContentResolver(); + String settings = settingsAdapter.getSecureStringForUser(contentResolver, + Settings.Secure.AUDIO_DEVICE_INVENTORY, UserHandle.USER_CURRENT); + if (settings == null) { + Log.i(TAG, "reading spatial audio device settings from legacy key" + + Settings.Secure.SPATIAL_AUDIO_ENABLED); + // legacy string format for key SPATIAL_AUDIO_ENABLED has the same order of fields like + // the strings for key AUDIO_DEVICE_INVENTORY. This will ensure to construct valid + // device settings when calling {@link #setDeviceSettings()} + settings = settingsAdapter.getSecureStringForUser(contentResolver, + Settings.Secure.SPATIAL_AUDIO_ENABLED, UserHandle.USER_CURRENT); + if (settings == null) { + Log.i(TAG, "no spatial audio device settings stored with legacy key"); + } else if (!settings.equals("")) { + // Delete old key value and update the new key + if (!settingsAdapter.putSecureStringForUser(contentResolver, + Settings.Secure.SPATIAL_AUDIO_ENABLED, + /*value=*/"", + UserHandle.USER_CURRENT)) { + Log.w(TAG, "cannot erase the legacy audio device settings with key " + + Settings.Secure.SPATIAL_AUDIO_ENABLED); + } + if (!settingsAdapter.putSecureStringForUser(contentResolver, + Settings.Secure.AUDIO_DEVICE_INVENTORY, + settings, + UserHandle.USER_CURRENT)) { + Log.e(TAG, "error updating the new audio device settings with key " + + Settings.Secure.AUDIO_DEVICE_INVENTORY); + } + } + } + + if (settings != null && !settings.equals("")) { + setDeviceSettings(settings); + } + } + + void setDeviceSettings(String settings) { + mDeviceInventory.setDeviceSettings(settings); + } + + /** Test only method. */ + String getDeviceSettings() { + return mDeviceInventory.getDeviceSettings(); + } + + List getImmutableDeviceInventory() { + return mDeviceInventory.getImmutableDeviceInventory(); + } + + void addDeviceStateToInventory(AdiDeviceState deviceState) { + mDeviceInventory.addDeviceStateToInventory(deviceState); + } + + AdiDeviceState findDeviceStateForAudioDeviceAttributes(AudioDeviceAttributes ada, + int canonicalType) { + return mDeviceInventory.findDeviceStateForAudioDeviceAttributes(ada, canonicalType); + } + + //------------------------------------------------ + // for testing purposes only + void clearDeviceInventory() { + mDeviceInventory.clearDeviceInventory(); + } } diff --git a/services/core/java/com/android/server/audio/AudioDeviceInventory.java b/services/core/java/com/android/server/audio/AudioDeviceInventory.java index a74f4154eb85..e94415f55ab7 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceInventory.java +++ b/services/core/java/com/android/server/audio/AudioDeviceInventory.java @@ -15,6 +15,8 @@ */ package com.android.server.audio; +import static android.media.AudioSystem.isBluetoothDevice; + import android.annotation.NonNull; import android.annotation.Nullable; import android.bluetooth.BluetoothAdapter; @@ -62,12 +64,54 @@ public class AudioDeviceInventory { private static final String TAG = "AS.AudioDeviceInventory"; + private static final String SETTING_DEVICE_SEPARATOR_CHAR = "|"; + private static final String SETTING_DEVICE_SEPARATOR = "\\|"; + // lock to synchronize all access to mConnectedDevices and mApmConnectedDevices private final Object mDevicesLock = new Object(); //Audio Analytics ids. private static final String mMetricsId = "audio.device."; + private final Object mDeviceInventoryLock = new Object(); + @GuardedBy("mDeviceInventoryLock") + private final ArrayList mDeviceInventory = new ArrayList<>(0); + + + List getImmutableDeviceInventory() { + synchronized (mDeviceInventoryLock) { + return List.copyOf(mDeviceInventory); + } + } + + void addDeviceStateToInventory(AdiDeviceState deviceState) { + synchronized (mDeviceInventoryLock) { + mDeviceInventory.add(deviceState); + } + } + + AdiDeviceState findDeviceStateForAudioDeviceAttributes(AudioDeviceAttributes ada, + int canonicalDeviceType) { + final boolean isWireless = isBluetoothDevice(ada.getInternalType()); + + synchronized (mDeviceInventoryLock) { + for (AdiDeviceState deviceSetting : mDeviceInventory) { + if (deviceSetting.getDeviceType() == canonicalDeviceType + && (!isWireless || ada.getAddress().equals( + deviceSetting.getDeviceAddress()))) { + return deviceSetting; + } + } + } + return null; + } + + void clearDeviceInventory() { + synchronized (mDeviceInventoryLock) { + mDeviceInventory.clear(); + } + } + // List of connected devices // Key for map created from DeviceInfo.makeDeviceListKey() @GuardedBy("mDevicesLock") @@ -263,6 +307,12 @@ public class AudioDeviceInventory { mPreferredDevicesForCapturePreset.forEach((capturePreset, devices) -> { pw.println(" " + prefix + "capturePreset:" + capturePreset + " devices:" + devices); }); + pw.println("\ndevices:\n"); + synchronized (mDeviceInventoryLock) { + for (AdiDeviceState device : mDeviceInventory) { + pw.println("\t" + device + "\n"); + } + } } //------------------------------------------------------------ @@ -1580,6 +1630,40 @@ public class AudioDeviceInventory { return null; } + /*package*/ String getDeviceSettings() { + int deviceCatalogSize = 0; + synchronized (mDeviceInventoryLock) { + deviceCatalogSize = mDeviceInventory.size(); + } + final StringBuilder settingsBuilder = new StringBuilder( + deviceCatalogSize * AdiDeviceState.getPeristedMaxSize()); + + synchronized (mDeviceInventoryLock) { + for (int i = 0; i < mDeviceInventory.size(); i++) { + settingsBuilder.append(mDeviceInventory.get(i).toPersistableString()); + if (i != mDeviceInventory.size() - 1) { + settingsBuilder.append(SETTING_DEVICE_SEPARATOR_CHAR); + } + } + } + return settingsBuilder.toString(); + } + + /*package*/ void setDeviceSettings(String settings) { + clearDeviceInventory(); + String[] devSettings = TextUtils.split(Objects.requireNonNull(settings), + SETTING_DEVICE_SEPARATOR); + // small list, not worth overhead of Arrays.stream(devSettings) + for (String setting : devSettings) { + AdiDeviceState devState = AdiDeviceState.fromPersistedString(setting); + // Note if the device is not compatible with spatialization mode or the device + // type is not canonical, it will be ignored in {@link SpatializerHelper}. + if (devState != null) { + addDeviceStateToInventory(devState); + } + } + } + //---------------------------------------------------------- // For tests only diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 3ddb0ae96a22..ea2a5f475235 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -372,7 +372,6 @@ public class AudioService extends IAudioService.Stub private static final int MSG_DISPATCH_AUDIO_MODE = 40; private static final int MSG_ROUTING_UPDATED = 41; private static final int MSG_INIT_HEADTRACKING_SENSORS = 42; - private static final int MSG_PERSIST_SPATIAL_AUDIO_DEVICE_SETTINGS = 43; private static final int MSG_ADD_ASSISTANT_SERVICE_UID = 44; private static final int MSG_REMOVE_ASSISTANT_SERVICE_UID = 45; private static final int MSG_UPDATE_ACTIVE_ASSISTANT_SERVICE_UID = 46; @@ -1005,6 +1004,8 @@ public class AudioService extends IAudioService.Stub mPlatformType = AudioSystem.getPlatformType(context); + mDeviceBroker = new AudioDeviceBroker(mContext, this, mAudioSystem); + mIsSingleVolume = AudioSystem.isSingleVolume(context); mUserManagerInternal = LocalServices.getService(UserManagerInternal.class); @@ -1017,13 +1018,14 @@ public class AudioService extends IAudioService.Stub mSfxHelper = new SoundEffectsHelper(mContext, playerBase -> ignorePlayerLogs(playerBase)); - final boolean binauralEnabledDefault = SystemProperties.getBoolean( + boolean binauralEnabledDefault = SystemProperties.getBoolean( "ro.audio.spatializer_binaural_enabled_default", true); - final boolean transauralEnabledDefault = SystemProperties.getBoolean( + boolean transauralEnabledDefault = SystemProperties.getBoolean( "ro.audio.spatializer_transaural_enabled_default", true); - final boolean headTrackingEnabledDefault = mContext.getResources().getBoolean( + boolean headTrackingEnabledDefault = mContext.getResources().getBoolean( com.android.internal.R.bool.config_spatial_audio_head_tracking_enabled_default); - mSpatializerHelper = new SpatializerHelper(this, mAudioSystem, + + mSpatializerHelper = new SpatializerHelper(this, mAudioSystem, mDeviceBroker, binauralEnabledDefault, transauralEnabledDefault, headTrackingEnabledDefault); mVibrator = (Vibrator) context.getSystemService(Context.VIBRATOR_SERVICE); @@ -1211,8 +1213,6 @@ public class AudioService extends IAudioService.Stub mUseFixedVolume = mContext.getResources().getBoolean( com.android.internal.R.bool.config_useFixedVolume); - mDeviceBroker = new AudioDeviceBroker(mContext, this, mAudioSystem); - mRecordMonitor = new RecordingActivityMonitor(mContext); mRecordMonitor.registerRecordingCallback(mVoiceRecordingActivityMonitor, true); @@ -6399,6 +6399,10 @@ public class AudioService extends IAudioService.Stub } } + /*package*/ SettingsAdapter getSettings() { + return mSettings; + } + /////////////////////////////////////////////////////////////////////////// // Internal methods /////////////////////////////////////////////////////////////////////////// @@ -8900,10 +8904,6 @@ public class AudioService extends IAudioService.Stub mSpatializerHelper.onInitSensors(); break; - case MSG_PERSIST_SPATIAL_AUDIO_DEVICE_SETTINGS: - onPersistSpatialAudioDeviceSettings(); - break; - case MSG_RESET_SPATIALIZER: mSpatializerHelper.reset(/* featureEnabled */ mHasSpatializerEffect); break; @@ -9897,41 +9897,11 @@ public class AudioService extends IAudioService.Stub } void onInitSpatializer() { - final String settings = mSettings.getSecureStringForUser(mContentResolver, - Settings.Secure.SPATIAL_AUDIO_ENABLED, UserHandle.USER_CURRENT); - if (settings == null) { - Log.e(TAG, "error reading spatial audio device settings"); - } - mSpatializerHelper.init(/*effectExpected*/ mHasSpatializerEffect, settings); + mDeviceBroker.onReadAudioDeviceSettings(); + mSpatializerHelper.init(/*effectExpected*/ mHasSpatializerEffect); mSpatializerHelper.setFeatureEnabled(mHasSpatializerEffect); } - /** - * post a message to persist the spatial audio device settings. - * Message is delayed by 1s on purpose in case of successive changes in quick succession (at - * init time for instance) - * Note this method is made public to work around a Mockito bug where it needs to be public - * in order to be mocked by a test a the same package - * (see https://code.google.com/archive/p/mockito/issues/127) - */ - public void persistSpatialAudioDeviceSettings() { - sendMsg(mAudioHandler, - MSG_PERSIST_SPATIAL_AUDIO_DEVICE_SETTINGS, - SENDMSG_REPLACE, /*arg1*/ 0, /*arg2*/ 0, TAG, - /*delay*/ 1000); - } - - void onPersistSpatialAudioDeviceSettings() { - final String settings = mSpatializerHelper.getSADeviceSettings(); - Log.v(TAG, "saving spatial audio device settings: " + settings); - boolean res = mSettings.putSecureStringForUser(mContentResolver, - Settings.Secure.SPATIAL_AUDIO_ENABLED, - settings, UserHandle.USER_CURRENT); - if (!res) { - Log.e(TAG, "error saving spatial audio device settings: " + settings); - } - } - //========================================================================================== private boolean readCameraSoundForced() { return SystemProperties.getBoolean("audio.camerasound.force", false) || diff --git a/services/core/java/com/android/server/audio/SpatializerHelper.java b/services/core/java/com/android/server/audio/SpatializerHelper.java index c9cdba970ccb..c30fc69fad87 100644 --- a/services/core/java/com/android/server/audio/SpatializerHelper.java +++ b/services/core/java/com/android/server/audio/SpatializerHelper.java @@ -16,6 +16,8 @@ package com.android.server.audio; +import static android.media.AudioSystem.isBluetoothDevice; + import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; @@ -47,12 +49,12 @@ import android.util.Pair; import android.util.SparseIntArray; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; import java.util.Locale; -import java.util.Objects; import java.util.UUID; /** @@ -72,11 +74,12 @@ public class SpatializerHelper { private final @NonNull AudioSystemAdapter mASA; private final @NonNull AudioService mAudioService; + private final @NonNull AudioDeviceBroker mDeviceBroker; private @Nullable SensorManager mSensorManager; //------------------------------------------------------------ - private static final SparseIntArray SPAT_MODE_FOR_DEVICE_TYPE = new SparseIntArray(15) { + /*package*/ static final SparseIntArray SPAT_MODE_FOR_DEVICE_TYPE = new SparseIntArray(15) { { append(AudioDeviceInfo.TYPE_BUILTIN_SPEAKER, SpatializationMode.SPATIALIZER_TRANSAURAL); append(AudioDeviceInfo.TYPE_WIRED_HEADSET, SpatializationMode.SPATIALIZER_BINAURAL); @@ -98,13 +101,6 @@ public class SpatializerHelper { } }; - private static final int[] WIRELESS_TYPES = { AudioDeviceInfo.TYPE_BLUETOOTH_SCO, - AudioDeviceInfo.TYPE_BLUETOOTH_A2DP, - AudioDeviceInfo.TYPE_BLE_HEADSET, - AudioDeviceInfo.TYPE_BLE_SPEAKER, - AudioDeviceInfo.TYPE_BLE_BROADCAST - }; - // Spatializer state machine /*package*/ static final int STATE_UNINITIALIZED = 0; /*package*/ static final int STATE_NOT_SUPPORTED = 1; @@ -114,10 +110,15 @@ public class SpatializerHelper { /*package*/ static final int STATE_DISABLED_AVAILABLE = 6; private int mState = STATE_UNINITIALIZED; + @VisibleForTesting boolean mBinauralEnabledDefault; + @VisibleForTesting boolean mTransauralEnabledDefault; + @VisibleForTesting boolean mHeadTrackingEnabledDefault; + private boolean mFeatureEnabled = false; /** current level as reported by native Spatializer in callback */ private int mSpatLevel = Spatializer.SPATIALIZER_IMMERSIVE_LEVEL_NONE; private int mCapableSpatLevel = Spatializer.SPATIALIZER_IMMERSIVE_LEVEL_NONE; + private boolean mTransauralSupported = false; private boolean mBinauralSupported = false; private boolean mIsHeadTrackingSupported = false; @@ -160,31 +161,21 @@ public class SpatializerHelper { */ private final ArrayList mSACapableDeviceTypes = new ArrayList<>(0); - /** - * List of devices where Spatial Audio is possible. Each device can be enabled or disabled - * (== user choice to use or not) - */ - @GuardedBy("this") - private final ArrayList mSADevices = new ArrayList<>(0); - //------------------------------------------------------ // initialization - @SuppressWarnings("StaticAssignmentInConstructor") SpatializerHelper(@NonNull AudioService mother, @NonNull AudioSystemAdapter asa, - boolean binauralEnabledDefault, - boolean transauralEnabledDefault, - boolean headTrackingEnabledDefault) { + @NonNull AudioDeviceBroker deviceBroker, boolean binauralEnabledDefault, + boolean transauralEnabledDefault, boolean headTrackingEnabledDefault) { mAudioService = mother; mASA = asa; - // "StaticAssignmentInConstructor" warning is suppressed as the SpatializerHelper being - // constructed here is the factory for SADeviceState, thus SADeviceState and its - // private static field sHeadTrackingEnabledDefault should never be accessed directly. - SADeviceState.sBinauralEnabledDefault = binauralEnabledDefault; - SADeviceState.sTransauralEnabledDefault = transauralEnabledDefault; - SADeviceState.sHeadTrackingEnabledDefault = headTrackingEnabledDefault; + mDeviceBroker = deviceBroker; + + mBinauralEnabledDefault = binauralEnabledDefault; + mTransauralEnabledDefault = transauralEnabledDefault; + mHeadTrackingEnabledDefault = headTrackingEnabledDefault; } - synchronized void init(boolean effectExpected, @Nullable String settings) { + synchronized void init(boolean effectExpected) { loglogi("init effectExpected=" + effectExpected); if (!effectExpected) { loglogi("init(): setting state to STATE_NOT_SUPPORTED due to effect not expected"); @@ -288,10 +279,11 @@ public class SpatializerHelper { } } - // When initialized from AudioService, the settings string will be non-null. - // Saved settings need to be applied after spatialization support is initialized above. - if (settings != null) { - setSADeviceSettings(settings); + // Log the saved device states that are compatible with SA + for (AdiDeviceState deviceState : mDeviceBroker.getImmutableDeviceInventory()) { + if (isSADevice(deviceState)) { + logDeviceState(deviceState, "setSADeviceSettings"); + } } // for both transaural / binaural, we are not forcing enablement as the init() method @@ -331,7 +323,7 @@ public class SpatializerHelper { mState = STATE_UNINITIALIZED; mSpatLevel = Spatializer.SPATIALIZER_IMMERSIVE_LEVEL_NONE; mActualHeadTrackingMode = Spatializer.HEAD_TRACKING_MODE_UNSUPPORTED; - init(true, null /* settings */); + init(/*effectExpected=*/true); setSpatializerEnabledInt(featureEnabled); } @@ -372,7 +364,7 @@ public class SpatializerHelper { final AudioDeviceAttributes currentDevice = sRoutingDevices.get(0); // is media routed to a new device? - if (isWireless(currentDevice.getType())) { + if (isBluetoothDevice(currentDevice.getInternalType())) { addWirelessDeviceIfNew(currentDevice); } @@ -520,8 +512,8 @@ public class SpatializerHelper { synchronized @NonNull List getCompatibleAudioDevices() { // build unionOf(mCompatibleAudioDevices, mEnabledDevice) - mDisabledAudioDevices ArrayList compatList = new ArrayList<>(); - for (SADeviceState deviceState : mSADevices) { - if (deviceState.mEnabled) { + for (AdiDeviceState deviceState : mDeviceBroker.getImmutableDeviceInventory()) { + if (deviceState.isSAEnabled() && isSADevice(deviceState)) { compatList.add(deviceState.getAudioDeviceAttributes()); } } @@ -548,29 +540,48 @@ public class SpatializerHelper { return; } loglogi("addCompatibleAudioDevice: dev=" + ada); - final SADeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); - SADeviceState deviceUpdated = null; // non-null on update. + final AdiDeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); + initSAState(deviceState); + AdiDeviceState updatedDevice = null; // non-null on update. if (deviceState != null) { - if (forceEnable && !deviceState.mEnabled) { - deviceUpdated = deviceState; - deviceUpdated.mEnabled = true; + if (forceEnable && !deviceState.isSAEnabled()) { + updatedDevice = deviceState; + updatedDevice.setSAEnabled(true); } } else { // When adding, force the device type to be a canonical one. - final int canonicalDeviceType = getCanonicalDeviceType(ada.getType()); + final int canonicalDeviceType = getCanonicalDeviceType(ada.getType(), + ada.getInternalType()); if (canonicalDeviceType == AudioDeviceInfo.TYPE_UNKNOWN) { Log.e(TAG, "addCompatibleAudioDevice with incompatible AudioDeviceAttributes " + ada); return; } - deviceUpdated = new SADeviceState(canonicalDeviceType, ada.getAddress()); - mSADevices.add(deviceUpdated); + updatedDevice = new AdiDeviceState(canonicalDeviceType, ada.getInternalType(), + ada.getAddress()); + initSAState(updatedDevice); + mDeviceBroker.addDeviceStateToInventory(updatedDevice); } - if (deviceUpdated != null) { + if (updatedDevice != null) { onRoutingUpdated(); - mAudioService.persistSpatialAudioDeviceSettings(); - logDeviceState(deviceUpdated, "addCompatibleAudioDevice"); + mDeviceBroker.persistAudioDeviceSettings(); + logDeviceState(updatedDevice, "addCompatibleAudioDevice"); + } + } + + private void initSAState(AdiDeviceState device) { + if (device == null) { + return; } + + int spatMode = SPAT_MODE_FOR_DEVICE_TYPE.get(device.getDeviceType(), + Integer.MIN_VALUE); + device.setSAEnabled(spatMode == SpatializationMode.SPATIALIZER_BINAURAL + ? mBinauralEnabledDefault + : spatMode == SpatializationMode.SPATIALIZER_TRANSAURAL + ? mTransauralEnabledDefault + : false); + device.setHeadTrackerEnabled(mHeadTrackingEnabledDefault); } private static final String METRICS_DEVICE_PREFIX = "audio.spatializer.device."; @@ -580,29 +591,30 @@ public class SpatializerHelper { // // There may be different devices with the same device type (aliasing). // We always send the full device state info on each change. - private void logDeviceState(SADeviceState deviceState, String event) { + static void logDeviceState(AdiDeviceState deviceState, String event) { final int deviceType = AudioDeviceInfo.convertDeviceTypeToInternalDevice( - deviceState.mDeviceType); + deviceState.getDeviceType()); final String deviceName = AudioSystem.getDeviceName(deviceType); new MediaMetrics.Item(METRICS_DEVICE_PREFIX + deviceName) - .set(MediaMetrics.Property.ADDRESS, deviceState.mDeviceAddress) - .set(MediaMetrics.Property.ENABLED, deviceState.mEnabled ? "true" : "false") - .set(MediaMetrics.Property.EVENT, TextUtils.emptyIfNull(event)) - .set(MediaMetrics.Property.HAS_HEAD_TRACKER, - deviceState.mHasHeadTracker ? "true" : "false") // this may be updated later. - .set(MediaMetrics.Property.HEAD_TRACKER_ENABLED, - deviceState.mHeadTrackerEnabled ? "true" : "false") - .record(); + .set(MediaMetrics.Property.ADDRESS, deviceState.getDeviceAddress()) + .set(MediaMetrics.Property.ENABLED, deviceState.isSAEnabled() ? "true" : "false") + .set(MediaMetrics.Property.EVENT, TextUtils.emptyIfNull(event)) + .set(MediaMetrics.Property.HAS_HEAD_TRACKER, + deviceState.hasHeadTracker() ? "true" + : "false") // this may be updated later. + .set(MediaMetrics.Property.HEAD_TRACKER_ENABLED, + deviceState.isHeadTrackerEnabled() ? "true" : "false") + .record(); } synchronized void removeCompatibleAudioDevice(@NonNull AudioDeviceAttributes ada) { loglogi("removeCompatibleAudioDevice: dev=" + ada); - final SADeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); - if (deviceState != null && deviceState.mEnabled) { - deviceState.mEnabled = false; + final AdiDeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); + if (deviceState != null && deviceState.isSAEnabled()) { + deviceState.setSAEnabled(false); onRoutingUpdated(); - mAudioService.persistSpatialAudioDeviceSettings(); + mDeviceBroker.persistAudioDeviceSettings(); logDeviceState(deviceState, "removeCompatibleAudioDevice"); } } @@ -611,8 +623,9 @@ public class SpatializerHelper { * Returns a possibly aliased device type which is used * for spatial audio settings (or TYPE_UNKNOWN if it doesn't exist). */ - private static @AudioDeviceInfo.AudioDeviceType int getCanonicalDeviceType(int deviceType) { - if (isWireless(deviceType)) return deviceType; + @AudioDeviceInfo.AudioDeviceType + private static int getCanonicalDeviceType(int deviceType, int internalDeviceType) { + if (isBluetoothDevice(internalDeviceType)) return deviceType; final int spatMode = SPAT_MODE_FOR_DEVICE_TYPE.get(deviceType, Integer.MIN_VALUE); if (spatMode == SpatializationMode.SPATIALIZER_TRANSAURAL) { @@ -629,18 +642,9 @@ public class SpatializerHelper { */ @GuardedBy("this") @Nullable - private SADeviceState findDeviceStateForAudioDeviceAttributes(AudioDeviceAttributes ada) { - final int deviceType = ada.getType(); - final boolean isWireless = isWireless(deviceType); - final int canonicalDeviceType = getCanonicalDeviceType(deviceType); - - for (SADeviceState deviceState : mSADevices) { - if (deviceState.mDeviceType == canonicalDeviceType - && (!isWireless || ada.getAddress().equals(deviceState.mDeviceAddress))) { - return deviceState; - } - } - return null; + private AdiDeviceState findDeviceStateForAudioDeviceAttributes(AudioDeviceAttributes ada) { + return mDeviceBroker.findDeviceStateForAudioDeviceAttributes(ada, + getCanonicalDeviceType(ada.getType(), ada.getInternalType())); } /** @@ -662,14 +666,14 @@ public class SpatializerHelper { Log.e(TAG, "no spatialization mode found for device type:" + deviceType); return new Pair<>(false, false); } - final SADeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); + final AdiDeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); if (deviceState == null) { // no matching device state? Log.i(TAG, "no spatialization device state found for Spatial Audio device:" + ada); return new Pair<>(false, false); } // found the matching device state. - return new Pair<>(deviceState.mEnabled, true /* available */); + return new Pair<>(deviceState.isSAEnabled(), true /* available */); } private synchronized void addWirelessDeviceIfNew(@NonNull AudioDeviceAttributes ada) { @@ -678,16 +682,19 @@ public class SpatializerHelper { } if (findDeviceStateForAudioDeviceAttributes(ada) == null) { // wireless device types should be canonical, but we translate to be sure. - final int canonicalDeviceType = getCanonicalDeviceType((ada.getType())); + final int canonicalDeviceType = getCanonicalDeviceType(ada.getType(), + ada.getInternalType()); if (canonicalDeviceType == AudioDeviceInfo.TYPE_UNKNOWN) { Log.e(TAG, "addWirelessDeviceIfNew with incompatible AudioDeviceAttributes " + ada); return; } - final SADeviceState deviceState = - new SADeviceState(canonicalDeviceType, ada.getAddress()); - mSADevices.add(deviceState); - mAudioService.persistSpatialAudioDeviceSettings(); + final AdiDeviceState deviceState = + new AdiDeviceState(canonicalDeviceType, ada.getInternalType(), + ada.getAddress()); + initSAState(deviceState); + mDeviceBroker.addDeviceStateToInventory(deviceState); + mDeviceBroker.persistAudioDeviceSettings(); logDeviceState(deviceState, "addWirelessDeviceIfNew"); // may be updated later. } } @@ -756,6 +763,12 @@ public class SpatializerHelper { return false; } + private boolean isSADevice(AdiDeviceState deviceState) { + return deviceState.getDeviceType() == getCanonicalDeviceType(deviceState.getDeviceType(), + deviceState.getInternalDeviceType()) && isDeviceCompatibleWithSpatializationModes( + deviceState.getAudioDeviceAttributes()); + } + synchronized void setFeatureEnabled(boolean enabled) { loglogi("setFeatureEnabled(" + enabled + ") was featureEnabled:" + mFeatureEnabled); if (mFeatureEnabled == enabled) { @@ -768,7 +781,7 @@ public class SpatializerHelper { return; } if (mState == STATE_UNINITIALIZED) { - init(true, null /* settings */); + init(true); } setSpatializerEnabledInt(true); } else { @@ -1137,16 +1150,16 @@ public class SpatializerHelper { Log.v(TAG, "no headtracking support, ignoring setHeadTrackerEnabled to " + enabled + " for " + ada); } - final SADeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); + final AdiDeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); if (deviceState == null) return; - if (!deviceState.mHasHeadTracker) { + if (!deviceState.hasHeadTracker()) { Log.e(TAG, "Called setHeadTrackerEnabled enabled:" + enabled + " device:" + ada + " on a device without headtracker"); return; } Log.i(TAG, "setHeadTrackerEnabled enabled:" + enabled + " device:" + ada); - deviceState.mHeadTrackerEnabled = enabled; - mAudioService.persistSpatialAudioDeviceSettings(); + deviceState.setHeadTrackerEnabled(enabled); + mDeviceBroker.persistAudioDeviceSettings(); logDeviceState(deviceState, "setHeadTrackerEnabled"); // check current routing to see if it affects the headtracking mode @@ -1170,8 +1183,8 @@ public class SpatializerHelper { Log.v(TAG, "no headtracking support, hasHeadTracker always false for " + ada); return false; } - final SADeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); - return deviceState != null && deviceState.mHasHeadTracker; + final AdiDeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); + return deviceState != null && deviceState.hasHeadTracker(); } /** @@ -1184,14 +1197,14 @@ public class SpatializerHelper { Log.v(TAG, "no headtracking support, setHasHeadTracker always false for " + ada); return false; } - final SADeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); + final AdiDeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); if (deviceState != null) { - if (!deviceState.mHasHeadTracker) { - deviceState.mHasHeadTracker = true; - mAudioService.persistSpatialAudioDeviceSettings(); + if (!deviceState.hasHeadTracker()) { + deviceState.setHasHeadTracker(true); + mDeviceBroker.persistAudioDeviceSettings(); logDeviceState(deviceState, "setHasHeadTracker"); } - return deviceState.mHeadTrackerEnabled; + return deviceState.isHeadTrackerEnabled(); } Log.e(TAG, "setHasHeadTracker: device not found for:" + ada); return false; @@ -1202,9 +1215,9 @@ public class SpatializerHelper { Log.v(TAG, "no headtracking support, isHeadTrackerEnabled always false for " + ada); return false; } - final SADeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); + final AdiDeviceState deviceState = findDeviceStateForAudioDeviceAttributes(ada); return deviceState != null - && deviceState.mHasHeadTracker && deviceState.mHeadTrackerEnabled; + && deviceState.hasHeadTracker() && deviceState.isHeadTrackerEnabled(); } synchronized boolean isHeadTrackerAvailable() { @@ -1543,144 +1556,6 @@ public class SpatializerHelper { pw.println("\tsupports binaural:" + mBinauralSupported + " / transaural:" + mTransauralSupported); pw.println("\tmSpatOutput:" + mSpatOutput); - pw.println("\tdevices:"); - for (SADeviceState device : mSADevices) { - pw.println("\t\t" + device); - } - } - - /*package*/ static final class SADeviceState { - private static boolean sBinauralEnabledDefault = true; - private static boolean sTransauralEnabledDefault = true; - private static boolean sHeadTrackingEnabledDefault = false; - final @AudioDeviceInfo.AudioDeviceType int mDeviceType; - final @NonNull String mDeviceAddress; - boolean mEnabled; - boolean mHasHeadTracker = false; - boolean mHeadTrackerEnabled; - static final String SETTING_FIELD_SEPARATOR = ","; - static final String SETTING_DEVICE_SEPARATOR_CHAR = "|"; - static final String SETTING_DEVICE_SEPARATOR = "\\|"; - - /** - * Constructor - * @param deviceType - * @param address must be non-null for wireless devices - * @throws NullPointerException if a null address is passed for a wireless device - */ - SADeviceState(@AudioDeviceInfo.AudioDeviceType int deviceType, @Nullable String address) { - mDeviceType = deviceType; - mDeviceAddress = isWireless(deviceType) ? Objects.requireNonNull(address) : ""; - final int spatMode = SPAT_MODE_FOR_DEVICE_TYPE.get(deviceType, Integer.MIN_VALUE); - mEnabled = spatMode == SpatializationMode.SPATIALIZER_BINAURAL - ? sBinauralEnabledDefault - : spatMode == SpatializationMode.SPATIALIZER_TRANSAURAL - ? sTransauralEnabledDefault - : false; - mHeadTrackerEnabled = sHeadTrackingEnabledDefault; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj == null) { - return false; - } - // type check and cast - if (getClass() != obj.getClass()) { - return false; - } - final SADeviceState sads = (SADeviceState) obj; - return mDeviceType == sads.mDeviceType - && mDeviceAddress.equals(sads.mDeviceAddress) - && mEnabled == sads.mEnabled - && mHasHeadTracker == sads.mHasHeadTracker - && mHeadTrackerEnabled == sads.mHeadTrackerEnabled; - } - - @Override - public int hashCode() { - return Objects.hash(mDeviceType, mDeviceAddress, mEnabled, mHasHeadTracker, - mHeadTrackerEnabled); - } - - @Override - public String toString() { - return "type: " + mDeviceType + " addr: " + mDeviceAddress + " enabled: " + mEnabled - + " HT: " + mHasHeadTracker + " HTenabled: " + mHeadTrackerEnabled; - } - - String toPersistableString() { - return (new StringBuilder().append(mDeviceType) - .append(SETTING_FIELD_SEPARATOR).append(mDeviceAddress) - .append(SETTING_FIELD_SEPARATOR).append(mEnabled ? "1" : "0") - .append(SETTING_FIELD_SEPARATOR).append(mHasHeadTracker ? "1" : "0") - .append(SETTING_FIELD_SEPARATOR).append(mHeadTrackerEnabled ? "1" : "0") - .toString()); - } - - static @Nullable SADeviceState fromPersistedString(@Nullable String persistedString) { - if (persistedString == null) { - return null; - } - if (persistedString.isEmpty()) { - return null; - } - String[] fields = TextUtils.split(persistedString, SETTING_FIELD_SEPARATOR); - if (fields.length != 5) { - // expecting all fields, fewer may mean corruption, ignore those settings - return null; - } - try { - final int deviceType = Integer.parseInt(fields[0]); - final SADeviceState deviceState = new SADeviceState(deviceType, fields[1]); - deviceState.mEnabled = Integer.parseInt(fields[2]) == 1; - deviceState.mHasHeadTracker = Integer.parseInt(fields[3]) == 1; - deviceState.mHeadTrackerEnabled = Integer.parseInt(fields[4]) == 1; - return deviceState; - } catch (NumberFormatException e) { - Log.e(TAG, "unable to parse setting for SADeviceState: " + persistedString, e); - return null; - } - } - - public AudioDeviceAttributes getAudioDeviceAttributes() { - return new AudioDeviceAttributes(AudioDeviceAttributes.ROLE_OUTPUT, - mDeviceType, mDeviceAddress == null ? "" : mDeviceAddress); - } - - } - - /*package*/ synchronized String getSADeviceSettings() { - // expected max size of each String for each SADeviceState is 25 (accounting for separator) - final StringBuilder settingsBuilder = new StringBuilder(mSADevices.size() * 25); - for (int i = 0; i < mSADevices.size(); i++) { - settingsBuilder.append(mSADevices.get(i).toPersistableString()); - if (i != mSADevices.size() - 1) { - settingsBuilder.append(SADeviceState.SETTING_DEVICE_SEPARATOR_CHAR); - } - } - return settingsBuilder.toString(); - } - - /*package*/ synchronized void setSADeviceSettings(@NonNull String persistedSettings) { - String[] devSettings = TextUtils.split(Objects.requireNonNull(persistedSettings), - SADeviceState.SETTING_DEVICE_SEPARATOR); - // small list, not worth overhead of Arrays.stream(devSettings) - for (String setting : devSettings) { - SADeviceState devState = SADeviceState.fromPersistedString(setting); - // Note if the device is not compatible with spatialization mode - // or the device type is not canonical, it is ignored. - if (devState != null - && devState.mDeviceType == getCanonicalDeviceType(devState.mDeviceType) - && isDeviceCompatibleWithSpatializationModes( - devState.getAudioDeviceAttributes())) { - mSADevices.add(devState); - logDeviceState(devState, "setSADeviceSettings"); - } - } } private static String spatStateString(int state) { @@ -1702,15 +1577,6 @@ public class SpatializerHelper { } } - private static boolean isWireless(@AudioDeviceInfo.AudioDeviceType int deviceType) { - for (int type : WIRELESS_TYPES) { - if (type == deviceType) { - return true; - } - } - return false; - } - private int getHeadSensorHandleUpdateTracker() { int headHandle = -1; if (sRoutingDevices.isEmpty()) { @@ -1780,11 +1646,6 @@ public class SpatializerHelper { //------------------------------------------------ // for testing purposes only - - /*package*/ void clearSADevices() { - mSADevices.clear(); - } - /*package*/ synchronized void forceStateForTest(int state) { mState = state; } diff --git a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java index 31599eed539d..aba24fbd55b7 100644 --- a/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/AudioDeviceBrokerTest.java @@ -29,13 +29,14 @@ import android.bluetooth.BluetoothDevice; import android.content.Context; import android.content.Intent; import android.media.AudioDeviceAttributes; +import android.media.AudioDeviceInfo; import android.media.AudioManager; import android.media.AudioSystem; import android.media.BluetoothProfileConnectionInfo; import android.util.Log; -import androidx.test.InstrumentationRegistry; import androidx.test.filters.MediumTest; +import androidx.test.platform.app.InstrumentationRegistry; import androidx.test.runner.AndroidJUnit4; import org.junit.After; @@ -54,7 +55,6 @@ public class AudioDeviceBrokerTest { private static final String TAG = "AudioDeviceBrokerTest"; private static final int MAX_MESSAGE_HANDLING_DELAY_MS = 100; - private Context mContext; // the actual class under test private AudioDeviceBroker mAudioDeviceBroker; @@ -67,13 +67,13 @@ public class AudioDeviceBrokerTest { @Before public void setUp() throws Exception { - mContext = InstrumentationRegistry.getTargetContext(); + Context context = InstrumentationRegistry.getInstrumentation().getTargetContext(); mMockAudioService = mock(AudioService.class); mSpyAudioSystem = spy(new NoOpAudioSystemAdapter()); mSpyDevInventory = spy(new AudioDeviceInventory(mSpyAudioSystem)); mSpySystemServer = spy(new NoOpSystemServerAdapter()); - mAudioDeviceBroker = new AudioDeviceBroker(mContext, mMockAudioService, mSpyDevInventory, + mAudioDeviceBroker = new AudioDeviceBroker(context, mMockAudioService, mSpyDevInventory, mSpySystemServer, mSpyAudioSystem); mSpyDevInventory.setDeviceBroker(mAudioDeviceBroker); @@ -197,6 +197,37 @@ public class AudioDeviceBrokerTest { any(Intent.class)); } + /** + * Test that constructing an AdiDeviceState instance requires a non-null address for a + * wireless type, but can take null for a non-wireless type; + * @throws Exception + */ + @Test + public void testAdiDeviceStateNullAddressCtor() throws Exception { + try { + new AdiDeviceState(AudioDeviceInfo.TYPE_BUILTIN_SPEAKER, + AudioManager.DEVICE_OUT_SPEAKER, null); + new AdiDeviceState(AudioDeviceInfo.TYPE_BLUETOOTH_A2DP, + AudioManager.DEVICE_OUT_BLUETOOTH_A2DP, null); + Assert.fail(); + } catch (NullPointerException e) { } + } + + @Test + public void testAdiDeviceStateStringSerialization() throws Exception { + Log.i(TAG, "starting testAdiDeviceStateStringSerialization"); + final AdiDeviceState devState = new AdiDeviceState( + AudioDeviceInfo.TYPE_BUILTIN_SPEAKER, AudioManager.DEVICE_OUT_SPEAKER, "bla"); + devState.setHasHeadTracker(false); + devState.setHeadTrackerEnabled(false); + devState.setSAEnabled(true); + final String persistString = devState.toPersistableString(); + final AdiDeviceState result = AdiDeviceState.fromPersistedString(persistString); + Log.i(TAG, "original:" + devState); + Log.i(TAG, "result :" + result); + Assert.assertEquals(devState, result); + } + private void doTestConnectionDisconnectionReconnection(int delayAfterDisconnection, boolean mockMediaPlayback, boolean guaranteeSingleConnection) throws Exception { when(mMockAudioService.getDeviceForStream(AudioManager.STREAM_MUSIC)) diff --git a/services/tests/servicestests/src/com/android/server/audio/SpatializerHelperTest.java b/services/tests/servicestests/src/com/android/server/audio/SpatializerHelperTest.java index 3ad24de4cdca..ad09ef0ccdc1 100644 --- a/services/tests/servicestests/src/com/android/server/audio/SpatializerHelperTest.java +++ b/services/tests/servicestests/src/com/android/server/audio/SpatializerHelperTest.java @@ -15,8 +15,6 @@ */ package com.android.server.audio; -import com.android.server.audio.SpatializerHelper.SADeviceState; - import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.doNothing; @@ -26,12 +24,12 @@ import static org.mockito.Mockito.when; import android.media.AudioAttributes; import android.media.AudioDeviceAttributes; -import android.media.AudioDeviceInfo; import android.media.AudioFormat; import android.media.AudioSystem; import android.util.Log; import androidx.test.filters.MediumTest; +import androidx.test.platform.app.InstrumentationRegistry; import androidx.test.runner.AndroidJUnit4; import org.junit.Assert; @@ -55,72 +53,25 @@ public class SpatializerHelperTest { @Mock private AudioService mMockAudioService; @Spy private AudioSystemAdapter mSpyAudioSystem; - @Mock private AudioSystemAdapter mMockAudioSystem; + @Spy private AudioDeviceBroker mSpyDeviceBroker; @Before public void setUp() throws Exception { mMockAudioService = mock(AudioService.class); - } - - /** - * Initializes mSpatHelper, the SpatizerHelper instance under test, to use the mock or spy - * AudioSystemAdapter - * @param useSpyAudioSystem true to use the spy adapter, mSpyAudioSystem, or false to use - * the mock adapter, mMockAudioSystem. - */ - private void setUpSpatHelper(boolean useSpyAudioSystem) { - final AudioSystemAdapter asAdapter; - if (useSpyAudioSystem) { - mSpyAudioSystem = spy(new NoOpAudioSystemAdapter()); - asAdapter = mSpyAudioSystem; - mMockAudioSystem = null; - } else { - mSpyAudioSystem = null; - mMockAudioSystem = mock(NoOpAudioSystemAdapter.class); - asAdapter = mMockAudioSystem; - } - mSpatHelper = new SpatializerHelper(mMockAudioService, asAdapter, - true /*binauralEnabledDefault*/, - true /*transauralEnabledDefault*/, - false /*headTrackingEnabledDefault*/); - - } - /** - * Test that constructing an SADeviceState instance requires a non-null address for a - * wireless type, but can take null for a non-wireless type; - * @throws Exception - */ - @Test - public void testSADeviceStateNullAddressCtor() throws Exception { - setUpSpatHelper(true /*useSpyAudioSystem*/); - try { - SADeviceState devState = new SADeviceState(AudioDeviceInfo.TYPE_BUILTIN_SPEAKER, null); - devState = new SADeviceState(AudioDeviceInfo.TYPE_BLUETOOTH_A2DP, null); - Assert.fail(); - } catch (NullPointerException e) { } - } - - @Test - public void testSADeviceStateStringSerialization() throws Exception { - Log.i(TAG, "starting testSADeviceStateStringSerialization"); - setUpSpatHelper(true /*useSpyAudioSystem*/); - final SADeviceState devState = new SADeviceState( - AudioDeviceInfo.TYPE_BUILTIN_SPEAKER, "bla"); - devState.mHasHeadTracker = false; - devState.mHeadTrackerEnabled = false; - devState.mEnabled = true; - final String persistString = devState.toPersistableString(); - final SADeviceState result = SADeviceState.fromPersistedString(persistString); - Log.i(TAG, "original:" + devState); - Log.i(TAG, "result :" + result); - Assert.assertEquals(devState, result); + mSpyAudioSystem = spy(new NoOpAudioSystemAdapter()); + mSpyDeviceBroker = spy( + new AudioDeviceBroker( + InstrumentationRegistry.getInstrumentation().getTargetContext(), + mMockAudioService, mSpyAudioSystem)); + mSpatHelper = new SpatializerHelper(mMockAudioService, mSpyAudioSystem, + mSpyDeviceBroker, /*binauralEnabledDefault=*/true, /*transauralEnabledDefault=*/ + true, /*headTrackingEnabledDefault*/false); } @Test - public void testSADeviceSettings() throws Exception { + public void testAdiDeviceStateSettings() throws Exception { Log.i(TAG, "starting testSADeviceSettings"); - setUpSpatHelper(true /*useSpyAudioSystem*/); final AudioDeviceAttributes dev1 = new AudioDeviceAttributes(AudioSystem.DEVICE_OUT_SPEAKER, ""); final AudioDeviceAttributes dev2 = @@ -128,7 +79,7 @@ public class SpatializerHelperTest { final AudioDeviceAttributes dev3 = new AudioDeviceAttributes(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, "R2:D2:bloop"); - doNothing().when(mMockAudioService).persistSpatialAudioDeviceSettings(); + doNothing().when(mSpyDeviceBroker).persistAudioDeviceSettings(); mSpatHelper.initForTest(true /*binaural*/, true /*transaural*/); // test with single device @@ -163,11 +114,11 @@ public class SpatializerHelperTest { * the original one. */ private void checkAddSettings() throws Exception { - String settings = mSpatHelper.getSADeviceSettings(); + String settings = mSpyDeviceBroker.getDeviceSettings(); Log.i(TAG, "device settings: " + settings); - mSpatHelper.clearSADevices(); - mSpatHelper.setSADeviceSettings(settings); - String settingsRestored = mSpatHelper.getSADeviceSettings(); + mSpyDeviceBroker.clearDeviceInventory(); + mSpyDeviceBroker.setDeviceSettings(settings); + String settingsRestored = mSpyDeviceBroker.getDeviceSettings(); Log.i(TAG, "device settingsRestored: " + settingsRestored); Assert.assertEquals(settings, settingsRestored); } @@ -179,7 +130,6 @@ public class SpatializerHelperTest { @Test public void testNoRoutingCanBeSpatialized() throws Exception { Log.i(TAG, "Starting testNoRoutingCanBeSpatialized"); - setUpSpatHelper(false /*useSpyAudioSystem*/); mSpatHelper.forceStateForTest(SpatializerHelper.STATE_ENABLED_AVAILABLE); final ArrayList emptyList = new ArrayList<>(0); @@ -191,12 +141,12 @@ public class SpatializerHelperTest { .setEncoding(AudioFormat.ENCODING_PCM_16BIT) .setChannelMask(AudioFormat.CHANNEL_OUT_5POINT1).build(); - when(mMockAudioSystem.getDevicesForAttributes(any(AudioAttributes.class), anyBoolean())) + when(mSpyAudioSystem.getDevicesForAttributes(any(AudioAttributes.class), anyBoolean())) .thenReturn(emptyList); Assert.assertFalse("can be spatialized on empty routing", mSpatHelper.canBeSpatialized(media, spatialFormat)); - when(mMockAudioSystem.getDevicesForAttributes(any(AudioAttributes.class), anyBoolean())) + when(mSpyAudioSystem.getDevicesForAttributes(any(AudioAttributes.class), anyBoolean())) .thenReturn(listWithNull); Assert.assertFalse("can be spatialized on null routing", mSpatHelper.canBeSpatialized(media, spatialFormat)); -- cgit v1.2.3 From beaff8b17df1c718a7877619045722037a7cc2fc Mon Sep 17 00:00:00 2001 From: Eric Laurent Date: Fri, 13 Oct 2023 18:37:23 +0200 Subject: AudioService: anonymize Bluetooth MAC addresses Make sure APIs returning AudioDeviceAttributes from AudioService anonymize the Bluetooth MAC addresses because those are considered privacy sensitive. Only expose the full MAC address to system and apps with BLUETOOTH_CONNECT permission. setters, getters and listeners for preferred device for strategy, preferred device for capture preset and mute await connection are modified: - when entering AudioService, full MAC addresses are retrieved based on the known Bluetooth devices stored in AudioDeviceInventory.mDeviceInventory - when exiting AudioService, MAC addresses are anonymized if the client app does not have BLUETOOTH_CONNECT permission or is not a system component APIs based on AudioDeviceInfo do not need to be modified as the AudioDeviceInfo MAC address is for the AudioPort cached in the app process and AudioPorts are anonymized by the native audioserver before being returned to client apps. Bug: 285588444 Test: atest AudioManagerTest Test: atest RoutingTest Test: atest AudioCommunicationDeviceTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:665342c5171a2a626bb96168ddccc19b734e3a44) Merged-In: I67bbba2ba941c97138a068d640079b17650e3d86 Change-Id: I67bbba2ba941c97138a068d640079b17650e3d86 --- .../java/android/media/AudioDeviceAttributes.java | 26 ++- .../com/android/server/audio/AdiDeviceState.java | 11 +- .../android/server/audio/AudioDeviceBroker.java | 14 +- .../android/server/audio/AudioDeviceInventory.java | 112 ++++++++---- .../com/android/server/audio/AudioService.java | 200 ++++++++++++++++++--- 5 files changed, 299 insertions(+), 64 deletions(-) diff --git a/media/java/android/media/AudioDeviceAttributes.java b/media/java/android/media/AudioDeviceAttributes.java index af3c295b8d6c..5a274353f68e 100644 --- a/media/java/android/media/AudioDeviceAttributes.java +++ b/media/java/android/media/AudioDeviceAttributes.java @@ -68,7 +68,7 @@ public final class AudioDeviceAttributes implements Parcelable { /** * The unique address of the device. Some devices don't have addresses, only an empty string. */ - private final @NonNull String mAddress; + private @NonNull String mAddress; /** * The non-unique name of the device. Some devices don't have names, only an empty string. * Should not be used as a unique identifier for a device. @@ -186,6 +186,21 @@ public final class AudioDeviceAttributes implements Parcelable { mAudioDescriptors = new ArrayList<>(); } + /** + * @hide + * Copy Constructor. + * @param ada the copied AudioDeviceAttributes + */ + public AudioDeviceAttributes(AudioDeviceAttributes ada) { + mRole = ada.getRole(); + mType = ada.getType(); + mAddress = ada.getAddress(); + mName = ada.getName(); + mNativeType = ada.getInternalType(); + mAudioProfiles = ada.getAudioProfiles(); + mAudioDescriptors = ada.getAudioDescriptors(); + } + /** * @hide * Returns the role of a device @@ -216,6 +231,15 @@ public final class AudioDeviceAttributes implements Parcelable { return mAddress; } + /** + * @hide + * Sets the device address. Only used by audio service. + */ + public void setAddress(@NonNull String address) { + Objects.requireNonNull(address); + mAddress = address; + } + /** * @hide * Returns the name of the audio device, or an empty string for devices without one diff --git a/services/core/java/com/android/server/audio/AdiDeviceState.java b/services/core/java/com/android/server/audio/AdiDeviceState.java index 683b3eb7a92e..1c456a781f7a 100644 --- a/services/core/java/com/android/server/audio/AdiDeviceState.java +++ b/services/core/java/com/android/server/audio/AdiDeviceState.java @@ -25,6 +25,7 @@ import android.media.AudioDeviceAttributes; import android.media.AudioDeviceInfo; import android.text.TextUtils; import android.util.Log; +import android.util.Pair; import java.util.Objects; @@ -43,6 +44,8 @@ import java.util.Objects; private final int mInternalDeviceType; @NonNull private final String mDeviceAddress; + /** Unique device id from internal device type and address. */ + private final Pair mDeviceId; private boolean mSAEnabled; private boolean mHasHeadTracker = false; private boolean mHeadTrackerEnabled; @@ -68,6 +71,11 @@ import java.util.Objects; } mDeviceAddress = isBluetoothDevice(mInternalDeviceType) ? Objects.requireNonNull( address) : ""; + mDeviceId = new Pair<>(mInternalDeviceType, mDeviceAddress); + } + + public Pair getDeviceId() { + return mDeviceId; } @AudioDeviceInfo.AudioDeviceType @@ -138,7 +146,8 @@ import java.util.Objects; @Override public String toString() { - return "type: " + mDeviceType + "internal type: " + mInternalDeviceType + return "type: " + mDeviceType + + " internal type: 0x" + Integer.toHexString(mInternalDeviceType) + " addr: " + mDeviceAddress + " enabled: " + mSAEnabled + " HT: " + mHasHeadTracker + " HTenabled: " + mHeadTrackerEnabled; } diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index ece6ff6e3ab5..f2fe04850433 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -938,8 +938,8 @@ public class AudioDeviceBroker { } /*package*/ void registerStrategyPreferredDevicesDispatcher( - @NonNull IStrategyPreferredDevicesDispatcher dispatcher) { - mDeviceInventory.registerStrategyPreferredDevicesDispatcher(dispatcher); + @NonNull IStrategyPreferredDevicesDispatcher dispatcher, boolean isPrivileged) { + mDeviceInventory.registerStrategyPreferredDevicesDispatcher(dispatcher, isPrivileged); } /*package*/ void unregisterStrategyPreferredDevicesDispatcher( @@ -957,8 +957,8 @@ public class AudioDeviceBroker { } /*package*/ void registerCapturePresetDevicesRoleDispatcher( - @NonNull ICapturePresetDevicesRoleDispatcher dispatcher) { - mDeviceInventory.registerCapturePresetDevicesRoleDispatcher(dispatcher); + @NonNull ICapturePresetDevicesRoleDispatcher dispatcher, boolean isPrivileged) { + mDeviceInventory.registerCapturePresetDevicesRoleDispatcher(dispatcher, isPrivileged); } /*package*/ void unregisterCapturePresetDevicesRoleDispatcher( @@ -966,6 +966,11 @@ public class AudioDeviceBroker { mDeviceInventory.unregisterCapturePresetDevicesRoleDispatcher(dispatcher); } + /* package */ List anonymizeAudioDeviceAttributesListUnchecked( + List devices) { + return mAudioService.anonymizeAudioDeviceAttributesListUnchecked(devices); + } + /*package*/ void registerCommunicationDeviceDispatcher( @NonNull ICommunicationDeviceDispatcher dispatcher) { mCommDevDispatchers.register(dispatcher); @@ -2181,4 +2186,5 @@ public class AudioDeviceBroker { void clearDeviceInventory() { mDeviceInventory.clearDeviceInventory(); } + } diff --git a/services/core/java/com/android/server/audio/AudioDeviceInventory.java b/services/core/java/com/android/server/audio/AudioDeviceInventory.java index e94415f55ab7..84345441db23 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceInventory.java +++ b/services/core/java/com/android/server/audio/AudioDeviceInventory.java @@ -41,6 +41,7 @@ import android.text.TextUtils; import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; +import android.util.Pair; import android.util.Slog; import com.android.internal.annotations.GuardedBy; @@ -48,7 +49,9 @@ import com.android.internal.annotations.VisibleForTesting; import java.io.PrintWriter; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Objects; @@ -75,31 +78,59 @@ public class AudioDeviceInventory { private final Object mDeviceInventoryLock = new Object(); @GuardedBy("mDeviceInventoryLock") - private final ArrayList mDeviceInventory = new ArrayList<>(0); - + private final HashMap, AdiDeviceState> mDeviceInventory = new HashMap<>(); List getImmutableDeviceInventory() { synchronized (mDeviceInventoryLock) { - return List.copyOf(mDeviceInventory); + return new ArrayList(mDeviceInventory.values()); } } void addDeviceStateToInventory(AdiDeviceState deviceState) { synchronized (mDeviceInventoryLock) { - mDeviceInventory.add(deviceState); + mDeviceInventory.put(deviceState.getDeviceId(), deviceState); } } + /** + * Adds a new entry in mDeviceInventory if the AudioDeviceAttributes passed is an sink + * Bluetooth device and no corresponding entry already exists. + * @param ada the device to add if needed + */ + void addAudioDeviceInInventoryIfNeeded(AudioDeviceAttributes ada) { + if (!AudioSystem.isBluetoothOutDevice(ada.getInternalType())) { + return; + } + synchronized (mDeviceInventoryLock) { + if (findDeviceStateForAudioDeviceAttributes(ada, ada.getType()) != null) { + return; + } + AdiDeviceState ads = new AdiDeviceState( + ada.getType(), ada.getInternalType(), ada.getAddress()); + mDeviceInventory.put(ads.getDeviceId(), ads); + } + mDeviceBroker.persistAudioDeviceSettings(); + } + + /** + * Finds the device state that matches the passed {@link AudioDeviceAttributes} and device + * type. Note: currently this method only returns a valid device for A2DP and BLE devices. + * + * @param ada attributes of device to match + * @param canonicalDeviceType external device type to match + * @return the found {@link AdiDeviceState} matching a cached A2DP or BLE device or + * {@code null} otherwise. + */ + @Nullable AdiDeviceState findDeviceStateForAudioDeviceAttributes(AudioDeviceAttributes ada, int canonicalDeviceType) { final boolean isWireless = isBluetoothDevice(ada.getInternalType()); - synchronized (mDeviceInventoryLock) { - for (AdiDeviceState deviceSetting : mDeviceInventory) { - if (deviceSetting.getDeviceType() == canonicalDeviceType + for (AdiDeviceState deviceState : mDeviceInventory.values()) { + if (deviceState.getDeviceType() == canonicalDeviceType && (!isWireless || ada.getAddress().equals( - deviceSetting.getDeviceAddress()))) { - return deviceSetting; + deviceState.getDeviceAddress()))) { + return deviceState; } } } @@ -309,7 +340,7 @@ public class AudioDeviceInventory { + " devices:" + devices); }); pw.println("\ndevices:\n"); synchronized (mDeviceInventoryLock) { - for (AdiDeviceState device : mDeviceInventory) { + for (AdiDeviceState device : mDeviceInventory.values()) { pw.println("\t" + device + "\n"); } } @@ -705,8 +736,8 @@ public class AudioDeviceInventory { } /*package*/ void registerStrategyPreferredDevicesDispatcher( - @NonNull IStrategyPreferredDevicesDispatcher dispatcher) { - mPrefDevDispatchers.register(dispatcher); + @NonNull IStrategyPreferredDevicesDispatcher dispatcher, boolean isPrivileged) { + mPrefDevDispatchers.register(dispatcher, isPrivileged); } /*package*/ void unregisterStrategyPreferredDevicesDispatcher( @@ -740,8 +771,8 @@ public class AudioDeviceInventory { } /*package*/ void registerCapturePresetDevicesRoleDispatcher( - @NonNull ICapturePresetDevicesRoleDispatcher dispatcher) { - mDevRoleCapturePresetDispatchers.register(dispatcher); + @NonNull ICapturePresetDevicesRoleDispatcher dispatcher, boolean isPrivileged) { + mDevRoleCapturePresetDispatchers.register(dispatcher, isPrivileged); } /*package*/ void unregisterCapturePresetDevicesRoleDispatcher( @@ -819,6 +850,10 @@ public class AudioDeviceInventory { mConnectedDevices.put(deviceKey, new DeviceInfo( device, deviceName, address, AudioSystem.AUDIO_FORMAT_DEFAULT)); mDeviceBroker.postAccessoryPlugMediaUnmute(device); + + if (AudioSystem.isBluetoothScoDevice(device)) { + addAudioDeviceInInventoryIfNeeded(attributes); + } mmi.set(MediaMetrics.Property.STATE, MediaMetrics.Value.CONNECTED).record(); return true; } else if (!connect && isConnected) { @@ -1049,8 +1084,9 @@ public class AudioDeviceInventory { mDeviceBroker.setBluetoothA2dpOnInt(true, true /*fromA2dp*/, eventSource); // at this point there could be another A2DP device already connected in APM, but it // doesn't matter as this new one will overwrite the previous one - final int res = mAudioSystem.setDeviceConnectionState(new AudioDeviceAttributes( - AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, address, name), + AudioDeviceAttributes ada = new AudioDeviceAttributes( + AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, address, name); + final int res = mAudioSystem.setDeviceConnectionState(ada, AudioSystem.DEVICE_STATE_AVAILABLE, a2dpCodec); // TODO: log in MediaMetrics once distinction between connection failure and @@ -1072,8 +1108,7 @@ public class AudioDeviceInventory { // The convention for head tracking sensors associated with A2DP devices is to // use a UUID derived from the MAC address as follows: // time_low = 0, time_mid = 0, time_hi = 0, clock_seq = 0, node = MAC Address - UUID sensorUuid = UuidUtils.uuidFromAudioDeviceAttributes( - new AudioDeviceAttributes(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, address)); + UUID sensorUuid = UuidUtils.uuidFromAudioDeviceAttributes(ada); final DeviceInfo di = new DeviceInfo(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP, name, address, a2dpCodec, sensorUuid); final String diKey = di.getKey(); @@ -1084,6 +1119,7 @@ public class AudioDeviceInventory { mDeviceBroker.postAccessoryPlugMediaUnmute(AudioSystem.DEVICE_OUT_BLUETOOTH_A2DP); setCurrentAudioRouteNameIfPossible(name, true /*fromA2dp*/); + addAudioDeviceInInventoryIfNeeded(ada); } @GuardedBy("mDevicesLock") @@ -1179,9 +1215,9 @@ public class AudioDeviceInventory { final int hearingAidVolIndex = mDeviceBroker.getVssVolumeForDevice(streamType, AudioSystem.DEVICE_OUT_HEARING_AID); mDeviceBroker.postSetHearingAidVolumeIndex(hearingAidVolIndex, streamType); - - mAudioSystem.setDeviceConnectionState(new AudioDeviceAttributes( - AudioSystem.DEVICE_OUT_HEARING_AID, address, name), + AudioDeviceAttributes ada = new AudioDeviceAttributes( + AudioSystem.DEVICE_OUT_HEARING_AID, address, name); + mAudioSystem.setDeviceConnectionState(ada, AudioSystem.DEVICE_STATE_AVAILABLE, AudioSystem.AUDIO_FORMAT_DEFAULT); mConnectedDevices.put( @@ -1192,6 +1228,7 @@ public class AudioDeviceInventory { mDeviceBroker.postApplyVolumeOnDevice(streamType, AudioSystem.DEVICE_OUT_HEARING_AID, "makeHearingAidDeviceAvailable"); setCurrentAudioRouteNameIfPossible(name, false /*fromA2dp*/); + addAudioDeviceInInventoryIfNeeded(ada); new MediaMetrics.Item(mMetricsId + "makeHearingAidDeviceAvailable") .set(MediaMetrics.Property.ADDRESS, address != null ? address : "") .set(MediaMetrics.Property.DEVICE, @@ -1243,9 +1280,8 @@ public class AudioDeviceInventory { * AUDIO_POLICY_FORCE_NO_BT_A2DP is not set */ mDeviceBroker.setBluetoothA2dpOnInt(true, false /*fromA2dp*/, eventSource); - - final int res = AudioSystem.setDeviceConnectionState(new AudioDeviceAttributes( - device, address, name), + AudioDeviceAttributes ada = new AudioDeviceAttributes(device, address, name); + final int res = AudioSystem.setDeviceConnectionState(ada, AudioSystem.DEVICE_STATE_AVAILABLE, AudioSystem.AUDIO_FORMAT_DEFAULT); if (res != AudioSystem.AUDIO_STATUS_OK) { @@ -1263,6 +1299,7 @@ public class AudioDeviceInventory { new DeviceInfo(device, name, address, AudioSystem.AUDIO_FORMAT_DEFAULT)); mDeviceBroker.postAccessoryPlugMediaUnmute(device); setCurrentAudioRouteNameIfPossible(name, /*fromA2dp=*/false); + addAudioDeviceInInventoryIfNeeded(ada); } if (streamType == AudioSystem.STREAM_DEFAULT) { @@ -1585,6 +1622,9 @@ public class AudioDeviceInventory { final int nbDispatchers = mPrefDevDispatchers.beginBroadcast(); for (int i = 0; i < nbDispatchers; i++) { try { + if (!((Boolean) mPrefDevDispatchers.getBroadcastCookie(i))) { + devices = mDeviceBroker.anonymizeAudioDeviceAttributesListUnchecked(devices); + } mPrefDevDispatchers.getBroadcastItem(i).dispatchPrefDevicesChanged( strategy, devices); } catch (RemoteException e) { @@ -1598,6 +1638,9 @@ public class AudioDeviceInventory { final int nbDispatchers = mDevRoleCapturePresetDispatchers.beginBroadcast(); for (int i = 0; i < nbDispatchers; ++i) { try { + if (!((Boolean) mDevRoleCapturePresetDispatchers.getBroadcastCookie(i))) { + devices = mDeviceBroker.anonymizeAudioDeviceAttributesListUnchecked(devices); + } mDevRoleCapturePresetDispatchers.getBroadcastItem(i).dispatchDevicesRoleChanged( capturePreset, role, devices); } catch (RemoteException e) { @@ -1634,19 +1677,20 @@ public class AudioDeviceInventory { int deviceCatalogSize = 0; synchronized (mDeviceInventoryLock) { deviceCatalogSize = mDeviceInventory.size(); - } - final StringBuilder settingsBuilder = new StringBuilder( - deviceCatalogSize * AdiDeviceState.getPeristedMaxSize()); - synchronized (mDeviceInventoryLock) { - for (int i = 0; i < mDeviceInventory.size(); i++) { - settingsBuilder.append(mDeviceInventory.get(i).toPersistableString()); - if (i != mDeviceInventory.size() - 1) { - settingsBuilder.append(SETTING_DEVICE_SEPARATOR_CHAR); - } + final StringBuilder settingsBuilder = new StringBuilder( + deviceCatalogSize * AdiDeviceState.getPeristedMaxSize()); + + Iterator iterator = mDeviceInventory.values().iterator(); + if (iterator.hasNext()) { + settingsBuilder.append(iterator.next().toPersistableString()); + } + while (iterator.hasNext()) { + settingsBuilder.append(SETTING_DEVICE_SEPARATOR_CHAR); + settingsBuilder.append(iterator.next().toPersistableString()); } + return settingsBuilder.toString(); } - return settingsBuilder.toString(); } /*package*/ void setDeviceSettings(String settings) { diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index ea2a5f475235..db148f62366d 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -2827,8 +2827,11 @@ public class AudioService extends IAudioService.Stub return AudioSystem.ERROR; } enforceModifyAudioRoutingPermission(); + + devices = retrieveBluetoothAddresses(devices); + final String logString = String.format( - "setPreferredDeviceForStrategy u/pid:%d/%d strat:%d dev:%s", + "setPreferredDevicesForStrategy u/pid:%d/%d strat:%d dev:%s", Binder.getCallingUid(), Binder.getCallingPid(), strategy, devices.stream().map(e -> e.toString()).collect(Collectors.joining(","))); sDeviceLogger.log(new AudioEventLogger.StringEvent(logString).printLog(TAG)); @@ -2876,7 +2879,7 @@ public class AudioService extends IAudioService.Stub status, strategy)); return new ArrayList(); } else { - return devices; + return anonymizeAudioDeviceAttributesList(devices); } } @@ -2889,7 +2892,8 @@ public class AudioService extends IAudioService.Stub return; } enforceModifyAudioRoutingPermission(); - mDeviceBroker.registerStrategyPreferredDevicesDispatcher(dispatcher); + mDeviceBroker.registerStrategyPreferredDevicesDispatcher( + dispatcher, isBluetoothPrividged()); } /** @see AudioManager#removeOnPreferredDevicesForStrategyChangedListener( @@ -2905,7 +2909,7 @@ public class AudioService extends IAudioService.Stub } /** - * @see AudioManager#setPreferredDeviceForCapturePreset(int, AudioDeviceAttributes) + * @see AudioManager#setPreferredDevicesForCapturePreset(int, AudioDeviceAttributes) */ public int setPreferredDevicesForCapturePreset( int capturePreset, List devices) { @@ -2924,6 +2928,8 @@ public class AudioService extends IAudioService.Stub return AudioSystem.ERROR; } + devices = retrieveBluetoothAddresses(devices); + final int status = mDeviceBroker.setPreferredDevicesForCapturePresetSync( capturePreset, devices); if (status != AudioSystem.SUCCESS) { @@ -2962,7 +2968,7 @@ public class AudioService extends IAudioService.Stub status, capturePreset)); return new ArrayList(); } else { - return devices; + return anonymizeAudioDeviceAttributesList(devices); } } @@ -2976,7 +2982,8 @@ public class AudioService extends IAudioService.Stub return; } enforceModifyAudioRoutingPermission(); - mDeviceBroker.registerCapturePresetDevicesRoleDispatcher(dispatcher); + mDeviceBroker.registerCapturePresetDevicesRoleDispatcher( + dispatcher, isBluetoothPrividged()); } /** @@ -2996,7 +3003,9 @@ public class AudioService extends IAudioService.Stub public @NonNull ArrayList getDevicesForAttributes( @NonNull AudioAttributes attributes) { enforceQueryStateOrModifyRoutingPermission(); - return getDevicesForAttributesInt(attributes, false /* forVolume */); + + return new ArrayList(anonymizeAudioDeviceAttributesList( + getDevicesForAttributesInt(attributes, false /* forVolume */))); } /** @see AudioManager#getAudioDevicesForAttributes(AudioAttributes) @@ -3006,7 +3015,8 @@ public class AudioService extends IAudioService.Stub */ public @NonNull ArrayList getDevicesForAttributesUnprotected( @NonNull AudioAttributes attributes) { - return getDevicesForAttributesInt(attributes, false /* forVolume */); + return new ArrayList(anonymizeAudioDeviceAttributesList( + getDevicesForAttributesInt(attributes, false /* forVolume */))); } /** @@ -7114,6 +7124,9 @@ public class AudioService extends IAudioService.Stub // verify arguments Objects.requireNonNull(device); AudioManager.enforceValidVolumeBehavior(deviceVolumeBehavior); + + device = retrieveBluetoothAddress(device); + sVolumeLogger.log(new AudioEventLogger.StringEvent("setDeviceVolumeBehavior: dev:" + AudioSystem.getOutputDeviceName(device.getInternalType()) + " addr:" + device.getAddress() + " behavior:" @@ -7190,6 +7203,8 @@ public class AudioService extends IAudioService.Stub // verify permissions enforceQueryStateOrModifyRoutingPermission(); + device = retrieveBluetoothAddress(device); + return getDeviceVolumeBehaviorInt(device); } @@ -7265,9 +7280,13 @@ public class AudioService extends IAudioService.Stub /** * see AudioManager.setWiredDeviceConnectionState() */ - public void setWiredDeviceConnectionState(AudioDeviceAttributes attributes, + public void setWiredDeviceConnectionState(@NonNull AudioDeviceAttributes attributes, @ConnectionState int state, String caller) { enforceModifyAudioRoutingPermission(); + Objects.requireNonNull(attributes); + + attributes = retrieveBluetoothAddress(attributes); + if (state != CONNECTION_STATE_CONNECTED && state != CONNECTION_STATE_DISCONNECTED) { throw new IllegalArgumentException("Invalid state " + state); @@ -7289,6 +7308,9 @@ public class AudioService extends IAudioService.Stub boolean connected) { Objects.requireNonNull(device); enforceModifyAudioRoutingPermission(); + + device = retrieveBluetoothAddress(device); + mDeviceBroker.setTestDeviceConnectionState(device, connected ? CONNECTION_STATE_CONNECTED : CONNECTION_STATE_DISCONNECTED); // simulate a routing update from native @@ -9902,6 +9924,100 @@ public class AudioService extends IAudioService.Stub mSpatializerHelper.setFeatureEnabled(mHasSpatializerEffect); } + private boolean isBluetoothPrividged() { + return PackageManager.PERMISSION_GRANTED == mContext.checkCallingOrSelfPermission( + android.Manifest.permission.BLUETOOTH_CONNECT) + || Binder.getCallingUid() == Process.SYSTEM_UID; + } + + List retrieveBluetoothAddresses(List devices) { + if (isBluetoothPrividged()) { + return devices; + } + + List checkedDevices = new ArrayList(); + for (AudioDeviceAttributes ada : devices) { + if (ada == null) { + continue; + } + checkedDevices.add(retrieveBluetoothAddressUncheked(ada)); + } + return checkedDevices; + } + + AudioDeviceAttributes retrieveBluetoothAddress(@NonNull AudioDeviceAttributes ada) { + if (isBluetoothPrividged()) { + return ada; + } + return retrieveBluetoothAddressUncheked(ada); + } + + AudioDeviceAttributes retrieveBluetoothAddressUncheked(@NonNull AudioDeviceAttributes ada) { + Objects.requireNonNull(ada); + if (AudioSystem.isBluetoothDevice(ada.getInternalType())) { + String anonymizedAddress = anonymizeBluetoothAddress(ada.getAddress()); + for (AdiDeviceState ads : mDeviceBroker.getImmutableDeviceInventory()) { + if (!(AudioSystem.isBluetoothDevice(ads.getInternalDeviceType()) + && (ada.getInternalType() == ads.getInternalDeviceType()) + && anonymizedAddress.equals(anonymizeBluetoothAddress( + ads.getDeviceAddress())))) { + continue; + } + ada.setAddress(ads.getDeviceAddress()); + break; + } + } + return ada; + } + + /** + * Convert a Bluetooth MAC address to an anonymized one when exposed to a non privileged app + * Must match the implementation of BluetoothUtils.toAnonymizedAddress() + * @param address Mac address to be anonymized + * @return anonymized mac address + */ + static String anonymizeBluetoothAddress(String address) { + if (address == null || address.length() != "AA:BB:CC:DD:EE:FF".length()) { + return null; + } + return "XX:XX:XX:XX" + address.substring("XX:XX:XX:XX".length()); + } + + private List anonymizeAudioDeviceAttributesList( + List devices) { + if (isBluetoothPrividged()) { + return devices; + } + return anonymizeAudioDeviceAttributesListUnchecked(devices); + } + + /* package */ List anonymizeAudioDeviceAttributesListUnchecked( + List devices) { + List anonymizedDevices = new ArrayList(); + for (AudioDeviceAttributes ada : devices) { + anonymizedDevices.add(anonymizeAudioDeviceAttributesUnchecked(ada)); + } + return anonymizedDevices; + } + + private AudioDeviceAttributes anonymizeAudioDeviceAttributesUnchecked( + AudioDeviceAttributes ada) { + if (!AudioSystem.isBluetoothDevice(ada.getInternalType())) { + return ada; + } + AudioDeviceAttributes res = new AudioDeviceAttributes(ada); + res.setAddress(anonymizeBluetoothAddress(ada.getAddress())); + return res; + } + + private AudioDeviceAttributes anonymizeAudioDeviceAttributes(AudioDeviceAttributes ada) { + if (isBluetoothPrividged()) { + return ada; + } + + return anonymizeAudioDeviceAttributesUnchecked(ada); + } + //========================================================================================== private boolean readCameraSoundForced() { return SystemProperties.getBoolean("audio.camerasound.force", false) || @@ -9929,13 +10045,16 @@ public class AudioService extends IAudioService.Stub Objects.requireNonNull(usages); Objects.requireNonNull(device); enforceModifyAudioRoutingPermission(); + + final AudioDeviceAttributes ada = retrieveBluetoothAddress(device); + if (timeOutMs <= 0 || usages.length == 0) { throw new IllegalArgumentException("Invalid timeOutMs/usagesToMute"); } Log.i(TAG, "muteAwaitConnection dev:" + device + " timeOutMs:" + timeOutMs + " usages:" + Arrays.toString(usages)); - if (mDeviceBroker.isDeviceConnected(device)) { + if (mDeviceBroker.isDeviceConnected(ada)) { // not throwing an exception as there could be a race between a connection (server-side, // notification of connection in flight) and a mute operation (client-side) Log.i(TAG, "muteAwaitConnection ignored, device (" + device + ") already connected"); @@ -9947,19 +10066,26 @@ public class AudioService extends IAudioService.Stub + mMutingExpectedDevice); throw new IllegalStateException("muteAwaitConnection already in progress"); } - mMutingExpectedDevice = device; + mMutingExpectedDevice = ada; mMutedUsagesAwaitingConnection = usages; - mPlaybackMonitor.muteAwaitConnection(usages, device, timeOutMs); + mPlaybackMonitor.muteAwaitConnection(usages, ada, timeOutMs); } - dispatchMuteAwaitConnection(cb -> { try { - cb.dispatchOnMutedUntilConnection(device, usages); } catch (RemoteException e) { } }); + dispatchMuteAwaitConnection((cb, isPrivileged) -> { + try { + AudioDeviceAttributes dev = ada; + if (!isPrivileged) { + dev = anonymizeAudioDeviceAttributesUnchecked(ada); + } + cb.dispatchOnMutedUntilConnection(dev, usages); + } catch (RemoteException e) { } + }); } /** @see AudioManager#getMutingExpectedDevice */ public @Nullable AudioDeviceAttributes getMutingExpectedDevice() { enforceModifyAudioRoutingPermission(); synchronized (mMuteAwaitConnectionLock) { - return mMutingExpectedDevice; + return anonymizeAudioDeviceAttributes(mMutingExpectedDevice); } } @@ -9968,6 +10094,9 @@ public class AudioService extends IAudioService.Stub public void cancelMuteAwaitConnection(@NonNull AudioDeviceAttributes device) { Objects.requireNonNull(device); enforceModifyAudioRoutingPermission(); + + final AudioDeviceAttributes ada = retrieveBluetoothAddress(device); + Log.i(TAG, "cancelMuteAwaitConnection for device:" + device); final int[] mutedUsages; synchronized (mMuteAwaitConnectionLock) { @@ -9977,7 +10106,7 @@ public class AudioService extends IAudioService.Stub Log.i(TAG, "cancelMuteAwaitConnection ignored, no expected device"); return; } - if (!device.equalTypeAddress(mMutingExpectedDevice)) { + if (!ada.equalTypeAddress(mMutingExpectedDevice)) { Log.e(TAG, "cancelMuteAwaitConnection ignored, got " + device + "] but expected device is" + mMutingExpectedDevice); throw new IllegalStateException("cancelMuteAwaitConnection for wrong device"); @@ -9987,8 +10116,14 @@ public class AudioService extends IAudioService.Stub mMutedUsagesAwaitingConnection = null; mPlaybackMonitor.cancelMuteAwaitConnection("cancelMuteAwaitConnection dev:" + device); } - dispatchMuteAwaitConnection(cb -> { try { cb.dispatchOnUnmutedEvent( - AudioManager.MuteAwaitConnectionCallback.EVENT_CANCEL, device, mutedUsages); + dispatchMuteAwaitConnection((cb, isPrivileged) -> { + try { + AudioDeviceAttributes dev = ada; + if (!isPrivileged) { + dev = anonymizeAudioDeviceAttributesUnchecked(ada); + } + cb.dispatchOnUnmutedEvent( + AudioManager.MuteAwaitConnectionCallback.EVENT_CANCEL, dev, mutedUsages); } catch (RemoteException e) { } }); } @@ -10000,7 +10135,7 @@ public class AudioService extends IAudioService.Stub boolean register) { enforceModifyAudioRoutingPermission(); if (register) { - mMuteAwaitConnectionDispatchers.register(cb); + mMuteAwaitConnectionDispatchers.register(cb, isBluetoothPrividged()); } else { mMuteAwaitConnectionDispatchers.unregister(cb); } @@ -10024,8 +10159,14 @@ public class AudioService extends IAudioService.Stub mPlaybackMonitor.cancelMuteAwaitConnection( "checkMuteAwaitConnection device " + device + " connected, unmuting"); } - dispatchMuteAwaitConnection(cb -> { try { cb.dispatchOnUnmutedEvent( - AudioManager.MuteAwaitConnectionCallback.EVENT_CONNECTION, device, mutedUsages); + dispatchMuteAwaitConnection((cb, isPrivileged) -> { + try { + AudioDeviceAttributes ada = device; + if (!isPrivileged) { + ada = anonymizeAudioDeviceAttributesUnchecked(device); + } + cb.dispatchOnUnmutedEvent(AudioManager.MuteAwaitConnectionCallback.EVENT_CONNECTION, + ada, mutedUsages); } catch (RemoteException e) { } }); } @@ -10045,7 +10186,8 @@ public class AudioService extends IAudioService.Stub mMutingExpectedDevice = null; mMutedUsagesAwaitingConnection = null; } - dispatchMuteAwaitConnection(cb -> { try { + dispatchMuteAwaitConnection((cb, isPrivileged) -> { + try { cb.dispatchOnUnmutedEvent( AudioManager.MuteAwaitConnectionCallback.EVENT_TIMEOUT, timedOutDevice, mutedUsages); @@ -10053,13 +10195,14 @@ public class AudioService extends IAudioService.Stub } private void dispatchMuteAwaitConnection( - java.util.function.Consumer callback) { + java.util.function.BiConsumer callback) { final int nbDispatchers = mMuteAwaitConnectionDispatchers.beginBroadcast(); // lazy initialization as errors unlikely ArrayList errorList = null; for (int i = 0; i < nbDispatchers; i++) { try { - callback.accept(mMuteAwaitConnectionDispatchers.getBroadcastItem(i)); + callback.accept(mMuteAwaitConnectionDispatchers.getBroadcastItem(i), + (Boolean) mMuteAwaitConnectionDispatchers.getBroadcastCookie(i)); } catch (Exception e) { if (errorList == null) { errorList = new ArrayList<>(1); @@ -12324,6 +12467,9 @@ public class AudioService extends IAudioService.Stub @NonNull AudioDeviceAttributes device, @IntRange(from = 0) long delayMillis) { Objects.requireNonNull(device, "device must not be null"); enforceModifyAudioRoutingPermission(); + + device = retrieveBluetoothAddress(device); + final String getterKey = "additional_output_device_delay=" + device.getInternalType() + "," + device.getAddress(); // "getter" key as an id. final String setterKey = getterKey + "," + delayMillis; // append the delay for setter @@ -12344,6 +12490,9 @@ public class AudioService extends IAudioService.Stub @IntRange(from = 0) public long getAdditionalOutputDeviceDelay(@NonNull AudioDeviceAttributes device) { Objects.requireNonNull(device, "device must not be null"); + + device = retrieveBluetoothAddress(device); + final String key = "additional_output_device_delay"; final String reply = AudioSystem.getParameters( key + "=" + device.getInternalType() + "," + device.getAddress()); @@ -12371,6 +12520,9 @@ public class AudioService extends IAudioService.Stub @IntRange(from = 0) public long getMaxAdditionalOutputDeviceDelay(@NonNull AudioDeviceAttributes device) { Objects.requireNonNull(device, "device must not be null"); + + device = retrieveBluetoothAddress(device); + final String key = "max_additional_output_device_delay"; final String reply = AudioSystem.getParameters( key + "=" + device.getInternalType() + "," + device.getAddress()); -- cgit v1.2.3 From bf2acc6bf66417790ece0985e46835f0f92b9233 Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Wed, 1 Nov 2023 13:36:31 +0000 Subject: Handle screen capture disabled state for multiple users Before this fix, the framework use one single integer to track screen capture disabled policy across the device. This can be either a real userId, or the symbolic USER_ALL or USER_NULL for all users or no user. This has the problem that it can't represent policies from more than one admin (or policies from both the admin and its parent). In addition, due to the way DevicePolicyManagerService refreshes policies, when a non-managed user is started, its default policy state overrides any existing policy, causing screen capture to be no longer disabled. Fix this by properly tracking the policy for each user in an set. We still use USER_ALL in the set to represent screen capture being disabled for all users. In Android 14, the screen capture logic is migrated to policy engine which already contains this fix. Bug: 305664128 Test: ScreenCaptureDisabledTest Test: FrameworksServicesTests:DevicePolicyManagerTest Test: DevicePolicyManagerServiceMigrationTest Test: MixedDeviceOwnerTest#testScreenCaptureDisabled_assist MixedProfileOwnerTest#testScreenCaptureDisabled_assist MixedManagedProfileOwnerTest#testScreenCaptureDisabled_assist Test: MixedManagedProfileOwnerTest#testScreenCaptureDisabled_assist_allowedPrimaryUser Test: MixedDeviceOwnerTest#testCreateAdminSupportIntent MixedProfileOwnerTest#testCreateAdminSupportIntent MixedManagedProfileOwnerTest#testCreateAdminSupportIntent (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5f3db5ae9b9cf69c8a4ea73b6ed49ce9d49ba223) Merged-In: Ib016b740927003f8bd81992f24c722c29ac723b5 Change-Id: Ib016b740927003f8bd81992f24c722c29ac723b5 --- .../server/devicepolicy/DevicePolicyCacheImpl.java | 26 +++++++++++++++++--- .../devicepolicy/DevicePolicyManagerService.java | 28 +++++++--------------- .../devicepolicy/DevicePolicyManagerTest.java | 2 +- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java index 304d148252b1..fa11fb2061df 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyCacheImpl.java @@ -25,6 +25,9 @@ import android.util.SparseIntArray; import com.android.internal.annotations.GuardedBy; +import java.util.HashSet; +import java.util.Set; + /** * Implementation of {@link DevicePolicyCache}, to which {@link DevicePolicyManagerService} pushes * policies. @@ -45,6 +48,13 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { @GuardedBy("mLock") private int mScreenCaptureDisallowedUser = UserHandle.USER_NULL; + /** + * Indicates if screen capture is disallowed on a specific user or all users if + * it contains {@link UserHandle#USER_ALL}. + */ + @GuardedBy("mLock") + private Set mScreenCaptureDisallowedUsers = new HashSet<>(); + @GuardedBy("mLock") private final SparseIntArray mPasswordQuality = new SparseIntArray(); @@ -71,8 +81,8 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { @Override public boolean isScreenCaptureAllowed(int userHandle) { synchronized (mLock) { - return mScreenCaptureDisallowedUser != UserHandle.USER_ALL - && mScreenCaptureDisallowedUser != userHandle; + return !mScreenCaptureDisallowedUsers.contains(userHandle) + && !mScreenCaptureDisallowedUsers.contains(UserHandle.USER_ALL); } } @@ -88,6 +98,16 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { } } + public void setScreenCaptureDisallowedUser(int userHandle, boolean disallowed) { + synchronized (mLock) { + if (disallowed) { + mScreenCaptureDisallowedUsers.add(userHandle); + } else { + mScreenCaptureDisallowedUsers.remove(userHandle); + } + } + } + @Override public int getPasswordQuality(@UserIdInt int userHandle) { synchronized (mLock) { @@ -136,7 +156,7 @@ public class DevicePolicyCacheImpl extends DevicePolicyCache { public void dump(IndentingPrintWriter pw) { pw.println("Device policy cache:"); pw.increaseIndent(); - pw.println("Screen capture disallowed user: " + mScreenCaptureDisallowedUser); + pw.println("Screen capture disallowed users: " + mScreenCaptureDisallowedUsers); pw.println("Password quality: " + mPasswordQuality.toString()); pw.println("Permission policy: " + mPermissionPolicy.toString()); pw.println("Admin can grant sensors permission: " diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index c07b058bb2ca..55af0fa9d372 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -7712,30 +7712,20 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { // be disabled device-wide. private void pushScreenCapturePolicy(int adminUserId) { // Update screen capture device-wide if disabled by the DO or COPE PO on the parent profile. - ActiveAdmin admin = - getDeviceOwnerOrProfileOwnerOfOrganizationOwnedDeviceParentLocked( + // Always do this regardless which user this method is called with. Probably a little + // wasteful but still safe. + ActiveAdmin admin = getDeviceOwnerOrProfileOwnerOfOrganizationOwnedDeviceParentLocked( UserHandle.USER_SYSTEM); - if (admin != null && admin.disableScreenCapture) { - setScreenCaptureDisabled(UserHandle.USER_ALL); - } else { - // Otherwise, update screen capture only for the calling user. - admin = getProfileOwnerAdminLocked(adminUserId); - if (admin != null && admin.disableScreenCapture) { - setScreenCaptureDisabled(adminUserId); - } else { - setScreenCaptureDisabled(UserHandle.USER_NULL); - } - } + setScreenCaptureDisabled(UserHandle.USER_ALL, admin != null && admin.disableScreenCapture); + // Update screen capture only for the calling user. + admin = getProfileOwnerAdminLocked(adminUserId); + setScreenCaptureDisabled(adminUserId, admin != null && admin.disableScreenCapture); } // Set the latest screen capture policy, overriding any existing ones. // userHandle can be one of USER_ALL, USER_NULL or a concrete userId. - private void setScreenCaptureDisabled(int userHandle) { - int current = mPolicyCache.getScreenCaptureDisallowedUser(); - if (userHandle == current) { - return; - } - mPolicyCache.setScreenCaptureDisallowedUser(userHandle); + private void setScreenCaptureDisabled(int userHandle, boolean disabled) { + mPolicyCache.setScreenCaptureDisallowedUser(userHandle, disabled); updateScreenCaptureDisabled(); } diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 7e5033200210..1a620031e8e4 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -5058,7 +5058,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { // Refresh strong auth timeout verify(getServices().lockSettingsInternal).refreshStrongAuthTimeout(UserHandle.USER_SYSTEM); // Refresh screen capture - verify(getServices().iwindowManager).refreshScreenCaptureDisabled(); + verify(getServices().iwindowManager, times(2)).refreshScreenCaptureDisabled(); // Unsuspend personal apps verify(getServices().packageManagerInternal) .unsuspendForSuspendingPackage(PLATFORM_PACKAGE_NAME, UserHandle.USER_SYSTEM); -- cgit v1.2.3 From 3e617ac8316905b131c2555812c1d142f180f516 Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Wed, 8 Nov 2023 11:01:32 +0100 Subject: Enforce persisted snoozed notifications limits Prevent DoS attack that causes boot-looping by serializing a huge amount of snoozed notifications: - Check snooze limits for persisted notifications - Remove persisted group summary notification when in-memory counterpart is removed - Prevent unpriviledged API calls that allow 3P apps to snooze notifications with context/criterion Test: atest SnoozeHelperTest Test: atest NotificationManagerServiceTest Bug: 307948424 Bug: 308414141 (cherry picked from commit 965ff2d3c5487f72a77f6153ed8542cb2621d93c) (cherry picked from commit da6a9ea6deece5b2505d5facdf5d44cfc08057f3) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5adc11e960ffd891a8e48448b50cb25dad74eb78) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:02fac79d219f5abbca842c448ea5440ae62f0783) Merged-In: I3571fa9207b778def652130d3ca840183a9a8414 Change-Id: I3571fa9207b778def652130d3ca840183a9a8414 --- .../android/server/notification/SnoozeHelper.java | 22 ++++++---------------- .../server/notification/SnoozeHelperTest.java | 5 ----- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index 73d4b147f90a..0044912bbcf4 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -119,28 +119,15 @@ public final class SnoozeHelper { protected boolean canSnooze(int numberToSnooze) { synchronized (mLock) { if ((mSnoozedNotifications.size() + numberToSnooze) > CONCURRENT_SNOOZE_LIMIT - || (countPersistedNotificationsLocked() + numberToSnooze) - > CONCURRENT_SNOOZE_LIMIT) { + || (mPersistedSnoozedNotifications.size() + + mPersistedSnoozedNotificationsWithContext.size() + numberToSnooze) + > CONCURRENT_SNOOZE_LIMIT) { return false; } } return true; } - private int countPersistedNotificationsLocked() { - int numNotifications = 0; - for (ArrayMap persistedWithContext : - mPersistedSnoozedNotificationsWithContext.values()) { - numNotifications += persistedWithContext.size(); - } - for (ArrayMap persistedWithDuration : - mPersistedSnoozedNotifications.values()) { - numNotifications += persistedWithDuration.size(); - } - return numNotifications; - } - - @NonNull protected Long getSnoozeTimeForUnpostedNotification(int userId, String pkg, String key) { Long time = null; @@ -359,6 +346,9 @@ public final class SnoozeHelper { if (groupSummaryKey != null) { NotificationRecord record = mSnoozedNotifications.remove(groupSummaryKey); + String trimmedKey = getTrimmedString(groupSummaryKey); + mPersistedSnoozedNotificationsWithContext.remove(trimmedKey); + mPersistedSnoozedNotifications.remove(trimmedKey); final String trimmedKey = getTrimmedString(groupSummaryKey); removeRecordLocked(pkg, trimmedKey, userId, mPersistedSnoozedNotifications); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java index 12590ac0d66a..0c004b65e5e8 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/SnoozeHelperTest.java @@ -81,8 +81,6 @@ public class SnoozeHelperTest extends UiServiceTestCase { private static final String XML_SNOOZED_NOTIFICATION_TIME = "time"; private static final String XML_SNOOZED_NOTIFICATION_CONTEXT_ID = "id"; private static final String XML_SNOOZED_NOTIFICATION_VERSION_LABEL = "version"; - private static final String XML_SNOOZED_NOTIFICATION_PKG = "pkg"; - private static final String XML_SNOOZED_NOTIFICATION_USER_ID = "user-id"; @Mock SnoozeHelper.Callback mCallback; @Mock AlarmManager mAm; @@ -344,9 +342,6 @@ public class SnoozeHelperTest extends UiServiceTestCase { } else { serializer.startTag(null, XML_SNOOZED_NOTIFICATION_CONTEXT); } - serializer.attribute(null, XML_SNOOZED_NOTIFICATION_PKG, "pkg"); - serializer.attributeInt(null, XML_SNOOZED_NOTIFICATION_USER_ID, - UserHandle.USER_SYSTEM); serializer.attributeInt(null, XML_SNOOZED_NOTIFICATION_VERSION_LABEL, 1); serializer.attribute(null, XML_SNOOZED_NOTIFICATION_KEY, "key" + i); if (timedNotification) { -- cgit v1.2.3 From 9148e325b7078d5cf8bcf0d57e069756874adb01 Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Wed, 8 Nov 2023 11:01:32 +0100 Subject: Enforce persisted snoozed notifications limits Prevent DoS attack that causes boot-looping by serializing a huge amount of snoozed notifications: - Check snooze limits for persisted notifications - Remove persisted group summary notification when in-memory counterpart is removed - Prevent unpriviledged API calls that allow 3P apps to snooze notifications with context/criterion Test: atest SnoozeHelperTest Test: atest NotificationManagerServiceTest Bug: 307948424 Bug: 308414141 (cherry picked from commit 965ff2d3c5487f72a77f6153ed8542cb2621d93c) (cherry picked from commit da6a9ea6deece5b2505d5facdf5d44cfc08057f3) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5adc11e960ffd891a8e48448b50cb25dad74eb78) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a34f94ffc60409a42a0d0576b9ed7bcafa594c67) Merged-In: I3571fa9207b778def652130d3ca840183a9a8414 Change-Id: I3571fa9207b778def652130d3ca840183a9a8414 --- services/core/java/com/android/server/notification/SnoozeHelper.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/services/core/java/com/android/server/notification/SnoozeHelper.java b/services/core/java/com/android/server/notification/SnoozeHelper.java index 0044912bbcf4..d291f5f97716 100644 --- a/services/core/java/com/android/server/notification/SnoozeHelper.java +++ b/services/core/java/com/android/server/notification/SnoozeHelper.java @@ -350,11 +350,6 @@ public final class SnoozeHelper { mPersistedSnoozedNotificationsWithContext.remove(trimmedKey); mPersistedSnoozedNotifications.remove(trimmedKey); - final String trimmedKey = getTrimmedString(groupSummaryKey); - removeRecordLocked(pkg, trimmedKey, userId, mPersistedSnoozedNotifications); - removeRecordLocked(pkg, trimmedKey, userId, - mPersistedSnoozedNotificationsWithContext); - if (record != null && !record.isCanceled) { Runnable runnable = () -> { MetricsLogger.action(record.getLogMaker() -- cgit v1.2.3 From a408d334752bb1a6d7bd4d5fca4bae742bccc8fe Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Sun, 27 Mar 2022 15:12:42 -0700 Subject: DisplayContent: Don't force-update parent if layering didn't change A recent change to provide IME support for SurfaceControlViewHost modified logic for setImeLayeringTargetInner. setImeLayeringTarget inner also updates the control target, which may depend on the input target. In the case of SurfaceControlViewHost the input target can change with the layering target not changing, and so we need to update the control target, which was done by extending the conditions on the return at the beginning of setImeLayeringTargetInner to also include the check for mLastImeInputTarget == mImeInputTarget. However in standard app cases, this can produce a scenario where we consider the IME parent as changed, even when it wasn't (when neither the layering or the control target update). This CL partially restores the previous logic by not forcing the IME parent update when the layering target didn't really change. Bug: 226338567 Test: Existing tests pass Change-Id: I8ec9583feca8a0be9c90b0a4a2a382bf915ae4be (cherry picked from commit 2a1ef7a72fa99133e9d61bfda7294edbfbab8836) --- services/core/java/com/android/server/wm/DisplayContent.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 45adc1bfa95f..8934ff9a3100 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -4187,6 +4187,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp } ProtoLog.i(WM_DEBUG_IME, "setInputMethodTarget %s", target); + final boolean layeringTargetChanged = target != mImeLayeringTarget; mImeLayeringTarget = target; // 1. Reparent the IME container window to the target root DA to get the correct bounds and @@ -4214,7 +4215,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp // 4. Update the IME control target to apply any inset change and animation. // 5. Reparent the IME container surface to either the input target app, or the IME window // parent. - updateImeControlTarget(true /* forceUpdateImeParent */); + updateImeControlTarget(layeringTargetChanged); } @VisibleForTesting -- cgit v1.2.3 From 25cca8f4e630281546e27962b95addc2792917cf Mon Sep 17 00:00:00 2001 From: Beverly Tai Date: Thu, 14 Sep 2023 20:51:56 +0000 Subject: Revert "On device lockdown, always show the keyguard" This reverts commit c851ef6d27bf7d3193c85170706a67fb6185981a. Reason for revert: b/300463732 regression Bug: 300463732 Bug: 218495634 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:eaa129138096bc00b663bca93a5af9786aa47154) Merged-In: I314e5615b798487d9a965eaa0937905b65aa85fc Change-Id: I314e5615b798487d9a965eaa0937905b65aa85fc --- .../com/android/systemui/keyguard/KeyguardViewMediator.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index cba717038871..3f027cb31f25 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -768,13 +768,6 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } } } - - @Override - public void onStrongAuthStateChanged(int userId) { - if (mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { - doKeyguardLocked(null); - } - } }; ViewMediatorCallback mViewMediatorCallback = new ViewMediatorCallback() { @@ -2163,9 +2156,8 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, * Enable the keyguard if the settings are appropriate. */ private void doKeyguardLocked(Bundle options) { - // if another app is disabling us, don't show unless we're in lockdown mode - if (!mExternallyEnabled - && !mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { + // if another app is disabling us, don't show + if (!mExternallyEnabled) { if (DEBUG) Log.d(TAG, "doKeyguard: not showing because externally disabled"); mNeedToReshowWhenReenabled = true; -- cgit v1.2.3 From afa3a67b20aed87456969258e30690ffc90b44c6 Mon Sep 17 00:00:00 2001 From: Beverly Tai Date: Tue, 19 Sep 2023 20:54:47 +0000 Subject: Updated: always show the keyguard on device lockdown Additionally, don't hide keyguard when it's disabled if the user has locked down the device. Manual test steps: 1. Enable app pinning and disable "Ask for PIN before unpinning" setting 2. Pin an app (ie: Settings) 3. Lockdown from the power menu 4. Observe: user is brought to the keyguard, primary auth is required to enter the device. => After entering correct credential, the device is still in app pinning mode. => After entering an incorrect credential, the keyguard remains showing and the user can attempt again up to the limit Bug: 300463732 Bug: 218495634 Test: atest KeyguardViewMediatorTest Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:d9c7c85c52c007fdedb177b9f5f98821d0a76090) Merged-In: I70fdae80f717712b3dfc9df54b9649959b4bb8f0 Change-Id: I70fdae80f717712b3dfc9df54b9649959b4bb8f0 --- .../com/android/systemui/keyguard/KeyguardViewMediator.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index 3f027cb31f25..cba717038871 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -768,6 +768,13 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, } } } + + @Override + public void onStrongAuthStateChanged(int userId) { + if (mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { + doKeyguardLocked(null); + } + } }; ViewMediatorCallback mViewMediatorCallback = new ViewMediatorCallback() { @@ -2156,8 +2163,9 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, * Enable the keyguard if the settings are appropriate. */ private void doKeyguardLocked(Bundle options) { - // if another app is disabling us, don't show - if (!mExternallyEnabled) { + // if another app is disabling us, don't show unless we're in lockdown mode + if (!mExternallyEnabled + && !mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) { if (DEBUG) Log.d(TAG, "doKeyguard: not showing because externally disabled"); mNeedToReshowWhenReenabled = true; -- cgit v1.2.3 From 8a47209f84c8377eb57c8635adf55542b7bf6c17 Mon Sep 17 00:00:00 2001 From: Kiran Ramachandra Date: Tue, 19 Dec 2023 21:33:56 +0000 Subject: RESTRICT AUTOMERGE Added limitations for attributions to handle invalid cases Bug: 304983146 Test: Modified and introduced new tests to verify change -> atest CtsAppOpsTestCases:AttributionTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:2806d263c0b74da36d6d2bcc1583ea641266fd43) Merged-In: Iee26fdb9cf1ca0fa8905e22732c32ec7d9b80fea Change-Id: Iee26fdb9cf1ca0fa8905e22732c32ec7d9b80fea --- .../com/android/server/appop/AppOpsService.java | 38 ++++++++++++++++++++++ .../pm/pkg/component/ParsedAttributionImpl.java | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index a110169ac8c2..33655f748230 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -2640,6 +2640,10 @@ public class AppOpsService extends IAppOpsService.Stub { return new SyncNotedAppOp(AppOpsManager.MODE_ERRORED, code, attributionTag, packageName); } + if (proxyAttributionTag != null + && !isAttributionTagDefined(packageName, proxyPackageName, proxyAttributionTag)) { + proxyAttributionTag = null; + } synchronized (this) { final Ops ops = getOpsLocked(uid, packageName, attributionTag, @@ -3177,6 +3181,10 @@ public class AppOpsService extends IAppOpsService.Stub { return new SyncNotedAppOp(AppOpsManager.MODE_ERRORED, code, attributionTag, packageName); } + if (proxyAttributionTag != null + && !isAttributionTagDefined(packageName, proxyPackageName, proxyAttributionTag)) { + proxyAttributionTag = null; + } boolean isRestricted = false; int startType = START_TYPE_FAILED; @@ -3909,6 +3917,36 @@ public class AppOpsService extends IAppOpsService.Stub { return false; } + /** + * Checks to see if the attribution tag is defined in either package or proxyPackage. + * This method is intended for ProxyAttributionTag validation and returns false + * if it does not exist in either one of them. + * + * @param packageName Name of the package + * @param proxyPackageName Name of the proxy package + * @param attributionTag attribution tag to be checked + * + * @return boolean specifying if attribution tag is valid or not + */ + private boolean isAttributionTagDefined(@Nullable String packageName, + @Nullable String proxyPackageName, + @Nullable String attributionTag) { + if (packageName == null) { + return false; + } else if (attributionTag == null) { + return true; + } + PackageManagerInternal pmInt = LocalServices.getService(PackageManagerInternal.class); + if (proxyPackageName != null) { + AndroidPackage proxyPkg = pmInt.getPackage(proxyPackageName); + if (proxyPkg != null && isAttributionInPackage(proxyPkg, attributionTag)) { + return true; + } + } + AndroidPackage pkg = pmInt.getPackage(packageName); + return isAttributionInPackage(pkg, attributionTag); + } + /** * Get (and potentially create) ops. * diff --git a/services/core/java/com/android/server/pm/pkg/component/ParsedAttributionImpl.java b/services/core/java/com/android/server/pm/pkg/component/ParsedAttributionImpl.java index b59f511afa57..0917eb6ee43f 100644 --- a/services/core/java/com/android/server/pm/pkg/component/ParsedAttributionImpl.java +++ b/services/core/java/com/android/server/pm/pkg/component/ParsedAttributionImpl.java @@ -38,7 +38,7 @@ import java.util.List; public class ParsedAttributionImpl implements ParsedAttribution, Parcelable { /** Maximum amount of attributions per package */ - static final int MAX_NUM_ATTRIBUTIONS = 10000; + static final int MAX_NUM_ATTRIBUTIONS = 1000; /** Tag of the attribution */ private @NonNull String tag; -- cgit v1.2.3 From 79d1fb8e0a8a657c10baa6ad083531be22a2e028 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 27 Dec 2023 11:26:49 -0500 Subject: Don't store invalid pkgs when migrating filters Test: NotificationManagerServiceTest Test: call method from test app, view policy xml file Flag: none Bug: 305926929 (cherry picked from commit bfa04e208995b05eee2a5336667f4e2dcd19fd30) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:85ccbd33e3766bbd22ced332cb5b9e983c2707db) Merged-In: Ib7fcb733edd2cf2cbac0a7699763a5fe993b467e Change-Id: Ib7fcb733edd2cf2cbac0a7699763a5fe993b467e --- .../notification/NotificationManagerService.java | 6 ++-- .../NotificationManagerServiceTest.java | 35 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 647087de1c04..d997c8940b2f 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -4809,8 +4809,10 @@ public class NotificationManagerService extends SystemService { for (int userId : mUm.getProfileIds(info.userid, false)) { try { int uid = getUidForPackageAndUser(pkg, UserHandle.of(userId)); - VersionedPackage vp = new VersionedPackage(pkg, uid); - nlf.addPackage(vp); + if (uid != INVALID_UID) { + VersionedPackage vp = new VersionedPackage(pkg, uid); + nlf.addPackage(vp); + } } catch (Exception e) { // pkg doesn't exist on that user; skip } 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 86d2b2fc6b56..c31d0d778025 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -99,6 +99,7 @@ import static junit.framework.Assert.assertSame; import static junit.framework.Assert.assertTrue; import static junit.framework.Assert.fail; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Matchers.anyBoolean; @@ -10248,6 +10249,40 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { assertTrue(captor.getValue().isPackageAllowed(new VersionedPackage("apples", 10000))); } + @Test + public void testMigrateNotificationFilter_invalidPackage() throws Exception { + int[] userIds = new int[] {mUserId, 1000}; + when(mUm.getProfileIds(anyInt(), anyBoolean())).thenReturn(userIds); + List disallowedApps = ImmutableList.of("apples", "bananas", "cherries"); + for (int userId : userIds) { + when(mPackageManager.getPackageUid("apples", 0, userId)).thenThrow( + new RemoteException("")); + when(mPackageManager.getPackageUid("bananas", 0, userId)).thenReturn(9000); + when(mPackageManager.getPackageUid("cherries", 0, userId)).thenReturn(9001); + } + + when(mListeners.getNotificationListenerFilter(any())).thenReturn( + new NotificationListenerFilter()); + + mBinderService.migrateNotificationFilter(null, + FLAG_FILTER_TYPE_CONVERSATIONS | FLAG_FILTER_TYPE_ONGOING, + disallowedApps); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(NotificationListenerFilter.class); + verify(mListeners).setNotificationListenerFilter(any(), captor.capture()); + + assertEquals(FLAG_FILTER_TYPE_CONVERSATIONS | FLAG_FILTER_TYPE_ONGOING, + captor.getValue().getTypes()); + // valid values stay + assertFalse(captor.getValue().isPackageAllowed(new VersionedPackage("bananas", 9000))); + assertFalse(captor.getValue().isPackageAllowed(new VersionedPackage("cherries", 9001))); + // don't store invalid values + for (VersionedPackage vp : captor.getValue().getDisallowedPackages()) { + assertNotEquals("apples", vp.getPackageName()); + } + } + @Test public void testMigrateNotificationFilter_noPreexistingFilter() throws Exception { int[] userIds = new int[] {mUserId}; -- cgit v1.2.3 From ac275076540476d0e40d16bccc341bf0d628ca4f Mon Sep 17 00:00:00 2001 From: Beverly Date: Thu, 18 Jan 2024 20:13:52 +0000 Subject: isUserInLockDown can be true when there are other strong auth requirements Bug: 315206668 Bug: 218495634 Flag: None Test: manual, atest LockPatternUtilsTest (cherry picked from commit d341f1ecdb011d24b17358f115391b3f997cb179) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:93149616ba8255ec82877e43d4b41c2ebd6abf24) Merged-In: I5e979a7822dd7254b4579ab28ecf96df1db44179 Change-Id: I5e979a7822dd7254b4579ab28ecf96df1db44179 --- .../android/internal/widget/LockPatternUtils.java | 4 +-- .../internal/util/LockPatternUtilsTest.java | 39 +++++++++++++++++++--- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index e60a5c36d761..8fde4c4ae225 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -1371,8 +1371,8 @@ public class LockPatternUtils { } public boolean isUserInLockdown(int userId) { - return getStrongAuthForUser(userId) - == StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN; + return (getStrongAuthForUser(userId) + & StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN) != 0; } private static class WrappedCallback extends ICheckCredentialProgressCallback.Stub { diff --git a/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java b/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java index 0b7019995acb..b4f76d1a5eaf 100644 --- a/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java +++ b/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java @@ -20,7 +20,9 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_MANAGED; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED; import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.SOME_AUTH_REQUIRED_AFTER_TRUSTAGENT_EXPIRED; +import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_NOT_REQUIRED; import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_LOCKOUT; +import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN; import static com.google.common.truth.Truth.assertThat; @@ -70,12 +72,15 @@ import java.util.List; @SmallTest public class LockPatternUtilsTest { + private ILockSettings mLockSettings; + private static final int USER_ID = 1; private static final int DEMO_USER_ID = 5; private LockPatternUtils mLockPatternUtils; private void configureTest(boolean isSecure, boolean isDemoUser, int deviceDemoMode) throws Exception { + mLockSettings = Mockito.mock(ILockSettings.class); final Context context = spy(new ContextWrapper(InstrumentationRegistry.getTargetContext())); final MockContentResolver cr = new MockContentResolver(context); @@ -83,15 +88,14 @@ public class LockPatternUtilsTest { when(context.getContentResolver()).thenReturn(cr); Settings.Global.putInt(cr, Settings.Global.DEVICE_DEMO_MODE, deviceDemoMode); - final ILockSettings ils = Mockito.mock(ILockSettings.class); - when(ils.getCredentialType(DEMO_USER_ID)).thenReturn( + when(mLockSettings.getCredentialType(DEMO_USER_ID)).thenReturn( isSecure ? LockPatternUtils.CREDENTIAL_TYPE_PASSWORD : LockPatternUtils.CREDENTIAL_TYPE_NONE); - when(ils.getLong("lockscreen.password_type", PASSWORD_QUALITY_UNSPECIFIED, DEMO_USER_ID)) - .thenReturn((long) PASSWORD_QUALITY_MANAGED); + when(mLockSettings.getLong("lockscreen.password_type", PASSWORD_QUALITY_UNSPECIFIED, + DEMO_USER_ID)).thenReturn((long) PASSWORD_QUALITY_MANAGED); // TODO(b/63758238): stop spying the class under test mLockPatternUtils = spy(new LockPatternUtils(context)); - when(mLockPatternUtils.getLockSettings()).thenReturn(ils); + when(mLockPatternUtils.getLockSettings()).thenReturn(mLockSettings); doReturn(true).when(mLockPatternUtils).hasSecureLockScreen(); final UserInfo userInfo = Mockito.mock(UserInfo.class); @@ -101,6 +105,31 @@ public class LockPatternUtilsTest { when(context.getSystemService(Context.USER_SERVICE)).thenReturn(um); } + @Test + public void isUserInLockDown() throws Exception { + configureTest(true, false, 2); + + // GIVEN strong auth not required + when(mLockSettings.getStrongAuthForUser(USER_ID)).thenReturn(STRONG_AUTH_NOT_REQUIRED); + + // THEN user isn't in lockdown + assertFalse(mLockPatternUtils.isUserInLockdown(USER_ID)); + + // GIVEN lockdown + when(mLockSettings.getStrongAuthForUser(USER_ID)).thenReturn( + STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN); + + // THEN user is in lockdown + assertTrue(mLockPatternUtils.isUserInLockdown(USER_ID)); + + // GIVEN lockdown and lockout + when(mLockSettings.getStrongAuthForUser(USER_ID)).thenReturn( + STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN | STRONG_AUTH_REQUIRED_AFTER_LOCKOUT); + + // THEN user is in lockdown + assertTrue(mLockPatternUtils.isUserInLockdown(USER_ID)); + } + @Test public void isLockScreenDisabled_isDemoUser_true() throws Exception { configureTest(false, true, 2); -- cgit v1.2.3 From dde955da4adc67a16088373f424b2305e86bed4f Mon Sep 17 00:00:00 2001 From: Alex Buynytskyy Date: Thu, 4 Jan 2024 00:18:16 +0000 Subject: Stop marking apps as privileged if they are not signed properly. Fixes: 311374917 Test: atest android.content.pm.cts.PackageManagerTest (cherry picked from commit 3ee5dfdcba047051ce81dca0696d6ddfeafe2d98) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:06775341ad7d77410798f95117cbee7a1a02c201) Merged-In: I5b5b81cf43b06837a22c8dfd170a112106dd64c1 Change-Id: I5b5b81cf43b06837a22c8dfd170a112106dd64c1 --- services/core/java/com/android/server/pm/InstallPackageHelper.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/pm/InstallPackageHelper.java b/services/core/java/com/android/server/pm/InstallPackageHelper.java index d0304b43215e..f26a9f8f3aed 100644 --- a/services/core/java/com/android/server/pm/InstallPackageHelper.java +++ b/services/core/java/com/android/server/pm/InstallPackageHelper.java @@ -4742,7 +4742,9 @@ final class InstallPackageHelper { private void assertPackageWithSharedUserIdIsPrivileged(AndroidPackage pkg) throws PackageManagerException { - if (!AndroidPackageUtils.isPrivileged(pkg) && (pkg.getSharedUserId() != null)) { + if (!AndroidPackageUtils.isPrivileged(pkg) + && (pkg.getSharedUserId() != null) + && !pkg.isLeavingSharedUser()) { SharedUserSetting sharedUserSetting = null; try { synchronized (mPm.mLock) { @@ -4783,7 +4785,8 @@ final class InstallPackageHelper { if (((scanFlags & SCAN_AS_PRIVILEGED) == 0) && !AndroidPackageUtils.isPrivileged(pkg) && (pkg.getSharedUserId() != null) - && !skipVendorPrivilegeScan) { + && !skipVendorPrivilegeScan + && !pkg.isLeavingSharedUser()) { SharedUserSetting sharedUserSetting = null; synchronized (mPm.mLock) { try { -- cgit v1.2.3 From 38318c9d89e68887d3d66f7a1470dd5680201c05 Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Wed, 10 Jan 2024 16:25:13 +0000 Subject: Fix security vulnerability that creates user with no restrictions when accountOptions are too long. Bug: 293602970 Test: atest UserManagerTest#testAddUserAccountData_validStringValuesAreSaved_validBundleIsSaved && atest UserManagerTest#testAddUserAccountData_invalidStringValuesAreTruncated_invalidBundleIsDropped (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:33a90988cf3b59a3b8dc9f699d155688be8d5623) Merged-In: I23c971f671546ac085060add89485cfac6691ca3 Change-Id: I23c971f671546ac085060add89485cfac6691ca3 --- core/java/android/os/PersistableBundle.java | 37 ++++++++ core/java/android/os/UserManager.java | 23 ++++- .../internal/app/ConfirmUserCreationActivity.java | 12 +++ .../com/android/server/pm/UserManagerService.java | 30 +++--- .../android/server/pm/UserManagerServiceTest.java | 2 +- .../src/com/android/server/pm/UserManagerTest.java | 102 +++++++++++++++++++++ 6 files changed, 188 insertions(+), 18 deletions(-) diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java index 02704f5b346b..236194d16ad8 100644 --- a/core/java/android/os/PersistableBundle.java +++ b/core/java/android/os/PersistableBundle.java @@ -294,6 +294,43 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa XmlUtils.writeMapXml(mMap, out, this); } + /** + * Checks whether all keys and values are within the given character limit. + * Note: Maximum character limit of String that can be saved to XML as part of bundle is 65535. + * Otherwise IOException is thrown. + * @param limit length of String keys and values in the PersistableBundle, including nested + * PersistableBundles to check against. + * + * @hide + */ + public boolean isBundleContentsWithinLengthLimit(int limit) { + unparcel(); + if (mMap == null) { + return true; + } + for (int i = 0; i < mMap.size(); i++) { + if (mMap.keyAt(i) != null && mMap.keyAt(i).length() > limit) { + return false; + } + final Object value = mMap.valueAt(i); + if (value instanceof String && ((String) value).length() > limit) { + return false; + } else if (value instanceof String[]) { + String[] stringArray = (String[]) value; + for (int j = 0; j < stringArray.length; j++) { + if (stringArray[j] != null + && stringArray[j].length() > limit) { + return false; + } + } + } else if (value instanceof PersistableBundle + && !((PersistableBundle) value).isBundleContentsWithinLengthLimit(limit)) { + return false; + } + } + return true; + } + /** @hide */ static class MyReadMapCallback implements XmlUtils.ReadMapCallback { @Override diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index 84f1880213b8..98fad75cd031 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -103,6 +103,21 @@ public class UserManager { /** Whether the device is in headless system user mode; null until cached. */ private static Boolean sIsHeadlessSystemUser = null; + /** Maximum length of username. + * @hide + */ + public static final int MAX_USER_NAME_LENGTH = 100; + + /** Maximum length of user property String value. + * @hide + */ + public static final int MAX_ACCOUNT_STRING_LENGTH = 500; + + /** Maximum length of account options String values. + * @hide + */ + public static final int MAX_ACCOUNT_OPTIONS_LENGTH = 1000; + /** * User type representing a {@link UserHandle#USER_SYSTEM system} user that is a human user. * This type of user cannot be created; it can only pre-exist on first boot. @@ -4204,15 +4219,15 @@ public class UserManager { * This API should only be called if the current user is an {@link #isAdminUser() admin} user, * as otherwise the returned intent will not be able to create a user. * - * @param userName Optional name to assign to the user. + * @param userName Optional name to assign to the user. Character limit is 100. * @param accountName Optional account name that will be used by the setup wizard to initialize - * the user. + * the user. Character limit is 500. * @param accountType Optional account type for the account to be created. This is required - * if the account name is specified. + * if the account name is specified. Character limit is 500. * @param accountOptions Optional bundle of data to be passed in during account creation in the * new user via {@link AccountManager#addAccount(String, String, String[], * Bundle, android.app.Activity, android.accounts.AccountManagerCallback, - * Handler)}. + * Handler)}. Character limit is 1000. * @return An Intent that can be launched from an Activity. * @see #USER_CREATION_FAILED_NOT_PERMITTED * @see #USER_CREATION_FAILED_NO_MORE_USERS diff --git a/core/java/com/android/internal/app/ConfirmUserCreationActivity.java b/core/java/com/android/internal/app/ConfirmUserCreationActivity.java index 0a28997885ff..b4e87498f09b 100644 --- a/core/java/com/android/internal/app/ConfirmUserCreationActivity.java +++ b/core/java/com/android/internal/app/ConfirmUserCreationActivity.java @@ -116,6 +116,14 @@ public class ConfirmUserCreationActivity extends AlertActivity if (cantCreateUser) { setResult(UserManager.USER_CREATION_FAILED_NOT_PERMITTED); return null; + } else if (!(isUserPropertyWithinLimit(mUserName, UserManager.MAX_USER_NAME_LENGTH) + && isUserPropertyWithinLimit(mAccountName, UserManager.MAX_ACCOUNT_STRING_LENGTH) + && isUserPropertyWithinLimit(mAccountType, UserManager.MAX_ACCOUNT_STRING_LENGTH)) + || (mAccountOptions != null && !mAccountOptions.isBundleContentsWithinLengthLimit( + UserManager.MAX_ACCOUNT_OPTIONS_LENGTH))) { + setResult(UserManager.USER_CREATION_FAILED_NOT_PERMITTED); + Log.i(TAG, "User properties must not exceed their character limits"); + return null; } else if (cantCreateAnyMoreUsers) { setResult(UserManager.USER_CREATION_FAILED_NO_MORE_USERS); return null; @@ -144,4 +152,8 @@ public class ConfirmUserCreationActivity extends AlertActivity } finish(); } + + private boolean isUserPropertyWithinLimit(String property, int limit) { + return property == null || property.length() <= limit; + } } diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 80e1c7e9697b..390a09b99e2e 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -284,8 +284,6 @@ public class UserManagerService extends IUserManager.Stub { private static final int USER_VERSION = 11; - private static final int MAX_USER_STRING_LENGTH = 500; - private static final long EPOCH_PLUS_30_YEARS = 30L * 365 * 24 * 60 * 60 * 1000L; // ms static final int WRITE_USER_MSG = 1; @@ -4261,16 +4259,18 @@ public class UserManagerService extends IUserManager.Stub { if (userData.persistSeedData) { if (userData.seedAccountName != null) { serializer.attribute(null, ATTR_SEED_ACCOUNT_NAME, - truncateString(userData.seedAccountName)); + truncateString(userData.seedAccountName, + UserManager.MAX_ACCOUNT_STRING_LENGTH)); } if (userData.seedAccountType != null) { serializer.attribute(null, ATTR_SEED_ACCOUNT_TYPE, - truncateString(userData.seedAccountType)); + truncateString(userData.seedAccountType, + UserManager.MAX_ACCOUNT_STRING_LENGTH)); } } if (userInfo.name != null) { serializer.startTag(null, TAG_NAME); - serializer.text(truncateString(userInfo.name)); + serializer.text(truncateString(userInfo.name, UserManager.MAX_USER_NAME_LENGTH)); serializer.endTag(null, TAG_NAME); } synchronized (mRestrictionsLock) { @@ -4319,11 +4319,11 @@ public class UserManagerService extends IUserManager.Stub { serializer.endDocument(); } - private String truncateString(String original) { - if (original == null || original.length() <= MAX_USER_STRING_LENGTH) { + private String truncateString(String original, int limit) { + if (original == null || original.length() <= limit) { return original; } - return original.substring(0, MAX_USER_STRING_LENGTH); + return original.substring(0, limit); } /* @@ -4771,8 +4771,7 @@ public class UserManagerService extends IUserManager.Stub { @UserIdInt int parentId, boolean preCreate, @Nullable String[] disallowedPackages, @NonNull TimingsTraceAndSlog t, @Nullable Object token) throws UserManager.CheckedUserOperationException { - - String truncatedName = truncateString(name); + String truncatedName = truncateString(name, UserManager.MAX_USER_NAME_LENGTH); final UserTypeDetails userTypeDetails = mUserTypes.get(userType); if (userTypeDetails == null) { throwCheckedUserOperationException( @@ -6373,9 +6372,14 @@ public class UserManagerService extends IUserManager.Stub { Slog.e(LOG_TAG, "No such user for settings seed data u=" + userId); return; } - userData.seedAccountName = truncateString(accountName); - userData.seedAccountType = truncateString(accountType); - userData.seedAccountOptions = accountOptions; + userData.seedAccountName = truncateString(accountName, + UserManager.MAX_ACCOUNT_STRING_LENGTH); + userData.seedAccountType = truncateString(accountType, + UserManager.MAX_ACCOUNT_STRING_LENGTH); + if (accountOptions != null && accountOptions.isBundleContentsWithinLengthLimit( + UserManager.MAX_ACCOUNT_OPTIONS_LENGTH)) { + userData.seedAccountOptions = accountOptions; + } userData.persistSeedData = persist; } if (persist) { diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java index c8ff758c014c..5f18e9e91ea1 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java @@ -331,7 +331,7 @@ public final class UserManagerServiceTest { @Test public void testCreateUserWithLongName_TruncatesName() { UserInfo user = mUms.createUserWithThrow(generateLongString(), USER_TYPE_FULL_SECONDARY, 0); - assertThat(user.name.length()).isEqualTo(500); + assertThat(user.name.length()).isEqualTo(UserManager.MAX_USER_NAME_LENGTH); UserInfo user1 = mUms.createUserWithThrow("Test", USER_TYPE_FULL_SECONDARY, 0); assertThat(user1.name.length()).isEqualTo(4); } diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java index 6bcda3fbcf43..fa0ef188ef41 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java @@ -19,6 +19,7 @@ package com.android.server.pm; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; import static org.testng.Assert.assertThrows; @@ -31,6 +32,7 @@ import android.content.pm.UserInfo; import android.content.pm.UserProperties; import android.content.res.Resources; import android.os.Bundle; +import android.os.PersistableBundle; import android.os.UserHandle; import android.os.UserManager; import android.platform.test.annotations.Postsubmit; @@ -1410,6 +1412,106 @@ public final class UserManagerTest { assertThat(userInfo.name).isEqualTo(newName); } + @Test + public void testAddUserAccountData_validStringValuesAreSaved_validBundleIsSaved() { + assumeManagedUsersSupported(); + + String userName = "User"; + String accountName = "accountName"; + String accountType = "accountType"; + String arrayKey = "StringArrayKey"; + String stringKey = "StringKey"; + String intKey = "IntKey"; + String nestedBundleKey = "PersistableBundleKey"; + String value1 = "Value 1"; + String value2 = "Value 2"; + String value3 = "Value 3"; + + UserInfo userInfo = mUserManager.createUser(userName, + UserManager.USER_TYPE_FULL_SECONDARY, 0); + + PersistableBundle accountOptions = new PersistableBundle(); + String[] stringArray = {value1, value2}; + accountOptions.putInt(intKey, 1234); + PersistableBundle nested = new PersistableBundle(); + nested.putString(stringKey, value3); + accountOptions.putPersistableBundle(nestedBundleKey, nested); + accountOptions.putStringArray(arrayKey, stringArray); + + mUserManager.clearSeedAccountData(); + mUserManager.setSeedAccountData(mContext.getUserId(), accountName, + accountType, accountOptions); + + //assert userName accountName and accountType were saved correctly + assertTrue(mUserManager.getUserInfo(userInfo.id).name.equals(userName)); + assertTrue(mUserManager.getSeedAccountName().equals(accountName)); + assertTrue(mUserManager.getSeedAccountType().equals(accountType)); + + //assert bundle with correct values was added + assertThat(mUserManager.getSeedAccountOptions().containsKey(arrayKey)).isTrue(); + assertThat(mUserManager.getSeedAccountOptions().getPersistableBundle(nestedBundleKey) + .getString(stringKey)).isEqualTo(value3); + assertThat(mUserManager.getSeedAccountOptions().getStringArray(arrayKey)[0]) + .isEqualTo(value1); + + mUserManager.removeUser(userInfo.id); + } + + @Test + public void testAddUserAccountData_invalidStringValuesAreTruncated_invalidBundleIsDropped() { + assumeManagedUsersSupported(); + + String tooLongString = generateLongString(); + String userName = "User " + tooLongString; + String accountType = "Account Type " + tooLongString; + String accountName = "accountName " + tooLongString; + String arrayKey = "StringArrayKey"; + String stringKey = "StringKey"; + String intKey = "IntKey"; + String nestedBundleKey = "PersistableBundleKey"; + String value1 = "Value 1"; + String value2 = "Value 2"; + + UserInfo userInfo = mUserManager.createUser(userName, + UserManager.USER_TYPE_FULL_SECONDARY, 0); + + PersistableBundle accountOptions = new PersistableBundle(); + String[] stringArray = {value1, value2}; + accountOptions.putInt(intKey, 1234); + PersistableBundle nested = new PersistableBundle(); + nested.putString(stringKey, tooLongString); + accountOptions.putPersistableBundle(nestedBundleKey, nested); + accountOptions.putStringArray(arrayKey, stringArray); + mUserManager.clearSeedAccountData(); + mUserManager.setSeedAccountData(mContext.getUserId(), accountName, + accountType, accountOptions); + + //assert userName was truncated + assertTrue(mUserManager.getUserInfo(userInfo.id).name.length() + == UserManager.MAX_USER_NAME_LENGTH); + + //assert accountName and accountType got truncated + assertTrue(mUserManager.getSeedAccountName().length() + == UserManager.MAX_ACCOUNT_STRING_LENGTH); + assertTrue(mUserManager.getSeedAccountType().length() + == UserManager.MAX_ACCOUNT_STRING_LENGTH); + + //assert bundle with invalid values was dropped + assertThat(mUserManager.getSeedAccountOptions() == null).isTrue(); + + mUserManager.removeUser(userInfo.id); + } + + private String generateLongString() { + String partialString = "Test Name Test Name Test Name Test Name Test Name Test Name Test " + + "Name Test Name Test Name Test Name "; //String of length 100 + StringBuilder resultString = new StringBuilder(); + for (int i = 0; i < 600; i++) { + resultString.append(partialString); + } + return resultString.toString(); + } + private boolean isPackageInstalledForUser(String packageName, int userId) { try { return mPackageManager.getPackageInfoAsUser(packageName, 0, userId) != null; -- cgit v1.2.3 From 2c4fc9da7289a649a8578de9f04bb2dd9b45ef0b Mon Sep 17 00:00:00 2001 From: Vaibhav Raut Date: Thu, 1 Feb 2024 15:21:38 +0530 Subject: Revert "Dynamically enable multi-peripheral mode" This reverts commit f58eb1d14fc386e1634bbe18ec1c7f953fa590eb. CRs-Fixed: 3721555 Change-Id: I47d135ddcc6cc7bac1bae4461690b7c3b82bda20 --- services/usb/java/com/android/server/usb/UsbAlsaManager.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/services/usb/java/com/android/server/usb/UsbAlsaManager.java b/services/usb/java/com/android/server/usb/UsbAlsaManager.java index de430c2338e3..17c354a8a80e 100644 --- a/services/usb/java/com/android/server/usb/UsbAlsaManager.java +++ b/services/usb/java/com/android/server/usb/UsbAlsaManager.java @@ -38,7 +38,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; -import android.os.SystemProperties; /** * UsbAlsaManager manages USB audio and MIDI devices. @@ -49,7 +48,7 @@ public final class UsbAlsaManager { // Flag to turn on/off multi-peripheral select mode // Set to true to have single-device-only mode - private static boolean mIsSingleMode = true; + private static final boolean mIsSingleMode = true; private static final String ALSA_DIRECTORY = "/dev/snd/"; @@ -138,10 +137,6 @@ public final class UsbAlsaManager { Slog.d(TAG, "selectAlsaDevice() " + alsaDevice); } - if ("true".equals(SystemProperties.get("vendor.audio.gaming.enabled", "false"))) { - mIsSingleMode = false; - } - // This must be where an existing USB audio device is deselected.... (I think) if (mIsSingleMode && mSelectedDevice != null) { deselectAlsaDevice(); -- cgit v1.2.3 From e239b4caedb69ca45e370e1b3bfcf6607587d7c5 Mon Sep 17 00:00:00 2001 From: ruiliu Date: Wed, 13 Mar 2024 19:46:49 +0800 Subject: SystemUI:check ServiceStatus for all subs to show Emergency button SystemUI use latest service status to determine whether emergency button should be shown. In some corner cases, different sub's service status's mEmergencyOnly is different. So we need to check mEmergencyOnly status for all subs to determine its visibility. Change-Id: Ia2a20485ccc2058dc0e20394857102948a123b1f CRs-Fixed: 3759441 --- .../android/keyguard/EmergencyButtonController.java | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/EmergencyButtonController.java b/packages/SystemUI/src/com/android/keyguard/EmergencyButtonController.java index 85502c44ef3c..2927c4e32567 100644 --- a/packages/SystemUI/src/com/android/keyguard/EmergencyButtonController.java +++ b/packages/SystemUI/src/com/android/keyguard/EmergencyButtonController.java @@ -58,7 +58,7 @@ import com.android.systemui.util.EmergencyDialerConstants; import com.android.systemui.util.ViewController; import java.util.concurrent.Executor; -import java.util.List; +import java.util.HashMap; import javax.inject.Inject; @@ -80,7 +80,7 @@ public class EmergencyButtonController extends ViewController { private Executor mBackgroundExecutor; private EmergencyButtonCallback mEmergencyButtonCallback; - private ServiceState mServiceState; + private HashMap mServiceStates = new HashMap<>(); private final KeyguardUpdateMonitorCallback mInfoCallback = new KeyguardUpdateMonitorCallback() { @@ -96,7 +96,7 @@ public class EmergencyButtonController extends ViewController { @Override public void onServiceStateChanged(int subId, ServiceState state) { - mServiceState = state; + mServiceStates.put(subId, state); updateEmergencyCallButton(); } }; @@ -220,8 +220,18 @@ public class EmergencyButtonController extends ViewController { } private boolean isEmergencyCapable() { - return (!mKeyguardUpdateMonitor.isOOS() - || (mServiceState !=null && mServiceState.isEmergencyOnly())); + if (!mKeyguardUpdateMonitor.isOOS()){ + return true; + } + for (int subId : mServiceStates.keySet()) { + ServiceState ss = mServiceStates.get(subId); + if (ss != null) { + if (ss.isEmergencyOnly()) { + return true; + } + } + } + return false; } /** */ -- cgit v1.2.3 From defd700a3a47d07595f9eb0aeb6cb434e84f5f46 Mon Sep 17 00:00:00 2001 From: Anand Mohan Date: Sun, 4 Feb 2024 17:46:59 +0530 Subject: base: allow only A2DP devices to SHO between them Added check to allow SHO to A2DP device only if previously connected device was also A2DP device. CRs-Fixed: 3721524 Change-Id: Ie7b7188c29df64faa2525308685a6a505e0a5b82 --- services/core/java/com/android/server/audio/AudioDeviceBroker.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioDeviceBroker.java b/services/core/java/com/android/server/audio/AudioDeviceBroker.java index a63ef75c6a03..ddadf53731c8 100644 --- a/services/core/java/com/android/server/audio/AudioDeviceBroker.java +++ b/services/core/java/com/android/server/audio/AudioDeviceBroker.java @@ -893,8 +893,9 @@ public class AudioDeviceBroker { BluetoothProfile.STATE_CONNECTED)); } } - } else if (data.mInfo.getProfile() == BluetoothProfile.A2DP && data.mPreviousDevice != null - && data.mNewDevice != null && !data.mPreviousDevice.equals(data.mNewDevice)) { + } else if (data.mPreviousDevice != null && data.mNewDevice != null + && data.mInfo.getProfile() == BluetoothProfile.A2DP + && mDeviceInventory.isA2dpDeviceConnected(data.mPreviousDevice)) { final String name = TextUtils.emptyIfNull(data.mNewDevice.getName()); new MediaMetrics.Item(MediaMetrics.Name.AUDIO_DEVICE + MediaMetrics.SEPARATOR + "queueOnBluetoothActiveDeviceChanged_update") -- cgit v1.2.3