From 390e4ea8106f9e741bc80fb962aaee94d5b28cbb Mon Sep 17 00:00:00 2001 From: Amith Yamasani Date: Sat, 25 Apr 2015 19:08:57 -0700 Subject: [PATCH] Blacklist uids for network access FirewallController can now be in blacklist mode (aka disabled) or whitelist mode (aka enabled). Some of the methods don't do anything when in blacklist mode. Uid rules updated to allow dropping packets to uids that shouldn't get any network access, usually for idle apps. Added a wait option to iptables calls to make sure it doesn't fail if there's contention. Fixes a flakiness I was seeing in removing rules. Bug: 20066058 Change-Id: I815bcb45aa06d04020e902df8c67bb3894e98f40 --- server/CommandListener.cpp | 26 ++++++++++++++++++++--- server/CommandListener.h | 1 + server/FirewallController.cpp | 49 +++++++++++++++++++++++++++++++++---------- server/FirewallController.h | 11 ++++++++-- server/NetdConstants.cpp | 4 ++++ 5 files changed, 75 insertions(+), 16 deletions(-) diff --git a/server/CommandListener.cpp b/server/CommandListener.cpp index 5fbac2a..6d1ff52 100644 --- a/server/CommandListener.cpp +++ b/server/CommandListener.cpp @@ -1275,8 +1275,22 @@ int CommandListener::FirewallCmd::sendGenericOkFail(SocketClient *cli, int cond) FirewallRule CommandListener::FirewallCmd::parseRule(const char* arg) { if (!strcmp(arg, "allow")) { return ALLOW; - } else { + } else if (!strcmp(arg, "deny")) { return DENY; + } else { + ALOGE("failed to parse uid rule (%s)", arg); + return ALLOW; + } +} + +FirewallType CommandListener::FirewallCmd::parseFirewallType(const char* arg) { + if (!strcmp(arg, "whitelist")) { + return WHITELIST; + } else if (!strcmp(arg, "blacklist")) { + return BLACKLIST; + } else { + ALOGE("failed to parse firewall type (%s)", arg); + return BLACKLIST; } } @@ -1288,7 +1302,14 @@ int CommandListener::FirewallCmd::runCommand(SocketClient *cli, int argc, } if (!strcmp(argv[1], "enable")) { - int res = sFirewallCtrl->enableFirewall(); + if (argc != 3) { + cli->sendMsg(ResponseCode::CommandSyntaxError, + "Usage: firewall enable ", false); + return 0; + } + FirewallType firewallType = parseFirewallType(argv[2]); + + int res = sFirewallCtrl->enableFirewall(firewallType); return sendGenericOkFail(cli, res); } if (!strcmp(argv[1], "disable")) { @@ -1357,7 +1378,6 @@ int CommandListener::FirewallCmd::runCommand(SocketClient *cli, int argc, int uid = atoi(argv[2]); FirewallRule rule = parseRule(argv[3]); - int res = sFirewallCtrl->setUidRule(uid, rule); return sendGenericOkFail(cli, res); } diff --git a/server/CommandListener.h b/server/CommandListener.h index b71bc45..99dea5a 100644 --- a/server/CommandListener.h +++ b/server/CommandListener.h @@ -136,6 +136,7 @@ private: protected: int sendGenericOkFail(SocketClient *cli, int cond); static FirewallRule parseRule(const char* arg); + static FirewallType parseFirewallType(const char* arg); }; class ClatdCmd : public NetdCommand { diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp index 17c6da4..3f71d02 100644 --- a/server/FirewallController.cpp +++ b/server/FirewallController.cpp @@ -32,22 +32,29 @@ const char* FirewallController::LOCAL_OUTPUT = "fw_OUTPUT"; const char* FirewallController::LOCAL_FORWARD = "fw_FORWARD"; FirewallController::FirewallController(void) { + // If no rules are set, it's in BLACKLIST mode + firewallType = BLACKLIST; } int FirewallController::setupIptablesHooks(void) { return 0; } -int FirewallController::enableFirewall(void) { +int FirewallController::enableFirewall(FirewallType ftype) { int res = 0; // flush any existing rules disableFirewall(); - // create default rule to drop all traffic - res |= execIptables(V4V6, "-A", LOCAL_INPUT, "-j", "DROP", NULL); - res |= execIptables(V4V6, "-A", LOCAL_OUTPUT, "-j", "REJECT", NULL); - res |= execIptables(V4V6, "-A", LOCAL_FORWARD, "-j", "REJECT", NULL); + if (ftype == WHITELIST) { + // create default rule to drop all traffic + res |= execIptables(V4V6, "-A", LOCAL_INPUT, "-j", "DROP", NULL); + res |= execIptables(V4V6, "-A", LOCAL_OUTPUT, "-j", "REJECT", NULL); + res |= execIptables(V4V6, "-A", LOCAL_FORWARD, "-j", "REJECT", NULL); + } + + // Set this after calling disableFirewall(), since it defaults to WHITELIST there + firewallType = ftype; return res; } @@ -55,6 +62,8 @@ int FirewallController::enableFirewall(void) { int FirewallController::disableFirewall(void) { int res = 0; + firewallType = WHITELIST; + // flush any existing rules res |= execIptables(V4V6, "-F", LOCAL_INPUT, NULL); res |= execIptables(V4V6, "-F", LOCAL_OUTPUT, NULL); @@ -69,6 +78,11 @@ int FirewallController::isFirewallEnabled(void) { } int FirewallController::setInterfaceRule(const char* iface, FirewallRule rule) { + if (firewallType == BLACKLIST) { + // Unsupported in BLACKLIST mode + return -1; + } + if (!isIfaceName(iface)) { errno = ENOENT; return -1; @@ -88,6 +102,11 @@ int FirewallController::setInterfaceRule(const char* iface, FirewallRule rule) { } int FirewallController::setEgressSourceRule(const char* addr, FirewallRule rule) { + if (firewallType == BLACKLIST) { + // Unsupported in BLACKLIST mode + return -1; + } + IptablesTarget target = V4; if (strchr(addr, ':')) { target = V6; @@ -108,6 +127,11 @@ int FirewallController::setEgressSourceRule(const char* addr, FirewallRule rule) int FirewallController::setEgressDestRule(const char* addr, int protocol, int port, FirewallRule rule) { + if (firewallType == BLACKLIST) { + // Unsupported in BLACKLIST mode + return -1; + } + IptablesTarget target = V4; if (strchr(addr, ':')) { target = V6; @@ -139,16 +163,19 @@ int FirewallController::setUidRule(int uid, FirewallRule rule) { sprintf(uidStr, "%d", uid); const char* op; - if (rule == ALLOW) { - op = "-I"; - } else { - op = "-D"; + const char* target; + if (firewallType == WHITELIST) { + target = "RETURN"; + op = (rule == ALLOW)? "-I" : "-D"; + } else { // BLACKLIST mode + target = "DROP"; + op = (rule == DENY)? "-I" : "-D"; } int res = 0; res |= execIptables(V4V6, op, LOCAL_INPUT, "-m", "owner", "--uid-owner", uidStr, - "-j", "RETURN", NULL); + "-j", target, NULL); res |= execIptables(V4V6, op, LOCAL_OUTPUT, "-m", "owner", "--uid-owner", uidStr, - "-j", "RETURN", NULL); + "-j", target, NULL); return res; } diff --git a/server/FirewallController.h b/server/FirewallController.h index 158e0fa..8051a73 100644 --- a/server/FirewallController.h +++ b/server/FirewallController.h @@ -19,7 +19,12 @@ #include -enum FirewallRule { ALLOW, DENY }; +enum FirewallRule { DENY, ALLOW }; + +// WHITELIST means the firewall denies all by default, uids must be explicitly ALLOWed +// BLACKLIST means the firewall allows all by default, uids must be explicitly DENYed + +enum FirewallType { WHITELIST, BLACKLIST }; #define PROTOCOL_TCP 6 #define PROTOCOL_UDP 17 @@ -34,7 +39,7 @@ public: int setupIptablesHooks(void); - int enableFirewall(void); + int enableFirewall(FirewallType); int disableFirewall(void); int isFirewallEnabled(void); @@ -51,6 +56,8 @@ public: static const char* LOCAL_OUTPUT; static const char* LOCAL_FORWARD; +private: + FirewallType firewallType; }; #endif diff --git a/server/NetdConstants.cpp b/server/NetdConstants.cpp index d2e0bbe..c86538b 100644 --- a/server/NetdConstants.cpp +++ b/server/NetdConstants.cpp @@ -73,6 +73,10 @@ static int execIptables(IptablesTarget target, bool silent, va_list args) { std::list argsList; argsList.push_back(NULL); const char* arg; + + // Wait to avoid failure due to another process holding the lock + argsList.push_back("-w"); + do { arg = va_arg(args, const char *); argsList.push_back(arg); -- 2.11.0