Make FirewallController::createChain use replaceUidChain.

This has two benefits:

1. It makes the behaviour of setting firewall chains via the
   firewallReplaceUidChain RPC match the behaviour of creating
   the chains on boot. (As a side effect, it reduces code
   duplication between the two.)
2. It makes creating firewall chains on boot use iptables-restore,
   which is substantially faster than running iptables commands
   one at a time.

This CL will allow the framework to switch to using
firewallReplaceUidChain when the framework starts, providing
substantial speedups over the current behaviour of running two
iptables commands for every app that is whitelisted or idle.

Bug: 26675191
Change-Id: Ifbd15bf9143efd526570dde8f88effc79d164630

diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index 2d2e3b6..0a06b9d 100644 (file)
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -290,35 +290,9 @@ 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);
-
-    // 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);
-
-    if (type == WHITELIST) {
-        // 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);
-        }
-
-        // 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::vector<int32_t> uids;
+    return replaceUidChain(childChain, type == WHITELIST, uids);
 }
 
 std::string FirewallController::makeUidRules(IptablesTarget target, const char *name,

diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index 7e3686b..c1226b2 100644 (file)
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -22,6 +22,8 @@
 
 #include <gtest/gtest.h>
 
+#include <android-base/strings.h>
+
 #include "FirewallController.h"
 #include "IptablesBaseTest.h"
 
@@ -47,35 +49,61 @@ protected:
 
 
 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) {