Fix permissions handling.
+ Rename the permissions as per: http://go/android-multinetwork-routing
+ Make the SYSTEM permission explicitly include NETWORK.
+ Grant the SYSTEM permission to system UIDs by default, but allow the framework
to override them if necessary.
+ Move the "string to permission" parsing to CommandListener.cpp, thus allowing
us to get rid of Permission.cpp.
+ There's no need to support multiple permissions string arguments, so tighten
that up.
Change-Id: I73d51b5e2f44a97e6d5ab5943ff198cebfbcc0c4
diff --git a/server/Android.mk b/server/Android.mk
index 541644e..28defba 100644
--- a/server/Android.mk
+++ b/server/Android.mk
@@ -56,7 +56,6 @@
NetlinkManager.cpp \
Network.cpp \
NetworkController.cpp \
- Permission.cpp \
PhysicalNetwork.cpp \
PppController.cpp \
ResolverController.cpp \
diff --git a/server/ClatdController.cpp b/server/ClatdController.cpp
index 4114de1..ca6908c 100644
--- a/server/ClatdController.cpp
+++ b/server/ClatdController.cpp
@@ -61,7 +61,7 @@
char netIdString[UINT32_STRLEN];
snprintf(netIdString, sizeof(netIdString), "%u", netId);
- Fwmark fwmark = { netId, true, true, PERMISSION_CONNECTIVITY_INTERNAL };
+ Fwmark fwmark = {netId, true, true, PERMISSION_SYSTEM};
char fwmarkString[UINT32_HEX_STRLEN];
snprintf(fwmarkString, sizeof(fwmarkString), "0x%x", fwmark.intValue);
diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp
index 9c91910..df2df46 100644
--- a/server/CommandListener.cpp
+++ b/server/CommandListener.cpp
@@ -52,18 +52,14 @@
namespace {
-// Parses string permissions in argv[*nextArg], argv[*nextArg + 1], etc. and converts them into a
-// Permission enum. On return, nextArg will point to one past the last valid permissions string.
-Permission parseMultiplePermissions(int argc, char** argv, int* nextArg) {
- Permission permission = PERMISSION_NONE;
- for (; *nextArg < argc; ++*nextArg) {
- Permission p = permissionFromString(argv[*nextArg]);
- if (p == PERMISSION_NONE) {
- break;
- }
- permission = static_cast<Permission>(permission | p);
+Permission stringToPermission(const char* arg) {
+ if (!strcmp(arg, "android.permission.CHANGE_NETWORK_STATE")) {
+ return PERMISSION_NETWORK;
}
- return permission;
+ if (!strcmp(arg, "android.permission.CONNECTIVITY_INTERNAL")) {
+ return PERMISSION_SYSTEM;
+ }
+ return PERMISSION_NONE;
}
} // namespace
@@ -1641,8 +1637,8 @@
return success(client);
}
- // 0 1 2 3
- // network create <netId> [<permission> ...]
+ // 0 1 2 3
+ // network create <netId> [permission]
//
// 0 1 2 3
// network create <netId> vpn
@@ -1656,11 +1652,15 @@
if (int ret = sNetCtrl->createVpn(netId)) {
return operationError(client, "createVpn() failed", ret);
}
+ } else if (argc > 4) {
+ return syntaxError(client, "Unknown trailing argument(s)");
} else {
- int nextArg = 3;
- Permission permission = parseMultiplePermissions(argc, argv, &nextArg);
- if (nextArg != argc) {
- return syntaxError(client, "Unknown trailing argument(s)");
+ Permission permission = PERMISSION_NONE;
+ if (argc == 4) {
+ permission = stringToPermission(argv[3]);
+ if (permission == PERMISSION_NONE) {
+ return syntaxError(client, "Unknown permission");
+ }
}
if (int ret = sNetCtrl->createNetwork(netId, permission)) {
return operationError(client, "createNetwork() failed", ret);
@@ -1706,11 +1706,11 @@
return success(client);
}
- // 0 1 2 3 4
- // network permission user set [<permission> ...] <uid> ...
- // network permission user clear <uid> ...
- // network permission network set [<permission> ...] <netId> ...
- // network permission network clear <netId> ...
+ // 0 1 2 3 4 5
+ // network permission user set <permission> <uid> ...
+ // network permission user clear <uid> ...
+ // network permission network set <permission> <netId> ...
+ // network permission network clear <netId> ...
if (!strcmp(argv[1], "permission")) {
if (argc < 5) {
return syntaxError(client, "Missing argument");
@@ -1718,10 +1718,17 @@
int nextArg = 4;
Permission permission = PERMISSION_NONE;
if (!strcmp(argv[3], "set")) {
- permission = parseMultiplePermissions(argc, argv, &nextArg);
+ permission = stringToPermission(argv[4]);
+ if (permission == PERMISSION_NONE) {
+ return syntaxError(client, "Unknown permission");
+ }
+ nextArg = 5;
} else if (strcmp(argv[3], "clear")) {
return syntaxError(client, "Unknown argument");
}
+ if (nextArg == argc) {
+ return syntaxError(client, "Missing id");
+ }
std::vector<unsigned> ids;
for (; nextArg < argc; ++nextArg) {
char* endPtr;
@@ -1731,9 +1738,6 @@
}
ids.push_back(id);
}
- if (ids.empty()) {
- return syntaxError(client, "Missing id");
- }
if (!strcmp(argv[2], "user")) {
sNetCtrl->setPermissionForUsers(permission, ids);
} else if (!strcmp(argv[2], "network")) {
diff --git a/server/FwmarkServer.cpp b/server/FwmarkServer.cpp
index 71d15f9..e2d2079 100644
--- a/server/FwmarkServer.cpp
+++ b/server/FwmarkServer.cpp
@@ -117,8 +117,8 @@
} else {
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 (permission & PERMISSION_CONNECTIVITY_INTERNAL) {
+ // non-system apps (e.g.: VpnService) may also protect sockets.
+ if ((permission & PERMISSION_SYSTEM) == PERMISSION_SYSTEM) {
fwmark.protectedFromVpn = true;
}
if (!mNetworkController->isValidNetwork(command.netId)) {
diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp
index 0e341e5..5638678 100644
--- a/server/NetworkController.cpp
+++ b/server/NetworkController.cpp
@@ -36,6 +36,7 @@
#include "RouteController.h"
#include "VirtualNetwork.h"
+#include "cutils/misc.h"
#define LOG_TAG "Netd"
#include "log/log.h"
#include "resolv_netid.h"
@@ -247,31 +248,29 @@
Permission NetworkController::getPermissionForUser(uid_t uid) const {
android::RWLock::AutoRLock lock(mRWLock);
- auto iter = mUsers.find(uid);
- return iter == mUsers.end() ? PERMISSION_NONE : iter->second;
+ return getPermissionForUserLocked(uid);
}
void NetworkController::setPermissionForUsers(Permission permission,
const std::vector<uid_t>& uids) {
android::RWLock::AutoWLock lock(mRWLock);
for (uid_t uid : uids) {
- if (permission == PERMISSION_NONE) {
- mUsers.erase(uid);
- } else {
- mUsers[uid] = permission;
- }
+ mUsers[uid] = permission;
}
}
// TODO: Handle VPNs.
bool NetworkController::isUserPermittedOnNetwork(uid_t uid, unsigned netId) const {
+ if (uid == INVALID_UID || netId == NETID_UNSET) {
+ return false;
+ }
+
android::RWLock::AutoRLock lock(mRWLock);
- auto userIter = mUsers.find(uid);
- Permission userPermission = (userIter == mUsers.end() ? PERMISSION_NONE : userIter->second);
Network* network = getNetworkLocked(netId);
if (!network || network->getType() != Network::PHYSICAL) {
return false;
}
+ Permission userPermission = getPermissionForUserLocked(uid);
Permission networkPermission = static_cast<PhysicalNetwork*>(network)->getPermission();
return (userPermission & networkPermission) == networkPermission;
}
@@ -348,6 +347,14 @@
return iter == mNetworks.end() ? NULL : iter->second;
}
+Permission NetworkController::getPermissionForUserLocked(uid_t uid) const {
+ auto iter = mUsers.find(uid);
+ if (iter != mUsers.end()) {
+ return iter->second;
+ }
+ return uid < FIRST_APPLICATION_UID ? PERMISSION_SYSTEM : PERMISSION_NONE;
+}
+
int NetworkController::modifyRoute(unsigned netId, const char* interface, const char* destination,
const char* nexthop, bool add, bool legacy, uid_t uid) {
unsigned existingNetId = getNetworkId(interface);
@@ -358,7 +365,7 @@
RouteController::TableType tableType;
if (legacy) {
- if (getPermissionForUser(uid) & PERMISSION_CONNECTIVITY_INTERNAL) {
+ if ((getPermissionForUser(uid) & PERMISSION_SYSTEM) == PERMISSION_SYSTEM) {
tableType = RouteController::PRIVILEGED_LEGACY;
} else {
tableType = RouteController::LEGACY;
diff --git a/server/NetworkController.h b/server/NetworkController.h
index 735704c..0418f96 100644
--- a/server/NetworkController.h
+++ b/server/NetworkController.h
@@ -83,6 +83,7 @@
private:
Network* getNetworkLocked(unsigned netId) const;
+ Permission getPermissionForUserLocked(uid_t uid) const;
int modifyRoute(unsigned netId, const char* interface, const char* destination,
const char* nexthop, bool add, bool legacy, uid_t uid) WARN_UNUSED_RESULT;
diff --git a/server/Permission.cpp b/server/Permission.cpp
deleted file mode 100644
index c5bf677..0000000
--- a/server/Permission.cpp
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Copyright (C) 2014 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include "Permission.h"
-
-#include <string.h>
-
-Permission permissionFromString(const char* permission) {
- if (permission) {
- if (!strcmp(permission, "android.permission.CHANGE_NETWORK_STATE")) {
- return PERMISSION_CHANGE_NETWORK_STATE;
- }
- if (!strcmp(permission, "android.permission.CONNECTIVITY_INTERNAL")) {
- return PERMISSION_CONNECTIVITY_INTERNAL;
- }
- }
- return PERMISSION_NONE;
-}
diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index 799a531..d2d1841 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -459,8 +459,8 @@
fwmark.netId = netId;
mask.netId = FWMARK_NET_ID_MASK;
- fwmark.permission = PERMISSION_CONNECTIVITY_INTERNAL;
- mask.permission = PERMISSION_CONNECTIVITY_INTERNAL;
+ fwmark.permission = PERMISSION_SYSTEM;
+ mask.permission = PERMISSION_SYSTEM;
if (int ret = modifyIpRule(action, RULE_PRIORITY_SECURE_VPN, table, fwmark.intValue,
mask.intValue, NULL, INVALID_UID, INVALID_UID)) {
@@ -613,9 +613,9 @@
return ret;
}
- // Add a rule to allow legacy routes from privileged apps to override VPNs.
- fwmark.permission = PERMISSION_CONNECTIVITY_INTERNAL;
- mask.permission = PERMISSION_CONNECTIVITY_INTERNAL;
+ // Add a rule to allow legacy routes from system apps to override VPNs.
+ fwmark.permission = PERMISSION_SYSTEM;
+ mask.permission = PERMISSION_SYSTEM;
if (int ret = modifyIpRule(RTM_NEWRULE, RULE_PRIORITY_VPN_OVERRIDES,
RouteController::ROUTE_TABLE_PRIVILEGED_LEGACY, fwmark.intValue,