diff options
author | Kristian Evensen <kristian.evensen@gmail.com> | 2019-08-19 14:45:57 +0200 |
---|---|---|
committer | Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> | 2019-08-19 20:40:35 +0100 |
commit | bf29c1e7e95c00953da9430e4c5144ef2b79a361 (patch) | |
tree | f95eaa960644754e3ae044ab8de8e69b57ebc771 | |
parent | 509e673dab011851ed084ca592c557ee395fddd4 (diff) | |
download | firewall3-bf29c1e7e95c00953da9430e4c5144ef2b79a361.tar.gz |
firewall3: ipset: Handle reload_set properly
The reload_set option was added in commit 509e673dab01 ("firewall3:
Improve ipset support"), and the purpose of the option is to control if
a set should be flushed or not on a firewall reload.
In some cases, the option unfortunately does not work properly. I had
fixed the errors locally, but failed to submit a v2 of "Improve ipset
support". This patch contains my local fixes, and after the following
changes are applied then the option (as well as ipset support) works as
at least I expect.
The following errors have been fixed:
* "family" was not written to the state file, causing all sets read from
this file was considered as ipv4. Save family to ensure that sets are
handled correctly on firewall reload.
* The default value of "reload_set" is false, meaning that the
reload-check in "fw3_create_ipsets()" is always true (on reload). A
consequence of this is that new sets are never created on firewall
reload. In order to ensure that new sets are created, only consider
"reload_set" if the set exists. If a set (from configuration) does not
exist, we always want to create it.
* On reload and before "fw3_destroy_ipsets()" are called, we need to
update run_state to ensure that sets are updated correctly. We need to
check if the sets in run_state is found in cfg_state, if not the set
should be destroyed (done by forcing reload_set to true). If the set is
found, then we copy the value of reload_set to the set in run_state so
that the elements are updated as the user expects.
Since we now always copy the value of reload_set from cfg_state, there
is no need to write reload_set to run_state.
Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
-rw-r--r-- | ipsets.c | 47 | ||||
-rw-r--r-- | ipsets.h | 4 | ||||
-rw-r--r-- | main.c | 1 | ||||
-rw-r--r-- | utils.c | 7 |
4 files changed, 55 insertions, 4 deletions
@@ -427,13 +427,16 @@ fw3_create_ipsets(struct fw3_state *state, enum fw3_family family, /* spawn ipsets */ list_for_each_entry(ipset, &state->ipsets, list) { - if (ipset->family != family || - (reload_set && !ipset->reload_set)) + if (ipset->family != family) continue; if (ipset->external) continue; + if (fw3_check_ipset(ipset) && + (reload_set && !ipset->reload_set)) + continue; + if (!exec) { exec = fw3_command_pipe(false, "ipset", "-exist", "-"); @@ -568,3 +571,43 @@ out: return rv; } + +void +fw3_ipsets_update_run_state(enum fw3_family family, struct fw3_state *run_state, + struct fw3_state *cfg_state) +{ + struct fw3_ipset *ipset_run, *ipset_cfg; + bool in_cfg; + + list_for_each_entry(ipset_run, &run_state->ipsets, list) { + if (ipset_run->family != family) + continue; + + in_cfg = false; + + list_for_each_entry(ipset_cfg, &cfg_state->ipsets, list) { + if (ipset_cfg->family != family) + continue; + + if (strlen(ipset_run->name) == + strlen(ipset_cfg->name) && + !strcmp(ipset_run->name, ipset_cfg->name)) { + in_cfg = true; + break; + } + } + + /* If a set is found in run_state, but not in cfg_state then the + * set has been deleted/renamed. Set reload_set to true to force + * the old set to be destroyed in the "stop" fase of the reload. + * If the set is found, then copy the reload_set value from the + * configuration state. This ensures that the elements are + * always updated according to the configuration, and not the + * runtime state (which the user might have forgotten). + */ + if (!in_cfg) + ipset_run->reload_set = true; + else + ipset_run->reload_set = ipset_cfg->reload_set; + } +} @@ -37,6 +37,10 @@ struct fw3_ipset * fw3_lookup_ipset(struct fw3_state *state, const char *name); bool fw3_check_ipset(struct fw3_ipset *set); +void +fw3_ipsets_update_run_state(enum fw3_family family, struct fw3_state *run_state, + struct fw3_state *cfg_state); + static inline void fw3_free_ipset(struct fw3_ipset *ipset) { list_del(&ipset->list); @@ -354,6 +354,7 @@ reload(void) fw3_ipt_close(handle); } + fw3_ipsets_update_run_state(family, run_state, cfg_state); fw3_destroy_ipsets(run_state, family, true); family_set(run_state, family, false); @@ -586,8 +586,11 @@ write_ipset_uci(struct uci_context *ctx, struct fw3_ipset *s, uci_set(ctx, &ptr); ptr.o = NULL; - ptr.option = "reload_set"; - ptr.value = s->reload_set ? "true" : "false"; + ptr.option = "family"; + if (s->family == FW3_FAMILY_V4) + ptr.value = "ipv4"; + else + ptr.value = "ipv6"; uci_set(ctx, &ptr); ptr.o = NULL; |