[netcf-devel] [PATCH 2/2] Remove code that modifies iptables config for bridges

Laine Stump laine at laine.org
Wed Sep 15 15:52:25 UTC 2010


Originally, if net.bridge.bridge-nf-call-iptables was set to 1, netcf
would check for the presence of the rule

   -I FORWARD -m physdev --physdev-is-bridged -j ACCEPT

in the iptables setup of a system, and add it if it wasn't there. For
several reasons, it's been agreed that this code should be removed:

1) This is firewall policy, not interface config, and shouldn't be
   modified unconditionally.

2) The default is for net.bridge.bridge-nf-call-iptables to be 0, so
   normally this code wouldn't be needed anyway (and if somebody has
   changed the setting from the default, they probably had a good
   reason, and we shouldn't be silently negating the effects of that
   change).

3) There are multiple places that this config may be located, and
   multiple ways it could be added (lokkit vs. manually), and trying
   to handle all of these has proven to be error prone. More than half
   of the bugs filed against netcf have been related to this code.
---
 src/drv_initscripts.c |  143 -------------------------------------------------
 src/dutil.c           |    5 --
 src/dutil_linux.c     |   31 -----------
 src/dutil_linux.h     |    4 --
 4 files changed, 0 insertions(+), 183 deletions(-)

diff --git a/src/drv_initscripts.c b/src/drv_initscripts.c
index 1b9261b..b2d96cb 100644
--- a/src/drv_initscripts.c
+++ b/src/drv_initscripts.c
@@ -86,25 +86,6 @@ static const struct augeas_xfm_table augeas_xfm_common =
     { .size = ARRAY_CARDINALITY(augeas_xfm_common_pv),
       .pv = augeas_xfm_common_pv };
 
-static const struct augeas_pv augeas_xfm_iptables_pv[] = {
-    /* iptables config */
-    { "/augeas/load/Iptables/lens", "Iptables.lns" },
-    { "/augeas/load/Iptables/incl", "/etc/sysconfig/iptables" },
-    /* lokkit */
-    { "/augeas/load/Lokkit/lens", "Lokkit.lns" },
-    { "/augeas/load/Lokkit/incl", "/etc/sysconfig/system-config-firewall" },
-};
-
-static const struct augeas_xfm_table augeas_xfm_iptables =
-    { .size = ARRAY_CARDINALITY(augeas_xfm_iptables_pv),
-      .pv = augeas_xfm_iptables_pv };
-
-static const char *const prog_lokkit = "/usr/sbin/lokkit";
-static const char *const lokkit_custom_rules =
-    "--custom-rules=ipv4:filter:" DATADIR "/netcf/iptables-forward-bridged";
-
-static const char *const prog_rc_d_iptables = "/etc/init.d/iptables";
-
 /* Entries in a ifcfg file that tell us that the interface
  * is not a toplevel interface
  */
@@ -358,127 +339,6 @@ static int list_interfaces(struct netcf *ncf, char ***intf) {
     return -1;
 }
 
