From 8000e03ec993c62227e0577ff0543d1360cfe751 Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Thu, 26 Nov 2020 23:42:25 +0900
Subject: [PATCH] Clear calling identity in registerNetworkAgent.

Much of registerNetworkAgent calls internal ConnectivityService
methods which generally assume that they are not processing an
IPC and are running under the system's calling identity.

However, only the call to makeNetworkMonitor is run with caller
identity cleared. Expand the scope of clearing the caller
identity over the creation of the nai.

Bug: 173331190
Test: passes existing tests in ConnectivityServiceTest
Change-Id: Icad28601a612fb5e1ed0451ec9e2066f4e766d0e
---
 .../android/server/ConnectivityService.java   | 33 ++++++++++++-------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index e96958eb32..b909b26096 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -5980,13 +5980,29 @@ public class ConnectivityService extends IConnectivityManager.Stub
             int currentScore, NetworkAgentConfig networkAgentConfig, int providerId) {
         if (networkCapabilities.hasTransport(TRANSPORT_TEST)) {
             enforceAnyPermissionOf(Manifest.permission.MANAGE_TEST_NETWORKS);
+        } else {
+            enforceNetworkFactoryPermission();
+        }
+
+        final int uid = Binder.getCallingUid();
+        final long token = Binder.clearCallingIdentity();
+        try {
+            return registerNetworkAgentInternal(messenger, networkInfo, linkProperties,
+                    networkCapabilities, currentScore, networkAgentConfig, providerId, uid);
+        } finally {
+            Binder.restoreCallingIdentity(token);
+        }
+    }
+
+    private Network registerNetworkAgentInternal(Messenger messenger, NetworkInfo networkInfo,
+            LinkProperties linkProperties, NetworkCapabilities networkCapabilities,
+            int currentScore, NetworkAgentConfig networkAgentConfig, int providerId, int uid) {
+        if (networkCapabilities.hasTransport(TRANSPORT_TEST)) {
             // Strictly, sanitizing here is unnecessary as the capabilities will be sanitized in
             // the call to mixInCapabilities below anyway, but sanitizing here means the NAI never
             // sees capabilities that may be malicious, which might prevent mistakes in the future.
             networkCapabilities = new NetworkCapabilities(networkCapabilities);
-            networkCapabilities.restrictCapabilitesForTestNetwork(Binder.getCallingUid());
-        } else {
-            enforceNetworkFactoryPermission();
+            networkCapabilities.restrictCapabilitesForTestNetwork(uid);
         }
 
         LinkProperties lp = new LinkProperties(linkProperties);
@@ -5997,7 +6013,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         final NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(),
                 new Network(mNetIdManager.reserveNetId()), new NetworkInfo(networkInfo), lp, nc,
                 currentScore, mContext, mTrackerHandler, new NetworkAgentConfig(networkAgentConfig),
-                this, mNetd, mDnsResolver, mNMS, providerId, Binder.getCallingUid());
+                this, mNetd, mDnsResolver, mNMS, providerId, uid);
 
         // Make sure the LinkProperties and NetworkCapabilities reflect what the agent info says.
         processCapabilitiesFromAgent(nai, nc);
@@ -6008,13 +6024,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
         final String name = TextUtils.isEmpty(extraInfo)
                 ? nai.networkCapabilities.getSsid() : extraInfo;
         if (DBG) log("registerNetworkAgent " + nai);
-        final long token = Binder.clearCallingIdentity();
-        try {
-            mDeps.getNetworkStack().makeNetworkMonitor(
-                    nai.network, name, new NetworkMonitorCallbacks(nai));
-        } finally {
-            Binder.restoreCallingIdentity(token);
-        }
+        mDeps.getNetworkStack().makeNetworkMonitor(
+                nai.network, name, new NetworkMonitorCallbacks(nai));
         // NetworkAgentInfo registration will finish when the NetworkMonitor is created.
         // If the network disconnects or sends any other event before that, messages are deferred by
         // NetworkAgent until nai.asyncChannel.connect(), which will be called when finalizing the