Commit 2298624d authored by Jussi Laakkonen's avatar Jussi Laakkonen

[connman] Use config file name in firewall rule sorting. JB#44071

This adds sorting of firewall rules using the config file name as sort
criteria. The rules are read from alphabetical list of configuration
files at startup and were added in correct order. But when reloading the
order must be kept and this commit, by adding each rule into sorted list
of rules, guarantees that the order of the rules is always consistent.
The rules that are added by the system (config_file is NULL) are sorted
on top of the list.

With currently active connections which have firewall enabled the rules
are simply appended at the end. The order is effective after the service
is reconnected.

The default static rules are ordered but no change for these is made
into iptables. The change of rule order is effective only after connman
restart. This issue should be addressed in the future.
parent d8b03fec
......@@ -1059,10 +1059,12 @@ 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 *config_file,
const char *table,
const char *chain,
const char *rule_fmt, ...);
int __connman_firewall_add_ipv6_rule(struct firewall_context *ctx,
const char *config_file,
const char *table,
const char *chain,
const char *rule_fmt, ...);
......
......@@ -63,6 +63,7 @@ struct fw_rule {
char *chain;
char *rule_spec;
char *ifname;
char *config_file;
};
struct firewall_context {
......@@ -139,6 +140,22 @@ static const char *supported_policiesv6[] = {
*/
static GHashTable *current_dynamic_rules = NULL;
static int firewall_rule_compare(gconstpointer a, gconstpointer b)
{
const struct fw_rule *rule_a;
const struct fw_rule *rule_b;
rule_a = a;
rule_b = b;
/*
* g_strcmp0 sorts NULLs before others, the system defined rules that
* are added by connman have no config_file and should be on top of
* other rules.
*/
return g_strcmp0(rule_a->config_file, rule_b->config_file);
}
static int chain_to_index(const char *chain_name)
{
if (!g_strcmp0(builtin_chains[NF_IP_PRE_ROUTING], chain_name))
......@@ -385,6 +402,7 @@ static void cleanup_fw_rule(gpointer user_data)
g_free(rule->rule_spec);
g_free(rule->chain);
g_free(rule->table);
g_free(rule->config_file);
g_free(rule);
}
......@@ -461,6 +479,7 @@ static int firewall_disable_rule(struct fw_rule *rule)
}
int __connman_firewall_add_rule(struct firewall_context *ctx,
const char *config_file,
const char *table,
const char *chain,
const char *rule_fmt, ...)
......@@ -480,15 +499,21 @@ int __connman_firewall_add_rule(struct firewall_context *ctx,
rule->id = firewall_rule_id++;
rule->type = AF_INET;
rule->enabled = false;
if (config_file)
rule->config_file = g_path_get_basename(config_file);
rule->table = g_strdup(table);
rule->chain = g_strdup(chain);
rule->rule_spec = rule_spec;
ctx->rules = g_list_append(ctx->rules, rule);
ctx->rules = g_list_insert_sorted(ctx->rules, rule,
firewall_rule_compare);
return rule->id;
}
int __connman_firewall_add_ipv6_rule(struct firewall_context *ctx,
const char *config_file,
const char *table,
const char *chain,
const char *rule_fmt, ...)
......@@ -508,11 +533,16 @@ int __connman_firewall_add_ipv6_rule(struct firewall_context *ctx,
rule->id = firewall_rule_id++;
rule->type = AF_INET6;
rule->enabled = false;
if (config_file)
rule->config_file = g_path_get_basename(config_file);
rule->table = g_strdup(table);
rule->chain = g_strdup(chain);
rule->rule_spec = rule_spec;
ctx->rules = g_list_append(ctx->rules, rule);
ctx->rules = g_list_insert_sorted(ctx->rules, rule,
firewall_rule_compare);
return rule->id;
}
......@@ -829,6 +859,10 @@ static gpointer copy_fw_rule(gconstpointer src, gpointer data)
new->id = firewall_rule_id++;
new->enabled = false;
new->type = old->type;
if (old->config_file)
new->config_file = g_strdup(old->config_file);
new->table = g_strdup(old->table);
new->chain = g_strdup(old->chain);
new->rule_spec = g_strdup(old->rule_spec);
......@@ -1016,21 +1050,20 @@ static int add_default_tethering_rules(struct firewall_context *ctx,
/* Add tethering rules for both IPv4 and IPv6 when using usb */
for (i = 0; tethering_rules[i]; i++) {
id = __connman_firewall_add_rule(ctx, "filter", "INPUT",
id = __connman_firewall_add_rule(ctx, NULL, "filter", "INPUT",
tethering_rules[i]);
if (id < 0)
DBG("cannot add IPv4 rule %s",
tethering_rules[i]);
id = __connman_firewall_add_ipv6_rule(ctx, "filter",
id = __connman_firewall_add_ipv6_rule(ctx, NULL, "filter",
"INPUT", tethering_rules[i]);
if (id < 0)
DBG("cannot add IPv6 rule %s",
tethering_rules[i]);
}
g_list_foreach(ctx->rules, setup_firewall_rule_interface,
ifname);
g_list_foreach(ctx->rules, setup_firewall_rule_interface, ifname);
return 0;
}
......@@ -1712,11 +1745,11 @@ static bool is_rule_in_context(struct firewall_context *ctx, int type,
return false;
}
typedef int (*add_rules_cb_t)(int type, const char *group, int chain_id,
char** rules);
typedef int (*add_rules_cb_t)(int type, const char *filename, const char *group,
int chain_id, char** rules);
static int add_dynamic_rules_cb(int type, const char *group, int chain_id,
char** rules)
static int add_dynamic_rules_cb(int type, const char *filename,
const char *group, int chain_id, char** rules)
{
enum connman_service_type service_type;
char table[] = "filter";
......@@ -1755,13 +1788,15 @@ static int add_dynamic_rules_cb(int type, const char *group, int chain_id,
case AF_INET:
id = __connman_firewall_add_rule(
dynamic_rules[service_type],
table, builtin_chains[chain_id],
filename, table,
builtin_chains[chain_id],
rules[i]);
break;
case AF_INET6:
id = __connman_firewall_add_ipv6_rule(
dynamic_rules[service_type],
table, builtin_chains[chain_id],
filename, table,
builtin_chains[chain_id],
rules[i]);
break;
default:
......@@ -1785,8 +1820,8 @@ static int add_dynamic_rules_cb(int type, const char *group, int chain_id,
return err;
}
static int add_general_rules_cb(int type, const char *group, int chain_id,
char** rules)
static int add_general_rules_cb(int type, const char *filename,
const char *group, int chain_id, char** rules)
{
char table[] = "filter";
int count = 0;
......@@ -1833,13 +1868,15 @@ static int add_general_rules_cb(int type, const char *group, int chain_id,
switch (type) {
case AF_INET:
id = __connman_firewall_add_rule(general_firewall->ctx,
table, builtin_chains[chain_id],
rules[i]);
filename, table,
builtin_chains[chain_id],
rules[i]);
break;
case AF_INET6:
id = __connman_firewall_add_ipv6_rule(
general_firewall->ctx, table,
builtin_chains[chain_id], rules[i]);
general_firewall->ctx, filename,
table, builtin_chains[chain_id],
rules[i]);
break;
default:
id = -1;
......@@ -1863,8 +1900,8 @@ static int add_general_rules_cb(int type, const char *group, int chain_id,
return err;
}
static int add_tethering_rules_cb(int type, const char *group, int chain_id,
char** rules)
static int add_tethering_rules_cb(int type, const char *filename,
const char *group, int chain_id, char** rules)
{
char table[] = "filter";
int count = 0;
......@@ -1908,13 +1945,16 @@ static int add_tethering_rules_cb(int type, const char *group, int chain_id,
switch (type) {
case AF_INET:
id = __connman_firewall_add_rule(tethering_firewall,
table, builtin_chains[chain_id],
rules[i]);
filename, table,
builtin_chains[chain_id],
rules[i]);
break;
case AF_INET6:
id = __connman_firewall_add_ipv6_rule(
tethering_firewall, table,
builtin_chains[chain_id], rules[i]);
tethering_firewall,
filename, table,
builtin_chains[chain_id],
rules[i]);
break;
default:
id = -1;
......@@ -1938,8 +1978,8 @@ static int add_tethering_rules_cb(int type, const char *group, int chain_id,
return err;
}
static int add_rules_from_group(GKeyFile *config, const char *group,
add_rules_cb_t cb)
static int add_rules_from_group(const char *filename, GKeyFile *config,
const char *group, add_rules_cb_t cb)
{
GError *error = NULL;
char** rules;
......@@ -1981,7 +2021,8 @@ static int add_rules_from_group(GKeyFile *config, const char *group,
DBG("found %d rules in group %s chain %s", len,
group, chain_name);
count = cb(types[i], group, chain, rules);
count = cb(types[i], filename, group, chain,
rules);
if (count < 0) {
DBG("cannot add rules from config");
......@@ -2387,7 +2428,7 @@ static int init_general_firewall_policies(GKeyFile *config)
return err;
}
static int init_general_firewall(GKeyFile *config)
static int init_general_firewall(const char *config_file, GKeyFile *config)
{
int err;
......@@ -2408,7 +2449,8 @@ static int init_general_firewall(GKeyFile *config)
if (err)
DBG("cannot initialize general policies"); // TODO react to this
err = add_rules_from_group(config, GROUP_GENERAL, add_general_rules_cb);
err = add_rules_from_group(config_file, config, GROUP_GENERAL,
add_general_rules_cb);
if (err)
DBG("cannot setup general firewall rules");
......@@ -2450,7 +2492,7 @@ static int init_dynamic_firewall_rules(const char *file)
goto out;
}
if (init_general_firewall(config))
if (init_general_firewall(file, config))
DBG("Cannot setup general firewall");
if (!dynamic_rules)
......@@ -2475,11 +2517,12 @@ static int init_dynamic_firewall_rules(const char *file)
if (!group)
continue;
if (add_rules_from_group(config, group, add_dynamic_rules_cb))
if (add_rules_from_group(file, config, group,
add_dynamic_rules_cb))
DBG("failed to process rules from group type %d", type);
}
if (add_rules_from_group(config, GROUP_TETHERING,
if (add_rules_from_group(file, config, GROUP_TETHERING,
add_tethering_rules_cb))
DBG("failed to add tethering rules");
......@@ -2728,7 +2771,8 @@ static int copy_new_dynamic_rules(struct firewall_context *dyn_ctx,
int err;
/* Go over dynamic rules for this type */
for (dyn_list = dyn_ctx->rules; dyn_list; dyn_list = dyn_list->next) {
for (dyn_list = g_list_first(dyn_ctx->rules); dyn_list;
dyn_list = dyn_list->next) {
dyn_rule = dyn_list->data;
/* If the dynamic rule is already added for service firewall */
......@@ -2739,7 +2783,8 @@ static int copy_new_dynamic_rules(struct firewall_context *dyn_ctx,
new_rule = copy_fw_rule(dyn_rule, ifname);
srv_ctx->rules = g_list_append(srv_ctx->rules, new_rule);
srv_ctx->rules = g_list_insert_sorted(srv_ctx->rules, new_rule,
firewall_rule_compare);
if (srv_ctx->enabled) {
err = firewall_enable_rule(new_rule);
......@@ -2748,7 +2793,7 @@ static int copy_new_dynamic_rules(struct firewall_context *dyn_ctx,
DBG("new rule not enabled %d", err);
}
}
return 0;
}
......
......@@ -98,7 +98,7 @@ static int enable_nat(struct connman_nat *nat)
nat->prefixlen,
nat->interface);
err = __connman_firewall_add_rule(nat->fw, "nat",
err = __connman_firewall_add_rule(nat->fw, NULL, "nat",
"POSTROUTING", cmd);
g_free(cmd);
if (err < 0)
......
......@@ -205,12 +205,12 @@ static int init_firewall(void)
fw = __connman_firewall_create();
err = __connman_firewall_add_rule(fw, "mangle", "INPUT",
err = __connman_firewall_add_rule(fw, NULL, "mangle", "INPUT",
"-j CONNMARK --restore-mark");
if (err < 0)
goto err;
err = __connman_firewall_add_rule(fw, "mangle", "POSTROUTING",
err = __connman_firewall_add_rule(fw, NULL, "mangle", "POSTROUTING",
"-j CONNMARK --save-mark");
if (err < 0)
goto err;
......@@ -258,13 +258,13 @@ static int init_firewall_session(struct connman_session *session)
switch (session->policy_config->id_type) {
case CONNMAN_SESSION_ID_TYPE_UID:
err = __connman_firewall_add_rule(fw, "mangle", "OUTPUT",
err = __connman_firewall_add_rule(fw, NULL, "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",
err = __connman_firewall_add_rule(fw, NULL, "mangle", "OUTPUT",
"-m owner --gid-owner %s -j MARK --set-mark %d",
session->policy_config->id,
session->mark);
......@@ -402,8 +402,8 @@ static void add_nat_rules(struct connman_session *session)
ifname = connman_inet_ifname(index);
addr = __connman_ipconfig_get_local(ipconfig);
id = __connman_firewall_add_rule(session->fw, "nat", "POSTROUTING",
"-o %s -j SNAT --to-source %s",
id = __connman_firewall_add_rule(session->fw, NULL, "nat",
"POSTROUTING", "-o %s -j SNAT --to-source %s",
ifname, addr);
g_free(ifname);
if (id < 0) {
......
......@@ -626,7 +626,7 @@ static void test_firewall_basic0(void)
ctx = __connman_firewall_create();
g_assert(ctx);
err = __connman_firewall_add_rule(ctx, "filter", "INPUT",
err = __connman_firewall_add_rule(ctx, NULL, "filter", "INPUT",
"-m mark --mark 999 -j LOG");
g_assert(err >= 0);
......@@ -657,11 +657,11 @@ static void test_firewall_basic1(void)
ctx = __connman_firewall_create();
g_assert(ctx);
err = __connman_firewall_add_rule(ctx, "filter", "INPUT",
err = __connman_firewall_add_rule(ctx, NULL, "filter", "INPUT",
"-m mark --mark 999 -j LOG");
g_assert(err >= 0);
err = __connman_firewall_add_rule(ctx, "filter", "OUTPUT",
err = __connman_firewall_add_rule(ctx, NULL, "filter", "OUTPUT",
"-m mark --mark 999 -j LOG");
g_assert(err >= 0);
......@@ -682,11 +682,11 @@ static void test_firewall_basic2(void)
ctx = __connman_firewall_create();
g_assert(ctx);
err = __connman_firewall_add_rule(ctx, "mangle", "INPUT",
err = __connman_firewall_add_rule(ctx, NULL, "mangle", "INPUT",
"-j CONNMARK --restore-mark");
g_assert(err >= 0);
err = __connman_firewall_add_rule(ctx, "mangle", "POSTROUTING",
err = __connman_firewall_add_rule(ctx, NULL, "mangle", "POSTROUTING",
"-j CONNMARK --save-mark");
g_assert(err >= 0);
......@@ -707,7 +707,7 @@ static void test_firewall_basic3(void)
ctx = __connman_firewall_create();
g_assert(ctx);
id = __connman_firewall_add_rule(ctx, "mangle", "INPUT",
id = __connman_firewall_add_rule(ctx, NULL, "mangle", "INPUT",
"-j CONNMARK --restore-mark");
g_assert(id >= 0);
......@@ -1002,7 +1002,7 @@ static void test_firewall6_basic0(void)
ctx = __connman_firewall_create();
g_assert(ctx);
err = __connman_firewall_add_ipv6_rule(ctx, "filter", "INPUT",
err = __connman_firewall_add_ipv6_rule(ctx, NULL, "filter", "INPUT",
"-m mark --mark 999 -j LOG");
g_assert(err >= 0);
......@@ -1033,11 +1033,11 @@ static void test_firewall6_basic1(void)
ctx = __connman_firewall_create();
g_assert(ctx);
err = __connman_firewall_add_ipv6_rule(ctx, "filter", "INPUT",
err = __connman_firewall_add_ipv6_rule(ctx, NULL, "filter", "INPUT",
"-m mark --mark 999 -j LOG");
g_assert(err >= 0);
err = __connman_firewall_add_rule(ctx, "filter", "OUTPUT",
err = __connman_firewall_add_rule(ctx, NULL, "filter", "OUTPUT",
"-m mark --mark 999 -j LOG");
g_assert(err >= 0);
......@@ -1058,11 +1058,11 @@ static void test_firewall6_basic2(void)
ctx = __connman_firewall_create();
g_assert(ctx);
err = __connman_firewall_add_ipv6_rule(ctx, "mangle", "INPUT",
err = __connman_firewall_add_ipv6_rule(ctx, NULL, "mangle", "INPUT",
"-j CONNMARK --restore-mark");
g_assert(err >= 0);
err = __connman_firewall_add_ipv6_rule(ctx, "mangle", "POSTROUTING",
err = __connman_firewall_add_ipv6_rule(ctx, NULL, "mangle", "POSTROUTING",
"-j CONNMARK --save-mark");
g_assert(err >= 0);
......@@ -1083,7 +1083,7 @@ static void test_firewall6_basic3(void)
ctx = __connman_firewall_create();
g_assert(ctx);
id = __connman_firewall_add_rule(ctx, "mangle", "INPUT",
id = __connman_firewall_add_rule(ctx, NULL, "mangle", "INPUT",
"-j CONNMARK --restore-mark");
g_assert(id >= 0);
......@@ -1111,24 +1111,24 @@ static void test_firewall_4and6_basic0(void)
g_assert(ctx);
err = __connman_firewall_add_rule(ctx, "filter", "INPUT",
err = __connman_firewall_add_rule(ctx, NULL, "filter", "INPUT",
"-p icmp -m icmp "
"--icmp-type 8/0 -j DROP");
g_assert(err >= 0);
err = __connman_firewall_add_rule(ctx, "filter", "OUTPUT",
err = __connman_firewall_add_rule(ctx, NULL, "filter", "OUTPUT",
"-p icmp -m icmp "
"--icmp-type 0/0 -j DROP");
g_assert(err >= 0);
err = __connman_firewall_add_ipv6_rule(ctx, "filter", "INPUT",
err = __connman_firewall_add_ipv6_rule(ctx, NULL, "filter", "INPUT",
"-p icmpv6 -m icmpv6 "
"--icmpv6-type 128/0 -j DROP");
g_assert(err >= 0);
err = __connman_firewall_add_ipv6_rule(ctx, "filter", "OUTPUT",
err = __connman_firewall_add_ipv6_rule(ctx, NULL, "filter", "OUTPUT",
"-p icmpv6 -m icmpv6 "
"--icmpv6-type 129/0 -j DROP");
g_assert(err >= 0);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment