From: Jo-Philipp Wich Date: Sun, 10 Mar 2013 17:17:21 +0000 (+0100) Subject: Properly handle per zone user chain rules by fixing multiple logic errors X-Git-Url: https://git.archive.openwrt.org/?p=project%2Ffirewall3.git;a=commitdiff_plain;h=9d72f0ecb589960bfe21751fbef470116c041a3d 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 --- 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 7881375..1054658 100644 --- 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 1b9d672..5de2964 100644 --- 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 c81125f..86f7948 100644 --- 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); } }