From b7a1333b896e54dd893e3a87f3606d1bb5b85b73 Mon Sep 17 00:00:00 2001 From: Santiago Seifert Date: Wed, 4 Jan 2023 16:39:00 +0000 Subject: Simplify if/else branch in MR2ServiceImpl Non-functional change to deduplicate code. Test: Should be trivially correct. Bug: 205124386 Change-Id: I07f8182d4b4b3dfea2dc5cb63edd77858becd87a --- .../java/com/android/server/media/MediaRouter2ServiceImpl.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java index 1998af649070..5e01ddbfbf77 100644 --- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java +++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java @@ -1938,12 +1938,12 @@ class MediaRouter2ServiceImpl { @NonNull RoutingSessionInfo oldSession, @NonNull MediaRoute2Info route) { try { if (route.isSystemRoute() && !routerRecord.mHasModifyAudioRoutingPermission) { - routerRecord.mRouter.requestCreateSessionByManager(uniqueRequestId, - oldSession, mSystemProvider.getDefaultRoute()); - } else { - routerRecord.mRouter.requestCreateSessionByManager(uniqueRequestId, - oldSession, route); + // The router lacks permission to modify system routing, so we hide system + // route info from them. + route = mSystemProvider.getDefaultRoute(); } + routerRecord.mRouter.requestCreateSessionByManager( + uniqueRequestId, oldSession, route); } catch (RemoteException ex) { Slog.w(TAG, "getSessionHintsForCreatingSessionOnHandler: " + "Failed to request. Router probably died.", ex); -- cgit v1.2.3-59-g8ed1b From c5944b179e2fc98aa5fca114ccc49276b0b9566c Mon Sep 17 00:00:00 2001 From: Santiago Seifert Date: Wed, 4 Jan 2023 17:30:07 +0000 Subject: Enforce MEDIA_CONTENT_CONTROL in verifyPackageName Also rename verifyPackageName to verifyPackageExists, which probably reflects behavior a bit better. Before this change, an app can use a direct binder call to ask the system whether a package is installed without having any permissions. Since the method now requires MEDIA_CONTENT_CONTROL, we don't need enforceMediaContentControlPermission anymore. Bug: 205124386 Test: atest MediaRouter2HostSideTest CtsMediaBetterTogetherTestCases Change-Id: I66723a00cf8b22b40aa0cd680a7dbe4bb71fddb6 --- media/java/android/media/IMediaRouterService.aidl | 3 +-- media/java/android/media/MediaRouter2.java | 6 +++--- .../server/media/MediaRouter2ServiceImpl.java | 23 ++++++++-------------- .../android/server/media/MediaRouterService.java | 10 ++-------- 4 files changed, 14 insertions(+), 28 deletions(-) diff --git a/media/java/android/media/IMediaRouterService.aidl b/media/java/android/media/IMediaRouterService.aidl index f6a9162cda39..aa7e4df18186 100644 --- a/media/java/android/media/IMediaRouterService.aidl +++ b/media/java/android/media/IMediaRouterService.aidl @@ -50,8 +50,7 @@ interface IMediaRouterService { // MediaRouterService.java for readability. // Methods for MediaRouter2 - boolean verifyPackageName(String clientPackageName); - void enforceMediaContentControlPermission(); + boolean verifyPackageExists(String clientPackageName); List getSystemRoutes(); RoutingSessionInfo getSystemSessionInfo(); diff --git a/media/java/android/media/MediaRouter2.java b/media/java/android/media/MediaRouter2.java index 5faa794151b1..fa74a9f12d7c 100644 --- a/media/java/android/media/MediaRouter2.java +++ b/media/java/android/media/MediaRouter2.java @@ -208,9 +208,9 @@ public final class MediaRouter2 { IMediaRouterService.Stub.asInterface( ServiceManager.getService(Context.MEDIA_ROUTER_SERVICE)); try { - // SecurityException will be thrown if there's no permission. - serviceBinder.enforceMediaContentControlPermission(); - if (!serviceBinder.verifyPackageName(clientPackageName)) { + // verifyPackageExists throws SecurityException if the caller doesn't hold + // MEDIA_CONTENT_CONTROL permission. + if (!serviceBinder.verifyPackageExists(clientPackageName)) { Log.e(TAG, "Package " + clientPackageName + " not found. Ignoring."); return null; } diff --git a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java index 5e01ddbfbf77..2e31a4a4c86e 100644 --- a/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java +++ b/services/core/java/com/android/server/media/MediaRouter2ServiceImpl.java @@ -154,10 +154,17 @@ class MediaRouter2ServiceImpl { // Start of methods that implement MediaRouter2 operations. @NonNull - public boolean verifyPackageName(@NonNull String clientPackageName) { + public boolean verifyPackageExists(@NonNull String clientPackageName) { + final int pid = Binder.getCallingPid(); + final int uid = Binder.getCallingUid(); final long token = Binder.clearCallingIdentity(); try { + mContext.enforcePermission( + Manifest.permission.MEDIA_CONTENT_CONTROL, + pid, + uid, + "Must hold MEDIA_CONTENT_CONTROL permission."); PackageManager pm = mContext.getPackageManager(); pm.getPackageInfo(clientPackageName, PackageManager.PackageInfoFlags.of(0)); return true; @@ -168,20 +175,6 @@ class MediaRouter2ServiceImpl { } } - @NonNull - public void enforceMediaContentControlPermission() { - final int pid = Binder.getCallingPid(); - final int uid = Binder.getCallingUid(); - final long token = Binder.clearCallingIdentity(); - - try { - mContext.enforcePermission(Manifest.permission.MEDIA_CONTENT_CONTROL, pid, uid, - "Must hold MEDIA_CONTENT_CONTROL permission."); - } finally { - Binder.restoreCallingIdentity(token); - } - } - @NonNull public List getSystemRoutes() { final int uid = Binder.getCallingUid(); diff --git a/services/core/java/com/android/server/media/MediaRouterService.java b/services/core/java/com/android/server/media/MediaRouterService.java index ad82e1c786e3..3ad0e44e6ea3 100644 --- a/services/core/java/com/android/server/media/MediaRouterService.java +++ b/services/core/java/com/android/server/media/MediaRouterService.java @@ -380,14 +380,8 @@ public final class MediaRouterService extends IMediaRouterService.Stub // Binder call @Override - public boolean verifyPackageName(String clientPackageName) { - return mService2.verifyPackageName(clientPackageName); - } - - // Binder call - @Override - public void enforceMediaContentControlPermission() { - mService2.enforceMediaContentControlPermission(); + public boolean verifyPackageExists(String clientPackageName) { + return mService2.verifyPackageExists(clientPackageName); } // Binder call -- cgit v1.2.3-59-g8ed1b