Use IptablesRestoreController for UID rule updates.
Bug: 32073253
Test: netd_{unit,integration}_test passes
Test: bullhead builds, boots
Test: fw_powersave chain correctly updated when updating battery optimization whitelist
Test: fw_powersave chain correctly updated when bringing apps into foreground
Change-Id: I964b7664718f353057047c66e69351169b5cf453
diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index e2ddc74..4693206 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -173,9 +173,6 @@
}
int FirewallController::setUidRule(ChildChain chain, int uid, FirewallRule rule) {
- char uidStr[16];
- sprintf(uidStr, "%d", uid);
-
const char* op;
const char* target;
FirewallType firewallType = getFirewallType(chain);
@@ -189,31 +186,33 @@
op = (rule == DENY)? "-A" : "-D";
}
- int res = 0;
+ std::vector<std::string> chainNames;
switch(chain) {
case DOZABLE:
- res |= execIptables(V4V6, op, LOCAL_DOZABLE, "-m", "owner", "--uid-owner",
- uidStr, "-j", target, NULL);
+ chainNames = { LOCAL_DOZABLE };
break;
case STANDBY:
- res |= execIptables(V4V6, op, LOCAL_STANDBY, "-m", "owner", "--uid-owner",
- uidStr, "-j", target, NULL);
+ chainNames = { LOCAL_STANDBY };
break;
case POWERSAVE:
- res |= execIptables(V4V6, op, LOCAL_POWERSAVE, "-m", "owner", "--uid-owner",
- uidStr, "-j", target, NULL);
+ chainNames = { LOCAL_POWERSAVE };
break;
case NONE:
- res |= execIptables(V4V6, op, LOCAL_INPUT, "-m", "owner", "--uid-owner", uidStr,
- "-j", target, NULL);
- res |= execIptables(V4V6, op, LOCAL_OUTPUT, "-m", "owner", "--uid-owner", uidStr,
- "-j", target, NULL);
+ chainNames = { LOCAL_INPUT, LOCAL_OUTPUT };
break;
default:
ALOGW("Unknown child chain: %d", chain);
- break;
+ return -1;
}
- return res;
+
+ std::string command = "*filter\n";
+ for (std::string chainName : chainNames) {
+ StringAppendF(&command, "%s %s -m owner --uid-owner %d -j %s\n",
+ op, chainName.c_str(), uid, target);
+ }
+ StringAppendF(&command, "COMMIT\n");
+
+ return execIptablesRestore(V4V6, command);
}
int FirewallController::attachChain(const char* childChain, const char* parentChain) {
diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index f709cda..3f8ce12 100644
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -104,30 +104,50 @@
TEST_F(FirewallControllerTest, TestSetStandbyRule) {
ExpectedIptablesCommands expected = {
- { V4V6, "-D fw_standby -m owner --uid-owner 12345 -j DROP" }
+ { V4V6, "*filter\n-D fw_standby -m owner --uid-owner 12345 -j DROP\nCOMMIT\n" }
};
mFw.setUidRule(STANDBY, 12345, ALLOW);
- expectIptablesCommands(expected);
+ expectIptablesRestoreCommands(expected);
expected = {
- { V4V6, "-A fw_standby -m owner --uid-owner 12345 -j DROP" }
+ { V4V6, "*filter\n-A fw_standby -m owner --uid-owner 12345 -j DROP\nCOMMIT\n" }
};
mFw.setUidRule(STANDBY, 12345, DENY);
- expectIptablesCommands(expected);
+ expectIptablesRestoreCommands(expected);
}
TEST_F(FirewallControllerTest, TestSetDozeRule) {
ExpectedIptablesCommands expected = {
- { V4V6, "-I fw_dozable -m owner --uid-owner 54321 -j RETURN" }
+ { V4V6, "*filter\n-I fw_dozable -m owner --uid-owner 54321 -j RETURN\nCOMMIT\n" }
};
mFw.setUidRule(DOZABLE, 54321, ALLOW);
- expectIptablesCommands(expected);
+ expectIptablesRestoreCommands(expected);
expected = {
- { V4V6, "-D fw_dozable -m owner --uid-owner 54321 -j RETURN" }
+ { V4V6, "*filter\n-D fw_dozable -m owner --uid-owner 54321 -j RETURN\nCOMMIT\n" }
};
mFw.setUidRule(DOZABLE, 54321, DENY);
- expectIptablesCommands(expected);
+ expectIptablesRestoreCommands(expected);
+}
+
+TEST_F(FirewallControllerTest, TestSetFirewallRule) {
+ ExpectedIptablesCommands expected = {
+ { V4V6, "*filter\n"
+ "-A fw_INPUT -m owner --uid-owner 54321 -j DROP\n"
+ "-A fw_OUTPUT -m owner --uid-owner 54321 -j DROP\n"
+ "COMMIT\n" }
+ };
+ mFw.setUidRule(NONE, 54321, DENY);
+ expectIptablesRestoreCommands(expected);
+
+ expected = {
+ { V4V6, "*filter\n"
+ "-D fw_INPUT -m owner --uid-owner 54321 -j DROP\n"
+ "-D fw_OUTPUT -m owner --uid-owner 54321 -j DROP\n"
+ "COMMIT\n" }
+ };
+ mFw.setUidRule(NONE, 54321, ALLOW);
+ expectIptablesRestoreCommands(expected);
}
TEST_F(FirewallControllerTest, TestReplaceWhitelistUidRule) {
diff --git a/server/IptablesRestoreControllerTest.cpp b/server/IptablesRestoreControllerTest.cpp
index 515dee6..43041ec 100644
--- a/server/IptablesRestoreControllerTest.cpp
+++ b/server/IptablesRestoreControllerTest.cpp
@@ -29,6 +29,7 @@
#include "IptablesRestoreController.h"
#include "NetdConstants.h"
+#include "Stopwatch.h"
#define XT_LOCK_NAME "/system/etc/xtables.lock"
#define XT_LOCK_ATTEMPTS 10
@@ -239,3 +240,33 @@
EXPECT_EQ(0, con.execute(IptablesTarget::V4V6, commandString, &output));
EXPECT_EQ(expected, output);
}
+
+TEST_F(IptablesRestoreControllerTest, TestUidRuleBenchmark) {
+ const std::vector<int> ITERATIONS = { 1, 5, 10 };
+
+ const std::string IPTABLES_RESTORE_ADD =
+ "*filter\n-I fw_powersave -m owner --uid-owner 2000000000 -j RETURN\nCOMMIT\n";
+ const std::string IPTABLES_RESTORE_DEL =
+ "*filter\n-D fw_powersave -m owner --uid-owner 2000000000 -j RETURN\nCOMMIT\n";
+
+ for (const int iterations : ITERATIONS) {
+ Stopwatch s;
+ for (int i = 0; i < iterations; i++) {
+ EXPECT_EQ(0, con.execute(V4V6, IPTABLES_RESTORE_ADD, nullptr));
+ EXPECT_EQ(0, con.execute(V4V6, IPTABLES_RESTORE_DEL, nullptr));
+ }
+ float timeTaken = s.getTimeAndReset();
+ fprintf(stderr, " Add/del %d UID rules via restore: %.1fms (%.2fms per operation)\n",
+ iterations, timeTaken, timeTaken / 2 / iterations);
+
+ for (int i = 0; i < iterations; i++) {
+ EXPECT_EQ(0, execIptables(V4V6, "-I", "fw_powersave", "-m", "owner",
+ "--uid-owner", "2000000000", "-j", "RETURN", nullptr));
+ EXPECT_EQ(0, execIptables(V4V6, "-D", "fw_powersave", "-m", "owner",
+ "--uid-owner", "2000000000", "-j", "RETURN", nullptr));
+ }
+ timeTaken = s.getTimeAndReset();
+ fprintf(stderr, " Add/del %d UID rules via iptables: %.1fms (%.2fms per operation)\n",
+ iterations, timeTaken, timeTaken / 2 / iterations);
+ }
+}