commit d351bea99bc46011dae9291a7dc68efbf0979a12
author Lorenzo Colitti <lorenzo@google.com> Sun Jul 16 22:52:30 2017 +0900
committer Lorenzo Colitti <lorenzo@google.com> Mon Jul 17 02:12:09 2017 +0900
tree 7a9c868173a23e3eb6bcb00ffcef1e9b263437eb
parent cc1bb82f2e4edc987579655dc1babab5e721a126

Convert {enable,disable}Firewall to iptables-restore

Bug: 28362720
Test: netd_{unit,integration}_test pass
Change-Id: I7c3ddf0812f40124ac83f36d3fd3a8c595ce5472

server/FirewallController.cpp

diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index b235f91..4b6eca6 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -77,9 +77,13 @@
 
         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);
+            std::string command =
+                "*filter\n"
+                "-A fw_INPUT -j DROP\n"
+                "-A fw_OUTPUT -j REJECT\n"
+                "-A fw_FORWARD -j REJECT\n"
+                "COMMIT\n";
+            res = execIptablesRestore(V4V6, command.c_str());
         }
 
         // Set this after calling disableFirewall(), since it defaults to WHITELIST there
@@ -89,16 +93,17 @@
 }
 
 int FirewallController::disableFirewall(void) {
-    int res = 0;
-
     mFirewallType = WHITELIST;
 
     // flush any existing rules
-    res |= execIptables(V4V6, "-F", LOCAL_INPUT, NULL);
-    res |= execIptables(V4V6, "-F", LOCAL_OUTPUT, NULL);
-    res |= execIptables(V4V6, "-F", LOCAL_FORWARD, NULL);
+    std::string command =
+        "*filter\n"
+        ":fw_INPUT -\n"
+        ":fw_OUTPUT -\n"
+        ":fw_FORWARD -\n"
+        "COMMIT\n";
 
-    return res;
+    return execIptablesRestore(V4V6, command.c_str());
 }
 
 int FirewallController::enableChildChains(ChildChain chain, bool enable) {

server/FirewallControllerTest.cpp

diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index 65576a7..db9b31e 100644
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -27,6 +27,7 @@
 #include "FirewallController.h"
 #include "IptablesBaseTest.h"
 
+using android::base::Join;
 
 class FirewallControllerTest : public IptablesBaseTest {
 protected:
@@ -216,28 +217,32 @@
 
 TEST_F(FirewallControllerTest, TestEnableDisableFirewall) {
     std::vector<std::string> enableCommands = {
-        "-A fw_INPUT -j DROP",
-        "-A fw_OUTPUT -j REJECT",
-        "-A fw_FORWARD -j REJECT",
+        "*filter\n"
+        "-A fw_INPUT -j DROP\n"
+        "-A fw_OUTPUT -j REJECT\n"
+        "-A fw_FORWARD -j REJECT\n"
+        "COMMIT\n"
     };
     std::vector<std::string> disableCommands = {
-        "-F fw_INPUT",
-        "-F fw_OUTPUT",
-        "-F fw_FORWARD",
+        "*filter\n"
+        ":fw_INPUT -\n"
+        ":fw_OUTPUT -\n"
+        ":fw_FORWARD -\n"
+        "COMMIT\n"
     };
     std::vector<std::string> noCommands = {};
 
     EXPECT_EQ(0, mFw.disableFirewall());
-    expectIptablesCommands(disableCommands);
+    expectIptablesRestoreCommands(disableCommands);
 
     EXPECT_EQ(0, mFw.disableFirewall());
-    expectIptablesCommands(disableCommands);
+    expectIptablesRestoreCommands(disableCommands);
 
     EXPECT_EQ(0, mFw.enableFirewall(BLACKLIST));
-    expectIptablesCommands(disableCommands);
+    expectIptablesRestoreCommands(disableCommands);
 
     EXPECT_EQ(0, mFw.enableFirewall(BLACKLIST));
-    expectIptablesCommands(noCommands);
+    expectIptablesRestoreCommands(noCommands);
 
     std::vector<std::string> disableEnableCommands;
     disableEnableCommands.insert(
@@ -246,16 +251,16 @@
             disableEnableCommands.end(), enableCommands.begin(), enableCommands.end());
 
     EXPECT_EQ(0, mFw.enableFirewall(WHITELIST));
-    expectIptablesCommands(disableEnableCommands);
+    expectIptablesRestoreCommands(disableEnableCommands);
 
     EXPECT_EQ(0, mFw.enableFirewall(WHITELIST));
-    expectIptablesCommands(noCommands);
+    expectIptablesRestoreCommands(noCommands);
 
     EXPECT_EQ(0, mFw.disableFirewall());
-    expectIptablesCommands(disableCommands);
+    expectIptablesRestoreCommands(disableCommands);
 
     // TODO: calling disableFirewall and then enableFirewall(WHITELIST) does
     // nothing. This seems like a clear bug.
     EXPECT_EQ(0, mFw.enableFirewall(WHITELIST));
-    expectIptablesCommands(noCommands);
+    expectIptablesRestoreCommands(noCommands);
 }