firewall3: Improve ipset support

This patch is an attempt at improving the ipset support in firewall3.
The following changes have been made:

* The enabled option did not work properly for ipsets, as it was not
checked on create/destroy of a set. After this commit, sets are only
created/destroyed if enabled is set to true.

* Add support for reloading, or recreating, ipsets on firewall reload.
By setting "reload_set" to true, the set will be destroyed and then
re-created when the firewall is reloaded. My use-case for "reload_set"
was to reset sets populated by dnsmasq, without having to restart the
firewall or resort to scripts.

* Add support for the counters and comment extensions. By setting
"counters" or "comment" to true, then counters or comments are added to
the set.

Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
re-ordered additional variables
dropped enum OPT_COMMENT & OPT_COUNTERS as unused
implemented exponential delay whilst waiting for ipset deletion/creation
fixed delays made firewall unresponsive for too long on reloads
Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>

diff --git a/ipsets.c b/ipsets.c
index b73c3d28c64d99d7769f44ca9ad0da6a19241111..93bde0d30d998278a00b9c0d5cce57513c4810f6 100644 (file)
--- a/ipsets.c
+++ b/ipsets.c
@@ -21,6 +21,9 @@
 
 const struct fw3_option fw3_ipset_opts[] = {
        FW3_OPT("enabled",       bool,           ipset,     enabled),
 
 const struct fw3_option fw3_ipset_opts[] = {
        FW3_OPT("enabled",       bool,           ipset,     enabled),

+       FW3_OPT("reload_set",    bool,           ipset,     reload_set),
+       FW3_OPT("counters",      bool,           ipset,     counters),
+       FW3_OPT("comment",       bool,           ipset,     comment),

 
        FW3_OPT("name",          string,         ipset,     name),
        FW3_OPT("family",        family,         ipset,     family),
 
        FW3_OPT("name",          string,         ipset,     name),
        FW3_OPT("family",        family,         ipset,     family),


@@ -204,6 +207,10 @@ check_types(struct uci_element *e, struct fw3_ipset *ipset)
 static bool
 check_ipset(struct fw3_state *state, struct fw3_ipset *ipset, struct uci_element *e)
 {
 static bool
 check_ipset(struct fw3_state *state, struct fw3_ipset *ipset, struct uci_element *e)
 {

+       if (!ipset->enabled) {
+               return false;
+       }
+

        if (ipset->external)
        {
                if (!*ipset->external)
        if (ipset->external)
        {
                if (!*ipset->external)


@@ -252,8 +259,11 @@ fw3_alloc_ipset(struct fw3_state *state)
        INIT_LIST_HEAD(&ipset->datatypes);
        INIT_LIST_HEAD(&ipset->entries);
 
        INIT_LIST_HEAD(&ipset->datatypes);
        INIT_LIST_HEAD(&ipset->entries);
 

-       ipset->enabled = true;
-       ipset->family  = FW3_FAMILY_V4;
+       ipset->comment    = false;
+       ipset->counters   = false;
+       ipset->enabled    = true;
+       ipset->family     = FW3_FAMILY_V4;
+       ipset->reload_set = false;

 
        list_add_tail(&ipset->list, &state->ipsets);
 
 
        list_add_tail(&ipset->list, &state->ipsets);
 


@@ -389,6 +399,12 @@ create_ipset(struct fw3_ipset *ipset, struct fw3_state *state)
        if (ipset->hashsize > 0)
                fw3_pr(" hashsize %u", ipset->hashsize);
 
        if (ipset->hashsize > 0)
                fw3_pr(" hashsize %u", ipset->hashsize);
 

+       if (ipset->counters)
+               fw3_pr(" counters");
+
+       if (ipset->comment)
+               fw3_pr(" comment");
+

        fw3_pr("\n");
 
        list_for_each_entry(entry, &ipset->entries, list)
        fw3_pr("\n");
 
        list_for_each_entry(entry, &ipset->entries, list)


@@ -398,9 +414,10 @@ create_ipset(struct fw3_ipset *ipset, struct fw3_state *state)
 }
 
 void
 }
 
 void

-fw3_create_ipsets(struct fw3_state *state)
+fw3_create_ipsets(struct fw3_state *state, enum fw3_family family,
+                 bool reload_set)

 {
 {

-       int tries;
+       unsigned int delay, tries;

        bool exec = false;
        struct fw3_ipset *ipset;
 
        bool exec = false;
        struct fw3_ipset *ipset;
 


@@ -410,6 +427,10 @@ fw3_create_ipsets(struct fw3_state *state)
        /* spawn ipsets */
        list_for_each_entry(ipset, &state->ipsets, list)
        {
        /* spawn ipsets */
        list_for_each_entry(ipset, &state->ipsets, list)
        {

+               if (ipset->family != family ||
+                   (reload_set && !ipset->reload_set))
+                       continue;
+

                if (ipset->external)
                        continue;
 
                if (ipset->external)
                        continue;
 


@@ -430,27 +451,36 @@ fw3_create_ipsets(struct fw3_state *state)
                fw3_command_close();
        }
 
                fw3_command_close();
        }
 

-       /* wait for ipsets to appear */
+       /* wait a little expontially for ipsets to appear */

        list_for_each_entry(ipset, &state->ipsets, list)
        {
                if (ipset->external)
                        continue;
 
        list_for_each_entry(ipset, &state->ipsets, list)
        {
                if (ipset->external)
                        continue;
 

+               delay = 5;

                for (tries = 0; !fw3_check_ipset(ipset) && tries < 10; tries++)
                for (tries = 0; !fw3_check_ipset(ipset) && tries < 10; tries++)

-                       usleep(50000);
+                       usleep(delay<<1);

        }
 }
 
 void
        }
 }
 
 void

-fw3_destroy_ipsets(struct fw3_state *state)
+fw3_destroy_ipsets(struct fw3_state *state, enum fw3_family family,
+                  bool reload_set)

 {
 {

-       int tries;
+       unsigned int delay, tries;

        bool exec = false;
        struct fw3_ipset *ipset;
 
        bool exec = false;
        struct fw3_ipset *ipset;
 

+       if (state->disable_ipsets)
+               return;
+

        /* destroy ipsets */
        list_for_each_entry(ipset, &state->ipsets, list)
        {
        /* destroy ipsets */
        list_for_each_entry(ipset, &state->ipsets, list)
        {

+               if (ipset->family != family ||
+                   (reload_set && !ipset->reload_set))
+                       continue;
+

                if (!exec)
                {
                        exec = fw3_command_pipe(false, "ipset", "-exist", "-");
                if (!exec)
                {
                        exec = fw3_command_pipe(false, "ipset", "-exist", "-");


@@ -477,8 +507,9 @@ fw3_destroy_ipsets(struct fw3_state *state)
                if (ipset->external)
                        continue;
 
                if (ipset->external)
                        continue;
 

+               delay = 5;

                for (tries = 0; fw3_check_ipset(ipset) && tries < 10; tries++)
                for (tries = 0; fw3_check_ipset(ipset) && tries < 10; tries++)

-                       usleep(50000);
+                       usleep(delay<<1);

        }
 }
 
        }
 }
 

diff --git a/ipsets.h b/ipsets.h
index 2ba862dbba0c8d0c70f53bdcd3f771d8caab431d..fec79f8754ad88eb43089d28752a9ac733589b34 100644 (file)
--- a/ipsets.h
+++ b/ipsets.h
@@ -28,8 +28,10 @@
 extern const struct fw3_option fw3_ipset_opts[];
 
 void fw3_load_ipsets(struct fw3_state *state, struct uci_package *p, struct blob_attr *a);
 extern const struct fw3_option fw3_ipset_opts[];
 
 void fw3_load_ipsets(struct fw3_state *state, struct uci_package *p, struct blob_attr *a);

-void fw3_create_ipsets(struct fw3_state *state);
-void fw3_destroy_ipsets(struct fw3_state *state);
+void fw3_create_ipsets(struct fw3_state *state, enum fw3_family family,
+                      bool reload_set);
+void fw3_destroy_ipsets(struct fw3_state *state, enum fw3_family family,
+                       bool reload_set);

 
 struct fw3_ipset * fw3_lookup_ipset(struct fw3_state *state, const char *name);
 
 
 struct fw3_ipset * fw3_lookup_ipset(struct fw3_state *state, const char *name);
 

diff --git a/main.c b/main.c
index 1410fef31f3c477363b01f7bf9537b4e80e089f8..8d9a2e8d432debc61c19d4b13f72a4556e3a9276 100644 (file)
--- a/main.c
+++ b/main.c
@@ -224,8 +224,10 @@ stop(bool complete)
                rv = 0;
        }
 
                rv = 0;
        }
 

-       if (run_state)
-               fw3_destroy_ipsets(run_state);
+       if (run_state) {
+               for (family = FW3_FAMILY_V4; family <= FW3_FAMILY_V6; family++)
+                       fw3_destroy_ipsets(run_state, family, false);
+       }

 
        if (complete)
                fw3_flush_conntrack(NULL);
 
        if (complete)
                fw3_flush_conntrack(NULL);


@@ -244,11 +246,11 @@ start(void)
        enum fw3_table table;
        struct fw3_ipt_handle *handle;
 
        enum fw3_table table;
        struct fw3_ipt_handle *handle;
 

-       if (!print_family)
-               fw3_create_ipsets(cfg_state);
-

        for (family = FW3_FAMILY_V4; family <= FW3_FAMILY_V6; family++)
        {
        for (family = FW3_FAMILY_V4; family <= FW3_FAMILY_V6; family++)
        {

+               if (!print_family)
+                       fw3_create_ipsets(cfg_state, family, false);
+

                if (family == FW3_FAMILY_V6 && cfg_state->defaults.disable_ipv6)
                        continue;
 
                if (family == FW3_FAMILY_V6 && cfg_state->defaults.disable_ipv6)
                        continue;
 


@@ -352,6 +354,8 @@ reload(void)
                        fw3_ipt_close(handle);
                }
 
                        fw3_ipt_close(handle);
                }
 

+               fw3_destroy_ipsets(run_state, family, true);
+

                family_set(run_state, family, false);
                family_set(cfg_state, family, false);
 
                family_set(run_state, family, false);
                family_set(cfg_state, family, false);
 


@@ -359,6 +363,8 @@ start:
                if (family == FW3_FAMILY_V6 && cfg_state->defaults.disable_ipv6)
                        continue;
 
                if (family == FW3_FAMILY_V6 && cfg_state->defaults.disable_ipv6)
                        continue;
 

+               fw3_create_ipsets(cfg_state, family, true);
+

                for (table = FW3_TABLE_FILTER; table <= FW3_TABLE_RAW; table++)
                {
                        if (!fw3_has_table(family == FW3_FAMILY_V6, fw3_flag_names[table]))
                for (table = FW3_TABLE_FILTER; table <= FW3_TABLE_RAW; table++)
                {
                        if (!fw3_has_table(family == FW3_FAMILY_V6, fw3_flag_names[table]))

diff --git a/options.h b/options.h
index a75cfa3536714c919f9243176ad1f844870ffa0f..cffc01cc4f6f294b09a44bb456b961cf623a7266 100644 (file)
--- a/options.h
+++ b/options.h
@@ -501,6 +501,10 @@ struct fw3_ipset
        struct list_head list;
 
        bool enabled;
        struct list_head list;
 
        bool enabled;

+       bool reload_set;
+       bool counters;
+       bool comment;
+

        const char *name;
        enum fw3_family family;
 
        const char *name;
        enum fw3_family family;
 

diff --git a/utils.c b/utils.c
index fc6bbd7a3ec82d93f66aaf051bcfe556f1f98573..147acf8b3707b5b06488d360ad7543b4723c6934 100644 (file)
--- a/utils.c
+++ b/utils.c
@@ -585,6 +585,11 @@ write_ipset_uci(struct uci_context *ctx, struct fw3_ipset *s,
        ptr.value  = s->name;
        uci_set(ctx, &ptr);
 
        ptr.value  = s->name;
        uci_set(ctx, &ptr);
 

+       ptr.o      = NULL;
+       ptr.option = "reload_set";
+       ptr.value  = s->reload_set ? "true" : "false";
+       uci_set(ctx, &ptr);
+

        ptr.o      = NULL;
        ptr.option = "storage";
        ptr.value  = fw3_ipset_method_names[s->method];
        ptr.o      = NULL;
        ptr.option = "storage";
        ptr.value  = fw3_ipset_method_names[s->method];