Revert "Check sound Uri permission when creating a notification channel"
This reverts commit 94fc92cf20b897f016c3a9718edf40818de4a11b.
Reason for revert: CVE-2025-22420 A-337775777 is being removed from ASB#2025-04 due to an possible app compatibility regression. Partners are recommended to remove this fix and should not reference it in any public documentation. An updated fix will be required as part of a future Android Security Bulletin.
Pixel Impact: Internal testing found WhatsApp java crashes with this fix. Detected on Dogfood builds. We recommend Pixel removal if possible. Crash report here b/390020418
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:21fe1d68d9f128681e9754ab5e7b8373c64a4d61)
Merged-In: Iaf59e34577bed2eba8da30037a77d15e4f4bd690
Change-Id: Iaf59e34577bed2eba8da30037a77d15e4f4bd690
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 7db403f..2eb42b1 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -2611,7 +2611,6 @@
mNotificationChannelLogger,
mAppOps,
mUserProfiles,
- mUgmInternal,
mShowReviewPermissionsNotification,
Clock.systemUTC());
mRankingHelper = new RankingHelper(getContext(), mRankingHandler, mPreferencesHelper,
@@ -6888,7 +6887,13 @@
final Uri originalSoundUri =
(originalChannel != null) ? originalChannel.getSound() : null;
if (soundUri != null && !Objects.equals(originalSoundUri, soundUri)) {
- PermissionHelper.grantUriPermission(mUgmInternal, soundUri, sourceUid);
+ Binder.withCleanCallingIdentity(() -> {
+ mUgmInternal.checkGrantUriPermission(sourceUid, null,
+ ContentProvider.getUriWithoutUserId(soundUri),
+ Intent.FLAG_GRANT_READ_URI_PERMISSION,
+ ContentProvider.getUserIdFromUri(soundUri,
+ UserHandle.getUserId(sourceUid)));
+ });
}
}
diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java
index 93f512b..a865798 100644
--- a/services/core/java/com/android/server/notification/NotificationRecord.java
+++ b/services/core/java/com/android/server/notification/NotificationRecord.java
@@ -1493,23 +1493,14 @@
final Notification notification = getNotification();
notification.visitUris((uri) -> {
- if (com.android.server.notification.Flags.notificationVerifyChannelSoundUri()) {
- visitGrantableUri(uri, false, false);
- } else {
- oldVisitGrantableUri(uri, false, false);
- }
+ visitGrantableUri(uri, false, false);
});
if (notification.getChannelId() != null) {
NotificationChannel channel = getChannel();
if (channel != null) {
- if (com.android.server.notification.Flags.notificationVerifyChannelSoundUri()) {
- visitGrantableUri(channel.getSound(), (channel.getUserLockedFields()
- & NotificationChannel.USER_LOCKED_SOUND) != 0, true);
- } else {
- oldVisitGrantableUri(channel.getSound(), (channel.getUserLockedFields()
- & NotificationChannel.USER_LOCKED_SOUND) != 0, true);
- }
+ visitGrantableUri(channel.getSound(), (channel.getUserLockedFields()
+ & NotificationChannel.USER_LOCKED_SOUND) != 0, true);
}
}
} finally {
@@ -1525,7 +1516,7 @@
* {@link #mGrantableUris}. Otherwise, this will either log or throw
* {@link SecurityException} depending on target SDK of enqueuing app.
*/
- private void oldVisitGrantableUri(Uri uri, boolean userOverriddenUri, boolean isSound) {
+ private void visitGrantableUri(Uri uri, boolean userOverriddenUri, boolean isSound) {
if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;
if (mGrantableUris != null && mGrantableUris.contains(uri)) {
@@ -1564,45 +1555,6 @@
}
}
- /**
- * Note the presence of a {@link Uri} that should have permission granted to
- * whoever will be rendering it.
- * <p>
- * If the enqueuing app has the ability to grant access, it will be added to
- * {@link #mGrantableUris}. Otherwise, this will either log or throw
- * {@link SecurityException} depending on target SDK of enqueuing app.
- */
- private void visitGrantableUri(Uri uri, boolean userOverriddenUri,
- boolean isSound) {
- if (mGrantableUris != null && mGrantableUris.contains(uri)) {
- return; // already verified this URI
- }
-
- final int sourceUid = getSbn().getUid();
- try {
- PermissionHelper.grantUriPermission(mUgmInternal, uri, sourceUid);
-
- if (mGrantableUris == null) {
- mGrantableUris = new ArraySet<>();
- }
- mGrantableUris.add(uri);
- } catch (SecurityException e) {
- if (!userOverriddenUri) {
- if (isSound) {
- mSound = Settings.System.DEFAULT_NOTIFICATION_URI;
- Log.w(TAG, "Replacing " + uri + " from " + sourceUid + ": " + e.getMessage());
- } else {
- if (mTargetSdkVersion >= Build.VERSION_CODES.P) {
- throw e;
- } else {
- Log.w(TAG,
- "Ignoring " + uri + " from " + sourceUid + ": " + e.getMessage());
- }
- }
- }
- }
- }
-
public LogMaker getLogMaker(long now) {
LogMaker lm = getSbn().getLogMaker()
.addTaggedData(MetricsEvent.FIELD_NOTIFICATION_CHANNEL_IMPORTANCE, mImportance)
diff --git a/services/core/java/com/android/server/notification/PermissionHelper.java b/services/core/java/com/android/server/notification/PermissionHelper.java
index 1464d48..b6f4889 100644
--- a/services/core/java/com/android/server/notification/PermissionHelper.java
+++ b/services/core/java/com/android/server/notification/PermissionHelper.java
@@ -25,25 +25,19 @@
import android.annotation.NonNull;
import android.annotation.UserIdInt;
import android.companion.virtual.VirtualDeviceManager;
-import android.content.ContentProvider;
-import android.content.ContentResolver;
import android.content.Context;
-import android.content.Intent;
import android.content.pm.IPackageManager;
import android.content.pm.PackageInfo;
import android.content.pm.PackageManager;
import android.content.pm.ParceledListSlice;
-import android.net.Uri;
import android.os.Binder;
import android.os.RemoteException;
-import android.os.UserHandle;
import android.permission.IPermissionManager;
import android.util.ArrayMap;
import android.util.Pair;
import android.util.Slog;
import com.android.internal.util.ArrayUtils;
-import com.android.server.uri.UriGrantsManagerInternal;
import java.util.Collections;
import java.util.HashSet;
@@ -64,7 +58,7 @@
private final IPermissionManager mPermManager;
public PermissionHelper(Context context, IPackageManager packageManager,
- IPermissionManager permManager) {
+ IPermissionManager permManager) {
mContext = context;
mPackageManager = packageManager;
mPermManager = permManager;
@@ -304,19 +298,6 @@
return false;
}
- static void grantUriPermission(final UriGrantsManagerInternal ugmInternal, Uri uri,
- int sourceUid) {
- if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;
-
- Binder.withCleanCallingIdentity(() -> {
- // This will throw a SecurityException if the caller can't grant.
- ugmInternal.checkGrantUriPermission(sourceUid, null,
- ContentProvider.getUriWithoutUserId(uri),
- Intent.FLAG_GRANT_READ_URI_PERMISSION,
- ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)));
- });
- }
-
public static class PackagePermission {
public final String packageName;
public final @UserIdInt int userId;
diff --git a/services/core/java/com/android/server/notification/PreferencesHelper.java b/services/core/java/com/android/server/notification/PreferencesHelper.java
index 15377d6..a5ecd8e 100644
--- a/services/core/java/com/android/server/notification/PreferencesHelper.java
+++ b/services/core/java/com/android/server/notification/PreferencesHelper.java
@@ -101,7 +101,6 @@
import com.android.modules.utils.TypedXmlPullParser;
import com.android.modules.utils.TypedXmlSerializer;
import com.android.server.notification.PermissionHelper.PackagePermission;
-import com.android.server.uri.UriGrantsManagerInternal;
import org.json.JSONArray;
import org.json.JSONException;
@@ -228,7 +227,6 @@
private final NotificationChannelLogger mNotificationChannelLogger;
private final AppOpsManager mAppOps;
private final ManagedServices.UserProfiles mUserProfiles;
- private final UriGrantsManagerInternal mUgmInternal;
private SparseBooleanArray mBadgingEnabled;
private SparseBooleanArray mBubblesEnabled;
@@ -249,7 +247,6 @@
ZenModeHelper zenHelper, PermissionHelper permHelper, PermissionManager permManager,
NotificationChannelLogger notificationChannelLogger,
AppOpsManager appOpsManager, ManagedServices.UserProfiles userProfiles,
- UriGrantsManagerInternal ugmInternal,
boolean showReviewPermissionsNotification, Clock clock) {
mContext = context;
mZenModeHelper = zenHelper;
@@ -260,7 +257,6 @@
mNotificationChannelLogger = notificationChannelLogger;
mAppOps = appOpsManager;
mUserProfiles = userProfiles;
- mUgmInternal = ugmInternal;
mShowReviewPermissionsNotification = showReviewPermissionsNotification;
mIsMediaNotificationFilteringEnabled = context.getResources()
.getBoolean(R.bool.config_quickSettingsShowMediaPlayer);
@@ -1168,13 +1164,6 @@
}
clearLockedFieldsLocked(channel);
- // Verify that the app has permission to read the sound Uri
- // Only check for new channels, as regular apps can only set sound
- // before creating. See: {@link NotificationChannel#setSound}
- if (Flags.notificationVerifyChannelSoundUri()) {
- PermissionHelper.grantUriPermission(mUgmInternal, channel.getSound(), uid);
- }
-
channel.setImportanceLockedByCriticalDeviceFunction(
r.defaultAppLockedImportance || r.fixedImportance);
diff --git a/services/core/java/com/android/server/notification/flags.aconfig b/services/core/java/com/android/server/notification/flags.aconfig
index 65a38ae..76ebb47 100644
--- a/services/core/java/com/android/server/notification/flags.aconfig
+++ b/services/core/java/com/android/server/notification/flags.aconfig
@@ -172,16 +172,6 @@
}
flag {
- name: "notification_verify_channel_sound_uri"
- namespace: "systemui"
- description: "Verify Uri permission for sound when creating a notification channel"
- bug: "337775777"
- metadata {
- purpose: PURPOSE_BUGFIX
- }
-}
-
-flag {
name: "notification_vibration_in_sound_uri_for_channel"
namespace: "systemui"
description: "Enables sound uri with vibration source in notification channel"
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 074cbb5..f536176 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -5186,41 +5186,6 @@
}
@Test
- public void
- updateNotificationChannelFromPrivilegedListener_oldSoundNoUriPerm_newSoundHasUriPerm()
- throws Exception {
- mService.setPreferencesHelper(mPreferencesHelper);
- when(mCompanionMgr.getAssociations(mPkg, mUserId))
- .thenReturn(singletonList(mock(AssociationInfo.class)));
- when(mPreferencesHelper.getNotificationChannel(eq(mPkg), anyInt(),
- eq(mTestNotificationChannel.getId()), anyBoolean()))
- .thenReturn(mTestNotificationChannel);
-
- // Missing Uri permissions for the old channel sound
- final Uri oldSoundUri = Settings.System.DEFAULT_NOTIFICATION_URI;
- doThrow(new SecurityException("no access")).when(mUgmInternal)
- .checkGrantUriPermission(eq(Process.myUid()), any(), eq(oldSoundUri),
- anyInt(), eq(Process.myUserHandle().getIdentifier()));
-
- // Has Uri permissions for the old channel sound
- final Uri newSoundUri = Uri.parse("content://media/test/sound/uri");
- final NotificationChannel updatedNotificationChannel = new NotificationChannel(
- TEST_CHANNEL_ID, TEST_CHANNEL_ID, IMPORTANCE_DEFAULT);
- updatedNotificationChannel.setSound(newSoundUri,
- updatedNotificationChannel.getAudioAttributes());
-
- mBinderService.updateNotificationChannelFromPrivilegedListener(
- null, mPkg, Process.myUserHandle(), updatedNotificationChannel);
-
- verify(mPreferencesHelper, times(1)).updateNotificationChannel(
- anyString(), anyInt(), any(), anyBoolean(), anyInt(), anyBoolean());
-
- verify(mListeners, never()).notifyNotificationChannelChanged(eq(mPkg),
- eq(Process.myUserHandle()), eq(mTestNotificationChannel),
- eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED));
- }
-
- @Test
public void testGetNotificationChannelFromPrivilegedListener_cdm_success() throws Exception {
mService.setPreferencesHelper(mPreferencesHelper);
when(mCompanionMgr.getAssociations(mPkg, mUserId))
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
index b015c8c..c8fa7c5 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java
@@ -46,13 +46,11 @@
import static android.app.NotificationManager.IMPORTANCE_NONE;
import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED;
import static android.app.NotificationManager.VISIBILITY_NO_OVERRIDE;
-import static android.content.ContentResolver.SCHEME_ANDROID_RESOURCE;
-import static android.content.ContentResolver.SCHEME_CONTENT;
-import static android.content.ContentResolver.SCHEME_FILE;
import static android.media.AudioAttributes.CONTENT_TYPE_SONIFICATION;
import static android.media.AudioAttributes.USAGE_NOTIFICATION;
import static android.os.UserHandle.USER_ALL;
import static android.os.UserHandle.USER_SYSTEM;
+
import static android.platform.test.flag.junit.SetFlagsRule.DefaultInitValueType.DEVICE_DEFAULT;
import static android.service.notification.Adjustment.TYPE_CONTENT_RECOMMENDATION;
import static android.service.notification.Adjustment.TYPE_NEWS;
@@ -66,7 +64,7 @@
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES__FSI_STATE__DENIED;
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES__FSI_STATE__GRANTED;
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES__FSI_STATE__NOT_REQUESTED;
-import static com.android.server.notification.Flags.FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI;
+import static com.android.server.notification.Flags.FLAG_ALL_NOTIFS_NEED_TTL;
import static com.android.server.notification.Flags.FLAG_PERSIST_INCOMPLETE_RESTORE_DATA;
import static com.android.server.notification.NotificationChannelLogger.NotificationChannelEvent.NOTIFICATION_CHANNEL_UPDATED_BY_USER;
import static com.android.server.notification.PreferencesHelper.DEFAULT_BUBBLE_PREFERENCE;
@@ -93,7 +91,6 @@
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
@@ -380,10 +377,10 @@
mHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, false, mClock);
+ false, mClock);
mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, false, mClock);
+ false, mClock);
resetZenModeHelper();
mAudioAttributes = new AudioAttributes.Builder()
@@ -795,7 +792,7 @@
public void testReadXml_oldXml_migrates() throws Exception {
mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, /* showReviewPermissionsNotification= */ true, mClock);
+ /* showReviewPermissionsNotification= */ true, mClock);
String xml = "<ranking version=\"2\">\n"
+ "<package name=\"" + PKG_N_MR1 + "\" uid=\"" + UID_N_MR1
@@ -931,7 +928,7 @@
public void testReadXml_newXml_noMigration_showPermissionNotification() throws Exception {
mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, /* showReviewPermissionsNotification= */ true, mClock);
+ /* showReviewPermissionsNotification= */ true, mClock);
String xml = "<ranking version=\"3\">\n"
+ "<package name=\"" + PKG_N_MR1 + "\" show_badge=\"true\">\n"
@@ -990,7 +987,7 @@
public void testReadXml_newXml_permissionNotificationOff() throws Exception {
mHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, /* showReviewPermissionsNotification= */ false, mClock);
+ /* showReviewPermissionsNotification= */ false, mClock);
String xml = "<ranking version=\"3\">\n"
+ "<package name=\"" + PKG_N_MR1 + "\" show_badge=\"true\">\n"
@@ -1049,7 +1046,7 @@
public void testReadXml_newXml_noMigration_noPermissionNotification() throws Exception {
mHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, /* showReviewPermissionsNotification= */ true, mClock);
+ /* showReviewPermissionsNotification= */ true, mClock);
String xml = "<ranking version=\"4\">\n"
+ "<package name=\"" + PKG_N_MR1 + "\" show_badge=\"true\">\n"
@@ -1643,7 +1640,7 @@
// simulate load after reboot
mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, false, mClock);
+ false, mClock);
loadByteArrayXml(baos.toByteArray(), false, USER_ALL);
// Trigger 2nd restore pass
@@ -1698,7 +1695,7 @@
// simulate load after reboot
mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, false, mClock);
+ false, mClock);
loadByteArrayXml(xml.getBytes(), false, USER_ALL);
// Trigger 2nd restore pass
@@ -1776,10 +1773,10 @@
mHelper = new PreferencesHelper(mContext, mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, false, mClock);
+ false, mClock);
mXmlHelper = new PreferencesHelper(mContext, mPm, mHandler, mMockZenModeHelper,
mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles,
- mUgmInternal, false, mClock);
+ false, mClock);
NotificationChannel channel =
new NotificationChannel("id", "name", IMPORTANCE_LOW);
@@ -3190,64 +3187,6 @@
}
@Test
- @EnableFlags(FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI)
- public void testCreateChannel_noSoundUriPermission_contentSchemeVerified() {
- final Uri sound = Uri.parse(SCHEME_CONTENT + "://media/test/sound/uri");
-
- doThrow(new SecurityException("no access")).when(mUgmInternal)
- .checkGrantUriPermission(eq(UID_N_MR1), any(), eq(sound),
- anyInt(), eq(Process.myUserHandle().getIdentifier()));
-
- final NotificationChannel channel = new NotificationChannel("id2", "name2",
- NotificationManager.IMPORTANCE_DEFAULT);
- channel.setSound(sound, mAudioAttributes);
-
- assertThrows(SecurityException.class,
- () -> mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1, channel,
- true, false, UID_N_MR1, false));
- assertThat(mHelper.getNotificationChannel(PKG_N_MR1, UID_N_MR1, channel.getId(), true))
- .isNull();
- }
-
- @Test
- @EnableFlags(FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI)
- public void testCreateChannel_noSoundUriPermission_fileSchemaIgnored() {
- final Uri sound = Uri.parse(SCHEME_FILE + "://path/sound");
-
- doThrow(new SecurityException("no access")).when(mUgmInternal)
- .checkGrantUriPermission(eq(UID_N_MR1), any(), any(),
- anyInt(), eq(Process.myUserHandle().getIdentifier()));
-
- final NotificationChannel channel = new NotificationChannel("id2", "name2",
- NotificationManager.IMPORTANCE_DEFAULT);
- channel.setSound(sound, mAudioAttributes);
-
- mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1, channel, true, false, UID_N_MR1,
- false);
- assertThat(mHelper.getNotificationChannel(PKG_N_MR1, UID_N_MR1, channel.getId(), true)
- .getSound()).isEqualTo(sound);
- }
-
- @Test
- @EnableFlags(FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI)
- public void testCreateChannel_noSoundUriPermission_resourceSchemaIgnored() {
- final Uri sound = Uri.parse(SCHEME_ANDROID_RESOURCE + "://resId/sound");
-
- doThrow(new SecurityException("no access")).when(mUgmInternal)
- .checkGrantUriPermission(eq(UID_N_MR1), any(), any(),
- anyInt(), eq(Process.myUserHandle().getIdentifier()));
-
- final NotificationChannel channel = new NotificationChannel("id2", "name2",
- NotificationManager.IMPORTANCE_DEFAULT);
- channel.setSound(sound, mAudioAttributes);
-
- mHelper.createNotificationChannel(PKG_N_MR1, UID_N_MR1, channel, true, false, UID_N_MR1,
- false);
- assertThat(mHelper.getNotificationChannel(PKG_N_MR1, UID_N_MR1, channel.getId(), true)
- .getSound()).isEqualTo(sound);
- }
-
- @Test
public void testPermanentlyDeleteChannels() throws Exception {
NotificationChannel channel1 =
new NotificationChannel("id1", "name1", NotificationManager.IMPORTANCE_HIGH);