[PATCH v4 3/6] firewall: Add explicit feature API

From: Daniel Wagner <daniel.wagner(a)bmw-carit.de&gt; Instead heaving a generic firewall API, we use an just provide a few features such as NAT, SNAT or MARK. That allows us to push down all the code into firewall.c. There are only two different users in ConnMan for this API: enabling global NAT or for session handling. Fortunately, the NAT handling is pretty simple. We just have one global table and we either have it enabled or disabled. The session handling is slightly more tricky. There are three different rule sets. The first one enables one global rule for CONNMARK save/restore. This one will be installed only when the first session calls __connman_firewall_enable_marking(). For this we use a global context and enable disable it only if there is a session using it. The SNAT rules are shared between sessions and there exists only one per output device. The session tracking is done in session.c. Note the SNAT rules have also their own struct firewall_context. The third set of rule for MARK is owned by the session, that is the rules are tracked it the session->fw context. Due to the separation of the struct firewall_context usage we can git rid of any special tracking. Either we enable all or disable all in one go. We don't have enable indiviual rules anymore with the FW_ALL_RULES. And we are all happy again. --- src/connman.h | 22 +++--- src/firewall.c | 190 +++++++++++++++++++++++++++++++++++--------------- src/nat.c | 21 +----- src/session.c | 123 ++++---------------------------- tools/iptables-unit.c | 112 ----------------------------- 5 files changed, 162 insertions(+), 306 deletions(-) diff --git a/src/connman.h b/src/connman.h index e849ed8..401e3d7 100644 --- a/src/connman.h +++ b/src/connman.h @@ -992,16 +992,18 @@ struct firewall_context; struct firewall_context *__connman_firewall_create(void); void __connman_firewall_destroy(struct firewall_context *ctx); -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); +int __connman_firewall_enable_nat(struct firewall_context *ctx, + char *address, unsigned char prefixlen, + char *interface); +int __connman_firewall_disable_nat(struct firewall_context *ctx); +int __connman_firewall_enable_snat(struct firewall_context *ctx, + int index, const char *ifname, + const char *addr); +int __connman_firewall_disable_snat(struct firewall_context *ctx); +int __connman_firewall_enable_marking(struct firewall_context *ctx, + enum connman_session_id_type id_type, + char *id, uint32_t mark); +int __connman_firewall_disable_marking(struct firewall_context *ctx); int __connman_firewall_init(void); void __connman_firewall_cleanup(void); diff --git a/src/firewall.c b/src/firewall.c index fed75a8..c58efc1 100644 --- a/src/firewall.c +++ b/src/firewall.c @@ -31,7 +31,6 @@ #include "connman.h" #define CHAIN_PREFIX "connman-" -#define FW_ALL_RULES -1 static const char *builtin_chains[] = { [NF_IP_PRE_ROUTING] = "PREROUTING", @@ -47,7 +46,6 @@ struct connman_managed_table { }; struct fw_rule { - int id; bool enabled; char *table; char *chain; @@ -59,9 +57,8 @@ struct firewall_context { }; static GSList *managed_tables; - -static bool firewall_is_up; -static unsigned int firewall_rule_id; +static struct firewall_context *connmark_ctx; +static unsigned int connmark_ref; static int chain_to_index(const char *chain_name) { @@ -271,7 +268,7 @@ void __connman_firewall_destroy(struct firewall_context *ctx) g_free(ctx); } -static int firewall_enable_rule(struct fw_rule *rule) +static int enable_rule(struct fw_rule *rule) { int err; @@ -293,7 +290,7 @@ static int firewall_enable_rule(struct fw_rule *rule) return 0; } -static int firewall_disable_rule(struct fw_rule *rule) +static int disable_rule(struct fw_rule *rule) { int err; @@ -319,7 +316,7 @@ static int firewall_disable_rule(struct fw_rule *rule) return 0; } -int __connman_firewall_add_rule(struct firewall_context *ctx, +static void firewall_add_rule(struct firewall_context *ctx, const char *table, const char *chain, const char *rule_fmt, ...) @@ -336,40 +333,29 @@ 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 rule->id; } -int __connman_firewall_remove_rule(struct firewall_context *ctx, int id) +static void firewall_remove_rules(struct firewall_context *ctx) { struct fw_rule *rule; GList *list; - int err = -ENOENT; for (list = g_list_last(ctx->rules); list; list = g_list_previous(list)) { rule = list->data; - 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; - } + ctx->rules = g_list_remove(ctx->rules, rule); + cleanup_fw_rule(rule); } - - return err; } -int __connman_firewall_enable_rule(struct firewall_context *ctx, int id) +static int firewall_enable_rules(struct firewall_context *ctx) { struct fw_rule *rule; GList *list; @@ -378,20 +364,15 @@ int __connman_firewall_enable_rule(struct firewall_context *ctx, int id) 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; - } + err = enable_rule(rule); + if (err < 0) + break; } return err; } -int __connman_firewall_disable_rule(struct firewall_context *ctx, int id) +static int firewall_disable_rules(struct firewall_context *ctx) { struct fw_rule *rule; GList *list; @@ -402,49 +383,149 @@ int __connman_firewall_disable_rule(struct firewall_context *ctx, int id) list = g_list_previous(list)) { rule = list->data; - if (rule->id == id || id == FW_ALL_RULES) { - e = firewall_disable_rule(rule); + e = disable_rule(rule); - /* Report last error back */ - if (e == 0 && err == -ENOENT) - err = 0; - else if (e < 0) - err = e; + /* Report last error back */ + if (e == 0 && err == -ENOENT) + err = 0; + else if (e < 0) + err = e; + } - if (id != FW_ALL_RULES) - break; - } + return err; +} + +int __connman_firewall_enable_nat(struct firewall_context *ctx, + char *address, unsigned char prefixlen, + char *interface) +{ + char *cmd; + int err; + + cmd = g_strdup_printf("-s %s/%d -o %s -j MASQUERADE", + address, prefixlen, interface); + + firewall_add_rule(ctx, "nat", "POSTROUTING", cmd); + g_free(cmd); + err = firewall_enable_rules(ctx); + if (err) + firewall_remove_rules(ctx); + return err; +} + +int __connman_firewall_disable_nat(struct firewall_context *ctx) +{ + int err; + + err = firewall_disable_rules(ctx); + if (err < 0) { + DBG("could not disable NAT rule"); + return err; } + firewall_remove_rules(ctx); + return 0; +} + +int __connman_firewall_enable_snat(struct firewall_context *ctx, + int index, const char *ifname, + const char *addr) +{ + int err; + + firewall_add_rule(ctx, "nat", "POSTROUTING", + "-o %s -j SNAT --to-source %s", + ifname, addr); + + err = firewall_enable_rules(ctx); + if (err) + firewall_remove_rules(ctx); return err; } -int __connman_firewall_enable(struct firewall_context *ctx) +int __connman_firewall_disable_snat(struct firewall_context *ctx) { int err; - err = __connman_firewall_enable_rule(ctx, FW_ALL_RULES); + err = firewall_disable_rules(ctx); if (err < 0) { - connman_warn("Failed to install iptables rules: %s", - strerror(-err)); - __connman_firewall_disable_rule(ctx, FW_ALL_RULES); + DBG("could not disable SNAT rule"); return err; } - firewall_is_up = true; + firewall_remove_rules(ctx); + return 0; +} + +static int firewall_enable_connmark(void) +{ + int err; + + if (connmark_ref > 0) { + connmark_ref++; + return 0; + } + + connmark_ctx = __connman_firewall_create(); + firewall_add_rule(connmark_ctx, "mangle", "INPUT", + "-j CONNMARK --restore-mark"); + firewall_add_rule(connmark_ctx, "mangle", "POSTROUTING", + "-j CONNMARK --save-mark"); + err = firewall_enable_rules(connmark_ctx); + if (err) { + __connman_firewall_destroy(connmark_ctx); + connmark_ctx = NULL; + return err; + } + connmark_ref++; return 0; } -int __connman_firewall_disable(struct firewall_context *ctx) +static void firewall_disable_connmark(void) +{ + connmark_ref--; + if (connmark_ref > 0) + return; + + firewall_disable_rules(connmark_ctx); + __connman_firewall_destroy(connmark_ctx); + connmark_ctx = NULL; +} + +int __connman_firewall_enable_marking(struct firewall_context *ctx, + enum connman_session_id_type id_type, + char *id, uint32_t mark) { - __connman_firewall_disable_rule(ctx, FW_ALL_RULES); - return __connman_firewall_remove_rule(ctx, FW_ALL_RULES); + int err; + + err = firewall_enable_connmark(); + if (err) + return err; + + switch (id_type) { + case CONNMAN_SESSION_ID_TYPE_UID: + firewall_add_rule(ctx, "mangle", "OUTPUT", + "-m owner --uid-owner %s -j MARK --set-mark %d", + id, mark); + break; + case CONNMAN_SESSION_ID_TYPE_GID: + firewall_add_rule(ctx, "mangle", "OUTPUT", + "-m owner --gid-owner %s -j MARK --set-mark %d", + id, mark); + break; + case CONNMAN_SESSION_ID_TYPE_LSM: + default: + return -EINVAL; + } + + return firewall_enable_rules(ctx); } -bool __connman_firewall_is_up(void) +int __connman_firewall_disable_marking(struct firewall_context *ctx) { - return firewall_is_up; + firewall_disable_connmark(); + return firewall_disable_rules(ctx); } static void iterate_chains_cb(const char *chain_name, void *user_data) @@ -514,12 +595,9 @@ static void flush_all_tables(void) if (!g_file_test("/proc/net/ip_tables_names", G_FILE_TEST_EXISTS | G_FILE_TEST_IS_REGULAR)) { - firewall_is_up = false; return; } - firewall_is_up = true; - flush_table("filter"); flush_table("mangle"); flush_table("nat"); diff --git a/src/nat.c b/src/nat.c index 33ae6df..4c1f858 100644 --- a/src/nat.c +++ b/src/nat.c @@ -80,28 +80,14 @@ static int enable_ip_forward(bool enable) static int enable_nat(struct connman_nat *nat) { - char *cmd; - int err; - g_free(nat->interface); nat->interface = g_strdup(default_interface); if (!nat->interface) return 0; - /* Enable masquerading */ - cmd = g_strdup_printf("-s %s/%d -o %s -j MASQUERADE", - nat->address, - nat->prefixlen, - nat->interface); - - err = __connman_firewall_add_rule(nat->fw, "nat", - "POSTROUTING", cmd); - g_free(cmd); - if (err < 0) - return err; - - return __connman_firewall_enable(nat->fw); + return __connman_firewall_enable_nat(nat->fw, nat->address, + nat->prefixlen, nat->interface); } static void disable_nat(struct connman_nat *nat) @@ -109,8 +95,7 @@ static void disable_nat(struct connman_nat *nat) if (!nat->interface) return; - /* Disable masquerading */ - __connman_firewall_disable(nat->fw); + __connman_firewall_disable_nat(nat->fw); } int __connman_nat_enable(const char *name, const char *address, diff --git a/src/session.c b/src/session.c index 1205935..b1e9e1b 100644 --- a/src/session.c +++ b/src/session.c @@ -37,7 +37,6 @@ static GHashTable *session_hash; static GHashTable *service_hash; static struct connman_session *ecall_session; static uint32_t session_mark = 256; -static struct firewall_context *global_firewall = NULL; enum connman_session_state { CONNMAN_SESSION_STATE_DISCONNECTED = 0, @@ -230,21 +229,13 @@ static int fw_snat_create(struct connman_session *session, fw_snat->fw = __connman_firewall_create(); fw_snat->index = index; - fw_snat->id = __connman_firewall_add_rule(fw_snat->fw, - "nat", "POSTROUTING", - "-o %s -j SNAT --to-source %s", - ifname, addr); + fw_snat->id = __connman_firewall_enable_snat(fw_snat->fw, + index, ifname, addr); if (fw_snat->id < 0) { err = fw_snat->id; goto err; } - err = __connman_firewall_enable_rule(fw_snat->fw, fw_snat->id); - if (err < 0) { - __connman_firewall_remove_rule(fw_snat->fw, fw_snat->id); - goto err; - } - fw_snat_list = g_slist_prepend(fw_snat_list, fw_snat); fw_snat->sessions = g_slist_prepend(fw_snat->sessions, session); @@ -266,69 +257,17 @@ static void fw_snat_ref(struct connman_session *session, static void fw_snat_unref(struct connman_session *session, struct fw_snat *fw_snat) { - int err; - fw_snat->sessions = g_slist_remove(fw_snat->sessions, session); if (fw_snat->sessions) return; fw_snat_list = g_slist_remove(fw_snat_list, fw_snat); - err = __connman_firewall_disable_rule(fw_snat->fw, fw_snat->id); - if (err < 0) - DBG("could not disable SNAT rule"); - - err = __connman_firewall_remove_rule(fw_snat->fw, fw_snat->id); - if (err < 0) - DBG("could not remove SNAT rule"); - + __connman_firewall_disable_snat(fw_snat->fw); __connman_firewall_destroy(fw_snat->fw); g_free(fw_snat); } -static int init_firewall(void) -{ - struct firewall_context *fw; - int err; - - if (global_firewall) - return 0; - - fw = __connman_firewall_create(); - - err = __connman_firewall_add_rule(fw, "mangle", "INPUT", - "-j CONNMARK --restore-mark"); - if (err < 0) - goto err; - - err = __connman_firewall_add_rule(fw, "mangle", "POSTROUTING", - "-j CONNMARK --save-mark"); - if (err < 0) - goto err; - - err = __connman_firewall_enable(fw); - if (err < 0) - goto err; - - global_firewall = fw; - - return 0; - -err: - __connman_firewall_destroy(fw); - - return err; -} - -static void cleanup_firewall(void) -{ - if (!global_firewall) - return; - - __connman_firewall_disable(global_firewall); - __connman_firewall_destroy(global_firewall); -} - static int init_firewall_session(struct connman_session *session) { struct firewall_context *fw; @@ -339,49 +278,22 @@ static int init_firewall_session(struct connman_session *session) DBG(""); - err = init_firewall(); - if (err < 0) - return err; - fw = __connman_firewall_create(); if (!fw) return -ENOMEM; - switch (session->policy_config->id_type) { - case CONNMAN_SESSION_ID_TYPE_UID: - err = __connman_firewall_add_rule(fw, "mangle", "OUTPUT", - "-m owner --uid-owner %s -j MARK --set-mark %d", - session->policy_config->id, - session->mark); - break; - case CONNMAN_SESSION_ID_TYPE_GID: - err = __connman_firewall_add_rule(fw, "mangle", "OUTPUT", - "-m owner --gid-owner %s -j MARK --set-mark %d", - session->policy_config->id, - session->mark); - break; - case CONNMAN_SESSION_ID_TYPE_LSM: - default: - err = -EINVAL; + err =__connman_firewall_enable_marking(fw, + session->policy_config->id_type, + session->policy_config->id, + session->mark); + if (err < 0) { + __connman_firewall_destroy(fw); + return err; } - - if (err < 0) - goto err; - session->id_type = session->policy_config->id_type; - - err = __connman_firewall_enable(fw); - if (err) - goto err; - session->fw = fw; return 0; - -err: - __connman_firewall_destroy(fw); - - return err; } static void cleanup_firewall_session(struct connman_session *session) @@ -389,7 +301,8 @@ static void cleanup_firewall_session(struct connman_session *session) if (!session->fw) return; - __connman_firewall_disable(session->fw); + __connman_firewall_disable_marking(session->fw); + __connman_firewall_disable_snat(session->fw); __connman_firewall_destroy(session->fw); session->fw = NULL; @@ -472,10 +385,8 @@ static void add_nat_rules(struct connman_session *session) struct connman_ipconfig *ipconfig; struct fw_snat *fw_snat; const char *addr; - char *ifname; int index, err; - - DBG(""); + char *ifname; if (!session->service) return; @@ -1877,12 +1788,6 @@ int __connman_session_init(void) service_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, cleanup_service); - if (__connman_firewall_is_up()) { - err = init_firewall(); - if (err < 0) - return err; - } - return 0; } @@ -1893,8 +1798,6 @@ void __connman_session_cleanup(void) if (!connection) return; - cleanup_firewall(); - connman_notifier_unregister(&session_notifier); g_hash_table_foreach(session_hash, release_session, NULL); diff --git a/tools/iptables-unit.c b/tools/iptables-unit.c index e919aa5..426631a 100644 --- a/tools/iptables-unit.c +++ b/tools/iptables-unit.c @@ -406,112 +406,6 @@ static void test_nat_basic1(void) g_free(service); } -static void test_firewall_basic0(void) -{ - struct firewall_context *ctx; - int err; - - ctx = __connman_firewall_create(); - g_assert(ctx); - - err = __connman_firewall_add_rule(ctx, "filter", "INPUT", - "-m mark --mark 999 -j LOG"); - g_assert(err >= 0); - - err = __connman_firewall_enable(ctx); - g_assert(err == 0); - - assert_rule_exists("filter", ":connman-INPUT - [0:0]"); - assert_rule_exists("filter", "-A INPUT -j connman-INPUT"); - assert_rule_exists("filter", "-A connman-INPUT -m mark --mark 0x3e7 -j LOG"); - - err = __connman_firewall_disable(ctx); - g_assert(err == 0); - - assert_rule_not_exists("filter", ":connman-INPUT - [0:0]"); - assert_rule_not_exists("filter", "-A INPUT -j connman-INPUT"); - assert_rule_not_exists("filter", "-A connman-INPUT -m mark --mark 0x3e7 -j LOG"); - - __connman_firewall_destroy(ctx); -} - -static void test_firewall_basic1(void) -{ - struct firewall_context *ctx; - int err; - - ctx = __connman_firewall_create(); - g_assert(ctx); - - err = __connman_firewall_add_rule(ctx, "filter", "INPUT", - "-m mark --mark 999 -j LOG"); - g_assert(err >= 0); - - err = __connman_firewall_add_rule(ctx, "filter", "OUTPUT", - "-m mark --mark 999 -j LOG"); - g_assert(err >= 0); - - err = __connman_firewall_enable(ctx); - g_assert(err == 0); - - err = __connman_firewall_disable(ctx); - g_assert(err == 0); - - __connman_firewall_destroy(ctx); -} - -static void test_firewall_basic2(void) -{ - struct firewall_context *ctx; - int err; - - ctx = __connman_firewall_create(); - g_assert(ctx); - - err = __connman_firewall_add_rule(ctx, "mangle", "INPUT", - "-j CONNMARK --restore-mark"); - g_assert(err >= 0); - - err = __connman_firewall_add_rule(ctx, "mangle", "POSTROUTING", - "-j CONNMARK --save-mark"); - g_assert(err >= 0); - - err = __connman_firewall_enable(ctx); - g_assert(err == 0); - - err = __connman_firewall_disable(ctx); - g_assert(err == 0); - - __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, @@ -558,7 +452,6 @@ int main(int argc, char *argv[]) "Unit Tests Connection Manager", VERSION); __connman_iptables_init(); - __connman_firewall_init(); __connman_nat_init(); g_test_add_func("/iptables/chain0", test_iptables_chain0); @@ -571,15 +464,10 @@ int main(int argc, char *argv[]) g_test_add_func("/iptables/target0", test_iptables_target0); g_test_add_func("/nat/basic0", test_nat_basic0); g_test_add_func("/nat/basic1", test_nat_basic1); - 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(); __connman_nat_cleanup(); - __connman_firewall_cleanup(); __connman_iptables_cleanup(); g_free(option_debug); -- 2.7.4