Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1588b3b0 authored by Jeremy Joslin's avatar Jeremy Joslin
Browse files

Cleaned up the permission checks in NetworkScoreService 

Refactored the existing code into 3 permission check methods
to clear up what was being checked and to centralize the logic.

Bug:64313251
Test: bit services FrameworksServicesTests:com.android.server.NetworkScoreServiceTest
Test: bit FrameworksWifiTests:*
Test: gts-tradefed run gts -m GtsGmscoreHostTestCases -t com.google.android.gts.netrec.NetRecHostTest
Change-Id: I77457ce7cf98252e83f4dfa6602da0179d638818
parent 211ec527

core/java/android/net/NetworkScoreManager.java

+26 −7
Original line number Diff line number Diff line
@@ -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);

services/core/java/com/android/server/NetworkScoreService.java

+44 −61
Original line number Diff line number Diff line
@@ -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,21 +639,10 @@ 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()) {

        enforceSystemOrIsActiveScorer(getCallingUid());

        final long token = Binder.clearCallingIdentity();

        try {

            clearInternal();
@@ -662,20 +650,11 @@ public class NetworkScoreService extends INetworkScoreService.Stub { 
        } finally {

            Binder.restoreCallingIdentity(token);

        }

        } else {

            throw new SecurityException(

                    "Caller is neither the active scorer nor the scorer manager.");

        }

    }



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

        enforceSystemOrHasScoreNetworks();

        synchronized (mServiceConnectionLock) {

            if (mServiceConnection != null) {

                return mServiceConnection.getPackageName();

            }

        }

        } else {

            throw new SecurityException(

                    "Caller is not a network scorer/requester.");

        }



        return null;

    }
@@ -725,16 +720,12 @@ public class NetworkScoreService extends INetworkScoreService.Stub { 
    @Override

    public NetworkScorerAppData getActiveScorer() {

        // Only the system can access this data.

        if (isCallerSystemProcess(getCallingUid()) || canCallerRequestScores()) {

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

services/tests/servicestests/src/com/android/server/NetworkScoreServiceTest.java

+8 −6
Original line number Diff line number Diff line
@@ -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();