diff --git a/include/Permission.h b/include/Permission.h index aa5ae811..19881df6 100644 --- a/include/Permission.h +++ b/include/Permission.h @@ -39,4 +39,14 @@ enum Permission { PERMISSION_SYSTEM = 0x3, // Includes PERMISSION_NETWORK. }; +inline const char *permissionToName(Permission permission) { + switch (permission) { + case PERMISSION_NONE: return "NONE"; + case PERMISSION_NETWORK: return "NETWORK"; + case PERMISSION_SYSTEM: return "SYSTEM"; + // No default statement. We want to see errors of the form: + // "enumeration value 'PERMISSION_SYSTEM' not handled in switch [-Werror,-Wswitch]". + } +} + #endif // NETD_INCLUDE_PERMISSION_H diff --git a/server/InterfaceController.cpp b/server/InterfaceController.cpp index eac4fbf2..1d0a8e0d 100644 --- a/server/InterfaceController.cpp +++ b/server/InterfaceController.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #define LOG_TAG "InterfaceController" #include @@ -29,6 +30,7 @@ #include "RouteController.h" using android::base::StringPrintf; +using android::base::ReadFileToString; using android::base::WriteStringToFile; namespace { @@ -39,15 +41,25 @@ const char ipv4_neigh_conf_dir[] = "/proc/sys/net/ipv4/neigh"; const char ipv6_neigh_conf_dir[] = "/proc/sys/net/ipv6/neigh"; +const char proc_net_path[] = "/proc/sys/net"; const char sys_net_path[] = "/sys/class/net"; const char wl_util_path[] = "/vendor/xbin/wlutil"; -bool isInterfaceName(const char *name) { - return strcmp(name, ".") && - strcmp(name, "..") && - strcmp(name, "default") && - strcmp(name, "all"); +inline bool isNormalPathComponent(const char *component) { + return (strcmp(component, ".") != 0) && + (strcmp(component, "..") != 0) && + (strchr(component, '/') == nullptr); +} + +inline bool isAddressFamilyPathComponent(const char *component) { + return strcmp(component, "ipv4") == 0 || strcmp(component, "ipv6") == 0; +} + +inline bool isInterfaceName(const char *name) { + return isNormalPathComponent(name) && + (strcmp(name, "default") != 0) && + (strcmp(name, "all") != 0); } int writeValueToPath( @@ -81,6 +93,21 @@ void setIPv6UseOutgoingInterfaceAddrsOnly(const char *value) { setOnAllInterfaces(ipv6_proc_path, "use_oif_addrs_only", value); } +std::string getParameterPathname( + const char *family, const char *which, const char *interface, const char *parameter) { + if (!isAddressFamilyPathComponent(family)) { + errno = EAFNOSUPPORT; + return ""; + } else if (!isNormalPathComponent(which) || + !isInterfaceName(interface) || + !isNormalPathComponent(parameter)) { + errno = EINVAL; + return ""; + } + + return StringPrintf("%s/%s/%s/%s/%s", proc_net_path, family, which, interface, parameter); +} + } // namespace void InterfaceController::initializeAll() { @@ -201,7 +228,6 @@ int InterfaceController::setMtu(const char *interface, const char *mtu) return writeValueToPath(sys_net_path, interface, "mtu", mtu); } - int InterfaceController::addAddress(const char *interface, const char *addrString, int prefixLength) { return ifc_add_address(interface, addrString, prefixLength); @@ -212,6 +238,26 @@ int InterfaceController::delAddress(const char *interface, return ifc_del_address(interface, addrString, prefixLength); } +int InterfaceController::getParameter( + const char *family, const char *which, const char *interface, const char *parameter, + std::string *value) { + const std::string path(getParameterPathname(family, which, interface, parameter)); + if (path.empty()) { + return -errno; + } + return ReadFileToString(path, value) ? 0 : -errno; +} + +int InterfaceController::setParameter( + const char *family, const char *which, const char *interface, const char *parameter, + const char *value) { + const std::string path(getParameterPathname(family, which, interface, parameter)); + if (path.empty()) { + return -errno; + } + return WriteStringToFile(value, path) ? 0 : -errno; +} + void InterfaceController::setBaseReachableTimeMs(unsigned int millis) { std::string value(StringPrintf("%u", millis)); setOnAllInterfaces(ipv4_neigh_conf_dir, "base_reachable_time_ms", value.c_str()); diff --git a/server/InterfaceController.h b/server/InterfaceController.h index 98c8b8f3..80ebe5c3 100644 --- a/server/InterfaceController.h +++ b/server/InterfaceController.h @@ -17,6 +17,8 @@ #ifndef _INTERFACE_CONTROLLER_H #define _INTERFACE_CONTROLLER_H +#include + class InterfaceController { public: static void initializeAll(); @@ -31,6 +33,15 @@ class InterfaceController { static int addAddress(const char *interface, const char *addrString, int prefixLength); static int delAddress(const char *interface, const char *addrString, int prefixLength); + // Read and write values in files of the form: + // /proc/sys/net//// + static int getParameter( + const char *family, const char *which, const char *interface, const char *parameter, + std::string *value); + static int setParameter( + const char *family, const char *which, const char *interface, const char *parameter, + const char *value); + private: static void setAcceptRA(const char* value); static void setAcceptRARouteTable(int tableOrOffset); diff --git a/server/NatController.cpp b/server/NatController.cpp index 8b4ee119..cda8f5fb 100644 --- a/server/NatController.cpp +++ b/server/NatController.cpp @@ -376,8 +376,7 @@ int NatController::setForwardRules(bool add, const char *intIface, const char *e goto err_return; } - // STOPSHIP: Make this an error. - if (runCmd(ARRAY_SIZE(cmd4), cmd4) && add && false /* STOPSHIP */) { + if (runCmd(ARRAY_SIZE(cmd4), cmd4) && add) { rc = -1; goto err_rpfilter; } diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp index 8dc4d93e..f8f300a6 100644 --- a/server/NetdNativeService.cpp +++ b/server/NetdNativeService.cpp @@ -29,6 +29,7 @@ #include "Controllers.h" #include "DumpWriter.h" +#include "InterfaceController.h" #include "NetdConstants.h" #include "NetdNativeService.h" #include "RouteController.h" @@ -235,5 +236,44 @@ binder::Status NetdNativeService::interfaceDelAddress(const std::string &ifName, return binder::Status::ok(); } +binder::Status NetdNativeService::setProcSysNet( + int32_t family, int32_t which, const std::string &ifname, const std::string ¶meter, + const std::string &value) { + ENFORCE_PERMISSION(CONNECTIVITY_INTERNAL); + + const char *familyStr; + switch (family) { + case INetd::IPV4: + familyStr = "ipv4"; + break; + case INetd::IPV6: + familyStr = "ipv6"; + break; + default: + return binder::Status::fromServiceSpecificError(EAFNOSUPPORT, String8("Bad family")); + } + + const char *whichStr; + switch (which) { + case INetd::CONF: + whichStr = "conf"; + break; + case INetd::NEIGH: + whichStr = "neigh"; + break; + default: + return binder::Status::fromServiceSpecificError(EINVAL, String8("Bad category")); + } + + const int err = InterfaceController::setParameter( + familyStr, whichStr, ifname.c_str(), parameter.c_str(), + value.c_str()); + if (err != 0) { + return binder::Status::fromServiceSpecificError(-err, + String8::format("ResolverController error: %s", strerror(-err))); + } + return binder::Status::ok(); +} + } // namespace net } // namespace android diff --git a/server/NetdNativeService.h b/server/NetdNativeService.h index c39e2a8e..7e1d75ec 100644 --- a/server/NetdNativeService.h +++ b/server/NetdNativeService.h @@ -55,6 +55,10 @@ class NetdNativeService : public BinderService, public BnNetd const std::string &addrString, int prefixLength) override; binder::Status interfaceDelAddress(const std::string &ifName, const std::string &addrString, int prefixLength) override; + + binder::Status setProcSysNet( + int32_t family, int32_t which, const std::string &ifname, const std::string ¶meter, + const std::string &value) override; }; } // namespace net diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp index 014d9267..c891391d 100644 --- a/server/NetworkController.cpp +++ b/server/NetworkController.cpp @@ -453,8 +453,6 @@ int NetworkController::setPermissionForNetworks(Permission permission, return -EINVAL; } - // TODO: ioctl(SIOCKILLADDR, ...) to kill socets on the network that don't have permission. - if (int ret = static_cast(network)->setPermission(permission)) { return ret; } @@ -538,7 +536,14 @@ void NetworkController::dump(DumpWriter& dw) { dw.println("Networks:"); dw.incIndent(); for (const auto& i : mNetworks) { - dw.println(i.second->toString().c_str()); + Network* network = i.second; + dw.println(network->toString().c_str()); + if (network->getType() == Network::PHYSICAL) { + dw.incIndent(); + Permission permission = reinterpret_cast(network)->getPermission(); + dw.println("Required permission: %s", permissionToName(permission)); + dw.decIndent(); + } android::net::gCtls->resolverCtrl.dump(dw, i.first); dw.blankline(); } diff --git a/server/PhysicalNetwork.cpp b/server/PhysicalNetwork.cpp index 495a93a0..ee0e7c7b 100644 --- a/server/PhysicalNetwork.cpp +++ b/server/PhysicalNetwork.cpp @@ -17,6 +17,7 @@ #include "PhysicalNetwork.h" #include "RouteController.h" +#include "SockDiag.h" #define LOG_TAG "Netd" #include "log/log.h" @@ -65,10 +66,33 @@ Permission PhysicalNetwork::getPermission() const { return mPermission; } +int PhysicalNetwork::destroySocketsLackingPermission(Permission permission) { + if (permission == PERMISSION_NONE) return 0; + + SockDiag sd; + if (!sd.open()) { + ALOGE("Error closing sockets for netId %d permission change", mNetId); + return -EBADFD; + } + if (int ret = sd.destroySocketsLackingPermission(mNetId, permission, + true /* excludeLoopback */)) { + ALOGE("Failed to close sockets changing netId %d to permission %d: %s", + mNetId, permission, strerror(-ret)); + return ret; + } + return 0; +} + int PhysicalNetwork::setPermission(Permission permission) { if (permission == mPermission) { return 0; } + if (mInterfaces.empty()) { + mPermission = permission; + return 0; + } + + destroySocketsLackingPermission(permission); for (const std::string& interface : mInterfaces) { if (int ret = RouteController::modifyPhysicalNetworkPermission(mNetId, interface.c_str(), mPermission, permission)) { @@ -87,6 +111,10 @@ int PhysicalNetwork::setPermission(Permission permission) { } } } + // Destroy sockets again in case any were opened after we called destroySocketsLackingPermission + // above and before we changed the permissions. These sockets won't be able to send any RST + // packets because they are now no longer routed, but at least the apps will get errors. + destroySocketsLackingPermission(permission); mPermission = permission; return 0; } diff --git a/server/PhysicalNetwork.h b/server/PhysicalNetwork.h index 2ef10dfe..cba3c6ee 100644 --- a/server/PhysicalNetwork.h +++ b/server/PhysicalNetwork.h @@ -46,6 +46,7 @@ class PhysicalNetwork : public Network { Type getType() const override; int addInterface(const std::string& interface) override WARN_UNUSED_RESULT; int removeInterface(const std::string& interface) override WARN_UNUSED_RESULT; + int destroySocketsLackingPermission(Permission permission); Delegate* const mDelegate; Permission mPermission; diff --git a/server/SockDiag.cpp b/server/SockDiag.cpp index e91c6d16..1517b94e 100644 --- a/server/SockDiag.cpp +++ b/server/SockDiag.cpp @@ -31,7 +31,9 @@ #include #include +#include "Fwmark.h" #include "NetdConstants.h" +#include "Permission.h" #include "SockDiag.h" #include @@ -40,6 +42,8 @@ #define SOCK_DESTROY 21 #endif +#define INET_DIAG_BC_MARK_COND 10 + namespace { int checkError(int fd) { @@ -186,9 +190,9 @@ int SockDiag::sendDumpRequest(uint8_t proto, uint8_t family, const char *addrstr attrs.nla.nla_len = sizeof(attrs) + addrlen; iovec iov[] = { - { nullptr, 0 }, - { &attrs, sizeof(attrs) }, - { addr, addrlen }, + { nullptr, 0 }, + { &attrs, sizeof(attrs) }, + { addr, addrlen }, }; uint32_t states = ~(1 << TCP_TIME_WAIT); @@ -316,18 +320,19 @@ int SockDiag::destroySockets(const char *addrstr) { return mSocketsDestroyed; } -int SockDiag::destroyLiveSockets(DumpCallback destroyFilter) { +int SockDiag::destroyLiveSockets(DumpCallback destroyFilter, const char *what, + iovec *iov, int iovcnt) { int proto = IPPROTO_TCP; for (const int family : {AF_INET, AF_INET6}) { const char *familyName = (family == AF_INET) ? "IPv4" : "IPv6"; uint32_t states = (1 << TCP_ESTABLISHED) | (1 << TCP_SYN_SENT) | (1 << TCP_SYN_RECV); - if (int ret = sendDumpRequest(proto, family, states)) { - ALOGE("Failed to dump %s sockets for UID: %s", familyName, strerror(-ret)); + if (int ret = sendDumpRequest(proto, family, states, iov, iovcnt)) { + ALOGE("Failed to dump %s sockets for %s: %s", familyName, what, strerror(-ret)); return ret; } if (int ret = readDiagMsg(proto, destroyFilter)) { - ALOGE("Failed to destroy %s sockets for UID: %s", familyName, strerror(-ret)); + ALOGE("Failed to destroy %s sockets for %s: %s", familyName, what, strerror(-ret)); return ret; } } @@ -377,7 +382,11 @@ int SockDiag::destroySockets(const UidRanges& uidRanges, const std::set& !(excludeLoopback && isLoopbackSocket(msg)); }; - if (int ret = destroyLiveSockets(shouldDestroy)) { + iovec iov[] = { + { nullptr, 0 }, + }; + + if (int ret = destroyLiveSockets(shouldDestroy, "UID", iov, ARRAY_SIZE(iov))) { return ret; } @@ -395,3 +404,95 @@ int SockDiag::destroySockets(const UidRanges& uidRanges, const std::set& return 0; } + +// Destroys all "live" (CONNECTED, SYN_SENT, SYN_RECV) TCP sockets on the specified netId where: +// 1. The opening app no longer has permission to use this network, or: +// 2. The opening app does have permission, but did not explicitly select this network. +// +// We destroy sockets without the explicit bit because we want to avoid the situation where a +// privileged app uses its privileges without knowing it is doing so. For example, a privileged app +// might have opened a socket on this network just because it was the default network at the +// time. If we don't kill these sockets, those apps could continue to use them without realizing +// that they are now sending and receiving traffic on a network that is now restricted. +int SockDiag::destroySocketsLackingPermission(unsigned netId, Permission permission, + bool excludeLoopback) { + struct markmatch { + inet_diag_bc_op op; + // TODO: switch to inet_diag_markcond + __u32 mark; + __u32 mask; + } __attribute__((packed)); + constexpr uint8_t matchlen = sizeof(markmatch); + + Fwmark netIdMark, netIdMask; + netIdMark.netId = netId; + netIdMask.netId = 0xffff; + + Fwmark controlMark; + controlMark.explicitlySelected = true; + controlMark.permission = permission; + + // A SOCK_DIAG bytecode program that accepts the sockets we intend to destroy. + struct bytecode { + markmatch netIdMatch; + markmatch controlMatch; + inet_diag_bc_op controlJump; + } __attribute__((packed)) bytecode; + + // The length of the INET_DIAG_BC_JMP instruction. + constexpr uint8_t jmplen = sizeof(inet_diag_bc_op); + // Jump exactly this far past the end of the program to reject. + constexpr uint8_t rejectoffset = sizeof(inet_diag_bc_op); + // Total length of the program. + constexpr uint8_t bytecodelen = sizeof(bytecode); + + bytecode = (struct bytecode) { + // If netId matches, continue, otherwise, reject (i.e., leave socket alone). + { { INET_DIAG_BC_MARK_COND, matchlen, bytecodelen + rejectoffset }, + netIdMark.intValue, netIdMask.intValue }, + + // If explicit and permission bits match, go to the JMP below which rejects the socket + // (i.e., we leave it alone). Otherwise, jump to the end of the program, which accepts the + // socket (so we destroy it). + { { INET_DIAG_BC_MARK_COND, matchlen, matchlen + jmplen }, + controlMark.intValue, controlMark.intValue }, + + // This JMP unconditionally rejects the packet by jumping to the reject target. It is + // necessary to keep the kernel bytecode verifier happy. If we don't have a JMP the bytecode + // is invalid because the target of every no jump must always be reachable by yes jumps. + // Without this JMP, the accept target is not reachable by yes jumps and the program will + // be rejected by the validator. + { INET_DIAG_BC_JMP, jmplen, jmplen + rejectoffset }, + + // We have reached the end of the program. Accept the socket, and destroy it below. + }; + + struct nlattr nla = { + .nla_type = INET_DIAG_REQ_BYTECODE, + .nla_len = sizeof(struct nlattr) + bytecodelen, + }; + + iovec iov[] = { + { nullptr, 0 }, + { &nla, sizeof(nla) }, + { &bytecode, bytecodelen }, + }; + + mSocketsDestroyed = 0; + Stopwatch s; + + auto shouldDestroy = [&] (uint8_t, const inet_diag_msg *msg) { + return msg != nullptr && !(excludeLoopback && isLoopbackSocket(msg)); + }; + + if (int ret = destroyLiveSockets(shouldDestroy, "permission change", iov, ARRAY_SIZE(iov))) { + return ret; + } + + if (mSocketsDestroyed > 0) { + ALOGI("Destroyed %d sockets for netId %d permission=%d in %.1f ms", + mSocketsDestroyed, netId, permission, s.timeTaken()); + } + + return 0; +} diff --git a/server/SockDiag.h b/server/SockDiag.h index 5dc77c1b..e561561b 100644 --- a/server/SockDiag.h +++ b/server/SockDiag.h @@ -24,6 +24,7 @@ #include #include +#include "Permission.h" #include "UidRanges.h" struct inet_diag_msg; @@ -58,6 +59,10 @@ class SockDiag { // Destroys all "live" (CONNECTED, SYN_SENT, SYN_RECV) TCP sockets for the given UID ranges. int destroySockets(const UidRanges& uidRanges, const std::set& skipUids, bool excludeLoopback); + // Destroys all "live" (CONNECTED, SYN_SENT, SYN_RECV) TCP sockets that no longer have + // the permissions required by the specified network. + int destroySocketsLackingPermission(unsigned netId, Permission permission, + bool excludeLoopback); private: friend class SockDiagTest; @@ -66,7 +71,7 @@ class SockDiag { int mSocketsDestroyed; int sendDumpRequest(uint8_t proto, uint8_t family, uint32_t states, iovec *iov, int iovcnt); int destroySockets(uint8_t proto, int family, const char *addrstr); - int destroyLiveSockets(DumpCallback destroy); + int destroyLiveSockets(DumpCallback destroy, const char *what, iovec *iov, int iovcnt); bool hasSocks() { return mSock != -1 && mWriteSock != -1; } void closeSocks() { close(mSock); close(mWriteSock); mSock = mWriteSock = -1; } static bool isLoopbackSocket(const inet_diag_msg *msg); diff --git a/server/SockDiagTest.cpp b/server/SockDiagTest.cpp index f9353f3e..70a199d9 100644 --- a/server/SockDiagTest.cpp +++ b/server/SockDiagTest.cpp @@ -25,6 +25,7 @@ #include +#include "Fwmark.h" #include "NetdConstants.h" #include "SockDiag.h" #include "UidRanges.h" @@ -278,6 +279,7 @@ enum MicroBenchmarkTestType { UID_EXCLUDE_LOOPBACK, UIDRANGE, UIDRANGE_EXCLUDE_LOOPBACK, + PERMISSION, }; const char *testTypeName(MicroBenchmarkTestType mode) { @@ -288,10 +290,30 @@ const char *testTypeName(MicroBenchmarkTestType mode) { TO_STRING_TYPE(UID_EXCLUDE_LOOPBACK); TO_STRING_TYPE(UIDRANGE); TO_STRING_TYPE(UIDRANGE_EXCLUDE_LOOPBACK); + TO_STRING_TYPE(PERMISSION); } #undef TO_STRING_TYPE } +static struct { + unsigned netId; + bool explicitlySelected; + Permission permission; +} permissionTestcases[] = { + { 42, false, PERMISSION_NONE, }, + { 42, false, PERMISSION_NETWORK, }, + { 42, false, PERMISSION_SYSTEM, }, + { 42, true, PERMISSION_NONE, }, + { 42, true, PERMISSION_NETWORK, }, + { 42, true, PERMISSION_SYSTEM, }, + { 43, false, PERMISSION_NONE, }, + { 43, false, PERMISSION_NETWORK, }, + { 43, false, PERMISSION_SYSTEM, }, + { 43, true, PERMISSION_NONE, }, + { 43, true, PERMISSION_NETWORK, }, + { 43, true, PERMISSION_SYSTEM, }, +}; + class SockDiagMicroBenchmarkTest : public ::testing::TestWithParam { public: @@ -305,10 +327,15 @@ class SockDiagMicroBenchmarkTest : public ::testing::TestWithParamsetProcSysNet( + td.family, td.which, td.ifname, td.parameter, + td.value); + + if (td.expectedReturnCode == 0) { + SCOPED_TRACE(String8::format("test case %d should have passed", i)); + EXPECT_EQ(0, status.exceptionCode()); + EXPECT_EQ(0, status.serviceSpecificErrorCode()); + } else { + SCOPED_TRACE(String8::format("test case %d should have failed", i)); + EXPECT_EQ(binder::Status::EX_SERVICE_SPECIFIC, status.exceptionCode()); + EXPECT_EQ(td.expectedReturnCode, status.serviceSpecificErrorCode()); + } + } +}