Merge "Cleaned up the permission checks in NetworkScoreService"

This commit is contained in:
TreeHugger Robot
2017-08-18 16:20:43 +00:00
committed by Android (Google) Code Review
3 changed files with 78 additions and 74 deletions

View File

@@ -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 <code>null</code> 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<NetworkScorerAppData> 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 {
*
* <p>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);

View File

@@ -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<NetworkScorerAppData> 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();

View File

@@ -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();