Merge "Check system_server PID in NetworkStack calls" into qt-dev am: 0e740ae97e

am: f045873ea3

Change-Id: I9975f695a2f5cb75961beac511879b575bdac469

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

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
+        if (caller == Process.SYSTEM_UID) {
-                && UserHandle.getAppId(caller) != Process.BLUETOOTH_UID
+            checkConsistentSystemPid();
-                && UserHandle.getAppId(caller) != Process.PHONE_UID) {
+            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.