Hi Wagi,
On ma, 2015-07-27 at 14:47 +0200, Daniel Wagner wrote:
> We like to add and remove rules while the filewall is up and running.
filewall -> firewall
> For example we need to insert per Session rule in the global
> NAT table. That could also be implemented destroying the whole table
> and recreate it when need but that is quite an overhead.
>
> Instead of taking down the whole table down we add an API to
> add and remove new rules during runtime.
> ---
> src/connman.h | 3 ++
> src/firewall.c | 112
> +++++++++++++++++++++++++++++++++++++++-----------
> tools/iptables-unit.c | 39 +++++++++++++++---
> 3 files changed, 126 insertions(+), 28 deletions(-)
>
> diff --git a/src/connman.h b/src/connman.h
> index 29151af..654b8fa 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -989,6 +989,9 @@ int __connman_firewall_add_rule(struct firewall_context
> *ctx,
> const char *table,
> const char *chain,
> const char *rule_fmt, ...);
> +int __connman_firewall_remove_rule(struct firewall_context *ctx, int id);
> +int __connman_firewall_enable_rule(struct firewall_context *ctx, int id);
> +int __connman_firewall_disable_rule(struct firewall_context *ctx, int id);
> int __connman_firewall_enable(struct firewall_context *ctx);
> int __connman_firewall_disable(struct firewall_context *ctx);
> bool __connman_firewall_is_up(void);
> diff --git a/src/firewall.c b/src/firewall.c
> index 90c3d3c..c25d086 100644
> --- a/src/firewall.c
> +++ b/src/firewall.c
> @@ -46,6 +46,7 @@ struct connman_managed_table {
> };
>
> struct fw_rule {
> + int id;
> char *table;
> char *chain;
> char *rule_spec;
> @@ -58,6 +59,7 @@ struct firewall_context {
> static GSList *managed_tables;
>
> static bool firewall_is_up;
> +static unsigned int firewall_rule_id;
>
> static int chain_to_index(const char *chain_name)
> {
> @@ -284,38 +286,109 @@ int __connman_firewall_add_rule(struct
> firewall_context *ctx,
>
> rule = g_new0(struct fw_rule, 1);
>
> + rule->id = firewall_rule_id++;
> rule->table = g_strdup(table);
> rule->chain = g_strdup(chain);
> rule->rule_spec = rule_spec;
>
> ctx->rules = g_list_append(ctx->rules, rule);
> + return rule->id;
> +}
> +
> +static int firewall_enable_rule(struct fw_rule *rule)
> +{
> + int err;
> +
> + DBG("%s %s %s", rule->table, rule->chain, rule->rule_spec);
> +
> + err = insert_managed_rule(rule->table, rule->chain, rule->rule_spec);
> + if (err < 0)
> + return err;
> +
> + return __connman_iptables_commit(rule->table);
> +}
> +
> +static int firewall_disable_rule(struct fw_rule *rule)
> +{
> + int err;
> +
> + err = delete_managed_rule(rule->table, rule->chain, rule->rule_spec);
> + if (err < 0) {
> + connman_error("Cannot remove previously installed "
> + "iptables rules: %s", strerror(-err));
> + return err;
> + }
> +
> + err = __connman_iptables_commit(rule->table);
> + if (err < 0) {
> + connman_error("Cannot remove previously installed "
> + "iptables rules: %s", strerror(-err));
> + return err;
> + }
>
> return 0;
> }
>
> +
> +int __connman_firewall_remove_rule(struct firewall_context *ctx, int id)
> +{
> + struct fw_rule *rule;
> + GList *list;
> +
> + for (list = ctx->rules; list; list = list->next) {
> + rule = list->data;
> + if (rule->id != id)
> + continue;
> +
> + ctx->rules = g_list_remove(ctx->rules, rule);
> + cleanup_fw_rule(rule);
> + return 0;
> + }
> +
> + return -ENOENT;
> +}
> +
> +int __connman_firewall_enable_rule(struct firewall_context *ctx, int id)
> +{
> + struct fw_rule *rule;
> + GList *list;
> +
> + for (list = ctx->rules; list; list = list->next) {
> + rule = list->data;
> + if (rule->id != id)
> + continue;
> +
> + return firewall_enable_rule(rule);
> + }
> +
> + return -ENOENT;
> +}
__connman_firewall_remove_rule() and __connman_firewall_enable_rule()
and __connman_firewall_disable_rule() are looking quite similar, perhaps
some refactoring could be done here?
> +
> +int __connman_firewall_disable_rule(struct firewall_context *ctx, int id)
> +{
> + struct fw_rule *rule;
> + GList *list;
> +
> + for (list = ctx->rules; list; list = list->next) {
> + rule = list->data;
> + if (rule->id != id)
> + continue;
> +
> + return firewall_disable_rule(rule);
> + }
> +
> + return -ENOENT;
> +}
> +
> static int firewall_disable(GList *rules)
> {
> struct fw_rule *rule;
> GList *list;
> - int err;
>
> for (list = rules; list; list = g_list_previous(list)) {
> rule = list->data;
>
> - err = delete_managed_rule(rule->table,
> - rule->chain, rule->rule_spec);
> - if (err < 0) {
> - connman_error("Cannot remove previously installed "
> - "iptables rules: %s", strerror(-err));
> - return err;
> - }
> -
> - err = __connman_iptables_commit(rule->table);
> - if (err < 0) {
> - connman_error("Cannot remove previously installed "
> - "iptables rules: %s", strerror(-err));
> - return err;
> - }
> + return firewall_disable_rule(rule);
> }
>
> return 0;
> @@ -331,14 +404,7 @@ int __connman_firewall_enable(struct firewall_context
> *ctx)
> list = g_list_next(list)) {
> rule = list->data;
>
> - DBG("%s %s %s", rule->table, rule->chain, rule->rule_spec);
> -
> - err = insert_managed_rule(rule->table,
> - rule->chain, rule->rule_spec);
> - if (err < 0)
> - goto err;
> -
> - err = __connman_iptables_commit(rule->table);
> + err = firewall_enable_rule(rule);
> if (err < 0)
> goto err;
> }
> diff --git a/tools/iptables-unit.c b/tools/iptables-unit.c
> index 7e427e2..58f53ae 100644
> --- a/tools/iptables-unit.c
> +++ b/tools/iptables-unit.c
> @@ -24,6 +24,7 @@
> #endif
>
> #include <glib.h>
> +#include <errno.h>
>
> #include "../src/connman.h"
>
> @@ -412,7 +413,7 @@ static void test_firewall_basic0(void)
>
> err = __connman_firewall_add_rule(ctx, "filter", "INPUT",
> "-m mark --mark 999 -j LOG");
> - g_assert(err == 0);
> + g_assert(err >= 0);
>
> err = __connman_firewall_enable(ctx);
> g_assert(err == 0);
> @@ -441,11 +442,11 @@ static void test_firewall_basic1(void)
>
> err = __connman_firewall_add_rule(ctx, "filter", "INPUT",
> "-m mark --mark 999 -j LOG");
> - g_assert(err == 0);
> + g_assert(err >= 0);
>
> err = __connman_firewall_add_rule(ctx, "filter", "OUTPUT",
> "-m mark --mark 999 -j LOG");
> - g_assert(err == 0);
> + g_assert(err >= 0);
>
> err = __connman_firewall_enable(ctx);
> g_assert(err == 0);
> @@ -466,11 +467,11 @@ static void test_firewall_basic2(void)
>
> err = __connman_firewall_add_rule(ctx, "mangle", "INPUT",
> "-j CONNMARK --restore-mark");
> - g_assert(err == 0);
> + g_assert(err >= 0);
>
> err = __connman_firewall_add_rule(ctx, "mangle", "POSTROUTING",
> "-j CONNMARK --save-mark");
> - g_assert(err == 0);
> + g_assert(err >= 0);
>
> err = __connman_firewall_enable(ctx);
> g_assert(err == 0);
> @@ -481,6 +482,33 @@ static void test_firewall_basic2(void)
> __connman_firewall_destroy(ctx);
> }
>
> +static void test_firewall_basic3(void)
> +{
> + struct firewall_context *ctx;
> + int err, id;
> +
> + ctx = __connman_firewall_create();
> + g_assert(ctx);
> +
> + id = __connman_firewall_add_rule(ctx, "mangle", "INPUT",
> + "-j CONNMARK --restore-mark");
> + g_assert(id >= 0);
> +
> + err = __connman_firewall_enable_rule(ctx, id);
> + g_assert(err == 0);
> +
> + err = __connman_firewall_disable_rule(ctx, id);
> + g_assert(err == 0);
> +
> + err = __connman_firewall_remove_rule(ctx, id);
> + g_assert(err == 0);
> +
> + err = __connman_firewall_disable(ctx);
> + g_assert(err == 0);
> +
> + __connman_firewall_destroy(ctx);
> +}
> +
> static gchar *option_debug = NULL;
>
> static bool parse_debug(const char *key, const char *value,
> @@ -543,6 +571,7 @@ int main(int argc, char *argv[])
> g_test_add_func("/firewall/basic0", test_firewall_basic0);
> g_test_add_func("/firewall/basic1", test_firewall_basic1);
> g_test_add_func("/firewall/basic2", test_firewall_basic2);
> + g_test_add_func("/firewall/basic3", test_firewall_basic3);
>
> err = g_test_run();
>
Cheers,
Jukka
_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
