commit a675b00423e5a370e72a7f10f6b632b4d15a1ff4
author Sreeram Ramachandran <sreeram@google.com> Sat Jul 05 11:00:55 2014 -0700
committer Sreeram Ramachandran <sreeram@google.com> Mon Jul 07 15:08:06 2014 +0000
tree 524c625470f144162a3c028f30dc87abd92968a4
parent 72999d6cfc1df23cd911b85730f1f532fb62bed4

Only add permissions in accept(); do not try to enforce anything.

Previously, we were enforcing that the user has access to the network over which
the socket is accepted. This has two problems:

1. We weren't handling the loopback interface ('lo') correctly. It's not part of
   any network, so the NetId in the fwmark is NETID_UNSET. In
   NetworkController::isUserPermittedOnNetwork(), we would fail to find a valid
   network, and so we would return false.

2. We have decided that in fact, we don't want to enforce this even for other
   interfaces, due to the SYN-ACK problem. See the comments and discussion at:
   http://go/android-multinetwork-routing

Bug: 16079376
Change-Id: I3f07f67d06dd7b48bfdfa5616ee22e098e31f8af

server/FwmarkServer.cpp

diff --git a/server/FwmarkServer.cpp b/server/FwmarkServer.cpp
index 60ee852..71d15f9 100644
--- a/server/FwmarkServer.cpp
+++ b/server/FwmarkServer.cpp
@@ -91,15 +91,13 @@
         return -errno;
     }
 
-    fwmark.permission = mNetworkController->getPermissionForUser(client->getUid());
+    Permission permission = mNetworkController->getPermissionForUser(client->getUid());
 
     switch (command.cmdId) {
         case FwmarkCommand::ON_ACCEPT: {
-            // Called after a socket accept(). The kernel would've marked the netId into the socket
-            // already, so we just need to check permissions here.
-            if (!mNetworkController->isUserPermittedOnNetwork(client->getUid(), fwmark.netId)) {
-                return -EPERM;
-            }
+            // Called after a socket accept(). The kernel would've marked the netId and necessary
+            // permissions bits, so we just add the rest of the user's permissions here.
+            permission = static_cast<Permission>(permission | fwmark.permission);
             break;
         }
 
@@ -120,7 +118,7 @@
                 fwmark.explicitlySelected = true;
                 // If the socket already has the protectedFromVpn bit set, don't reset it, because
                 // non-CONNECTIVITY_INTERNAL apps (e.g.: VpnService) may also protect sockets.
-                if (fwmark.permission & PERMISSION_CONNECTIVITY_INTERNAL) {
+                if (permission & PERMISSION_CONNECTIVITY_INTERNAL) {
                     fwmark.protectedFromVpn = true;
                 }
                 if (!mNetworkController->isValidNetwork(command.netId)) {
@@ -146,6 +144,8 @@
         }
     }
 
+    fwmark.permission = permission;
+
     if (setsockopt(*fd, SOL_SOCKET, SO_MARK, &fwmark.intValue, sizeof(fwmark.intValue)) == -1) {
         return -errno;
     }