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 973b3a7..95a9e22 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,
 
 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;
              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;
                          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])
 
 #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))
 
        /* 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));
 }
 
                     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[] = {
        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,
        };
 
        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)
                {
 
                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]);
                {
                        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)
                {
        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;
 
                }
                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;
                 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 */
 
        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);
 
 
        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));
 
                                         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
        {
                                         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);
                                         default_chains, ARRAY_SIZE(default_chains));
 
                delbit(d->flags, family);

diff --git a/options.h b/options.h
index 7881375..1054658 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_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,
 };
 
 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];
 };
 
 extern const char *fw3_flag_names[FW3_DEFAULT_DROP_INVALID + 1];


@@ -170,7 +171,7 @@ struct fw3_protocol
 
        bool any;
        bool invert;
 
        bool any;
        bool invert;

-       uint16_t protocol;
+       uint32_t protocol;

 };
 
 struct fw3_port
 };
 
 struct fw3_port


@@ -238,7 +239,7 @@ struct fw3_defaults
 
        bool disable_ipv6;
 
 
        bool disable_ipv6;
 

-       uint16_t flags;
+       uint32_t flags;

 };
 
 struct fw3_zone
 };
 
 struct fw3_zone


@@ -274,8 +275,11 @@ struct fw3_zone
 
        bool custom_chains;
 
 
        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
 };
 
 struct fw3_rule


@@ -393,7 +397,7 @@ struct fw3_ipset
 
        const char *external;
 
 
        const char *external;
 

-       uint16_t flags;
+       uint32_t flags;

 };
 
 struct fw3_include
 };
 
 struct fw3_include

diff --git a/utils.c b/utils.c
index 1b9d672..5de2964 100644 (file)
--- a/utils.c
+++ b/utils.c
@@ -356,7 +356,7 @@ fw3_read_statefile(void *state)
        char line[128];
        const char *p, *name;
 
        char line[128];
        const char *p, *name;
 

-       uint16_t flags[2];
+       uint32_t flags[2];

 
        struct fw3_state *s = state;
        struct fw3_zone *zone;
 
        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);
                        }
 
                                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;
 
                        list_add_tail(&zone->running_list, &s->running_zones);
                        break;
 

diff --git a/zones.c b/zones.c
index c81125f..86f7948 100644 (file)
--- a/zones.c
+++ b/zones.c
@@ -31,29 +31,49 @@ struct chain {
 };
 
 static const struct chain src_chains[] = {
 };
 
 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[] = {
 };
 
 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[] = {
 };
 
 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,
 
 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;
              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--)
        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)
 {
 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;
 
        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 */
        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)
 
        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,
 
        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,
                         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));
 
                         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);
        {
                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;
                            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)
 
        /* 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)
        {
 
        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",
                        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",
                             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);
 
                             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);
                }
        }
                                fw3_set_running(z, NULL);
                }
        }