Check system_server PID in NetworkStack calls

Add a check that callers with UID 1000 always have the same PID. This is
a proxy for checking that no system is designed to bind to the network
stack unless it is the system_server, as otherwise either the
system_server would start crashing, or that system would not have access
to binder calls.

Also remove access from PHONE_UID as it is not being used.

Test: Flashed, WiFi working, Bluetooth reverse tethering shows no
      permission issue.
Bug: 133209255
Change-Id: Ib848aaaedfd599c1d4437378846c7dda74352019

2 files changed, 33 insertions, 4 deletions

diff --git a/packages/NetworkStack/src/com/android/server/NetworkStackService.java b/packages/NetworkStack/src/com/android/server/NetworkStackService.java
index 2fae0c703084..9bf44579127e 100644
--- a/packages/NetworkStack/src/com/android/server/NetworkStackService.java
+++ b/packages/NetworkStack/src/com/android/server/NetworkStackService.java
@@ -195,6 +195,7 @@ public class NetworkStackService extends Service {
         @Override
         public void makeNetworkMonitor(Network network, String name, INetworkMonitorCallbacks cb)
                 throws RemoteException {
+            checkNetworkStackCallingPermission();
             updateSystemAidlVersion(cb.getInterfaceVersion());
             final SharedLog log = addValidationLogs(network, name);
             final NetworkMonitor nm = new NetworkMonitor(mContext, cb, network, log);
@@ -203,6 +204,7 @@ public class NetworkStackService extends Service {
 
         @Override
         public void makeIpClient(String ifName, IIpClientCallbacks cb) throws RemoteException {
+            checkNetworkStackCallingPermission();
             updateSystemAidlVersion(cb.getInterfaceVersion());
             final IpClient ipClient = new IpClient(mContext, ifName, cb, mObserverRegistry, this);
 
@@ -228,6 +230,7 @@ public class NetworkStackService extends Service {
         @Override
         public void fetchIpMemoryStore(@NonNull final IIpMemoryStoreCallbacks cb)
                 throws RemoteException {
+            checkNetworkStackCallingPermission();
             updateSystemAidlVersion(cb.getInterfaceVersion());
             cb.onIpMemoryStoreFetched(mIpMemoryStoreService);
         }
diff --git a/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java b/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java
index 6fbeeadb7e72..670138411bae 100644
--- a/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java
+++ b/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java
@@ -16,30 +16,56 @@
 
 package com.android.server.util;
 
+import static android.os.Binder.getCallingPid;
 import static android.os.Binder.getCallingUid;
 
 import android.os.Process;
 import android.os.UserHandle;
 
+import java.util.concurrent.atomic.AtomicInteger;
+
 /**
  * Utility class to check calling permissions on the network stack.
  */
 public final class PermissionUtil {
+    private static final AtomicInteger sSystemPid = new AtomicInteger(-1);
 
     /**
      * Check that the caller is allowed to communicate with the network stack.
      * @throws SecurityException The caller is not allowed to communicate with the network stack.
      */
     public static void checkNetworkStackCallingPermission() {
-        // TODO: check that the calling PID is the system server.
         final int caller = getCallingUid();
-        if (caller != Process.SYSTEM_UID
-                && UserHandle.getAppId(caller) != Process.BLUETOOTH_UID
-                && UserHandle.getAppId(caller) != Process.PHONE_UID) {
+        if (caller == Process.SYSTEM_UID) {
+            checkConsistentSystemPid();
+            return;
+        }
+
+        if (UserHandle.getAppId(caller) != Process.BLUETOOTH_UID) {
             throw new SecurityException("Invalid caller: " + caller);
         }
     }
 
+    private static void checkConsistentSystemPid() {
+        // Apart from the system server process, no process with a system UID should try to
+        // communicate with the network stack. This is to ensure that the network stack does not
+        // need to maintain behavior for clients it was not designed to work with.
+        // Checking that all calls from a system UID originate from the same PID loosely enforces
+        // this restriction as if another system process calls the network stack first, the system
+        // server would lose access to the network stack and cause obvious failures. If the system
+        // server calls the network stack first, other clients would lose access as expected.
+        final int systemPid = getCallingPid();
+        if (sSystemPid.compareAndSet(-1, systemPid)) {
+            // sSystemPid was unset (-1): this was the first call
+            return;
+        }
+
+        if (sSystemPid.get() != systemPid) {
+            throw new SecurityException("Invalid PID for the system server, expected "
+                    + sSystemPid.get() + " but was called from " + systemPid);
+        }
+    }
+
     /**
      * Check that the caller is allowed to dump the network stack, e.g. dumpsys.
      * @throws SecurityException The caller is not allowed to dump the network stack.