Never send packets with a source of ::1 on the wire.
Doing so is obviously invalid, and certain carriers will tear
down the connection if such packets are sent on their network.
This is done by adding an ip6tables rule to fw_OUTPUT that drops
all packets with a non-lo egress interface and a source of ::1.
Test: boot device, "adb root && adb shell ip6tables-save | egrep fw_OUTPUT"
Bug: 190368103
Bug: 198896920
Bug: 203096965
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ifb272d48705ba756ccd7bac806e4dc2dd7488cd5
Merged-In: Ifb272d48705ba756ccd7bac806e4dc2dd7488cd5
(cherry picked from commit 350dbdb697eac51310e48044ed57933944617cd5)
diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index 0a0f8d8..35fd1e2 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -90,7 +90,8 @@
}
int FirewallController::setupIptablesHooks(void) {
- int res = 0;
+ int res = flushRules();
+
// mUseBpfOwnerMatch should be removed, but it is still depended upon by test code.
mUseBpfOwnerMatch = true;
if (mUseBpfOwnerMatch) {
@@ -126,19 +127,22 @@
return res ? -EREMOTEIO : 0;
}
+int FirewallController::flushRules() {
+ std::string command =
+ "*filter\n"
+ ":fw_INPUT -\n"
+ ":fw_OUTPUT -\n"
+ ":fw_FORWARD -\n"
+ "-6 -A fw_OUTPUT ! -o lo -s ::1 -j DROP\n"
+ "COMMIT\n";
+
+ return (execIptablesRestore(V4V6, command.c_str()) == 0) ? 0 : -EREMOTEIO;
+}
+
int FirewallController::resetFirewall(void) {
mFirewallType = ALLOWLIST;
mIfaceRules.clear();
-
- // flush any existing rules
- std::string command =
- "*filter\n"
- ":fw_INPUT -\n"
- ":fw_OUTPUT -\n"
- ":fw_FORWARD -\n"
- "COMMIT\n";
-
- return (execIptablesRestore(V4V6, command.c_str()) == 0) ? 0 : -EREMOTEIO;
+ return flushRules();
}
int FirewallController::enableChildChains(ChildChain chain, bool enable) {
diff --git a/server/FirewallController.h b/server/FirewallController.h
index 6cabfb5..6de1b45 100644
--- a/server/FirewallController.h
+++ b/server/FirewallController.h
@@ -57,46 +57,46 @@
*/
class FirewallController {
public:
- FirewallController();
+ FirewallController();
- int setupIptablesHooks(void);
+ int setupIptablesHooks(void);
- int setFirewallType(FirewallType);
- int resetFirewall(void);
- int isFirewallEnabled(void);
+ int setFirewallType(FirewallType);
+ int resetFirewall(void);
+ int isFirewallEnabled(void);
- /* Match traffic going in/out over the given iface. */
- int setInterfaceRule(const char*, FirewallRule);
- /* Match traffic owned by given UID. This is specific to a particular chain. */
- int setUidRule(ChildChain, int, FirewallRule);
+ /* Match traffic going in/out over the given iface. */
+ int setInterfaceRule(const char*, FirewallRule);
+ /* Match traffic owned by given UID. This is specific to a particular chain. */
+ int setUidRule(ChildChain, int, FirewallRule);
- int enableChildChains(ChildChain, bool);
+ int enableChildChains(ChildChain, bool);
- int replaceUidChain(const std::string&, bool, const std::vector<int32_t>&);
+ int replaceUidChain(const std::string&, bool, const std::vector<int32_t>&);
- static std::string makeCriticalCommands(IptablesTarget target, const char* chainName);
- static uid_t discoverMaximumValidUid(const std::string& fileName);
+ static std::string makeCriticalCommands(IptablesTarget target, const char* chainName);
+ static uid_t discoverMaximumValidUid(const std::string& fileName);
- static const char* TABLE;
+ static const char* TABLE;
- static const char* LOCAL_INPUT;
- static const char* LOCAL_OUTPUT;
- static const char* LOCAL_FORWARD;
+ static const char* LOCAL_INPUT;
+ static const char* LOCAL_OUTPUT;
+ static const char* LOCAL_FORWARD;
- static const char* LOCAL_DOZABLE;
- static const char* LOCAL_STANDBY;
- static const char* LOCAL_POWERSAVE;
- static const char* LOCAL_RESTRICTED;
+ static const char* LOCAL_DOZABLE;
+ static const char* LOCAL_STANDBY;
+ static const char* LOCAL_POWERSAVE;
+ static const char* LOCAL_RESTRICTED;
- static const char* ICMPV6_TYPES[];
+ static const char* ICMPV6_TYPES[];
- std::mutex lock;
+ std::mutex lock;
protected:
- friend class FirewallControllerTest;
- std::string makeUidRules(IptablesTarget target, const char* name, bool isAllowlist,
- const std::vector<int32_t>& uids);
- static int (*execIptablesRestore)(IptablesTarget target, const std::string& commands);
+ friend class FirewallControllerTest;
+ std::string makeUidRules(IptablesTarget target, const char* name, bool isAllowlist,
+ const std::vector<int32_t>& uids);
+ static int (*execIptablesRestore)(IptablesTarget target, const std::string& commands);
private:
// Netd supports two cases, in both of which mMaxUid that derives from the uid mapping is const:
@@ -108,6 +108,7 @@
FirewallType mFirewallType;
bool mUseBpfOwnerMatch;
std::set<std::string> mIfaceRules;
+ int flushRules(void);
int attachChain(const char*, const char*);
int detachChain(const char*, const char*);
int createChain(const char*, FirewallType);
diff --git a/server/FirewallControllerTest.cpp b/server/FirewallControllerTest.cpp
index df6ca82..1f199af 100644
--- a/server/FirewallControllerTest.cpp
+++ b/server/FirewallControllerTest.cpp
@@ -227,19 +227,18 @@
TEST_F(FirewallControllerTest, TestFirewall) {
std::vector<std::string> enableCommands = {
- "*filter\n"
- "-A fw_INPUT -j DROP\n"
- "-A fw_OUTPUT -j REJECT\n"
- "-A fw_FORWARD -j REJECT\n"
- "COMMIT\n"
- };
+ "*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 = {
- "*filter\n"
- ":fw_INPUT -\n"
- ":fw_OUTPUT -\n"
- ":fw_FORWARD -\n"
- "COMMIT\n"
- };
+ "*filter\n"
+ ":fw_INPUT -\n"
+ ":fw_OUTPUT -\n"
+ ":fw_FORWARD -\n"
+ "-6 -A fw_OUTPUT ! -o lo -s ::1 -j DROP\n"
+ "COMMIT\n"};
std::vector<std::string> noCommands = {};
EXPECT_EQ(0, mFw.resetFirewall());
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index 22d1f22..e80296a 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -2420,11 +2420,14 @@
}
void expectFirewallDenylistMode() {
- for (const auto& binary : {IPTABLES_PATH, IP6TABLES_PATH}) {
- EXPECT_EQ(2, iptablesRuleLineLength(binary, FIREWALL_INPUT));
- EXPECT_EQ(2, iptablesRuleLineLength(binary, FIREWALL_OUTPUT));
- EXPECT_EQ(2, iptablesRuleLineLength(binary, FIREWALL_FORWARD));
- }
+ EXPECT_EQ(2, iptablesRuleLineLength(IPTABLES_PATH, FIREWALL_INPUT));
+ EXPECT_EQ(2, iptablesRuleLineLength(IPTABLES_PATH, FIREWALL_OUTPUT));
+ EXPECT_EQ(2, iptablesRuleLineLength(IPTABLES_PATH, FIREWALL_FORWARD));
+
+ // for IPv6 there is an extra OUTPUT rule to DROP ::1 sourced packets to non-loopback devices
+ EXPECT_EQ(2, iptablesRuleLineLength(IP6TABLES_PATH, FIREWALL_INPUT));
+ EXPECT_EQ(3, iptablesRuleLineLength(IP6TABLES_PATH, FIREWALL_OUTPUT));
+ EXPECT_EQ(2, iptablesRuleLineLength(IP6TABLES_PATH, FIREWALL_FORWARD));
}
bool iptablesFirewallInterfaceFirstRuleExists(const char* binary, const char* chainName,
