Unify permission checking for Netd binder service
PERM_CONNECTIVITY_INTERNAL is deprecated.
Use PERM_NETWORK_STACK instead. Also some trivial cleanup.
Bug: 150749456
Test: atest
Merged-In: I93b6bca3e71b4ee82e393c5048f2783b945bb3c4
(cherry picked from commit 09c929fdff41acab6d97cf5d07a35dd9b76404cb)
Change-Id: I9b6e895c59d893ac68a4783a770b378345c3444f
diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp
index 9c91249..59e8b27 100644
--- a/server/NetdNativeService.cpp
+++ b/server/NetdNativeService.cpp
@@ -73,9 +73,8 @@
// blocked on a thread that's waiting for us to complete, we deadlock. http://b/69389492
//
// From a security perspective, there is currently no difference, because:
- // 1. The only permissions we check in netd's binder interface are CONNECTIVITY_INTERNAL
- // and NETWORK_STACK, which the system server always has (or MAINLINE_NETWORK_STACK, which
- // is equivalent to having both CONNECTIVITY_INTERNAL and NETWORK_STACK).
+ // 1. The system server has the NETWORK_STACK permission, which grants access to all the
+ // IPCs in this file.
// 2. AID_SYSTEM always has all permissions. See ActivityManager#checkComponentPermission.
if (uid == AID_SYSTEM) {
return binder::Status::ok();
@@ -114,9 +113,6 @@
} \
} while (0)
-#define ENFORCE_INTERNAL_PERMISSIONS() \
- ENFORCE_ANY_PERMISSION(PERM_CONNECTIVITY_INTERNAL, PERM_MAINLINE_NETWORK_STACK)
-
#define ENFORCE_NETWORK_STACK_PERMISSIONS() \
ENFORCE_ANY_PERMISSION(PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK)
@@ -248,7 +244,7 @@
}
binder::Status NetdNativeService::isAlive(bool *alive) {
- NETD_BIG_LOCK_RPC(PERM_CONNECTIVITY_INTERNAL, PERM_MAINLINE_NETWORK_STACK);
+ NETD_BIG_LOCK_RPC(PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
*alive = true;
@@ -257,16 +253,14 @@
binder::Status NetdNativeService::firewallReplaceUidChain(const std::string& chainName,
bool isWhitelist, const std::vector<int32_t>& uids, bool *ret) {
- NETD_LOCKING_RPC(gCtls->firewallCtrl.lock, PERM_CONNECTIVITY_INTERNAL,
- PERM_MAINLINE_NETWORK_STACK);
+ NETD_LOCKING_RPC(gCtls->firewallCtrl.lock, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
int err = gCtls->firewallCtrl.replaceUidChain(chainName, isWhitelist, uids);
*ret = (err == 0);
return binder::Status::ok();
}
binder::Status NetdNativeService::bandwidthEnableDataSaver(bool enable, bool *ret) {
- NETD_LOCKING_RPC(gCtls->bandwidthCtrl.lock, PERM_CONNECTIVITY_INTERNAL,
- PERM_MAINLINE_NETWORK_STACK);
+ NETD_LOCKING_RPC(gCtls->bandwidthCtrl.lock, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
int err = gCtls->bandwidthCtrl.enableDataSaver(enable);
*ret = (err == 0);
return binder::Status::ok();
@@ -333,7 +327,7 @@
}
binder::Status NetdNativeService::networkCreatePhysical(int32_t netId, int32_t permission) {
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
int ret = gCtls->netCtrl.createPhysicalNetwork(netId, convertPermission(permission));
return statusFromErrcode(ret);
}
@@ -352,13 +346,13 @@
}
binder::Status NetdNativeService::networkAddInterface(int32_t netId, const std::string& iface) {
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
int ret = gCtls->netCtrl.addInterfaceToNetwork(netId, iface.c_str());
return statusFromErrcode(ret);
}
binder::Status NetdNativeService::networkRemoveInterface(int32_t netId, const std::string& iface) {
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
int ret = gCtls->netCtrl.removeInterfaceFromNetwork(netId, iface.c_str());
return statusFromErrcode(ret);
}
@@ -366,7 +360,7 @@
binder::Status NetdNativeService::networkAddUidRanges(
int32_t netId, const std::vector<UidRangeParcel>& uidRangeArray) {
// NetworkController::addUsersToNetwork is thread-safe.
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
int ret = gCtls->netCtrl.addUsersToNetwork(netId, UidRanges(uidRangeArray));
return statusFromErrcode(ret);
}
@@ -374,7 +368,7 @@
binder::Status NetdNativeService::networkRemoveUidRanges(
int32_t netId, const std::vector<UidRangeParcel>& uidRangeArray) {
// NetworkController::removeUsersFromNetwork is thread-safe.
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
int ret = gCtls->netCtrl.removeUsersFromNetwork(netId, UidRanges(uidRangeArray));
return statusFromErrcode(ret);
}
@@ -386,7 +380,7 @@
// the CommandListener "network" command will need to hold this lock too, not just the ones that
// read/modify network internal state (that is sufficient for ::dump() because it doesn't
// look at routes, but it's not enough here).
- NETD_BIG_LOCK_RPC(PERM_CONNECTIVITY_INTERNAL, PERM_MAINLINE_NETWORK_STACK);
+ NETD_BIG_LOCK_RPC(PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
UidRanges uidRanges(uidRangeArray);
int err;
@@ -400,7 +394,7 @@
binder::Status NetdNativeService::socketDestroy(const std::vector<UidRangeParcel>& uids,
const std::vector<int32_t>& skipUids) {
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
SockDiag sd;
if (!sd.open()) {
@@ -489,7 +483,7 @@
binder::Status NetdNativeService::interfaceAddAddress(const std::string &ifName,
const std::string &addrString, int prefixLength) {
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
const int err = InterfaceController::addAddress(
ifName.c_str(), addrString.c_str(), prefixLength);
if (err != 0) {
@@ -501,7 +495,7 @@
binder::Status NetdNativeService::interfaceDelAddress(const std::string &ifName,
const std::string &addrString, int prefixLength) {
- ENFORCE_INTERNAL_PERMISSIONS();
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
const int err = InterfaceController::delAddress(
ifName.c_str(), addrString.c_str(), prefixLength);
if (err != 0) {
@@ -582,7 +576,6 @@
binder::Status NetdNativeService::ipSecSetEncapSocketOwner(const ParcelFileDescriptor& socket,
int newUid) {
ENFORCE_NETWORK_STACK_PERMISSIONS();
- gLog.log("ipSecSetEncapSocketOwner()");
uid_t callerUid = IPCThreadState::self()->getCallingUid();
return asBinderStatus(
@@ -596,8 +589,7 @@
int32_t inSpi,
int32_t* outSpi) {
// Necessary locking done in IpSecService and kernel
- ENFORCE_INTERNAL_PERMISSIONS();
- gLog.log("ipSecAllocateSpi()");
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
return asBinderStatus(gCtls->xfrmCtrl.ipSecAllocateSpi(
transformId,
sourceAddress,
@@ -615,8 +607,7 @@
const std::vector<uint8_t>& aeadKey, int32_t aeadIcvBits, int32_t encapType,
int32_t encapLocalPort, int32_t encapRemotePort, int32_t interfaceId) {
// Necessary locking done in IpSecService and kernel
- ENFORCE_INTERNAL_PERMISSIONS();
- gLog.log("ipSecAddSecurityAssociation()");
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
return asBinderStatus(gCtls->xfrmCtrl.ipSecAddSecurityAssociation(
transformId, mode, sourceAddress, destinationAddress, underlyingNetId, spi, markValue,
markMask, authAlgo, authKey, authTruncBits, cryptAlgo, cryptKey, cryptTruncBits,
@@ -629,8 +620,7 @@
const std::string& destinationAddress, int32_t spi, int32_t markValue, int32_t markMask,
int32_t interfaceId) {
// Necessary locking done in IpSecService and kernel
- ENFORCE_INTERNAL_PERMISSIONS();
- gLog.log("ipSecDeleteSecurityAssociation()");
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
return asBinderStatus(gCtls->xfrmCtrl.ipSecDeleteSecurityAssociation(
transformId, sourceAddress, destinationAddress, spi, markValue, markMask, interfaceId));
}
@@ -639,8 +629,7 @@
const ParcelFileDescriptor& socket, int32_t transformId, int32_t direction,
const std::string& sourceAddress, const std::string& destinationAddress, int32_t spi) {
// Necessary locking done in IpSecService and kernel
- ENFORCE_INTERNAL_PERMISSIONS();
- gLog.log("ipSecApplyTransportModeTransform()");
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
return asBinderStatus(gCtls->xfrmCtrl.ipSecApplyTransportModeTransform(
socket.get(), transformId, direction, sourceAddress, destinationAddress, spi));
}
@@ -648,8 +637,7 @@
binder::Status NetdNativeService::ipSecRemoveTransportModeTransform(
const ParcelFileDescriptor& socket) {
// Necessary locking done in IpSecService and kernel
- ENFORCE_INTERNAL_PERMISSIONS();
- gLog.log("ipSecRemoveTransportModeTransform()");
+ ENFORCE_NETWORK_STACK_PERMISSIONS();
return asBinderStatus(gCtls->xfrmCtrl.ipSecRemoveTransportModeTransform(socket.get()));
}
@@ -661,7 +649,6 @@
int32_t markMask, int32_t interfaceId) {
// Necessary locking done in IpSecService and kernel
ENFORCE_NETWORK_STACK_PERMISSIONS();
- gLog.log("ipSecAddSecurityPolicy()");
return asBinderStatus(gCtls->xfrmCtrl.ipSecAddSecurityPolicy(
transformId, selAddrFamily, direction, tmplSrcAddress, tmplDstAddress, spi, markValue,
markMask, interfaceId));
@@ -673,7 +660,6 @@
int32_t markValue, int32_t markMask, int32_t interfaceId) {
// Necessary locking done in IpSecService and kernel
ENFORCE_NETWORK_STACK_PERMISSIONS();
- gLog.log("ipSecAddSecurityPolicy()");
return asBinderStatus(gCtls->xfrmCtrl.ipSecUpdateSecurityPolicy(
transformId, selAddrFamily, direction, tmplSrcAddress, tmplDstAddress, spi, markValue,
markMask, interfaceId));
@@ -685,7 +671,6 @@
int32_t markMask, int32_t interfaceId) {
// Necessary locking done in IpSecService and kernel
ENFORCE_NETWORK_STACK_PERMISSIONS();
- gLog.log("ipSecAddSecurityPolicy()");
return asBinderStatus(gCtls->xfrmCtrl.ipSecDeleteSecurityPolicy(
transformId, selAddrFamily, direction, markValue, markMask, interfaceId));
}
@@ -838,6 +823,7 @@
}
namespace {
+
std::string addCurlyBrackets(const std::string& s) {
return "{" + s + "}";
}