commit b4a4fa1be51357a8c6c58cb5a72c03421ebabf44
author Chenbo Feng <fengc@google.com> Wed Jan 09 17:20:45 2019 -0800
committer Chenbo Feng <fengc@google.com> Tue Jan 29 19:25:02 2019 -0800
tree f4ed8abfad1a7522a8442e9409debc8ca7dcd803
parent 48eaed3745017204667641bd71782674e032d682

Don't call into system_server for permissions check

With the recently added permission information in traffic controller,
netd can check if the calling process has permission UPDATE_DEVICE_STATS
without calling into system_server. Update the code path and add some
test cases for it.

Bug: 111560570
Bug: 111560739
Test: netd_unit_test, netd_integration_test
Change-Id: I79eee1321f32154e91466f023f7952db23df8494

server/FwmarkServer.cpp

diff --git a/server/FwmarkServer.cpp b/server/FwmarkServer.cpp
index dc05093..51d5398 100644
--- a/server/FwmarkServer.cpp
+++ b/server/FwmarkServer.cpp
@@ -37,7 +37,6 @@
 namespace android {
 namespace net {
 
-constexpr const char *UPDATE_DEVICE_STATS = "android.permission.UPDATE_DEVICE_STATS";
 constexpr const char *SYSTEM_SERVER_CONTEXT = "u:r:system_server:s0";
 
 bool isSystemServer(SocketClient* client) {
@@ -58,14 +57,6 @@
     return ret;
 }
 
-bool hasUpdateDeviceStatsPermission(SocketClient* client) {
-    // If the caller is the system server, allow without any further checks.
-    // Otherwise, if the system server's binder thread pool is full, and all the threads are
-    // blocked on a thread that's waiting for us to complete, we deadlock. http://b/69389492
-    return isSystemServer(client) ||
-           checkPermission(String16(UPDATE_DEVICE_STATS), client->getPid(), client->getUid());
-}
-
 FwmarkServer::FwmarkServer(NetworkController* networkController, EventReporter* eventReporter,
                            TrafficController* trafficCtrl)
     : SocketListener(SOCKET_NAME, true),
@@ -133,17 +124,11 @@
     }
 
     if (command.cmdId == FwmarkCommand::SET_COUNTERSET) {
-        if (!hasUpdateDeviceStatsPermission(client)) {
-            return -EPERM;
-        }
-        return mTrafficCtrl->setCounterSet(command.trafficCtrlInfo, command.uid);
+        return mTrafficCtrl->setCounterSet(command.trafficCtrlInfo, command.uid, client->getUid());
     }
 
     if (command.cmdId == FwmarkCommand::DELETE_TAGDATA) {
-        if (!hasUpdateDeviceStatsPermission(client)) {
-            return -EPERM;
-        }
-        return mTrafficCtrl->deleteTagData(command.trafficCtrlInfo, command.uid);
+        return mTrafficCtrl->deleteTagData(command.trafficCtrlInfo, command.uid, client->getUid());
     }
 
     cmsghdr* const cmsgh = CMSG_FIRSTHDR(&message);
@@ -307,10 +292,8 @@
             if (static_cast<int>(command.uid) == -1) {
                 command.uid = client->getUid();
             }
-            if (command.uid != client->getUid() && !hasUpdateDeviceStatsPermission(client)) {
-                return -EPERM;
-            }
-            return mTrafficCtrl->tagSocket(*socketFd, command.trafficCtrlInfo, command.uid);
+            return mTrafficCtrl->tagSocket(*socketFd, command.trafficCtrlInfo, command.uid,
+                                           client->getUid());
         }
 
         case FwmarkCommand::UNTAG_SOCKET: {