Allow TCP RSTs to make it through firewall rules.

This allows us to cleanly close apps' TCP connections when we
remove their network connectivity.

Bug: 27824851
Change-Id: I69ae0e860536139d30d14d580a36c82f79dc2f82

diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index 8f07a56..7bb883d 100644 (file)
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -30,6 +30,10 @@
 
 using android::base::StringAppendF;
 
+auto FirewallController::execIptables = ::execIptables;
+auto FirewallController::execIptablesSilently = ::execIptables;
+auto FirewallController::execIptablesRestore = ::execIptablesRestore;
+
 const char* FirewallController::TABLE = "filter";
 
 const char* FirewallController::LOCAL_INPUT = "fw_INPUT";
@@ -241,10 +245,12 @@ int FirewallController::setUidRule(ChildChain chain, int uid, FirewallRule rule)
     FirewallType firewallType = getFirewallType(chain);
     if (firewallType == WHITELIST) {
         target = "RETURN";
+        // When adding, insert RETURN rules at the front, before the catch-all DROP at the end.
         op = (rule == ALLOW)? "-I" : "-D";
     } else { // BLACKLIST mode
         target = "DROP";
-        op = (rule == DENY)? "-I" : "-D";
+        // When adding, append DROP rules at the end, after the RETURN rule that matches TCP RSTs.
+        op = (rule == DENY)? "-A" : "-D";
     }
 
     int res = 0;
@@ -290,6 +296,12 @@ int FirewallController::createChain(const char* childChain,
     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++) {

diff --git a/server/FirewallController.h b/server/FirewallController.h
index 3af6081..0854c20 100644 (file)
--- a/server/FirewallController.h
+++ b/server/FirewallController.h
@@ -22,6 +22,8 @@
 
 #include <utils/RWLock.h>
 
+#include "NetdConstants.h"
+
 enum FirewallRule { DENY, ALLOW };
 
 // WHITELIST means the firewall denies all by default, uids must be explicitly ALLOWed
@@ -82,6 +84,9 @@ public:
 protected:
     friend class FirewallControllerTest;
     std::string makeUidRules(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);
 
 private:
     FirewallType mFirewallType;

diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index 2a4e7e1..b909833 100644 (file)
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -23,18 +23,89 @@
 #include <gtest/gtest.h>
 
 #include "FirewallController.h"
+#include "IptablesBaseTest.h"
 
 
-class FirewallControllerTest : public ::testing::Test {
+class FirewallControllerTest : public IptablesBaseTest {
 protected:
+    FirewallControllerTest() {
+        FirewallController::execIptables = fakeExecIptables;
+        FirewallController::execIptablesSilently = fakeExecIptables;
+        FirewallController::execIptablesRestore = fakeExecIptablesRestore;
+    }
     FirewallController mFw;
+
     std::string makeUidRules(const char *a, bool b, const std::vector<int32_t>& c) {
         return mFw.makeUidRules(a, b, c);
     }
+
+    int createChain(const char* a, const char*b , FirewallType c) {
+        return mFw.createChain(a, b, c);
+    }
 };
 
 
-TEST_F(FirewallControllerTest, TestWhitelist) {
+TEST_F(FirewallControllerTest, TestCreateWhitelistChain) {
+    ExpectedIptablesCommands expected = {
+        { 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" },
+    };
+    createChain("fw_whitelist", "INPUT", WHITELIST);
+    expectIptablesCommands(expected);
+}
+
+TEST_F(FirewallControllerTest, TestCreateBlacklistChain) {
+    ExpectedIptablesCommands expected = {
+        { 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" },
+    };
+    createChain("fw_blacklist", "INPUT", BLACKLIST);
+    expectIptablesCommands(expected);
+}
+
+TEST_F(FirewallControllerTest, TestSetStandbyRule) {
+    ExpectedIptablesCommands expected = {
+        { V4V6, "-D fw_standby -m owner --uid-owner 12345 -j DROP" }
+    };
+    mFw.setUidRule(STANDBY, 12345, ALLOW);
+    expectIptablesCommands(expected);
+
+    expected = {
+        { V4V6, "-A fw_standby -m owner --uid-owner 12345 -j DROP" }
+    };
+    mFw.setUidRule(STANDBY, 12345, DENY);
+    expectIptablesCommands(expected);
+}
+
+TEST_F(FirewallControllerTest, TestSetDozeRule) {
+    ExpectedIptablesCommands expected = {
+        { V4V6, "-I fw_dozable -m owner --uid-owner 54321 -j RETURN" }
+    };
+    mFw.setUidRule(DOZABLE, 54321, ALLOW);
+    expectIptablesCommands(expected);
+
+    expected = {
+        { V4V6, "-D fw_dozable -m owner --uid-owner 54321 -j RETURN" }
+    };
+    mFw.setUidRule(DOZABLE, 54321, DENY);
+    expectIptablesCommands(expected);
+}
+
+TEST_F(FirewallControllerTest, TestReplaceWhitelistUidRule) {
     std::string expected =
             "*filter\n"
             ":FW_whitechain -\n"
@@ -53,7 +124,7 @@ TEST_F(FirewallControllerTest, TestWhitelist) {
     EXPECT_EQ(expected, makeUidRules("FW_whitechain", true, uids));
 }
 
-TEST_F(FirewallControllerTest, TestBlacklist) {
+TEST_F(FirewallControllerTest, TestReplaceBlacklistUidRule) {
     std::string expected =
             "*filter\n"
             ":FW_blackchain -\n"