[ipsec-doze] Add fchown capabilities, and fw rules
Add some firewall rules to allow doze mode packets to be sent/received
on ESP & no-socket packets. No-socket packets are no security risk
because they are either forwarded, going to be forwarded, or will be
dropped at routing tables (unless they are ESP).
Bug: 62994731
Test: New tests added, run
Change-Id: I2d8704498b564403d94123e4938091dee8fb98c1
diff --git a/libnetdutils/Syscalls.cpp b/libnetdutils/Syscalls.cpp
index 5354341..9a05e3b 100644
--- a/libnetdutils/Syscalls.cpp
+++ b/libnetdutils/Syscalls.cpp
@@ -65,6 +65,15 @@
return status::ok;
}
+ Status getsockopt(Fd sock, int level, int optname, void* optval,
+ socklen_t* optlen) const override {
+ auto rv = ::getsockopt(sock.get(), level, optname, optval, optlen);
+ if (rv == -1) {
+ return statusFromErrno(errno, "getsockopt() failed");
+ }
+ return status::ok;
+ }
+
Status setsockopt(Fd sock, int level, int optname, const void* optval,
socklen_t optlen) const override {
auto rv = ::setsockopt(sock.get(), level, optname, optval, optlen);
diff --git a/libnetdutils/SyscallsTest.cpp b/libnetdutils/SyscallsTest.cpp
index a754d1c..da93b76 100644
--- a/libnetdutils/SyscallsTest.cpp
+++ b/libnetdutils/SyscallsTest.cpp
@@ -108,6 +108,26 @@
EXPECT_EQ(kError, sys.setsockopt(kFd, kLevel, kOptname, expected));
}
+TEST_F(SyscallsTest, getsockopt) {
+ constexpr Fd kFd(40);
+ constexpr int kLevel = 50;
+ constexpr int kOptname = 70;
+ sockaddr_nl expected = {};
+ socklen_t optLen = 0;
+ auto& sys = sSyscalls.get();
+
+ // Success
+ EXPECT_CALL(mSyscalls, getsockopt(kFd, kLevel, kOptname, &expected, &optLen))
+ .WillOnce(Return(status::ok));
+ EXPECT_EQ(status::ok, sys.getsockopt(kFd, kLevel, kOptname, &expected, &optLen));
+
+ // Failure
+ const Status kError = statusFromErrno(EINVAL, "test");
+ EXPECT_CALL(mSyscalls, getsockopt(kFd, kLevel, kOptname, &expected, &optLen))
+ .WillOnce(Return(kError));
+ EXPECT_EQ(kError, sys.getsockopt(kFd, kLevel, kOptname, &expected, &optLen));
+}
+
TEST_F(SyscallsTest, bind) {
constexpr Fd kFd(40);
sockaddr_nl expected = {};
diff --git a/libnetdutils/include/netdutils/MockSyscalls.h b/libnetdutils/include/netdutils/MockSyscalls.h
index 149ba59..06ca859 100644
--- a/libnetdutils/include/netdutils/MockSyscalls.h
+++ b/libnetdutils/include/netdutils/MockSyscalls.h
@@ -37,6 +37,8 @@
StatusOr<UniqueFd>(const std::string& pathname, int flags, mode_t mode));
MOCK_CONST_METHOD3(socket, StatusOr<UniqueFd>(int domain, int type, int protocol));
MOCK_CONST_METHOD3(getsockname, Status(Fd sock, sockaddr* addr, socklen_t* addrlen));
+ MOCK_CONST_METHOD5(getsockopt, Status(Fd sock, int level, int optname, void* optval,
+ socklen_t *optlen));
MOCK_CONST_METHOD5(setsockopt, Status(Fd sock, int level, int optname, const void* optval,
socklen_t optlen));
diff --git a/libnetdutils/include/netdutils/Syscalls.h b/libnetdutils/include/netdutils/Syscalls.h
index 0e336b6..4c9a004 100644
--- a/libnetdutils/include/netdutils/Syscalls.h
+++ b/libnetdutils/include/netdutils/Syscalls.h
@@ -47,6 +47,9 @@
virtual Status getsockname(Fd sock, sockaddr* addr, socklen_t* addrlen) const = 0;
+ virtual Status getsockopt(Fd sock, int level, int optname, void *optval,
+ socklen_t *optlen) const = 0;
+
virtual Status setsockopt(Fd sock, int level, int optname, const void* optval,
socklen_t optlen) const = 0;
@@ -115,6 +118,11 @@
}
template <typename SockoptT>
+ Status getsockopt(Fd sock, int level, int optname, void* optval, socklen_t* optlen) const {
+ return getsockopt(sock, level, optname, optval, optlen);
+ }
+
+ template <typename SockoptT>
Status setsockopt(Fd sock, int level, int optname, const SockoptT& opt) const {
return setsockopt(sock, level, optname, &opt, sizeof(opt));
}
diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index f5da069..dc4fa36 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -16,6 +16,7 @@
#include <set>
+#include <cstdint>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
@@ -266,6 +267,14 @@
// Always whitelist system UIDs.
StringAppendF(&commands,
"-A %s -m owner --uid-owner %d-%d -j RETURN\n", name, 0, MAX_SYSTEM_UID);
+
+ // This rule inverts the match for all UIDs; ie, if there is no UID match here,
+ // there is no socket to be found
+ StringAppendF(&commands,
+ "-A %s -m owner ! --uid-owner %d-%u -j RETURN\n", name, 0, UINT32_MAX-1);
+
+ // Always whitelist traffic with protocol ESP, or no known socket - required for IPSec
+ StringAppendF(&commands, "-A %s -p esp -j RETURN\n", name);
}
// Always allow networking on loopback.
diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index 74dbbad..c1f43eb 100644
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -54,6 +54,8 @@
"*filter",
":fw_whitelist -",
"-A fw_whitelist -m owner --uid-owner 0-9999 -j RETURN",
+ "-A fw_whitelist -m owner ! --uid-owner 0-4294967294 -j RETURN",
+ "-A fw_whitelist -p esp -j RETURN",
"-A fw_whitelist -i lo -j RETURN",
"-A fw_whitelist -o lo -j RETURN",
"-A fw_whitelist -p tcp --tcp-flags RST RST -j RETURN",
@@ -64,6 +66,8 @@
"*filter",
":fw_whitelist -",
"-A fw_whitelist -m owner --uid-owner 0-9999 -j RETURN",
+ "-A fw_whitelist -m owner ! --uid-owner 0-4294967294 -j RETURN",
+ "-A fw_whitelist -p esp -j RETURN",
"-A fw_whitelist -i lo -j RETURN",
"-A fw_whitelist -o lo -j RETURN",
"-A fw_whitelist -p tcp --tcp-flags RST RST -j RETURN",
@@ -163,6 +167,8 @@
"-A FW_whitechain -m owner --uid-owner 210153 -j RETURN\n"
"-A FW_whitechain -m owner --uid-owner 210024 -j RETURN\n"
"-A FW_whitechain -m owner --uid-owner 0-9999 -j RETURN\n"
+ "-A FW_whitechain -m owner ! --uid-owner 0-4294967294 -j RETURN\n"
+ "-A FW_whitechain -p esp -j RETURN\n"
"-A FW_whitechain -i lo -j RETURN\n"
"-A FW_whitechain -o lo -j RETURN\n"
"-A FW_whitechain -p tcp --tcp-flags RST RST -j RETURN\n"
diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp
index 0afecde..0cb740f 100644
--- a/server/NetdNativeService.cpp
+++ b/server/NetdNativeService.cpp
@@ -393,6 +393,16 @@
: binder::Status::fromExceptionCode(binder::Status::EX_ILLEGAL_ARGUMENT);
}
+binder::Status NetdNativeService::ipSecSetEncapSocketOwner(const android::base::unique_fd& socket,
+ int newUid) {
+ ENFORCE_PERMISSION(NETWORK_STACK)
+ ALOGD("ipSecSetEncapSocketOwner()");
+
+ uid_t callerUid = IPCThreadState::self()->getCallingUid();
+ return asBinderStatus(gCtls->xfrmCtrl.ipSecSetEncapSocketOwner(socket, newUid, callerUid));
+}
+
+
binder::Status NetdNativeService::ipSecAllocateSpi(
int32_t transformId,
int32_t direction,
diff --git a/server/NetdNativeService.h b/server/NetdNativeService.h
index 0d7f721..6842f1a 100644
--- a/server/NetdNativeService.h
+++ b/server/NetdNativeService.h
@@ -76,6 +76,8 @@
binder::Status getMetricsReportingLevel(int *reportingLevel) override;
binder::Status setMetricsReportingLevel(const int reportingLevel) override;
+ binder::Status ipSecSetEncapSocketOwner(const android::base::unique_fd& socket, int newUid);
+
binder::Status ipSecAllocateSpi(
int32_t transformId,
int32_t direction,
diff --git a/server/XfrmController.cpp b/server/XfrmController.cpp
index e70ed49..7f0c761 100644
--- a/server/XfrmController.cpp
+++ b/server/XfrmController.cpp
@@ -340,6 +340,39 @@
//
XfrmController::XfrmController(void) {}
+netdutils::Status XfrmController::ipSecSetEncapSocketOwner(const android::base::unique_fd& socket,
+ int newUid, uid_t callerUid) {
+ ALOGD("XfrmController:%s, line=%d", __FUNCTION__, __LINE__);
+
+ const int fd = socket.get();
+ struct stat info;
+ if (fstat(fd, &info)) {
+ return netdutils::statusFromErrno(errno, "Failed to stat socket file descriptor");
+ }
+ if (info.st_uid != callerUid) {
+ return netdutils::statusFromErrno(EPERM, "fchown disabled for non-owner calls");
+ }
+ if (S_ISSOCK(info.st_mode) == 0){
+ return netdutils::statusFromErrno(EINVAL, "File descriptor was not a socket");
+ }
+
+ int optval;
+ socklen_t optlen;
+ netdutils::Status status = getSyscallInstance().getsockopt(Fd(socket), IPPROTO_UDP, UDP_ENCAP,
+ &optval, &optlen);
+ if (status != netdutils::status::ok) {
+ return status;
+ }
+ if (optval != UDP_ENCAP_ESPINUDP && optval != UDP_ENCAP_ESPINUDP_NON_IKE){
+ return netdutils::statusFromErrno(EINVAL, "Socket did not have UDP-encap sockopt set");
+ }
+ if (fchown(fd, newUid, -1)) {
+ return netdutils::statusFromErrno(errno, "Failed to fchown socket file descriptor");
+ }
+
+ return netdutils::status::ok;
+}
+
netdutils::Status XfrmController::ipSecAllocateSpi(int32_t transformId, int32_t direction,
const std::string& localAddress,
const std::string& remoteAddress, int32_t inSpi,
diff --git a/server/XfrmController.h b/server/XfrmController.h
index a881b64..56c1847 100644
--- a/server/XfrmController.h
+++ b/server/XfrmController.h
@@ -122,6 +122,9 @@
public:
XfrmController();
+ netdutils::Status ipSecSetEncapSocketOwner(const android::base::unique_fd& socket, int newUid,
+ uid_t callerUid);
+
netdutils::Status ipSecAllocateSpi(int32_t transformId, int32_t direction,
const std::string& localAddress,
const std::string& remoteAddress, int32_t inSpi,
diff --git a/server/XfrmControllerTest.cpp b/server/XfrmControllerTest.cpp
index 1a14a72..26dcb47 100644
--- a/server/XfrmControllerTest.cpp
+++ b/server/XfrmControllerTest.cpp
@@ -142,6 +142,73 @@
return (info.param == AF_INET) ? "AF_INET" : "AF_INET6";
}
+/* Generate function to set value refered to by 3rd argument.
+ *
+ * This allows us to mock functions that pass in a pointer, expecting the result to be put into
+ * that location.
+ */
+ACTION_P(SetArg3IntValue, value) { *static_cast<int*>(arg3) = value; }
+
+TEST_F(XfrmControllerTest, TestFchown) {
+ XfrmController ctrl;
+ unique_fd sockFd(socket(AF_INET, SOCK_DGRAM, 0));
+
+ EXPECT_CALL(mockSyscalls, getsockopt(Fd(sockFd), IPPROTO_UDP, UDP_ENCAP, _, _))
+ .WillOnce(DoAll(SetArg3IntValue(UDP_ENCAP_ESPINUDP), Return(netdutils::status::ok)));
+
+ netdutils::Status res = ctrl.ipSecSetEncapSocketOwner(sockFd, 1001, getuid());
+ EXPECT_EQ(netdutils::status::ok, res);
+
+ struct stat info;
+ EXPECT_EQ(0, fstat(sockFd.get(), &info));
+ EXPECT_EQ(1001, (int)info.st_uid);
+}
+
+TEST_F(XfrmControllerTest, TestFchownInvalidFd) {
+ XfrmController ctrl;
+
+ netdutils::Status res = ctrl.ipSecSetEncapSocketOwner(unique_fd(), 1001, getuid());
+ EXPECT_EQ(netdutils::statusFromErrno(errno, "Failed to stat socket file descriptor"), res);
+}
+
+TEST_F(XfrmControllerTest, TestFchownIncorrectCallerUid) {
+ XfrmController ctrl;
+ unique_fd sockFd(socket(AF_INET, SOCK_DGRAM, 0));
+
+ netdutils::Status res = ctrl.ipSecSetEncapSocketOwner(sockFd, 1001, 1001);
+ EXPECT_EQ(netdutils::statusFromErrno(EPERM, "fchown disabled for non-owner calls"), res);
+}
+
+TEST_F(XfrmControllerTest, TestFchownNonSocketFd) {
+ XfrmController ctrl;
+ unique_fd fd(open("/dev/null", 0));
+
+ netdutils::Status res = ctrl.ipSecSetEncapSocketOwner(fd, 1001, getuid());
+ EXPECT_EQ(netdutils::statusFromErrno(EINVAL, "File descriptor was not a socket"), res);
+}
+
+TEST_F(XfrmControllerTest, TestFchownNonUdp) {
+ XfrmController ctrl;
+ unique_fd sockFd(socket(AF_INET, SOCK_STREAM, 0));
+
+ EXPECT_CALL(mockSyscalls, getsockopt(Fd(sockFd), IPPROTO_UDP, UDP_ENCAP, _, _))
+ .WillOnce(DoAll(SetArg3IntValue(0), Return(netdutils::status::ok)));
+
+ netdutils::Status res = ctrl.ipSecSetEncapSocketOwner(sockFd, 1001, getuid());
+ EXPECT_EQ(netdutils::statusFromErrno(EINVAL, "Socket was not a UDP socket"), res);
+}
+
+TEST_F(XfrmControllerTest, TestFchownNonUdpEncap) {
+ XfrmController ctrl;
+ unique_fd sockFd(socket(AF_INET, SOCK_DGRAM, 0));
+
+ EXPECT_CALL(mockSyscalls, getsockopt(Fd(sockFd), IPPROTO_UDP, UDP_ENCAP, _, _))
+ .WillOnce(DoAll(SetArg3IntValue(0), Return(netdutils::status::ok)));
+
+ netdutils::Status res = ctrl.ipSecSetEncapSocketOwner(sockFd, 1001, getuid());
+ EXPECT_EQ(netdutils::statusFromErrno(EINVAL, "Socket did not have UDP-encap sockopt set"), res);
+}
+
// The TEST_P cases below will run with each of the following value parameters.
INSTANTIATE_TEST_CASE_P(ByFamily, XfrmControllerParameterizedTest, Values(AF_INET, AF_INET6),
FamilyName);
diff --git a/server/binder/android/net/INetd.aidl b/server/binder/android/net/INetd.aidl
index e6f23dc..45274cb 100644
--- a/server/binder/android/net/INetd.aidl
+++ b/server/binder/android/net/INetd.aidl
@@ -226,6 +226,15 @@
void setMetricsReportingLevel(int level);
/**
+ * Sets owner of socket FileDescriptor to the new UID, checking to ensure that the caller's
+ * uid is that of the old owner's, and that this is a UDP-encap socket
+ *
+ * @param FileDescriptor socket Socket file descriptor
+ * @param int newUid UID of the new socket fd owner
+ */
+ void ipSecSetEncapSocketOwner(in FileDescriptor socket, int newUid);
+
+ /**
* Reserve an SPI from the kernel
*
* @param transformId a unique identifier for allocated resources
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index b3a160d..f5a8d43 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -66,6 +66,9 @@
static const char* IP_RULE_V4 = "-4";
static const char* IP_RULE_V6 = "-6";
+static const std::string NO_SOCKET_ALLOW_RULE("! owner UID match 0-4294967294");
+static const std::string ESP_ALLOW_RULE("esp");
+
class BinderTest : public ::testing::Test {
public:
@@ -159,6 +162,28 @@
return listIptablesRule(binary, chainName).size();
}
+static bool iptablesRuleExists(const char *binary,
+ const char *chainName,
+ const std::string expectedRule) {
+ std::vector<std::string> rules = listIptablesRule(binary, chainName);
+ for(std::string &rule: rules) {
+ if(rule.find(expectedRule) != std::string::npos) {
+ return true;
+ }
+ }
+ return false;
+}
+
+static bool iptablesNoSocketAllowRuleExists(const char *chainName){
+ return iptablesRuleExists(IPTABLES_PATH, chainName, NO_SOCKET_ALLOW_RULE) &&
+ iptablesRuleExists(IP6TABLES_PATH, chainName, NO_SOCKET_ALLOW_RULE);
+}
+
+static bool iptablesEspAllowRuleExists(const char *chainName){
+ return iptablesRuleExists(IPTABLES_PATH, chainName, ESP_ALLOW_RULE) &&
+ iptablesRuleExists(IP6TABLES_PATH, chainName, ESP_ALLOW_RULE);
+}
+
TEST_F(BinderTest, TestFirewallReplaceUidChain) {
std::string chainName = StringPrintf("netd_binder_test_%u", arc4random_uniform(10000));
const int kNumUids = 500;
@@ -174,8 +199,10 @@
mNetd->firewallReplaceUidChain(String16(chainName.c_str()), true, uids, &ret);
}
EXPECT_EQ(true, ret);
- EXPECT_EQ((int) uids.size() + 7, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
- EXPECT_EQ((int) uids.size() + 13, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
+ EXPECT_EQ((int) uids.size() + 9, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
+ EXPECT_EQ((int) uids.size() + 15, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
+ EXPECT_EQ(true, iptablesNoSocketAllowRuleExists(chainName.c_str()));
+ EXPECT_EQ(true, iptablesEspAllowRuleExists(chainName.c_str()));
{
TimedOperation op("Clearing whitelist chain");
mNetd->firewallReplaceUidChain(String16(chainName.c_str()), false, noUids, &ret);
@@ -191,6 +218,8 @@
EXPECT_EQ(true, ret);
EXPECT_EQ((int) uids.size() + 5, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
EXPECT_EQ((int) uids.size() + 5, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
+ EXPECT_EQ(false, iptablesNoSocketAllowRuleExists(chainName.c_str()));
+ EXPECT_EQ(false, iptablesEspAllowRuleExists(chainName.c_str()));
{
TimedOperation op("Clearing blacklist chain");