Hi Daniel,
On ti, 2015-07-28 at 08:57 +0200, Daniel Wagner wrote:
> We like to add and remove rules while the firewall is up and running.
> 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 | 152
> ++++++++++++++++++++++++++++++++++++++------------
> tools/iptables-unit.c | 39 +++++++++++--
> 3 files changed, 152 insertions(+), 42 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..8f14c44 100644
> --- a/src/firewall.c
> +++ b/src/firewall.c
> @@ -2,7 +2,7 @@
> *
> * Connection Manager
> *
> - * Copyright (C) 2013 BMW Car IT GmbH.
> + * Copyright (C) 2013,2015 BMW Car IT GmbH.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -31,6 +31,7 @@
> #include "connman.h"
>
> #define CHAIN_PREFIX "connman-"
> +#define FW_ALL_RULES -1
>
> static const char *builtin_chains[] = {
> [NF_IP_PRE_ROUTING] = "PREROUTING",
> @@ -46,6 +47,8 @@ struct connman_managed_table {
> };
>
> struct fw_rule {
> + int id;
> + bool enabled;
> char *table;
> char *chain;
> char *rule_spec;
> @@ -58,6 +61,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)
> {
> @@ -267,6 +271,51 @@ void __connman_firewall_destroy(struct firewall_context
> *ctx)
> g_free(ctx);
> }
>
> +static int firewall_enable_rule(struct fw_rule *rule)
> +{
> + int err;
> +
Should we check if the rule is already enabled here?
> + 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;
> +
> + err = __connman_iptables_commit(rule->table);
> + if (err < 0)
> + return err;
> +
> + rule->enabled = true;
> +
> + return 0;
> +}
> +
> +static int firewall_disable_rule(struct fw_rule *rule)
> +{
> + int err;
> +
> + if (!rule->enabled)
> + return 0;
> +
> + 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;
> + }
> +
> + rule->enabled = false;
> +
> + return 0;
> +}
> +
> int __connman_firewall_add_rule(struct firewall_context *ctx,
> const char *table,
> const char *chain,
> @@ -284,80 +333,109 @@ int __connman_firewall_add_rule(struct
> firewall_context *ctx,
>
> rule = g_new0(struct fw_rule, 1);
>
> + rule->id = firewall_rule_id++;
> + rule->enabled = false;
> rule->table = g_strdup(table);
> rule->chain = g_strdup(chain);
> rule->rule_spec = rule_spec;
>
> ctx->rules = g_list_append(ctx->rules, rule);
> -
> - return 0;
> + return rule->id;
> }
>
> -static int firewall_disable(GList *rules)
> +int __connman_firewall_remove_rule(struct firewall_context *ctx, int id)
> {
> struct fw_rule *rule;
> GList *list;
> - int err;
> + int err = -ENOENT;
>
> - for (list = rules; list; list = g_list_previous(list)) {
> + for (list = g_list_last(ctx->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;
> + if (rule->id == id || id == FW_ALL_RULES) {
> + ctx->rules = g_list_remove(ctx->rules, rule);
> + cleanup_fw_rule(rule);
> + err = 0;
> +
> + if (id != FW_ALL_RULES)
> + break;
> }
> + }
>
> - err = __connman_iptables_commit(rule->table);
> - if (err < 0) {
> - connman_error("Cannot remove previously installed "
> - "iptables rules: %s", strerror(-err));
> - return err;
> + return err;
> +}
> +
> +int __connman_firewall_enable_rule(struct firewall_context *ctx, int id)
> +{
> + struct fw_rule *rule;
> + GList *list;
> + int err = -ENOENT;
> +
> + for (list = g_list_first(ctx->rules); list; list = g_list_next(list)) {
> + rule = list->data;
> +
> + if (rule->id == id || id == FW_ALL_RULES) {
> + err = firewall_enable_rule(rule);
> + if (err < 0)
> + break;
> +
> + if (id != FW_ALL_RULES)
> + break;
> }
> }
>
> - return 0;
> + return err;
> }
>
> -int __connman_firewall_enable(struct firewall_context *ctx)
> +int __connman_firewall_disable_rule(struct firewall_context *ctx, int id)
> {
> struct fw_rule *rule;
> GList *list;
> - int err;
> + int e;
> + int err = -ENOENT;
>
> - for (list = g_list_first(ctx->rules); list;
> - list = g_list_next(list)) {
> + for (list = g_list_last(ctx->rules); list;
> + list = g_list_previous(list)) {
> rule = list->data;
>
> - DBG("%s %s %s", rule->table, rule->chain, rule->rule_spec);
> + if (rule->id == id || id == FW_ALL_RULES) {
> + e = firewall_disable_rule(rule);
>
> - err = insert_managed_rule(rule->table,
> - rule->chain, rule->rule_spec);
> - if (err < 0)
> - goto err;
> + /* Report last error back */
> + if (e == 0 && err == -ENOENT)
> + err = 0;
> + else if (e < 0)
> + err = e;
>
> - err = __connman_iptables_commit(rule->table);
> - if (err < 0)
> - goto err;
> + if (id != FW_ALL_RULES)
> + break;
> + }
> }
>
> - firewall_is_up = true;
> + return err;
> +}
>
> - return 0;
> +int __connman_firewall_enable(struct firewall_context *ctx)
> +{
> + int err;
>
> -err:
> - connman_warn("Failed to install iptables rules: %s", strerror(-err));
> + err = __connman_firewall_enable_rule(ctx, FW_ALL_RULES);
> + if (err < 0) {
> + connman_warn("Failed to install iptables rules: %s",
> + strerror(-err));
> + __connman_firewall_disable_rule(ctx, FW_ALL_RULES);
> + return err;
> + }
>
> - firewall_disable(g_list_previous(list));
> + firewall_is_up = true;
>
> - return err;
> + return 0;
> }
>
> int __connman_firewall_disable(struct firewall_context *ctx)
> {
> - return firewall_disable(g_list_last(ctx->rules));
> + return __connman_firewall_disable_rule(ctx, FW_ALL_RULES);
> }
>
> bool __connman_firewall_is_up(void)
> diff --git a/tools/iptables-unit.c b/tools/iptables-unit.c
> index 7e427e2..9c09867 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 == -ENOENT);
> +
> + __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
[email protected]
https://lists.connman.net/mailman/listinfo/connman
