Consolidate iptables chain management.

Move creation and management of module iptables chains up into
CommandListener, which gives better visibility into ordering.

Change-Id: If0c94187c6e59a20840b035d7241057f45a0f74b

BandwidthController.cpp

@@ -51,6 +51,11 @@ extern "C" int system_nosh(const char *command);
 #define ALERT_IPT_TEMPLATE "%s %s %s -m quota2 ! --quota %lld --name %s"
 const int  BandwidthController::ALERT_RULE_POS_IN_COSTLY_CHAIN = 4;
 const char BandwidthController::ALERT_GLOBAL_NAME[] = "globalAlert";
+const char* BandwidthController::LOCAL_INPUT = "bw_INPUT";
+const char* BandwidthController::LOCAL_FORWARD = "bw_FORWARD";
+const char* BandwidthController::LOCAL_OUTPUT = "bw_OUTPUT";
+const char* BandwidthController::LOCAL_RAW_PREROUTING = "bw_raw_PREROUTING";
+const char* BandwidthController::LOCAL_MANGLE_POSTROUTING = "bw_mangle_POSTROUTING";
 const int  BandwidthController::MAX_CMD_ARGS = 32;
 const int  BandwidthController::MAX_CMD_LEN = 1024;
 const int  BandwidthController::MAX_IFACENAME_LEN = 64;
