diff --git a/core/java/android/net/NetworkScoreManager.java b/core/java/android/net/NetworkScoreManager.java index 54e1899e9a004..50dd468aa9053 100644 --- a/core/java/android/net/NetworkScoreManager.java +++ b/core/java/android/net/NetworkScoreManager.java @@ -226,6 +226,8 @@ public class NetworkScoreManager { * the {@link #ACTION_CHANGE_ACTIVE} intent. * @return the full package name of the current active scorer, or null if there is no active * scorer. + * @throws SecurityException if the caller doesn't hold either {@link permission#SCORE_NETWORKS} + * or {@link permission#REQUEST_NETWORK_SCORES} permissions. */ @RequiresPermission(anyOf = {android.Manifest.permission.SCORE_NETWORKS, android.Manifest.permission.REQUEST_NETWORK_SCORES}) @@ -240,6 +242,8 @@ public class NetworkScoreManager { /** * Returns metadata about the active scorer or null if there is no active scorer. * + * @throws SecurityException if the caller does not hold the + * {@link permission#REQUEST_NETWORK_SCORES} permission. * @hide */ @Nullable @@ -256,8 +260,11 @@ public class NetworkScoreManager { * Returns the list of available scorer apps. The list will be empty if there are * no valid scorers. * + * @throws SecurityException if the caller does not hold the + * {@link permission#REQUEST_NETWORK_SCORES} permission. * @hide */ + @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) public List getAllValidScorers() { try { return mService.getAllValidScorers(); @@ -296,9 +303,11 @@ public class NetworkScoreManager { * from one scorer cannot be compared to those from another scorer. * * @return whether the clear was successful. - * @throws SecurityException if the caller is not the active scorer or privileged. + * @throws SecurityException if the caller is not the active scorer or if the caller doesn't + * hold the {@link permission#REQUEST_NETWORK_SCORES} permission. */ - @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) + @RequiresPermission(anyOf = {android.Manifest.permission.SCORE_NETWORKS, + android.Manifest.permission.REQUEST_NETWORK_SCORES}) public boolean clearScores() throws SecurityException { try { return mService.clearScores(); @@ -314,12 +323,13 @@ public class NetworkScoreManager { * the {@link #ACTION_CHANGE_ACTIVE} broadcast, or using a custom configuration activity. * * @return true if the operation succeeded, or false if the new package is not a valid scorer. - * @throws SecurityException if the caller is not a system process or does not hold the - * {@link permission#SCORE_NETWORKS} permission + * @throws SecurityException if the caller doesn't hold either {@link permission#SCORE_NETWORKS} + * or {@link permission#REQUEST_NETWORK_SCORES} permissions. * @hide */ @SystemApi - @RequiresPermission(android.Manifest.permission.SCORE_NETWORKS) + @RequiresPermission(anyOf = {android.Manifest.permission.SCORE_NETWORKS, + android.Manifest.permission.REQUEST_NETWORK_SCORES}) public boolean setActiveScorer(String packageName) throws SecurityException { try { return mService.setActiveScorer(packageName); @@ -333,9 +343,11 @@ public class NetworkScoreManager { * *

May only be called by the current scorer app, or the system. * - * @throws SecurityException if the caller is neither the active scorer nor the system. + * @throws SecurityException if the caller is not the active scorer or if the caller doesn't + * hold the {@link permission#REQUEST_NETWORK_SCORES} permission. */ - @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) + @RequiresPermission(anyOf = {android.Manifest.permission.SCORE_NETWORKS, + android.Manifest.permission.REQUEST_NETWORK_SCORES}) public void disableScoring() throws SecurityException { try { mService.disableScoring(); @@ -352,6 +364,7 @@ public class NetworkScoreManager { * {@link permission#REQUEST_NETWORK_SCORES} permission. * @hide */ + @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) public boolean requestScores(NetworkKey[] networks) throws SecurityException { try { return mService.requestScores(networks); @@ -371,6 +384,7 @@ public class NetworkScoreManager { * @deprecated equivalent to registering for cache updates with CACHE_FILTER_NONE. * @hide */ + @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) @Deprecated // migrate to registerNetworkScoreCache(int, INetworkScoreCache, int) public void registerNetworkScoreCache(int networkType, INetworkScoreCache scoreCache) { registerNetworkScoreCache(networkType, scoreCache, CACHE_FILTER_NONE); @@ -387,6 +401,7 @@ public class NetworkScoreManager { * @throws IllegalArgumentException if a score cache is already registered for this type. * @hide */ + @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) public void registerNetworkScoreCache(int networkType, INetworkScoreCache scoreCache, @CacheUpdateFilter int filterType) { try { @@ -406,6 +421,7 @@ public class NetworkScoreManager { * @throws IllegalArgumentException if a score cache is already registered for this type. * @hide */ + @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) public void unregisterNetworkScoreCache(int networkType, INetworkScoreCache scoreCache) { try { mService.unregisterNetworkScoreCache(networkType, scoreCache); @@ -419,8 +435,11 @@ public class NetworkScoreManager { * * @param callingUid the UID to check * @return true if the provided UID is the active scorer, false otherwise. + * @throws SecurityException if the caller does not hold the + * {@link permission#REQUEST_NETWORK_SCORES} permission. * @hide */ + @RequiresPermission(android.Manifest.permission.REQUEST_NETWORK_SCORES) public boolean isCallerActiveScorer(int callingUid) { try { return mService.isCallerActiveScorer(callingUid); diff --git a/services/core/java/com/android/server/NetworkScoreService.java b/services/core/java/com/android/server/NetworkScoreService.java index e438620faeb77..44c02270fe6cc 100644 --- a/services/core/java/com/android/server/NetworkScoreService.java +++ b/services/core/java/com/android/server/NetworkScoreService.java @@ -46,7 +46,6 @@ import android.os.Handler; import android.os.IBinder; import android.os.Looper; import android.os.Message; -import android.os.Process; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.UserHandle; @@ -640,42 +639,22 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } } - private boolean canCallerRequestScores() { - // REQUEST_NETWORK_SCORES is a signature only permission. - return mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES) == - PackageManager.PERMISSION_GRANTED; - } - - private boolean canCallerScoreNetworks() { - return mContext.checkCallingOrSelfPermission(permission.SCORE_NETWORKS) == - PackageManager.PERMISSION_GRANTED; - } - @Override public boolean clearScores() { // Only the active scorer or the system should be allowed to flush all scores. - if (isCallerActiveScorer(getCallingUid()) || canCallerRequestScores()) { - final long token = Binder.clearCallingIdentity(); - try { - clearInternal(); - return true; - } finally { - Binder.restoreCallingIdentity(token); - } - } else { - throw new SecurityException( - "Caller is neither the active scorer nor the scorer manager."); + enforceSystemOrIsActiveScorer(getCallingUid()); + final long token = Binder.clearCallingIdentity(); + try { + clearInternal(); + return true; + } finally { + Binder.restoreCallingIdentity(token); } } @Override public boolean setActiveScorer(String packageName) { - // Only the system can set the active scorer - if (!isCallerSystemProcess(getCallingUid()) && !canCallerScoreNetworks()) { - throw new SecurityException( - "Caller is neither the system process or a network scorer."); - } - + enforceSystemOrHasScoreNetworks(); return mNetworkScorerAppManager.setActiveScorer(packageName); } @@ -693,8 +672,29 @@ public class NetworkScoreService extends INetworkScoreService.Stub { } } - private boolean isCallerSystemProcess(int callingUid) { - return callingUid == Process.SYSTEM_UID; + private void enforceSystemOnly() throws SecurityException { + // REQUEST_NETWORK_SCORES is a signature only permission. + mContext.enforceCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES, + "Caller must be granted REQUEST_NETWORK_SCORES."); + } + + private void enforceSystemOrHasScoreNetworks() throws SecurityException { + if (mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES) + != PackageManager.PERMISSION_GRANTED + && mContext.checkCallingOrSelfPermission(permission.SCORE_NETWORKS) + != PackageManager.PERMISSION_GRANTED) { + throw new SecurityException( + "Caller is neither the system process or a network scorer."); + } + } + + private void enforceSystemOrIsActiveScorer(int callingUid) throws SecurityException { + if (mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES) + != PackageManager.PERMISSION_GRANTED + && !isCallerActiveScorer(callingUid)) { + throw new SecurityException( + "Caller is neither the system process or the active network scorer."); + } } /** @@ -705,17 +705,12 @@ public class NetworkScoreService extends INetworkScoreService.Stub { */ @Override public String getActiveScorerPackage() { - if (canCallerRequestScores() || canCallerScoreNetworks()) { - synchronized (mServiceConnectionLock) { - if (mServiceConnection != null) { - return mServiceConnection.getPackageName(); - } + enforceSystemOrHasScoreNetworks(); + synchronized (mServiceConnectionLock) { + if (mServiceConnection != null) { + return mServiceConnection.getPackageName(); } - } else { - throw new SecurityException( - "Caller is not a network scorer/requester."); } - return null; } @@ -725,15 +720,11 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public NetworkScorerAppData getActiveScorer() { // Only the system can access this data. - if (isCallerSystemProcess(getCallingUid()) || canCallerRequestScores()) { - synchronized (mServiceConnectionLock) { - if (mServiceConnection != null) { - return mServiceConnection.getAppData(); - } + enforceSystemOnly(); + synchronized (mServiceConnectionLock) { + if (mServiceConnection != null) { + return mServiceConnection.getAppData(); } - } else { - throw new SecurityException( - "Caller is neither the system process nor a score requester."); } return null; @@ -746,22 +737,14 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public List getAllValidScorers() { // Only the system can access this data. - if (!isCallerSystemProcess(getCallingUid()) && !canCallerRequestScores()) { - throw new SecurityException( - "Caller is neither the system process nor a score requester."); - } - + enforceSystemOnly(); return mNetworkScorerAppManager.getAllValidScorers(); } @Override public void disableScoring() { // Only the active scorer or the system should be allowed to disable scoring. - if (!isCallerActiveScorer(getCallingUid()) && !canCallerRequestScores()) { - throw new SecurityException( - "Caller is neither the active scorer nor the scorer manager."); - } - + enforceSystemOrIsActiveScorer(getCallingUid()); // no-op for now but we could write to the setting if needed. } @@ -785,7 +768,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { public void registerNetworkScoreCache(int networkType, INetworkScoreCache scoreCache, int filterType) { - mContext.enforceCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES, TAG); + enforceSystemOnly(); final long token = Binder.clearCallingIdentity(); try { synchronized (mScoreCaches) { @@ -810,7 +793,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public void unregisterNetworkScoreCache(int networkType, INetworkScoreCache scoreCache) { - mContext.enforceCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES, TAG); + enforceSystemOnly(); final long token = Binder.clearCallingIdentity(); try { synchronized (mScoreCaches) { @@ -831,7 +814,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub { @Override public boolean requestScores(NetworkKey[] networks) { - mContext.enforceCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES, TAG); + enforceSystemOnly(); final long token = Binder.clearCallingIdentity(); try { final INetworkRecommendationProvider provider = getRecommendationProvider(); diff --git a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java index be6a1a5d15b04..3b7db9f730103 100644 --- a/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java @@ -367,6 +367,8 @@ public class NetworkScoreServiceTest { @Test public void testSetActiveScorer_noScoreNetworksPermission() { + when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES)) + .thenReturn(PackageManager.PERMISSION_DENIED); when(mContext.checkCallingOrSelfPermission(permission.SCORE_NETWORKS)) .thenReturn(PackageManager.PERMISSION_DENIED); @@ -411,8 +413,8 @@ public class NetworkScoreServiceTest { @Test public void testGetAllValidScorer_noRequestNetworkScoresPermission() { - when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES)) - .thenReturn(PackageManager.PERMISSION_DENIED); + doThrow(new SecurityException()).when(mContext) + .enforceCallingOrSelfPermission(eq(permission.REQUEST_NETWORK_SCORES), anyString()); try { mNetworkScoreService.getAllValidScorers(); @@ -805,8 +807,8 @@ public class NetworkScoreServiceTest { @Test public void testGetActiveScorer_notConnected_canNotRequestScores() throws Exception { - when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES)) - .thenReturn(PackageManager.PERMISSION_DENIED); + doThrow(new SecurityException()).when(mContext) + .enforceCallingOrSelfPermission(eq(permission.REQUEST_NETWORK_SCORES), anyString()); try { mNetworkScoreService.getActiveScorer(); fail("SecurityException expected."); @@ -830,8 +832,8 @@ public class NetworkScoreServiceTest { @Test public void testGetActiveScorer_connected_canNotRequestScores() throws Exception { - when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES)) - .thenReturn(PackageManager.PERMISSION_DENIED); + doThrow(new SecurityException()).when(mContext) + .enforceCallingOrSelfPermission(eq(permission.REQUEST_NETWORK_SCORES), anyString()); bindToScorer(false); try { mNetworkScoreService.getActiveScorer();