diff options
author | Beth Thibodeau <ethibodeau@google.com> | 2023-08-08 16:19:48 -0500 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-11-09 21:20:09 +0000 |
commit | 80e565eab30582df4afddf4e1b1658628a5bdfa9 (patch) | |
tree | 0e7f86009394e3ac445779ebb3befafbaa7e75fe | |
parent | f693e48ae534acf1b4619149a3e58b404190683c (diff) | |
download | base-80e565eab30582df4afddf4e1b1658628a5bdfa9.tar.gz |
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:c8b7d562cfd8006e12ffcd621ec7811a393025f6)
Merged-In: Ie79915d3d1712f08dc2e8dfbd5bc7fd32bb308a3
Change-Id: Ie79915d3d1712f08dc2e8dfbd5bc7fd32bb308a3
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 6b993ce9e7bf..69f02f785b03 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 @@ -22,11 +22,13 @@ import android.app.Notification import android.app.Notification.EXTRA_SUBSTITUTE_APP_NAME import android.app.PendingIntent import android.app.StatusBarManager +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 @@ -699,10 +701,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) { @@ -711,9 +716,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 && @@ -1260,6 +1263,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 56698e0ec41c..9a415f571bd5 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<SmartspaceConfig> + @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>(UriGrantsManager::class.java) + .mockStatic<ImageDecoder>(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( @@ -2199,6 +2218,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 01fdc8800c0e..1faba099a547 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<UriPermission> providePersistentUriGrants() { final ArrayList<UriPermission> result = new ArrayList<>(); |