@@ -113,42 +118,13 @@ const char *BandwidthController::IPT_FLUSH_COMMANDS[] = {
 
 /* The cleanup commands assume flushing has been done. */
 const char *BandwidthController::IPT_CLEANUP_COMMANDS[] = {
-    /* Delete hooks to custom chains. */
-    "-D INPUT -j bw_INPUT",
-    "-D OUTPUT -j bw_OUTPUT",
-    "-D FORWARD -j bw_FORWARD",
-
-    "-t raw -D bw_raw_PREROUTING",
-    "-t mangle -D bw_mangle_POSTROUTING",
-
-    "-X bw_INPUT",
-    "-X bw_OUTPUT",
-    "-X bw_FORWARD",
     "-X penalty_box",
     "-X costly_shared",
-
-    "-t raw -X bw_raw_PREROUTING",
-    "-t mangle -X bw_mangle_POSTROUTING",
 };
 
 const char *BandwidthController::IPT_SETUP_COMMANDS[] = {
-    /* Created needed chains. */
-    "-N bw_INPUT",
-    "-A INPUT -j bw_INPUT",
-
-    "-N bw_OUTPUT",
-    "-A OUTPUT -j bw_OUTPUT",
-
-    "-N bw_FORWARD",
-    "-I FORWARD -j bw_FORWARD",
-
     "-N costly_shared",
     "-N penalty_box",
-
-    "-t raw -N bw_raw_PREROUTING",
-    "-t raw -A PREROUTING -j bw_raw_PREROUTING",
-    "-t mangle -N bw_mangle_POSTROUTING",
-    "-t mangle -A POSTROUTING -j bw_mangle_POSTROUTING",
 };
 
 const char *BandwidthController::IPT_BASIC_ACCOUNTING_COMMANDS[] = {
@@ -253,7 +229,6 @@ int BandwidthController::setupIptablesHooks(void) {
             IPT_SETUP_COMMANDS, RunCmdFailureBad);
 
     return 0;
-
 }
 
 int BandwidthController::enableBandwidthControl(bool force) {

BandwidthController.h

@@ -80,6 +80,12 @@ class BandwidthController {
      */
     int getTetherStats(TetherStats &stats, std::string &extraProcessingInfo);
 
+    static const char* LOCAL_INPUT;
+    static const char* LOCAL_FORWARD;
+    static const char* LOCAL_OUTPUT;
+    static const char* LOCAL_RAW_PREROUTING;
+    static const char* LOCAL_MANGLE_POSTROUTING;
+
 protected:
     class QuotaInfo {
     public:

CommandListener.cpp

@@ -40,7 +40,7 @@
 #include "IdletimerController.h"
 #include "SecondaryTableController.h"
 #include "oem_iptables_hook.h"
-
+#include "NetdConstants.h"
 
 TetherController *CommandListener::sTetherCtrl = NULL;
 NatController *CommandListener::sNatCtrl = NULL;
@@ -52,6 +52,70 @@ IdletimerController * CommandListener::sIdletimerCtrl = NULL;
 ResolverController *CommandListener::sResolverCtrl = NULL;
 SecondaryTableController *CommandListener::sSecondaryTableCtrl = NULL;
 
+/**
+ * List of module chains to be created, along with explicit ordering. ORDERING
+ * IS CRITICAL, AND SHOULD BE TRIPLE-CHECKED WITH EACH CHANGE.
+ */
+static const char* FILTER_INPUT[] = {
+        BandwidthController::LOCAL_INPUT,
+        NULL,
+};
+
+static const char* FILTER_FORWARD[] = {
+        OEM_IPTABLES_FILTER_FORWARD,
+        BandwidthController::LOCAL_FORWARD,
+        NatController::LOCAL_FORWARD,
+        NULL,
+};
+
+static const char* FILTER_OUTPUT[] = {
+        OEM_IPTABLES_FILTER_OUTPUT,
+        BandwidthController::LOCAL_OUTPUT,
+        NULL,
+};
+
+static const char* RAW_PREROUTING[] = {
+        BandwidthController::LOCAL_RAW_PREROUTING,
+        NULL,
+};
+
+static const char* MANGLE_POSTROUTING[] = {
+        BandwidthController::LOCAL_MANGLE_POSTROUTING,
+        NULL,
+};
+
+static const char* NAT_PREROUTING[] = {
+        OEM_IPTABLES_NAT_PREROUTING,
+        IdletimerController::LOCAL_NAT_PREROUTING,
+        NULL,
+};
+
+static const char* NAT_POSTROUTING[] = {
+        IdletimerController::LOCAL_NAT_POSTROUTING,
+        NatController::LOCAL_NAT_POSTROUTING,
+        NULL,
+};
+
+static void createChildChains(IptablesTarget target, const char* table, const char* parentChain,
+        const char** childChains) {
+    const char** childChain = childChains;
+    do {
+        // Order is important:
+        // -D to delete any pre-existing jump rule (removes references
+        //    that would prevent -X from working)
+        // -F to flush any existing chain
+        // -X to delete any existing chain
+        // -N to create the chain
+        // -A to append the chain to parent
+
+        execIptablesSilently(target, "-t", table, "-D", parentChain, "-j", *childChain, NULL);
+        execIptablesSilently(target, "-t", table, "-F", *childChain, NULL);
+        execIptablesSilently(target, "-t", table, "-X", *childChain, NULL);
+        execIptables(target, "-t", table, "-N", *childChain, NULL);
+        execIptables(target, "-t", table, "-A", parentChain, "-j", *childChain, NULL);
+    } while (*(++childChain) != NULL);
+}
+
 CommandListener::CommandListener() :
                  FrameworkListener("netd", true) {
     registerCmd(new InterfaceCmd());
@@ -86,14 +150,27 @@ CommandListener::CommandListener() :
         sResolverCtrl = new ResolverController();
 
     /*
-     * This is the only time controllers are allowed to touch
-     * top-level chains in iptables.
-     * Each controller should setup custom chains and hook them into
-     * the top-level ones.
-     * THE ORDER IS IMPORTANT. TRIPPLE CHECK EACH setup function.
+     * This is the only time we touch top-level chains in iptables; controllers
+     * should only mutate rules inside of their children chains, as created by
+     * the constants above.
+     *
+     * Modules should never ACCEPT packets (except in well-justified cases);
+     * they should instead defer to any remaining modules using RETURN, or
+     * otherwise DROP/REJECT.
      */
-    /* Does DROP in nat: PREROUTING, FORWARD, OUTPUT */
+
+    // Create chains for children modules
+    createChildChains(V4V6, "filter", "INPUT", FILTER_INPUT);
+    createChildChains(V4V6, "filter", "FORWARD", FILTER_FORWARD);
+    createChildChains(V4V6, "filter", "OUTPUT", FILTER_OUTPUT);
+    createChildChains(V4V6, "raw", "PREROUTING", RAW_PREROUTING);
+    createChildChains(V4V6, "mangle", "POSTROUTING", MANGLE_POSTROUTING);
+    createChildChains(V4, "nat", "PREROUTING", NAT_PREROUTING);
+    createChildChains(V4, "nat", "POSTROUTING", NAT_POSTROUTING);
+
+    // Let each module setup their child chains
     setupOemIptablesHook();
+
     /* Does DROPs in FORWARD by default */
     sNatCtrl->setupIptablesHooks();
     /*

IdletimerController.cpp

@@ -90,6 +90,9 @@
 
 extern "C" int system_nosh(const char *command);
 
+const char* IdletimerController::LOCAL_NAT_PREROUTING = "idletimer_nat_PREROUTING";
+const char* IdletimerController::LOCAL_NAT_POSTROUTING = "idletimer_nat_POSTROUTING";
+
 IdletimerController::IdletimerController() {
 }
 
@@ -115,18 +118,6 @@ int IdletimerController::runIpxtablesCmd(const char *cmd) {
 }
 
 bool IdletimerController::setupIptablesHooks() {
-    runIpxtablesCmd("-t nat -D PREROUTING -j idletimer_nat_PREROUTING");
-    runIpxtablesCmd("-t nat -F idletimer_nat_PREROUTING");
-    runIpxtablesCmd("-t nat -N idletimer_nat_PREROUTING");
-
-    runIpxtablesCmd("-t nat -D POSTROUTING -j idletimer_nat_POSTROUTING");
-    runIpxtablesCmd("-t nat -F idletimer_nat_POSTROUTING");
-    runIpxtablesCmd("-t nat -N idletimer_nat_POSTROUTING");
-
-    if (runIpxtablesCmd("-t nat -I PREROUTING -j idletimer_nat_PREROUTING")
-        || runIpxtablesCmd("-t nat -I POSTROUTING -j idletimer_nat_POSTROUTING")) {
-        return false;
-    }
     return true;
 }
 

IdletimerController.h

@@ -30,6 +30,9 @@ class IdletimerController {
                                  const char *classLabel);
     bool setupIptablesHooks();
 
+    static const char* LOCAL_NAT_PREROUTING;
+    static const char* LOCAL_NAT_POSTROUTING;
+
  private:
     enum IptOp { IptOpAdd, IptOpDelete };
     int setDefaults();

NatController.cpp

@@ -35,6 +35,9 @@
 
 extern "C" int system_nosh(const char *command);
 
+const char* NatController::LOCAL_FORWARD = "natctrl_FORWARD";
+const char* NatController::LOCAL_NAT_POSTROUTING = "natctrl_nat_POSTROUTING";
+
 NatController::NatController(SecondaryTableController *ctrl) {
     secondaryTableCtrl = ctrl;
 }
@@ -61,30 +64,6 @@ int NatController::runCmd(const char *path, const char *cmd) {
 }
 
 int NatController::setupIptablesHooks() {
-    if (runCmd(IPTABLES_PATH, "-P INPUT ACCEPT"))
-        return -1;
-    if (runCmd(IPTABLES_PATH, "-P OUTPUT ACCEPT"))
-        return -1;
-    if (runCmd(IPTABLES_PATH, "-P FORWARD ACCEPT"))
-        return -1;
-
-    // Order is important!
-    // -D to delete any pre-existing jump rule, to prevent dupes (no-op if doesn't exist)
-    // -F to flush the chain (no-op if doesn't exist).
-    // -N to create the chain (no-op if already exist).
-
-    runCmd(IPTABLES_PATH, "-D FORWARD -j natctrl_FORWARD");
-    runCmd(IPTABLES_PATH, "-F natctrl_FORWARD");
-    runCmd(IPTABLES_PATH, "-N natctrl_FORWARD");
-    if (runCmd(IPTABLES_PATH, "-A FORWARD -j natctrl_FORWARD"))
-        return -1;
-
-    runCmd(IPTABLES_PATH, "-t nat -D POSTROUTING -j natctrl_nat_POSTROUTING");
-    runCmd(IPTABLES_PATH, "-t nat -F natctrl_nat_POSTROUTING");
-    runCmd(IPTABLES_PATH, "-t nat -N natctrl_nat_POSTROUTING");
-    if (runCmd(IPTABLES_PATH, "-t nat -A POSTROUTING -j natctrl_nat_POSTROUTING"))
-        return -1;
-
     setDefaults();
     return 0;
 }

NatController.h

@@ -31,6 +31,9 @@ class NatController {
     int disableNat(const int argc, char **argv);
     int setupIptablesHooks();
 
+    static const char* LOCAL_FORWARD;
+    static const char* LOCAL_NAT_POSTROUTING;
+
 private:
     int natCount;
     SecondaryTableController *secondaryTableCtrl;

NetdConstants.cpp

@@ -14,6 +14,10 @@
  * limitations under the License.
  */
 
+#include <string.h>
+
+#include <cutils/log.h>
+
 #include "NetdConstants.h"
 
 const char * const OEM_SCRIPT_PATH = "/system/bin/oem-iptables-init.sh";
@@ -23,3 +27,71 @@ const char * const TC_PATH = "/system/bin/tc";
 const char * const IP_PATH = "/system/bin/ip";
 const char * const ADD = "add";
 const char * const DEL = "del";
+
+static void logExecError(const char* argv[], int res) {
+    const char** argp = argv;
+    std::string args = "";
+    while (*argp) {
+        args += *argp;
+        args += ' ';
+        argp++;
+    }
+    ALOGE("exec() res=%d for %s", res, args.c_str());
+}
+
+static int execIptables(IptablesTarget target, bool silent, va_list args) {
+    /* Read arguments from incoming va_list; we expect the list to be NULL terminated. */
+    std::list<const char*> argsList;
+    argsList.push_back(NULL);
+    const char* arg;
+    do {
+        arg = va_arg(args, const char *);
+        argsList.push_back(arg);
+    } while (arg);
+
+    int i = 0;
+    const char* argv[argsList.size()];
+    std::list<const char*>::iterator it;
+    for (it = argsList.begin(); it != argsList.end(); it++, i++) {
+        argv[i] = *it;
+    }
+
+    int res = 0;
+    if (target == V4 || target == V4V6) {
+        argv[0] = IPTABLES_PATH;
+        int localRes = fork_and_execve(argv[0], argv);
+        if (localRes) {
+            if (!silent) {
+                logExecError(argv, localRes);
+            }
+            res |= localRes;
+        }
+    }
+    if (target == V6 || target == V4V6) {
+        argv[0] = IP6TABLES_PATH;
+        int localRes = fork_and_execve(argv[0], argv);
+        if (localRes) {
+            if (!silent) {
+                logExecError(argv, localRes);
+            }
+            res |= localRes;
+        }
+    }
+    return res;
+}
+
+int execIptables(IptablesTarget target, ...) {
+    va_list args;
+    va_start(args, target);
+    int res = execIptables(target, false, args);
+    va_end(args);
+    return res;
+}
+
+int execIptablesSilently(IptablesTarget target, ...) {
+    va_list args;
+    va_start(args, target);
+    int res = execIptables(target, true, args);
+    va_end(args);
+    return res;
+}

NetdConstants.h

@@ -17,6 +17,9 @@
 #ifndef _NETD_CONSTANTS_H
 #define _NETD_CONSTANTS_H
 
+#include <string>
+#include <list>
+#include <stdarg.h>
 
 extern const char * const IPTABLES_PATH;
 extern const char * const IP6TABLES_PATH;
@@ -26,4 +29,11 @@ extern const char * const OEM_SCRIPT_PATH;
 extern const char * const ADD;
 extern const char * const DEL;
 
+extern "C" int fork_and_execve(const char*, const char*[]);
+
+enum IptablesTarget { V4, V6, V4V6 };
+
+int execIptables(IptablesTarget target, ...);
+int execIptablesSilently(IptablesTarget target, ...);
+
 #endif