[NETD-TC#6] Clean up the dependency between FirewallController and
TrafficController.
Test: m; flash; boot
Test: atest FirewallControllerTest TrafficControllerTest
Change-Id: I0a8f3f2e9c1f4510021570e7894a56e4998f3ede
diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index 31e34b3..baea708 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -39,7 +39,6 @@
using android::base::Join;
using android::base::StringAppendF;
using android::base::StringPrintf;
-using android::net::gCtls;
namespace android {
namespace net {
@@ -52,11 +51,6 @@
const char* FirewallController::LOCAL_OUTPUT = "fw_OUTPUT";
const char* FirewallController::LOCAL_FORWARD = "fw_FORWARD";
-const char* FirewallController::LOCAL_DOZABLE = "fw_dozable";
-const char* FirewallController::LOCAL_STANDBY = "fw_standby";
-const char* FirewallController::LOCAL_POWERSAVE = "fw_powersave";
-const char* FirewallController::LOCAL_RESTRICTED = "fw_restricted";
-
// ICMPv6 types that are required for any form of IPv6 connectivity to work. Note that because the
// fw_dozable chain is called from both INPUT and OUTPUT, this includes both packets that we need
// to be able to send (e.g., RS, NS), and packets that we need to receive (e.g., RA, NA).
@@ -120,10 +114,6 @@
return flushRules();
}
-int FirewallController::enableChildChains(ChildChain chain, bool enable) {
- return gCtls->trafficCtrl.toggleUidOwnerMap(chain, enable);
-}
-
int FirewallController::isFirewallEnabled(void) {
// TODO: verify that rules are still in place near top
return -1;
@@ -163,28 +153,6 @@
return (execIptablesRestore(V4V6, command) == 0) ? 0 : -EREMOTEIO;
}
-FirewallType FirewallController::getFirewallType(ChildChain chain) {
- switch(chain) {
- case DOZABLE:
- return ALLOWLIST;
- case STANDBY:
- return DENYLIST;
- case POWERSAVE:
- return ALLOWLIST;
- case RESTRICTED:
- return ALLOWLIST;
- case NONE:
- return mFirewallType;
- default:
- return DENYLIST;
- }
-}
-
-int FirewallController::setUidRule(ChildChain chain, int uid, FirewallRule rule) {
- FirewallType firewallType = getFirewallType(chain);
- return gCtls->trafficCtrl.changeUidOwnerRule(chain, uid, rule, firewallType);
-}
-
/* static */
std::string FirewallController::makeCriticalCommands(IptablesTarget target, const char* chainName) {
// Allow ICMPv6 packets necessary to make IPv6 connectivity work. http://b/23158230 .
diff --git a/server/FirewallController.h b/server/FirewallController.h
index 0379222..0de96cf 100644
--- a/server/FirewallController.h
+++ b/server/FirewallController.h
@@ -23,30 +23,12 @@
#include <string>
#include <vector>
-#include "android/net/INetd.h"
-
#include "NetdConstants.h"
#include "bpf/BpfUtils.h"
namespace android {
namespace net {
-enum FirewallRule { ALLOW = INetd::FIREWALL_RULE_ALLOW, DENY = INetd::FIREWALL_RULE_DENY };
-
-// ALLOWLIST means the firewall denies all by default, uids must be explicitly ALLOWed
-// DENYLIST means the firewall allows all by default, uids must be explicitly DENYed
-
-enum FirewallType { ALLOWLIST = INetd::FIREWALL_ALLOWLIST, DENYLIST = INetd::FIREWALL_DENYLIST };
-
-enum ChildChain {
- NONE = INetd::FIREWALL_CHAIN_NONE,
- DOZABLE = INetd::FIREWALL_CHAIN_DOZABLE,
- STANDBY = INetd::FIREWALL_CHAIN_STANDBY,
- POWERSAVE = INetd::FIREWALL_CHAIN_POWERSAVE,
- RESTRICTED = INetd::FIREWALL_CHAIN_RESTRICTED,
- INVALID_CHAIN
-};
-
/*
* Simple firewall that drops all packets except those matching explicitly
* defined ALLOW rules.
@@ -82,11 +64,6 @@
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* ICMPV6_TYPES[];
std::mutex lock;
@@ -99,7 +76,6 @@
FirewallType mFirewallType;
std::set<std::string> mIfaceRules;
int flushRules(void);
- FirewallType getFirewallType(ChildChain);
};
} // namespace net
diff --git a/server/IptablesBaseTest.cpp b/server/IptablesBaseTest.cpp
index b3748cd..ef4d743 100644
--- a/server/IptablesBaseTest.cpp
+++ b/server/IptablesBaseTest.cpp
@@ -24,10 +24,10 @@
#include <android-base/stringprintf.h>
+#define LOG_TAG "IptablesBaseTest"
#include "IptablesBaseTest.h"
#include "NetdConstants.h"
-#define LOG_TAG "IptablesBaseTest"
#include <log/log.h>
using android::base::StringPrintf;
diff --git a/server/IptablesRestoreController.cpp b/server/IptablesRestoreController.cpp
index 10cedfa..b37c7c6 100644
--- a/server/IptablesRestoreController.cpp
+++ b/server/IptablesRestoreController.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#define LOG_TAG "IptablesRestoreController"
#include "IptablesRestoreController.h"
#include <poll.h>
@@ -21,7 +22,6 @@
#include <sys/wait.h>
#include <unistd.h>
-#define LOG_TAG "IptablesRestoreController"
#include <android-base/logging.h>
#include <android-base/file.h>
#include <netdutils/Syscalls.h>
diff --git a/server/IptablesRestoreControllerTest.cpp b/server/IptablesRestoreControllerTest.cpp
index 3881124..a05c76d 100644
--- a/server/IptablesRestoreControllerTest.cpp
+++ b/server/IptablesRestoreControllerTest.cpp
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+#define LOG_TAG "IptablesRestoreControllerTest"
+#include "IptablesRestoreController.h"
+
#include <fcntl.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -25,14 +28,12 @@
#include <iostream>
#include <string>
-#define LOG_TAG "IptablesRestoreControllerTest"
#include <android-base/stringprintf.h>
#include <android-base/strings.h>
#include <log/log.h>
#include <netdutils/MockSyscalls.h>
#include <netdutils/Stopwatch.h>
-#include "IptablesRestoreController.h"
#include "NetdConstants.h"
#include "bpf/BpfUtils.h"
diff --git a/server/NetdConstants.h b/server/NetdConstants.h
index c273e1b..2bcc44d 100644
--- a/server/NetdConstants.h
+++ b/server/NetdConstants.h
@@ -24,6 +24,8 @@
#include <mutex>
#include <string>
+#include "android/net/INetd.h"
+
#include <netdutils/UidConstants.h>
#include <private/android_filesystem_config.h>
@@ -74,4 +76,20 @@
*/
extern std::mutex gBigNetdLock;
+enum FirewallRule { ALLOW = INetd::FIREWALL_RULE_ALLOW, DENY = INetd::FIREWALL_RULE_DENY };
+
+// ALLOWLIST means the firewall denies all by default, uids must be explicitly ALLOWed
+// DENYLIST means the firewall allows all by default, uids must be explicitly DENYed
+
+enum FirewallType { ALLOWLIST = INetd::FIREWALL_ALLOWLIST, DENYLIST = INetd::FIREWALL_DENYLIST };
+
+enum ChildChain {
+ NONE = INetd::FIREWALL_CHAIN_NONE,
+ DOZABLE = INetd::FIREWALL_CHAIN_DOZABLE,
+ STANDBY = INetd::FIREWALL_CHAIN_STANDBY,
+ POWERSAVE = INetd::FIREWALL_CHAIN_POWERSAVE,
+ RESTRICTED = INetd::FIREWALL_CHAIN_RESTRICTED,
+ INVALID_CHAIN
+};
+
} // namespace android::net
diff --git a/server/NetdNativeService.cpp b/server/NetdNativeService.cpp
index 0d64236..1a8d820 100644
--- a/server/NetdNativeService.cpp
+++ b/server/NetdNativeService.cpp
@@ -276,7 +276,7 @@
const std::vector<int32_t>& uids,
bool* ret) {
NETD_LOCKING_RPC(gCtls->firewallCtrl.lock, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
- int err = gCtls->firewallCtrl.replaceUidChain(chainName, isAllowlist, uids);
+ int err = gCtls->trafficCtrl.replaceUidOwnerMap(chainName, isAllowlist, uids);
*ret = (err == 0);
return binder::Status::ok();
}
@@ -1239,8 +1239,9 @@
NETD_LOCKING_RPC(gCtls->firewallCtrl.lock, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
auto chain = static_cast<ChildChain>(childChain);
auto rule = static_cast<FirewallRule>(firewallRule);
+ FirewallType fType = gCtls->trafficCtrl.getFirewallType(chain);
- int res = gCtls->firewallCtrl.setUidRule(chain, uid, rule);
+ int res = gCtls->trafficCtrl.changeUidOwnerRule(chain, uid, rule, fType);
return statusFromErrcode(res);
}
@@ -1248,7 +1249,7 @@
NETD_LOCKING_RPC(gCtls->firewallCtrl.lock, PERM_NETWORK_STACK, PERM_MAINLINE_NETWORK_STACK);
auto chain = static_cast<ChildChain>(childChain);
- int res = gCtls->firewallCtrl.enableChildChains(chain, enable);
+ int res = gCtls->trafficCtrl.toggleUidOwnerMap(chain, enable);
return statusFromErrcode(res);
}
diff --git a/server/TrafficController.cpp b/server/TrafficController.cpp
index 6ff0806..37adcda 100644
--- a/server/TrafficController.cpp
+++ b/server/TrafficController.cpp
@@ -46,7 +46,6 @@
#include "TrafficController.h"
#include "bpf/BpfMap.h"
-#include "FirewallController.h"
#include "InterfaceController.h"
#include "NetlinkListener.h"
#include "netdutils/DumpWriter.h"
@@ -81,6 +80,11 @@
// map with tagged traffic entries.
constexpr int TOTAL_UID_STATS_ENTRIES_LIMIT = STATS_MAP_SIZE * 0.9;
+const char* TrafficController::LOCAL_DOZABLE = "fw_dozable";
+const char* TrafficController::LOCAL_STANDBY = "fw_standby";
+const char* TrafficController::LOCAL_POWERSAVE = "fw_powersave";
+const char* TrafficController::LOCAL_RESTRICTED = "fw_restricted";
+
static_assert(BPF_PERMISSION_INTERNET == INetd::PERMISSION_INTERNET,
"Mismatch between BPF and AIDL permissions: PERMISSION_INTERNET");
static_assert(BPF_PERMISSION_UPDATE_DEVICE_STATS == INetd::PERMISSION_UPDATE_DEVICE_STATS,
@@ -574,6 +578,22 @@
return netdutils::status::ok;
}
+FirewallType TrafficController::getFirewallType(ChildChain chain) {
+ switch (chain) {
+ case DOZABLE:
+ return ALLOWLIST;
+ case STANDBY:
+ return DENYLIST;
+ case POWERSAVE:
+ return ALLOWLIST;
+ case RESTRICTED:
+ return ALLOWLIST;
+ case NONE:
+ default:
+ return DENYLIST;
+ }
+}
+
int TrafficController::changeUidOwnerRule(ChildChain chain, uid_t uid, FirewallRule rule,
FirewallType type) {
Status res;
@@ -660,13 +680,13 @@
// FirewallRule rule = isAllowlist ? ALLOW : DENY;
// FirewallType type = isAllowlist ? ALLOWLIST : DENYLIST;
Status res;
- if (!name.compare(FirewallController::LOCAL_DOZABLE)) {
+ if (!name.compare(LOCAL_DOZABLE)) {
res = replaceRulesInMap(DOZABLE_MATCH, uids);
- } else if (!name.compare(FirewallController::LOCAL_STANDBY)) {
+ } else if (!name.compare(LOCAL_STANDBY)) {
res = replaceRulesInMap(STANDBY_MATCH, uids);
- } else if (!name.compare(FirewallController::LOCAL_POWERSAVE)) {
+ } else if (!name.compare(LOCAL_POWERSAVE)) {
res = replaceRulesInMap(POWERSAVE_MATCH, uids);
- } else if (!name.compare(FirewallController::LOCAL_RESTRICTED)) {
+ } else if (!name.compare(LOCAL_RESTRICTED)) {
res = replaceRulesInMap(RESTRICTED_MATCH, uids);
} else {
ALOGE("unknown chain name: %s", name.c_str());
diff --git a/server/TrafficController.h b/server/TrafficController.h
index 4e01fbe..367c0b2 100644
--- a/server/TrafficController.h
+++ b/server/TrafficController.h
@@ -19,7 +19,6 @@
#include <linux/bpf.h>
-#include "FirewallController.h"
#include "NetlinkListener.h"
#include "Network.h"
#include "android-base/thread_annotations.h"
@@ -122,6 +121,13 @@
void setPermissionForUids(int permission, const std::vector<uid_t>& uids) EXCLUDES(mMutex);
+ FirewallType getFirewallType(ChildChain);
+
+ static const char* LOCAL_DOZABLE;
+ static const char* LOCAL_STANDBY;
+ static const char* LOCAL_POWERSAVE;
+ static const char* LOCAL_RESTRICTED;
+
private:
/*
* mCookieTagMap: Store the corresponding tag and uid for a specific socket.
diff --git a/server/TrafficControllerTest.cpp b/server/TrafficControllerTest.cpp
index 12a3901..29ce250 100644
--- a/server/TrafficControllerTest.cpp
+++ b/server/TrafficControllerTest.cpp
@@ -35,7 +35,6 @@
#include <netdutils/MockSyscalls.h>
-#include "FirewallController.h"
#include "TrafficController.h"
#include "bpf/BpfUtils.h"