Properly handle per zone user chain rules by fixing multiple logic errors

 * Track running zone state in separate bit fields
 * Track IPv4 and IPv6 custom chain state separately
 * Extend flag bitfields to 32 bit

diff --git a/defaults.c b/defaults.c
index 973b3a7cadfa2cab7a6eb0ef4a40a0b6ab177f0f..95a9e22a041b3507ac23c5a4403a17e9389c52c1 100644 (file)
--- a/defaults.c
+++ b/defaults.c
@@ -88,7 +88,7 @@ const struct fw3_option fw3_default_opts[] = {
 
 static bool
 print_chains(enum fw3_table table, enum fw3_family family,
-             const char *fmt, uint16_t flags,
+             const char *fmt, uint32_t flags,
              const struct chain *chains, int n)
 {
        bool rv = false;
@@ -180,7 +180,7 @@ fw3_print_default_chains(enum fw3_table table, enum fw3_family family,
                          struct fw3_state *state)
 {
        struct fw3_defaults *defs = &state->defaults;
-       uint16_t mask = ~0;
+       uint32_t custom_mask = ~0;
 
 #define policy(t) \
        ((t == FW3_TARGET_REJECT) ? "DROP" : fw3_flag_names[t])
@@ -194,9 +194,9 @@ fw3_print_default_chains(enum fw3_table table, enum fw3_family family,
 
        /* user chains already loaded, don't create again */
        if (hasbit(state->running_defaults.flags, FW3_DEFAULT_CUSTOM_CHAINS))
-               delbit(mask, FW3_DEFAULT_CUSTOM_CHAINS);
+               delbit(custom_mask, FW3_DEFAULT_CUSTOM_CHAINS);
 
-       print_chains(table, family, ":%s - [0:0]\n", defs->flags & mask,
+       print_chains(table, family, ":%s - [0:0]\n", defs->flags & custom_mask,
                     default_chains, ARRAY_SIZE(default_chains));
 }
 
@@ -207,9 +207,9 @@ fw3_print_default_head_rules(enum fw3_table table, enum fw3_family family,
        int i;
        struct fw3_defaults *defs = &state->defaults;
        const char *chains[] = {
-               "input",
-               "output",
-               "forward",
+               "input", "input",
+               "output", "output",
+               "forward", "forwarding",
        };
 
        print_chains(table, family, "-A %s\n", 0,
@@ -223,17 +223,15 @@ fw3_print_default_head_rules(enum fw3_table table, enum fw3_family family,
 
                if (defs->custom_chains)
                {
-                       fw3_pr("-A delegate_input -j input_rule "
-                              "-m comment --comment \"user chain for input\"\n");
-
-                       fw3_pr("-A delegate_output -j output_rule "
-                              "-m comment --comment \"user chain for output\"\n");
-
-                       fw3_pr("-A delegate_forward -j forwarding_rule "
-                              "-m comment --comment \"user chain for forwarding\"\n");
+                       for (i = 0; i < ARRAY_SIZE(chains); i += 2)
+                       {
+                               fw3_pr("-A delegate_%s -m comment "
+                                      "--comment \"user chain for %s\" -j %s_rule\n",
+                                          chains[i], chains[i+1], chains[i+1]);
+                       }
                }
 
-               for (i = 0; i < ARRAY_SIZE(chains); i++)
+               for (i = 0; i < ARRAY_SIZE(chains); i += 2)
                {
                        fw3_pr("-A delegate_%s -m conntrack --ctstate RELATED,ESTABLISHED "
                               "-j ACCEPT\n", chains[i]);
@@ -263,11 +261,13 @@ fw3_print_default_head_rules(enum fw3_table table, enum fw3_family family,
        case FW3_TABLE_NAT:
                if (defs->custom_chains)
                {
-                       fw3_pr("-A delegate_prerouting -j prerouting_rule "
-                              "-m comment --comment \"user chain for prerouting\"\n");
+                       fw3_pr("-A delegate_prerouting "
+                              "-m comment --comment \"user chain for prerouting\" "
+                              "-j prerouting_rule\n");
 
-                       fw3_pr("-A delegate_postrouting -j postrouting_rule "
-                              "-m comment --comment \"user chain for postrouting\"\n");
+                       fw3_pr("-A delegate_postrouting "
+                              "-m comment --comment \"user chain for postrouting\" "
+                              "-j postrouting_rule\n");
                }
                break;
 
@@ -341,27 +341,27 @@ fw3_flush_rules(enum fw3_table table, enum fw3_family family,
                 bool pass2, struct fw3_state *state, enum fw3_target policy)
 {
        struct fw3_defaults *d = &state->running_defaults;
-       uint16_t mask = ~0;
+       uint32_t custom_mask = ~0;
 
        if (!hasbit(d->flags, family))
                return;
 
        /* don't touch user chains on selective stop */
-       delbit(mask, FW3_DEFAULT_CUSTOM_CHAINS);
+       delbit(custom_mask, FW3_DEFAULT_CUSTOM_CHAINS);
 
        if (!pass2)
        {
                reset_policy(table, policy);
 
-               print_chains(table, family, "-D %s\n", d->flags & mask,
+               print_chains(table, family, "-D %s\n", d->flags & custom_mask,
                                         toplevel_rules, ARRAY_SIZE(toplevel_rules));
 
-               print_chains(table, family, "-F %s\n", d->flags & mask,
+               print_chains(table, family, "-F %s\n", d->flags & custom_mask,
                                         default_chains, ARRAY_SIZE(default_chains));
        }
        else
        {
-               print_chains(table, family, "-X %s\n", d->flags & mask,
+               print_chains(table, family, "-X %s\n", d->flags & custom_mask,
                                         default_chains, ARRAY_SIZE(default_chains));
 
                delbit(d->flags, family);

diff --git a/options.h b/options.h
index 7881375f41b589fc3646dab026a30280e8a17428..10546585845f7620aa29cf2348c7850a4cc15f57 100644 (file)
--- a/options.h
+++ b/options.h
@@ -70,16 +70,17 @@ enum fw3_target
        FW3_TARGET_NOTRACK       = 9,
        FW3_TARGET_DNAT          = 10,
        FW3_TARGET_SNAT          = 11,
-       FW3_TARGET_CUSTOM_CHAINS = 12, /* alias to FW3_DEFAULT_CUSTOM_CHAINS */
+       FW3_TARGET_CUSTOM_CNS_V4 = 12,
+       FW3_TARGET_CUSTOM_CNS_V6 = 13,
 };
 
 enum fw3_default
 {
        FW3_DEFAULT_UNSPEC        = 0,
-       FW3_DEFAULT_CUSTOM_CHAINS = 12,
-       FW3_DEFAULT_SYN_FLOOD     = 13,
-       FW3_DEFAULT_MTU_FIX       = 14,
-       FW3_DEFAULT_DROP_INVALID  = 15,
+       FW3_DEFAULT_CUSTOM_CHAINS = 14,
+       FW3_DEFAULT_SYN_FLOOD     = 15,
+       FW3_DEFAULT_MTU_FIX       = 16,
+       FW3_DEFAULT_DROP_INVALID  = 17,
 };
 
 extern const char *fw3_flag_names[FW3_DEFAULT_DROP_INVALID + 1];
@@ -170,7 +171,7 @@ struct fw3_protocol
 
        bool any;
        bool invert;
-       uint16_t protocol;
+       uint32_t protocol;
 };
 
 struct fw3_port
@@ -238,7 +239,7 @@ struct fw3_defaults
 
        bool disable_ipv6;
 
-       uint16_t flags;
+       uint32_t flags;
 };
 
 struct fw3_zone
@@ -274,8 +275,11 @@ struct fw3_zone
 
        bool custom_chains;
 
-       uint16_t src_flags;
-       uint16_t dst_flags;
+       uint32_t src_flags;
+       uint32_t dst_flags;
+
+       uint32_t running_src_flags;
+       uint32_t running_dst_flags;
 };
 
 struct fw3_rule
@@ -393,7 +397,7 @@ struct fw3_ipset
 
        const char *external;
 
-       uint16_t flags;
+       uint32_t flags;
 };
 
 struct fw3_include

diff --git a/utils.c b/utils.c
index 1b9d6722b406b1144804331ff9d8572e7e0de555..5de2964f286a3294a3acaa40a3b7230d98a69ecd 100644 (file)
--- a/utils.c
+++ b/utils.c
@@ -356,7 +356,7 @@ fw3_read_statefile(void *state)
        char line[128];
        const char *p, *name;
 
-       uint16_t flags[2];
+       uint32_t flags[2];
 
        struct fw3_state *s = state;
        struct fw3_zone *zone;
@@ -405,8 +405,8 @@ fw3_read_statefile(void *state)
                                list_add_tail(&zone->list, &s->zones);
                        }
 
-                       zone->src_flags = flags[0];
-                       zone->dst_flags = flags[1];
+                       zone->running_src_flags = flags[0];
+                       zone->running_dst_flags = flags[1];
                        list_add_tail(&zone->running_list, &s->running_zones);
                        break;
 

diff --git a/zones.c b/zones.c
index c81125f0eb71f8dd66636f64f689bd125ce21953..86f794816f5d26fd6a3daa25a9844c85b6d81c45 100644 (file)
--- a/zones.c
+++ b/zones.c
@@ -31,29 +31,49 @@ struct chain {
 };
 
 static const struct chain src_chains[] = {
-       C(ANY, FILTER, UNSPEC,  "zone_%s_input"),
-       C(ANY, FILTER, UNSPEC,  "zone_%s_output"),
-       C(ANY, FILTER, UNSPEC,  "zone_%s_forward"),
+       C(ANY, FILTER, UNSPEC,  "zone_%1$s_input"),
+       C(ANY, FILTER, UNSPEC,  "zone_%1$s_output"),
+       C(ANY, FILTER, UNSPEC,  "zone_%1$s_forward"),
 
-       C(ANY, FILTER, ACCEPT,  "zone_%s_src_ACCEPT"),
-       C(ANY, FILTER, REJECT,  "zone_%s_src_REJECT"),
-       C(ANY, FILTER, DROP,    "zone_%s_src_DROP"),
+       C(ANY, FILTER, ACCEPT,  "zone_%1$s_src_ACCEPT"),
+       C(ANY, FILTER, REJECT,  "zone_%1$s_src_REJECT"),
+       C(ANY, FILTER, DROP,    "zone_%1$s_src_DROP"),
 };
 
 static const struct chain dst_chains[] = {
-       C(ANY, FILTER, ACCEPT,  "zone_%s_dest_ACCEPT"),
-       C(ANY, FILTER, REJECT,  "zone_%s_dest_REJECT"),
-       C(ANY, FILTER, DROP,    "zone_%s_dest_DROP"),
+       C(ANY, FILTER, ACCEPT,  "zone_%1$s_dest_ACCEPT"),
+       C(ANY, FILTER, REJECT,  "zone_%1$s_dest_REJECT"),
+       C(ANY, FILTER, DROP,    "zone_%1$s_dest_DROP"),
+
+       C(V4,  NAT,    SNAT,    "zone_%1$s_postrouting"),
+       C(V4,  NAT,    DNAT,    "zone_%1$s_prerouting"),
+
+       C(ANY, FILTER, CUSTOM_CNS_V4, "input_%1$s_rule"),
+       C(ANY, FILTER, CUSTOM_CNS_V4, "output_%1$s_rule"),
+       C(ANY, FILTER, CUSTOM_CNS_V4, "forwarding_%1$s_rule"),
+       C(ANY, FILTER, CUSTOM_CNS_V6, "input_%1$s_rule"),
+       C(ANY, FILTER, CUSTOM_CNS_V6, "output_%1$s_rule"),
+       C(ANY, FILTER, CUSTOM_CNS_V6, "forwarding_%1$s_rule"),
+
+       C(V4,  NAT,    CUSTOM_CNS_V4, "prerouting_%1$s_rule"),
+       C(V4,  NAT,    CUSTOM_CNS_V4, "postrouting_%1$s_rule"),
+};
+
 
-       C(V4,  NAT,    SNAT,    "zone_%s_postrouting"),
-       C(V4,  NAT,    DNAT,    "zone_%s_prerouting"),
+#define R(dir1, dir2) \
+       "zone_%1$s_" #dir1 " -m comment --comment \"user chain for %1$s " \
+       #dir2 "\" -j " #dir2 "_%1$s_rule"
 
-       C(ANY, FILTER, CUSTOM_CHAINS, "input_%s_rule"),
-       C(ANY, FILTER, CUSTOM_CHAINS, "output_%s_rule"),
-       C(ANY, FILTER, CUSTOM_CHAINS, "forwarding_%s_rule"),
+static const struct chain def_rules[] = {
+       C(ANY, FILTER, CUSTOM_CNS_V4, R(input, input)),
+       C(ANY, FILTER, CUSTOM_CNS_V4, R(output, output)),
+       C(ANY, FILTER, CUSTOM_CNS_V4, R(forward, forwarding)),
+       C(ANY, FILTER, CUSTOM_CNS_V6, R(input, input)),
+       C(ANY, FILTER, CUSTOM_CNS_V6, R(output, output)),
+       C(ANY, FILTER, CUSTOM_CNS_V6, R(forward, forwarding)),
 
-       C(V4,  NAT,    CUSTOM_CHAINS, "prerouting_%s_rule"),
-       C(V4,  NAT,    CUSTOM_CHAINS, "postrouting_%s_rule"),
+       C(V4,  NAT,    CUSTOM_CNS_V4, R(prerouting, prerouting)),
+       C(V4,  NAT,    CUSTOM_CNS_V4, R(postrouting, postrouting)),
 };
 
 const struct fw3_option fw3_zone_opts[] = {
@@ -91,11 +111,11 @@ const struct fw3_option fw3_zone_opts[] = {
 
 static bool
 print_chains(enum fw3_table table, enum fw3_family family,
-             const char *fmt, const char *name, uint16_t targets,
+             const char *fmt, const char *name, uint32_t targets,
              const struct chain *chains, int n)
 {
        bool rv = false;
-       char cn[128] = { 0 };
+       char cn[256] = { 0 };
        const struct chain *c;
 
        for (c = chains; n > 0; c++, n--)
@@ -258,8 +278,9 @@ static void
 print_zone_chain(enum fw3_table table, enum fw3_family family,
                  struct fw3_zone *zone, struct fw3_state *state)
 {
-       bool s, d;
-       uint16_t mask = ~0;
+       bool s, d, r;
+       enum fw3_target f;
+       uint32_t custom_mask = ~0;
 
        if (!fw3_is_family(zone, family))
                return;
@@ -267,52 +288,30 @@ print_zone_chain(enum fw3_table table, enum fw3_family family,
        setbit(zone->dst_flags, family);
 
        /* user chains already loaded, don't create again */
-       if (hasbit(zone->dst_flags, FW3_TARGET_CUSTOM_CHAINS))
-               delbit(mask, FW3_TARGET_CUSTOM_CHAINS);
+       for (f = FW3_TARGET_CUSTOM_CNS_V4; f <= FW3_TARGET_CUSTOM_CNS_V6; f++)
+               if (hasbit(zone->running_dst_flags, f))
+                       delbit(custom_mask, f);
 
        if (zone->custom_chains)
-               setbit(zone->dst_flags, FW3_TARGET_CUSTOM_CHAINS);
+               setbit(zone->dst_flags, (family == FW3_FAMILY_V4) ?
+                      FW3_TARGET_CUSTOM_CNS_V4 : FW3_TARGET_CUSTOM_CNS_V6);
 
        if (!zone->conntrack && !state->defaults.drop_invalid)
                setbit(zone->dst_flags, FW3_TARGET_NOTRACK);
 
        s = print_chains(table, family, ":%s - [0:0]\n", zone->name,
-                        zone->src_flags & mask,
+                        zone->src_flags,
                         src_chains, ARRAY_SIZE(src_chains));
 
        d = print_chains(table, family, ":%s - [0:0]\n", zone->name,
-                        zone->dst_flags & mask,
+                        zone->dst_flags & custom_mask,
                         dst_chains, ARRAY_SIZE(dst_chains));
 
-       if (zone->custom_chains)
-       {
-               if (table == FW3_TABLE_FILTER)
-               {
-                       fw3_pr("-A zone_%s_input -j input_%s_rule "
-                                  "-m comment --comment \"user chain for %s input\"\n",
-                              zone->name, zone->name, zone->name);
-
-                       fw3_pr("-A zone_%s_output -j output_%s_rule "
-                                  "-m comment --comment \"user chain for %s output\"\n",
-                              zone->name, zone->name, zone->name);
-
-                       fw3_pr("-A zone_%s_forward -j forwarding_%s_rule "
-                                  "-m comment --comment \"user chain for %s forwarding\"\n",
-                              zone->name, zone->name, zone->name);
-               }
-               else if (table == FW3_TABLE_NAT)
-               {
-                       fw3_pr("-A zone_%s_prerouting -j prerouting_%s_rule "
-                              "-m comment --comment \"user chain for %s prerouting\"\n",
-                              zone->name, zone->name, zone->name);
+       r = print_chains(table, family, "-A %s\n", zone->name,
+                        zone->dst_flags,
+                        def_rules, ARRAY_SIZE(def_rules));
 
-                       fw3_pr("-A zone_%s_postrouting -j postrouting_%s_rule "
-                              "-m comment --comment \"user chain for %s postrouting\"\n",
-                              zone->name, zone->name, zone->name);
-               }
-       }
-
-       if (s || d)
+       if (s || d || r)
        {
                info("   * Zone '%s'", zone->name);
                fw3_set_running(zone, &state->running_zones);
@@ -540,12 +539,15 @@ fw3_flush_zones(enum fw3_table table, enum fw3_family family,
                            bool pass2, bool reload, struct fw3_state *state)
 {
        struct fw3_zone *z, *tmp;
-       uint16_t mask = ~0;
-       uint16_t families = (1 << FW3_FAMILY_V4) | (1 << FW3_FAMILY_V6);
+       uint32_t custom_mask = ~0;
+       uint32_t family_mask = (1 << FW3_FAMILY_V4) | (1 << FW3_FAMILY_V6);
 
        /* don't touch user chains on selective stop */
        if (reload)
-               delbit(mask, FW3_DEFAULT_CUSTOM_CHAINS);
+       {
+               delbit(custom_mask, FW3_TARGET_CUSTOM_CNS_V4);
+               delbit(custom_mask, FW3_TARGET_CUSTOM_CNS_V6);
+       }
 
        list_for_each_entry_safe(z, tmp, &state->running_zones, running_list)
        {
@@ -553,18 +555,18 @@ fw3_flush_zones(enum fw3_table table, enum fw3_family family,
                        continue;
 
                print_chains(table, family, pass2 ? "-X %s\n" : "-F %s\n",
-                            z->name, z->src_flags & mask,
+                            z->name, z->running_src_flags,
                             src_chains, ARRAY_SIZE(src_chains));
 
                print_chains(table, family, pass2 ? "-X %s\n" : "-F %s\n",
-                            z->name, z->dst_flags & mask,
+                            z->name, z->running_dst_flags & custom_mask,
                             dst_chains, ARRAY_SIZE(dst_chains));
 
                if (pass2)
                {
                        delbit(z->dst_flags, family);
 
-                       if (!(z->dst_flags & families))
+                       if (!(z->dst_flags & family_mask))
                                fw3_set_running(z, NULL);
                }
        }