Really always allow networking on loopback.

https://android-review.googlesource.com/#/c/294359/ attempted to
allow networking on loopback, but actually does not do anything
because no packet has both -i lo and -o lo: loopback packets have
-i lo in INPUT and -o lo in OUTPUT.

(cherry picked from commit d7e2e8a1238ebd4396e28524ca2104770fbbcf17)

Test: bullhead builds, boots
Test: netd_{unit,integration}_test pass
Test: loopback traffic is matched by new "-i lo" and "-o lo" rules
Test: originated and received traffic is not matched by new rules
Bug: 34444781
Change-Id: Ib7f1d04ea4ae85506c2b0a87bd5aa1378057f07f

3 files changed, 20 insertions, 14 deletions

diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index 9cab90a8..38843762 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -292,7 +292,8 @@ std::string FirewallController::makeUidRules(IptablesTarget target, const char *
     StringAppendF(&commands, "*filter\n:%s -\n", name);
 
     // Always allow networking on loopback.
-    StringAppendF(&commands, "-A %s -i lo -o lo -j RETURN\n", name);
+    StringAppendF(&commands, "-A %s -i lo -j RETURN\n", name);
+    StringAppendF(&commands, "-A %s -o lo -j RETURN\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.
diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index 9d436362..f709cda7 100644
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -52,7 +52,8 @@ TEST_F(FirewallControllerTest, TestCreateWhitelistChain) {
     std::vector<std::string> expectedRestore4 = {
         "*filter",
         ":fw_whitelist -",
-        "-A fw_whitelist -i lo -o lo -j RETURN",
+        "-A fw_whitelist -i lo -j RETURN",
+        "-A fw_whitelist -o lo -j RETURN",
         "-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",
@@ -61,7 +62,8 @@ TEST_F(FirewallControllerTest, TestCreateWhitelistChain) {
     std::vector<std::string> expectedRestore6 = {
         "*filter",
         ":fw_whitelist -",
-        "-A fw_whitelist -i lo -o lo -j RETURN",
+        "-A fw_whitelist -i lo -j RETURN",
+        "-A fw_whitelist -o lo -j RETURN",
         "-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",
@@ -86,7 +88,8 @@ TEST_F(FirewallControllerTest, TestCreateBlacklistChain) {
     std::vector<std::string> expectedRestore = {
         "*filter",
         ":fw_blacklist -",
-        "-A fw_blacklist -i lo -o lo -j RETURN",
+        "-A fw_blacklist -i lo -j RETURN",
+        "-A fw_blacklist -o lo -j RETURN",
         "-A fw_blacklist -p tcp --tcp-flags RST RST -j RETURN",
         "COMMIT\n"
     };
@@ -131,7 +134,8 @@ TEST_F(FirewallControllerTest, TestReplaceWhitelistUidRule) {
     std::string expected =
             "*filter\n"
             ":FW_whitechain -\n"
-            "-A FW_whitechain -i lo -o lo -j RETURN\n"
+            "-A FW_whitechain -i lo -j RETURN\n"
+            "-A FW_whitechain -o lo -j RETURN\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"
@@ -158,7 +162,8 @@ TEST_F(FirewallControllerTest, TestReplaceBlacklistUidRule) {
     std::string expected =
             "*filter\n"
             ":FW_blackchain -\n"
-            "-A FW_blackchain -i lo -o lo -j RETURN\n"
+            "-A FW_blackchain -i lo -j RETURN\n"
+            "-A FW_blackchain -o lo -j RETURN\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"
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index 1481186a..65de0c3c 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -168,31 +168,31 @@ TEST_F(BinderTest, TestFirewallReplaceUidChain) {
         mNetd->firewallReplaceUidChain(String16(chainName.c_str()), true, uids, &ret);
     }
     EXPECT_EQ(true, ret);
-    EXPECT_EQ((int) uids.size() + 6, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
-    EXPECT_EQ((int) uids.size() + 12, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
+    EXPECT_EQ((int) uids.size() + 7, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
+    EXPECT_EQ((int) uids.size() + 13, 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(4, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
-    EXPECT_EQ(4, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
+    EXPECT_EQ(5, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
+    EXPECT_EQ(5, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
 
     {
         TimedOperation op(StringPrintf("Programming %d-UID blacklist chain", kNumUids));
         mNetd->firewallReplaceUidChain(String16(chainName.c_str()), false, 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() + 5, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
 
     {
         TimedOperation op("Clearing blacklist chain");
         mNetd->firewallReplaceUidChain(String16(chainName.c_str()), false, noUids, &ret);
     }
     EXPECT_EQ(true, ret);
-    EXPECT_EQ(4, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
-    EXPECT_EQ(4, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
+    EXPECT_EQ(5, iptablesRuleLineLength(IPTABLES_PATH, chainName.c_str()));
+    EXPECT_EQ(5, iptablesRuleLineLength(IP6TABLES_PATH, chainName.c_str()));
 
     // Check that the call fails if iptables returns an error.
     std::string veryLongStringName = "netd_binder_test_UnacceptablyLongIptablesChainName";