-/* Ensure we have an iptables rule to bridge physdevs. We take care of both
- * systems using iptables directly, and systems using lokkit (even if it's
- * only installed, but not used)
- */
-static void bridge_physdevs(struct netcf *ncf) {
-    struct augeas *aug = NULL;
-    char *path = NULL, *p = NULL;
-    const char *argv[5];
-    int have_lokkit, use_lokkit;
-    int r, nmatches;
-
-    /* If bridge packets never hit iptables, nothing to worry about */
-    if (! bridge_nf_call_iptables(ncf))
-        return;
-    ERR_BAIL(ncf);
-
-    MEMZERO(argv, ARRAY_CARDINALITY(argv));
-
-    add_augeas_xfm_table(ncf, &augeas_xfm_iptables);
-
-    aug = get_augeas(ncf);
-    ERR_BAIL(ncf);
-
-    defnode(ncf, "iptables", NULL, "/files/etc/sysconfig/iptables");
-    ERR_BAIL(ncf);
-
-    nmatches = aug_match(aug,
-      "$iptables/table[ . = 'filter']/*[. = 'FORWARD'][match = 'physdev']", NULL);
-    ERR_THROW(nmatches < 0, ncf, EOTHER, "failed to look for bridge");
-    if (nmatches > 0)
-        goto error;
-
-    have_lokkit = access(prog_lokkit, X_OK) == 0;
-    use_lokkit = aug_match(aug,
-      "$iptables/#comment[. = 'Firewall configuration written by system-config-firewall']", NULL);
-    ERR_THROW(use_lokkit < 0, ncf, EOTHER, "failed to look for lokkit");
-
-    if (have_lokkit) {
-        const char *rules_file = strrchr(lokkit_custom_rules, ':') + 1;
-        int created;
-
-        defnode(ncf, "fw", NULL, "/files/etc/sysconfig/system-config-firewall");
-        ERR_BAIL(ncf);
-
-        created = defnode(ncf, "fw_custom", rules_file,
-                          "$fw/custom-rules[. = '%s']", rules_file);
-        ERR_BAIL(ncf);
-
-        if (created) {
-            r = aug_set(aug, "$fw_custom", rules_file);
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-
-            r = aug_set(aug, "$fw_custom/type", "ipv4");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            FREE(p);
-
-            r = aug_set(aug, "$fw_custom/table", "filter");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-
-            FREE(p);
-
-            r = aug_save(aug);
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-        }
-        FREE(path);
-
-        if (use_lokkit) {
-            argv[0] = prog_lokkit;
-            argv[1] = "--update";
-            r = run_program(ncf, argv);
-            ERR_BAIL(ncf);
-        }
-    }
-
-    if (! use_lokkit) {
-        int created = defnode(ncf, "ipt_filter", NULL,
-                              "$iptables/table[. = 'filter']");
-        ERR_BAIL(ncf);
-
-        if (created) {
-            r = aug_set(aug, "$ipt_filter", "filter");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            r = aug_set(aug, "$ipt_filter/chain[1]", "INPUT");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            r = aug_set(aug, "$ipt_filter/chain[1]/policy", "ACCEPT");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            r = aug_set(aug, "$ipt_filter/chain[2]", "FORWARD");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            r = aug_set(aug, "$ipt_filter/chain[2]/policy", "ACCEPT");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            r = aug_set(aug, "$ipt_filter/chain[3]", "OUTPUT");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-            r = aug_set(aug, "$ipt_filter/chain[3]/policy", "ACCEPT");
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-        } else {
-            r = aug_insert(aug, "$ipt_filter/chain[last()]", "append", 0);
-            ERR_COND_BAIL(r < 0, ncf, EOTHER);
-        }
-        r = aug_set(aug, "$ipt_filter/append[1]", "FORWARD");
-        r = aug_set(aug, "$ipt_filter/append[1]/match", "physdev");
-        ERR_COND_BAIL(r < 0, ncf, EOTHER);
-        r = aug_set(aug, "$ipt_filter/append[1]/physdev-is-bridged", NULL);
-        ERR_COND_BAIL(r < 0, ncf, EOTHER);
-        r = aug_set(aug, "$ipt_filter/append[1]/jump", "ACCEPT");
-        ERR_COND_BAIL(r < 0, ncf, EOTHER);
-
-        r = aug_save(aug);
-        ERR_COND_BAIL(r < 0, ncf, EOTHER);
-
-        argv[0] = prog_rc_d_iptables;
-        argv[1] = "condrestart";
-        r = run_program(ncf, argv);
-        ERR_BAIL(ncf);
-    }
- error:
-    remove_augeas_xfm_table(ncf, &augeas_xfm_iptables);
-    free(path);
-    free(p);
-    return;
-}
-
 int drv_init(struct netcf *ncf) {
     int r;
     struct stat stats;
@@ -506,9 +366,6 @@ int drv_init(struct netcf *ncf) {
     ncf->driver->rng = rng_parse(ncf, "interface.rng");
     ERR_BAIL(ncf);
 
-    bridge_physdevs(ncf);
-    ERR_BAIL(ncf);
-
     /* open a socket for interface ioctls */
     ncf->driver->ioctl_fd = init_ioctl_fd(ncf);
     if (ncf->driver->ioctl_fd < 0)
diff --git a/src/dutil.c b/src/dutil.c
index 2a6eaa0..fd37e16 100644
--- a/src/dutil.c
+++ b/src/dutil.c
@@ -166,11 +166,6 @@ struct augeas *get_augeas(struct netcf *ncf) {
 
     if (ncf->driver->load_augeas) {
         struct augeas *aug = ncf->driver->augeas;
-        /* Undefine all our variables to work around bug 79 in Augeas */
-        aug_defvar(aug, "iptables", NULL);
-        aug_defvar(aug, "fw", NULL);
-        aug_defvar(aug, "fw_custom", NULL);
-        aug_defvar(aug, "ipt_filter", NULL);
 
         r = aug_load(aug);
         ERR_THROW(r < 0, ncf, EOTHER, "failed to load config files");
diff --git a/src/dutil_linux.c b/src/dutil_linux.c
index 5054958..06a9650 100644
--- a/src/dutil_linux.c
+++ b/src/dutil_linux.c
@@ -151,37 +151,6 @@ void modprobed_unalias_bond(struct netcf *ncf, const char *name) {
     FREE(path);
 }
 
-bool bridge_nf_call_iptables(struct netcf *ncf) {
-    static const char *const proc_bridge_nf_call_iptables =
-        "proc/sys/net/bridge/bridge-nf-call-iptables";
-    char *path = NULL;
-    FILE *proc = NULL;
-    int r, c;
-    bool result = false;
-
-    r = xasprintf(&path, "%s%s", ncf->root, proc_bridge_nf_call_iptables);
-    ERR_NOMEM(r < 0, ncf);
-
-    proc = fopen(path, "r");
-    /* If the file does not exist, assume the bridge module is not loaded
-     * and just report that no bridge packets will hit iptables */
-    if (proc == NULL && errno == ENOENT)
-        goto error;
-
-    ERR_THROW(proc == NULL, ncf, EFILE, "can not open %s: %s",
-              path, strerror(errno));
-    c = fgetc(proc);
-    ERR_THROW(c == EOF, ncf, EFILE, "nothing to read from %s: %s",
-              path, strerror(errno));
-    result = (c == '1');
-
- error:
-    FREE(path);
-    if (proc != NULL)
-        fclose(proc);
-    return result;
-}
-
 /*
  * Local variables:
  *  indent-tabs-mode: nil
diff --git a/src/dutil_linux.h b/src/dutil_linux.h
index 315820e..af76c3e 100644
--- a/src/dutil_linux.h
+++ b/src/dutil_linux.h
@@ -45,10 +45,6 @@ void modprobed_alias_bond(struct netcf *ncf, const char *name);
 /* Remove an 'alias NAME bonding' as created by modprobed_alias_bond */
 void modprobed_unalias_bond(struct netcf *ncf, const char *name);
 
-/* Look at /proc/sys/net/bridge/bridge-nf-call-iptables and return TRUE
- * if bridge packets pass through iptables, FALSE otherwise
- */
-bool bridge_nf_call_iptables(struct netcf *ncf);
 #endif
 
 /*
-- 
1.7.2.2



More information about the netcf-devel mailing list