diff options
author | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-10-15 11:55:13 +0000 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2021-10-15 11:55:13 +0000 |
commit | 1b94e954369103493b52f9cb78860590e63d0304 (patch) | |
tree | 7c671c07cab1211792eba3b292b33899fb1ab099 | |
parent | 4580b915386f585e72ce0afa567450c78d6f4d65 (diff) | |
parent | 25e561c47d1bad3f4edb9ee4c457b6254965db5e (diff) | |
download | netd-android-12.0.0_r13.tar.gz |
Merge cherrypicks of [16064396, 16064361] into sparse-7825780-L03300000951566358android-12.0.0_r13android12-d1-s5-release
Change-Id: I7bdc1616b957b801df66deeebf54f9c86eb939f1
-rw-r--r-- | server/FirewallController.cpp | 26 | ||||
-rw-r--r-- | server/FirewallController.h | 55 | ||||
-rw-r--r-- | server/FirewallControllerTest.cpp | 23 | ||||
-rw-r--r-- | tests/binder_test.cpp | 13 |
4 files changed, 62 insertions, 55 deletions
diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp index 0a0f8d82..35fd1e20 100644 --- a/server/FirewallController.cpp +++ b/server/FirewallController.cpp @@ -90,7 +90,8 @@ FirewallController::FirewallController(void) : mMaxUid(discoverMaximumValidUid(k } 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,21 +127,24 @@ int FirewallController::setFirewallType(FirewallType ftype) { return res ? -EREMOTEIO : 0; } -int FirewallController::resetFirewall(void) { - mFirewallType = ALLOWLIST; - mIfaceRules.clear(); - - // flush any existing rules +int FirewallController::flushRules() { std::string command = - "*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"; return (execIptablesRestore(V4V6, command.c_str()) == 0) ? 0 : -EREMOTEIO; } +int FirewallController::resetFirewall(void) { + mFirewallType = ALLOWLIST; + mIfaceRules.clear(); + return flushRules(); +} + int FirewallController::enableChildChains(ChildChain chain, bool enable) { int res = 0; const char* name; diff --git a/server/FirewallController.h b/server/FirewallController.h index 6cabfb52..6de1b45a 100644 --- a/server/FirewallController.h +++ b/server/FirewallController.h @@ -57,46 +57,46 @@ enum ChildChain { */ 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 @@ private: 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 df6ca82d..1f199af6 100644 --- a/server/FirewallControllerTest.cpp +++ b/server/FirewallControllerTest.cpp @@ -227,19 +227,18 @@ TEST_F(FirewallControllerTest, TestEnableChildChains) { 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 22d1f226..e80296a5 100644 --- a/tests/binder_test.cpp +++ b/tests/binder_test.cpp @@ -2420,11 +2420,14 @@ void expectFirewallAllowlistMode() { } 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, |