Security-related cleanup for network scoring.

-Perform additional checks for the SCORE_NETWORKS permission when
broadcasting scoring requests to the active scorer and when accepting
score updates. In theory, these checks are unnecessary as we manually
check package manager when obtaining the list of valid scorers, but
they cannot hurt to add.

-Fix multi-user. Since the active scorer is a global setting, we
ensure that scoring can only be done by apps available to the primary
user / owner of the phone, and that the request scores broadcast is
sent to that user's profile. When the scorer is changed, we send that
to all user profiles as it's just informational, although it's
unlikely that apps outside the primary user's profile would need to
respond.

Bug: 14117916
Bug: 16399238
Change-Id: Iaf06bda244eec730b590a30a3f4ffab4965bde96

2 files changed, 15 insertions, 3 deletions

diff --git a/core/java/android/net/NetworkScoreManager.java b/core/java/android/net/NetworkScoreManager.java
index 921585321c38..3f68a4431d25 100644
--- a/core/java/android/net/NetworkScoreManager.java
+++ b/core/java/android/net/NetworkScoreManager.java
@@ -16,6 +16,7 @@
 
 package android.net;
 
+import android.Manifest;
 import android.annotation.SdkConstant;
 import android.annotation.SdkConstant.SdkConstantType;
 import android.annotation.SystemApi;
@@ -25,6 +26,7 @@ import android.net.NetworkScorerAppManager.NetworkScorerAppData;
 import android.os.IBinder;
 import android.os.RemoteException;
 import android.os.ServiceManager;
+import android.os.UserHandle;
 
 /**
  * Class that manages communication between network subsystems and a network scorer.
@@ -238,7 +240,9 @@ public class NetworkScoreManager {
         intent.setPackage(activeScorer);
         intent.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT);
         intent.putExtra(EXTRA_NETWORKS_TO_SCORE, networks);
-        mContext.sendBroadcast(intent);
+        // A scorer should never become active if its package doesn't hold SCORE_NETWORKS, but
+        // ensure the package still holds it to be extra safe.
+        mContext.sendBroadcastAsUser(intent, UserHandle.OWNER, Manifest.permission.SCORE_NETWORKS);
         return true;
     }
 
diff --git a/core/java/android/net/NetworkScorerAppManager.java b/core/java/android/net/NetworkScorerAppManager.java
index 87a68f739a46..c33f5ecf59c4 100644
--- a/core/java/android/net/NetworkScorerAppManager.java
+++ b/core/java/android/net/NetworkScorerAppManager.java
@@ -16,6 +16,7 @@
 
 package android.net;
 
+import android.Manifest;
 import android.Manifest.permission;
 import android.annotation.Nullable;
 import android.app.AppOpsManager;
@@ -24,6 +25,7 @@ import android.content.Intent;
 import android.content.pm.ActivityInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
+import android.os.UserHandle;
 import android.provider.Settings;
 import android.text.TextUtils;
 import android.util.Log;
@@ -86,7 +88,9 @@ public final class NetworkScorerAppManager {
         List<NetworkScorerAppData> scorers = new ArrayList<>();
 
         PackageManager pm = context.getPackageManager();
-        List<ResolveInfo> receivers = pm.queryBroadcastReceivers(SCORE_INTENT, 0 /* flags */);
+        // Only apps installed under the primary user of the device can be scorers.
+        List<ResolveInfo> receivers =
+                pm.queryBroadcastReceivers(SCORE_INTENT, 0 /* flags */, UserHandle.USER_OWNER);
         for (ResolveInfo receiver : receivers) {
             // This field is a misnomer, see android.content.pm.ResolveInfo#activityInfo
             final ActivityInfo receiverInfo = receiver.activityInfo;
@@ -186,10 +190,14 @@ public final class NetworkScorerAppManager {
         AppOpsManager appOpsMgr = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
         try {
             appOpsMgr.checkPackage(callingUid, defaultApp.mPackageName);
-            return true;
         } catch (SecurityException e) {
             return false;
         }
+
+        // To be extra safe, ensure the caller holds the SCORE_NETWORKS permission. It always
+        // should, since it couldn't become the active scorer otherwise, but this can't hurt.
+        return context.checkCallingPermission(Manifest.permission.SCORE_NETWORKS) ==
+                PackageManager.PERMISSION_GRANTED;
     }
 
     /** Returns the {@link NetworkScorerAppData} for the given app, or null if it's not a scorer. */