Skip to content

GitLab

Commit d52ab4f1 authored by Lorenzo Colitti's avatar Lorenzo Colitti Committed by android-build-merger
Browse files

Merge changes Ifbd15bf9,I985e6861,I54860c7c into nyc-dev 

am: 7ad3c888

* commit '7ad3c888':
  Make FirewallController::createChain use replaceUidChain.
  Make firewallReplaceUidChain match the behaviour of createChain.
  Don't crash the test if expecting more commands than were run.

Change-Id: Ib9dac62413187fc9b4978dfe14b7d29dbf328328
parents 5092efba 7ad3c888
Hide whitespace changes
Inline Side-by-side
Showing with 109 additions and 73 deletions
server/FirewallController.cpp
View file @ d52ab4f1
......@@ -290,59 +290,44 @@ int FirewallController::detachChain(const char* childChain, const char* parentCh
int FirewallController::createChain(const char* childChain,
const char* parentChain, FirewallType type) {
// Order is important, otherwise later steps may fail.
execIptablesSilently(V4V6, "-t", TABLE, "-D", parentChain, "-j", childChain, NULL);
execIptablesSilently(V4V6, "-t", TABLE, "-F", childChain, NULL);
execIptablesSilently(V4V6, "-t", TABLE, "-X", childChain, NULL);
int res = 0;
res |= execIptables(V4V6, "-t", TABLE, "-N", childChain, NULL);
std::vector<int32_t> uids;
return replaceUidChain(childChain, type == WHITELIST, uids);
}
std::string FirewallController::makeUidRules(IptablesTarget target, const char *name,
bool isWhitelist, const std::vector<int32_t>& uids) {
std::string commands;
StringAppendF(&commands, "*filter\n:%s -\n", name);
// Allow TCP RSTs so we can cleanly close TCP connections of apps that no longer have network
// access. Both incoming and outgoing RSTs are allowed.
res |= execIptables(V4V6, "-A", childChain, "-p", "tcp",
"--tcp-flags", "RST", "RST", "-j", "RETURN", NULL);
StringAppendF(&commands, "-A %s -p tcp --tcp-flags RST RST -j RETURN\n", name);
if (type == WHITELIST) {
if (isWhitelist) {
// Allow ICMPv6 packets necessary to make IPv6 connectivity work. http://b/23158230 .
for (size_t i = 0; i < ARRAY_SIZE(ICMPV6_TYPES); i++) {
res |= execIptables(V6, "-A", childChain, "-p", "icmpv6", "--icmpv6-type",
ICMPV6_TYPES[i], "-j", "RETURN", NULL);
if (target == V6) {
for (size_t i = 0; i < ARRAY_SIZE(ICMPV6_TYPES); i++) {
StringAppendF(&commands, "-A %s -p icmpv6 --icmpv6-type %s -j RETURN\n",
name, ICMPV6_TYPES[i]);
}
}
// create default white list for system uid range
char uidStr[16];
sprintf(uidStr, "0-%d", MAX_SYSTEM_UID);
res |= execIptables(V4V6, "-A", childChain, "-m", "owner", "--uid-owner",
uidStr, "-j", "RETURN", NULL);
// create default rule to drop all traffic
res |= execIptables(V4V6, "-A", childChain, "-j", "DROP", NULL);
}
return res;
}
std::string FirewallController::makeUidRules(
const char *name, bool isWhitelist, const std::vector<int32_t>& uids) {
const char *action = isWhitelist ? "RETURN" : "DROP";
const char *defaultAction = isWhitelist ? "DROP" : "RETURN";
std::string commands;
StringAppendF(&commands, "*filter\n:%s -\n", name);
if (isWhitelist) {
// Always whitelist system UIDs.
StringAppendF(&commands,
"-A %s -m owner --uid-owner %d-%d -j %s\n", name, 0, MAX_SYSTEM_UID, action);
"-A %s -m owner --uid-owner %d-%d -j RETURN\n", name, 0, MAX_SYSTEM_UID);
}
// Whitelist or blacklist the specified UIDs.
const char *action = isWhitelist ? "RETURN" : "DROP";
for (auto uid : uids) {
StringAppendF(&commands, "-A %s -m owner --uid-owner %d -j %s\n", name, uid, action);
}
// If it's a blacklist chain that blacklists nothing, then don't add a default action.
if (isWhitelist || uids.size() > 0) {
StringAppendF(&commands, "-A %s -j %s\n", name, defaultAction);
// If it's a whitelist chain, add a default DROP at the end. This is not necessary for a
// blacklist chain, because all user-defined chains implicitly RETURN at the end.
if (isWhitelist) {
StringAppendF(&commands, "-A %s -j DROP\n", name);
}
StringAppendF(&commands, "COMMIT\n\x04"); // EOT.
......@@ -352,6 +337,7 @@ std::string FirewallController::makeUidRules(
int FirewallController::replaceUidChain(
const char *name, bool isWhitelist, const std::vector<int32_t>& uids) {
std::string commands = makeUidRules(name, isWhitelist, uids);
return execIptablesRestore(V4V6, commands.c_str());
std::string commands4 = makeUidRules(V4, name, isWhitelist, uids);
std::string commands6 = makeUidRules(V6, name, isWhitelist, uids);
return execIptablesRestore(V4, commands4.c_str()) | execIptablesRestore(V6, commands6.c_str());
}
server/FirewallController.h
View file @ d52ab4f1
......@@ -83,7 +83,8 @@ public:
protected:
friend class FirewallControllerTest;
std::string makeUidRules(const char *name, bool isWhitelist, const std::vector<int32_t>& uids);
std::string makeUidRules(IptablesTarget target, const char *name, bool isWhitelist,
const std::vector<int32_t>& uids);
static int (*execIptables)(IptablesTarget target, ...);
static int (*execIptablesSilently)(IptablesTarget target, ...);
static int (*execIptablesRestore)(IptablesTarget target, const std::string& commands);
......
server/FirewallControllerTest.cpp
View file @ d52ab4f1
......@@ -22,6 +22,8 @@
#include <gtest/gtest.h>
#include <android-base/strings.h>
#include "FirewallController.h"
#include "IptablesBaseTest.h"
......@@ -35,46 +37,73 @@ protected:
}
FirewallController mFw;
std::string makeUidRules(const char *a, bool b, const std::vector<int32_t>& c) {
return mFw.makeUidRules(a, b, c);
std::string makeUidRules(IptablesTarget a, const char* b, bool c,
const std::vector<int32_t>& d) {
return mFw.makeUidRules(a, b, c, d);
}
int createChain(const char* a, const char*b , FirewallType c) {
int createChain(const char* a, const char* b , FirewallType c) {
return mFw.createChain(a, b, c);
}
};
TEST_F(FirewallControllerTest, TestCreateWhitelistChain) {
ExpectedIptablesCommands expected = {
ExpectedIptablesCommands expectedCommands = {
{ V4V6, "-t filter -D INPUT -j fw_whitelist" },
{ V4V6, "-t filter -F fw_whitelist" },
{ V4V6, "-t filter -X fw_whitelist" },
{ V4V6, "-t filter -N fw_whitelist" },
{ V4V6, "-A fw_whitelist -p tcp --tcp-flags RST RST -j RETURN" },
{ V6, "-A fw_whitelist -p icmpv6 --icmpv6-type packet-too-big -j RETURN" },
{ V6, "-A fw_whitelist -p icmpv6 --icmpv6-type router-solicitation -j RETURN" },
{ V6, "-A fw_whitelist -p icmpv6 --icmpv6-type router-advertisement -j RETURN" },
{ V6, "-A fw_whitelist -p icmpv6 --icmpv6-type neighbour-solicitation -j RETURN" },
{ V6, "-A fw_whitelist -p icmpv6 --icmpv6-type neighbour-advertisement -j RETURN" },
{ V6, "-A fw_whitelist -p icmpv6 --icmpv6-type redirect -j RETURN" },
{ V4V6, "-A fw_whitelist -m owner --uid-owner 0-9999 -j RETURN" },
{ V4V6, "-A fw_whitelist -j DROP" },
};
std::vector<std::string> expectedRestore4 = {
"*filter",
":fw_whitelist -",
"-A fw_whitelist -p tcp --tcp-flags RST RST -j RETURN",
"-A fw_whitelist -m owner --uid-owner 0-9999 -j RETURN",
"-A fw_whitelist -j DROP",
"COMMIT\n\x04"
};
std::vector<std::string> expectedRestore6 = {
"*filter",
":fw_whitelist -",
"-A fw_whitelist -p tcp --tcp-flags RST RST -j RETURN",
"-A fw_whitelist -p icmpv6 --icmpv6-type packet-too-big -j RETURN",
"-A fw_whitelist -p icmpv6 --icmpv6-type router-solicitation -j RETURN",
"-A fw_whitelist -p icmpv6 --icmpv6-type router-advertisement -j RETURN",
"-A fw_whitelist -p icmpv6 --icmpv6-type neighbour-solicitation -j RETURN",
"-A fw_whitelist -p icmpv6 --icmpv6-type neighbour-advertisement -j RETURN",
"-A fw_whitelist -p icmpv6 --icmpv6-type redirect -j RETURN",
"-A fw_whitelist -m owner --uid-owner 0-9999 -j RETURN",
"-A fw_whitelist -j DROP",
"COMMIT\n\x04"
};
std::vector<std::pair<IptablesTarget, std::string>> expectedRestoreCommands = {
{ V4, android::base::Join(expectedRestore4, '\n') },
{ V6, android::base::Join(expectedRestore6, '\n') },
};
createChain("fw_whitelist", "INPUT", WHITELIST);
expectIptablesCommands(expected);
expectIptablesCommands(expectedCommands);
expectIptablesRestoreCommands(expectedRestoreCommands);
}
TEST_F(FirewallControllerTest, TestCreateBlacklistChain) {
ExpectedIptablesCommands expected = {
ExpectedIptablesCommands expectedCommands = {
{ V4V6, "-t filter -D INPUT -j fw_blacklist" },
{ V4V6, "-t filter -F fw_blacklist" },
{ V4V6, "-t filter -X fw_blacklist" },
{ V4V6, "-t filter -N fw_blacklist" },
{ V4V6, "-A fw_blacklist -p tcp --tcp-flags RST RST -j RETURN" },
};
std::vector<std::string> expectedRestore = {
"*filter",
":fw_blacklist -",
"-A fw_blacklist -p tcp --tcp-flags RST RST -j RETURN",
"COMMIT\n\x04"
};
std::vector<std::pair<IptablesTarget, std::string>> expectedRestoreCommands = {
{ V4, android::base::Join(expectedRestore, '\n') },
{ V6, android::base::Join(expectedRestore, '\n') },
};
createChain("fw_blacklist", "INPUT", BLACKLIST);
expectIptablesCommands(expected);
expectIptablesCommands(expectedCommands);
expectIptablesRestoreCommands(expectedRestoreCommands);
}
TEST_F(FirewallControllerTest, TestSetStandbyRule) {
......@@ -109,6 +138,13 @@ TEST_F(FirewallControllerTest, TestReplaceWhitelistUidRule) {
std::string expected =
"*filter\n"
":FW_whitechain -\n"
"-A FW_whitechain -p tcp --tcp-flags RST RST -j RETURN\n"
"-A FW_whitechain -p icmpv6 --icmpv6-type packet-too-big -j RETURN\n"
"-A FW_whitechain -p icmpv6 --icmpv6-type router-solicitation -j RETURN\n"
"-A FW_whitechain -p icmpv6 --icmpv6-type router-advertisement -j RETURN\n"
"-A FW_whitechain -p icmpv6 --icmpv6-type neighbour-solicitation -j RETURN\n"
"-A FW_whitechain -p icmpv6 --icmpv6-type neighbour-advertisement -j RETURN\n"
"-A FW_whitechain -p icmpv6 --icmpv6-type redirect -j RETURN\n"
"-A FW_whitechain -m owner --uid-owner 0-9999 -j RETURN\n"
"-A FW_whitechain -m owner --uid-owner 10023 -j RETURN\n"
"-A FW_whitechain -m owner --uid-owner 10059 -j RETURN\n"
......@@ -121,19 +157,19 @@ TEST_F(FirewallControllerTest, TestReplaceWhitelistUidRule) {
"COMMIT\n\x04";
std::vector<int32_t> uids = { 10023, 10059, 10124, 10111, 110122, 210153, 210024 };
EXPECT_EQ(expected, makeUidRules("FW_whitechain", true, uids));
EXPECT_EQ(expected, makeUidRules(V6, "FW_whitechain", true, uids));
}
TEST_F(FirewallControllerTest, TestReplaceBlacklistUidRule) {
std::string expected =
"*filter\n"
":FW_blackchain -\n"
"-A FW_blackchain -p tcp --tcp-flags RST RST -j RETURN\n"
"-A FW_blackchain -m owner --uid-owner 10023 -j DROP\n"
"-A FW_blackchain -m owner --uid-owner 10059 -j DROP\n"
"-A FW_blackchain -m owner --uid-owner 10124 -j DROP\n"
"-A FW_blackchain -j RETURN\n"
"COMMIT\n\x04";
std::vector<int32_t> uids = { 10023, 10059, 10124 };
EXPECT_EQ(expected, makeUidRules("FW_blackchain", false, uids));
EXPECT_EQ(expected, makeUidRules(V4 ,"FW_blackchain", false, uids));
}
server/IptablesBaseTest.cpp
View file @ d52ab4f1
......@@ -70,12 +70,20 @@ int IptablesBaseTest::fakeExecIptablesRestore(IptablesTarget target, const std::
int IptablesBaseTest::expectIptablesCommand(IptablesTarget target, int pos,
const std::string& cmd) {
if ((unsigned) pos >= sCmds.size()) {
ADD_FAILURE() << "Expected too many iptables commands, want command "
<< pos + 1 << "/" << sCmds.size();
return -1;
}
if (target == V4 || target == V4V6) {
EXPECT_EQ("/system/bin/iptables -w " + cmd, sCmds[pos++]);
}
if (target == V6 || target == V4V6) {
EXPECT_EQ("/system/bin/ip6tables -w " + cmd, sCmds[pos++]);
}
return target == V4V6 ? 2 : 1;
}
......@@ -92,7 +100,12 @@ void IptablesBaseTest::expectIptablesCommands(const ExpectedIptablesCommands& ex
for (size_t i = 0; i < expectedCmds.size(); i ++) {
auto target = expectedCmds[i].first;
auto cmd = expectedCmds[i].second;
pos += expectIptablesCommand(target, pos, cmd);
int numConsumed = expectIptablesCommand(target, pos, cmd);
if (numConsumed < 0) {
// Read past the end of the array.
break;
}
pos += numConsumed;
}
EXPECT_EQ(pos, sCmds.size());
......
tests/binder_test.cpp
View file @ d52ab4f1
......@@ -140,15 +140,15 @@ TEST_F(BinderTest, TestFirewallReplaceUidChain) {
mNetd->firewallReplaceUidChain(String16(chainName.c_str()), true, uids, &ret);
}
EXPECT_EQ(true, ret);
EXPECT_EQ((int) uids.size() + 4, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
EXPECT_EQ((int) uids.size() + 4, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
EXPECT_EQ((int) uids.size() + 5, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
EXPECT_EQ((int) uids.size() + 11, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
{
TimedOperation op("Clearing whitelist chain");
mNetd->firewallReplaceUidChain(String16(chainName.c_str()), false, noUids, &ret);
}
EXPECT_EQ(true, ret);
EXPECT_EQ(2, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
EXPECT_EQ(2, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
EXPECT_EQ(3, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
EXPECT_EQ(3, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
{
TimedOperation op(StringPrintf("Programming %d-UID blacklist chain", kNumUids));
......@@ -163,8 +163,8 @@ TEST_F(BinderTest, TestFirewallReplaceUidChain) {
mNetd->firewallReplaceUidChain(String16(chainName.c_str()), false, noUids, &ret);
}
EXPECT_EQ(true, ret);
EXPECT_EQ(2, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
EXPECT_EQ(2, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
EXPECT_EQ(3, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
EXPECT_EQ(3, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
// Check that the call fails if iptables returns an error.
std::string veryLongStringName = "netd_binder_test_UnacceptablyLongIptablesChainName";
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment