diff options
7 files changed, 204 insertions, 24 deletions
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index c3a714b0eef0..79633f19715b 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -2579,6 +2579,7 @@ public class NotificationManagerService extends SystemService { mNotificationChannelLogger, mAppOps, mUserProfiles, + mUgmInternal, mShowReviewPermissionsNotification, Clock.systemUTC()); mRankingHelper = new RankingHelper(getContext(), mRankingHandler, mPreferencesHelper, @@ -6672,13 +6673,7 @@ public class NotificationManagerService extends SystemService { final Uri originalSoundUri = (originalChannel != null) ? originalChannel.getSound() : null; if (soundUri != null && !Objects.equals(originalSoundUri, soundUri)) { - Binder.withCleanCallingIdentity(() -> { - mUgmInternal.checkGrantUriPermission(sourceUid, null, - ContentProvider.getUriWithoutUserId(soundUri), - Intent.FLAG_GRANT_READ_URI_PERMISSION, - ContentProvider.getUserIdFromUri(soundUri, - UserHandle.getUserId(sourceUid))); - }); + PermissionHelper.grantUriPermission(mUgmInternal, soundUri, sourceUid); } } diff --git a/services/core/java/com/android/server/notification/NotificationRecord.java b/services/core/java/com/android/server/notification/NotificationRecord.java index b9f0968b5864..3ba93845a290 100644 --- a/services/core/java/com/android/server/notification/NotificationRecord.java +++ b/services/core/java/com/android/server/notification/NotificationRecord.java @@ -1493,14 +1493,23 @@ public final class NotificationRecord { final Notification notification = getNotification(); notification.visitUris((uri) -> { - visitGrantableUri(uri, false, false); + if (com.android.server.notification.Flags.notificationVerifyChannelSoundUri()) { + visitGrantableUri(uri, false, false); + } else { + oldVisitGrantableUri(uri, false, false); + } }); if (notification.getChannelId() != null) { NotificationChannel channel = getChannel(); if (channel != null) { - visitGrantableUri(channel.getSound(), (channel.getUserLockedFields() - & NotificationChannel.USER_LOCKED_SOUND) != 0, true); + 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); + } } } } finally { @@ -1516,7 +1525,7 @@ public final class NotificationRecord { * {@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) { + private void oldVisitGrantableUri(Uri uri, boolean userOverriddenUri, boolean isSound) { if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return; if (mGrantableUris != null && mGrantableUris.contains(uri)) { @@ -1555,6 +1564,45 @@ public final class NotificationRecord { } } + /** + * 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 b6f48890c528..1464d481311a 100644 --- a/services/core/java/com/android/server/notification/PermissionHelper.java +++ b/services/core/java/com/android/server/notification/PermissionHelper.java @@ -25,19 +25,25 @@ import android.Manifest; 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; @@ -58,7 +64,7 @@ public final class PermissionHelper { private final IPermissionManager mPermManager; public PermissionHelper(Context context, IPackageManager packageManager, - IPermissionManager permManager) { + IPermissionManager permManager) { mContext = context; mPackageManager = packageManager; mPermManager = permManager; @@ -298,6 +304,19 @@ public final class PermissionHelper { 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 85c395781d0a..9e70f815dff9 100644 --- a/services/core/java/com/android/server/notification/PreferencesHelper.java +++ b/services/core/java/com/android/server/notification/PreferencesHelper.java @@ -94,6 +94,7 @@ import com.android.internal.util.XmlUtils; 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; @@ -219,6 +220,7 @@ public class PreferencesHelper implements RankingConfig { private final NotificationChannelLogger mNotificationChannelLogger; private final AppOpsManager mAppOps; private final ManagedServices.UserProfiles mUserProfiles; + private final UriGrantsManagerInternal mUgmInternal; private SparseBooleanArray mBadgingEnabled; private SparseBooleanArray mBubblesEnabled; @@ -239,6 +241,7 @@ public class PreferencesHelper implements RankingConfig { ZenModeHelper zenHelper, PermissionHelper permHelper, PermissionManager permManager, NotificationChannelLogger notificationChannelLogger, AppOpsManager appOpsManager, ManagedServices.UserProfiles userProfiles, + UriGrantsManagerInternal ugmInternal, boolean showReviewPermissionsNotification, Clock clock) { mContext = context; mZenModeHelper = zenHelper; @@ -249,6 +252,7 @@ public class PreferencesHelper implements RankingConfig { mNotificationChannelLogger = notificationChannelLogger; mAppOps = appOpsManager; mUserProfiles = userProfiles; + mUgmInternal = ugmInternal; mShowReviewPermissionsNotification = showReviewPermissionsNotification; mIsMediaNotificationFilteringEnabled = context.getResources() .getBoolean(R.bool.config_quickSettingsShowMediaPlayer); @@ -1169,6 +1173,13 @@ public class PreferencesHelper implements RankingConfig { } 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 7043ceb7a525..0b34177d7413 100644 --- a/services/core/java/com/android/server/notification/flags.aconfig +++ b/services/core/java/com/android/server/notification/flags.aconfig @@ -170,3 +170,13 @@ flag { description: "This flag enables sound uri with vibration source" bug: "358524009" } + +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 + } +} 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 6c9015d72d5a..44770d21892c 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -4652,7 +4652,42 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { doThrow(new SecurityException("no access")).when(mUgmInternal) .checkGrantUriPermission(eq(Process.myUid()), any(), eq(soundUri), - anyInt(), eq(Process.myUserHandle().getIdentifier())); + anyInt(), eq(Process.myUserHandle().getIdentifier())); + + 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 + testUpdateNotificationChannelFromPrivilegedListener_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); 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 a0c0df8853f9..d64b9e858c64 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java @@ -45,11 +45,13 @@ import static android.app.NotificationManager.IMPORTANCE_MAX; 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.Flags.FLAG_NOTIFICATION_CLASSIFICATION; import static android.service.notification.Flags.notificationClassification; @@ -59,6 +61,7 @@ import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_P 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_ALL_NOTIFS_NEED_TTL; +import static com.android.server.notification.Flags.FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI; 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; @@ -84,6 +87,7 @@ import static org.mockito.ArgumentMatchers.eq; 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; @@ -369,10 +373,10 @@ public class PreferencesHelperTest extends UiServiceTestCase { mHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - false, mClock); + mUgmInternal, false, mClock); mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - false, mClock); + mUgmInternal, false, mClock); resetZenModeHelper(); mAudioAttributes = new AudioAttributes.Builder() @@ -783,7 +787,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { public void testReadXml_oldXml_migrates() throws Exception { mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - /* showReviewPermissionsNotification= */ true, mClock); + mUgmInternal, /* showReviewPermissionsNotification= */ true, mClock); String xml = "<ranking version=\"2\">\n" + "<package name=\"" + PKG_N_MR1 + "\" uid=\"" + UID_N_MR1 @@ -919,7 +923,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { public void testReadXml_newXml_noMigration_showPermissionNotification() throws Exception { mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - /* showReviewPermissionsNotification= */ true, mClock); + mUgmInternal, /* showReviewPermissionsNotification= */ true, mClock); String xml = "<ranking version=\"3\">\n" + "<package name=\"" + PKG_N_MR1 + "\" show_badge=\"true\">\n" @@ -978,7 +982,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { public void testReadXml_newXml_permissionNotificationOff() throws Exception { mHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - /* showReviewPermissionsNotification= */ false, mClock); + mUgmInternal, /* showReviewPermissionsNotification= */ false, mClock); String xml = "<ranking version=\"3\">\n" + "<package name=\"" + PKG_N_MR1 + "\" show_badge=\"true\">\n" @@ -1037,7 +1041,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { public void testReadXml_newXml_noMigration_noPermissionNotification() throws Exception { mHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - /* showReviewPermissionsNotification= */ true, mClock); + mUgmInternal, /* showReviewPermissionsNotification= */ true, mClock); String xml = "<ranking version=\"4\">\n" + "<package name=\"" + PKG_N_MR1 + "\" show_badge=\"true\">\n" @@ -1709,7 +1713,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { // simulate load after reboot mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - false, mClock); + mUgmInternal, false, mClock); loadByteArrayXml(baos.toByteArray(), false, USER_ALL); // Trigger 2nd restore pass @@ -1764,7 +1768,7 @@ public class PreferencesHelperTest extends UiServiceTestCase { // simulate load after reboot mXmlHelper = new PreferencesHelper(getContext(), mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - false, mClock); + mUgmInternal, false, mClock); loadByteArrayXml(xml.getBytes(), false, USER_ALL); // Trigger 2nd restore pass @@ -1842,10 +1846,10 @@ public class PreferencesHelperTest extends UiServiceTestCase { mHelper = new PreferencesHelper(mContext, mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - false, mClock); + mUgmInternal, false, mClock); mXmlHelper = new PreferencesHelper(mContext, mPm, mHandler, mMockZenModeHelper, mPermissionHelper, mPermissionManager, mLogger, mAppOpsManager, mUserProfiles, - false, mClock); + mUgmInternal, false, mClock); NotificationChannel channel = new NotificationChannel("id", "name", IMPORTANCE_LOW); @@ -3049,6 +3053,64 @@ public class PreferencesHelperTest extends UiServiceTestCase { } @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); |