colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

core: refactor tracking of shared-rules to use NMUtilsShareRules

Browse Source 
It's a bit ugly that NMActRequest also tracks the shared rules.
Why? It's just some additional state (the rules) and some additional
actions that should be done when activating/deactivating the profile.
NMActRequest also doesn't track the NMDhcpClient, so why these shared
rules?

Also, removing the rules from an object destructor is ugly. NMActRequest
is a GObject and ref-counted. We should not make assumptions when the
last reference gets releases, at least not in cases like this, where
we hand out the reference and the object is passed around through large
parts of the source code.

For now, still let NMActRequest keep track of NMUtilsShareRules.
Later this will be refactored too.
This commit is contained in:
Thomas Haller
2020-10-27 17:07:43 +01:00
parent 0c2d5fc972
commit 701654b930
3 changed files with 87 additions and 159 deletions
Download Patch File Download Diff File Expand all files Collapse all files

116
src/devices/nm-device.c
View File

@@ -11589,15 +11589,6 @@ share_init(NMDevice *self, GError **error)
return TRUE;
}
#define add_share_rule(req, table, ...) \
G_STMT_START \
{ \
char *_cmd = g_strdup_printf(__VA_ARGS__); \
nm_act_request_add_share_rule(req, table, _cmd); \
g_free(_cmd); \
} \
G_STMT_END
static gboolean
start_sharing(NMDevice *self, NMIP4Config *config, GError **error)
{
@@ -11612,6 +11603,7 @@ start_sharing(NMDevice *self, NMIP4Config *config, GError **error)
NMConnection * conn;
NMSettingConnection * s_con;
gboolean announce_android_metered;
NMUtilsShareRules * share_rules;
g_return_val_if_fail(config, FALSE);
@@ -11642,51 +11634,67 @@ start_sharing(NMDevice *self, NMIP4Config *config, GError **error)
network = ip4_addr->address & netmask;
_nm_utils_inet4_ntop(network, str_addr);
add_share_rule(req,
"nat",
"POSTROUTING --source %s/%s ! --destination %s/%s --jump MASQUERADE",
str_addr,
str_mask,
str_addr,
str_mask);
add_share_rule(req,
"filter",
"FORWARD --destination %s/%s --out-interface %s --match state --state "
"ESTABLISHED,RELATED --jump ACCEPT",
str_addr,
str_mask,
ip_iface);
add_share_rule(req,
"filter",
"FORWARD --source %s/%s --in-interface %s --jump ACCEPT",
str_addr,
str_mask,
ip_iface);
add_share_rule(req,
"filter",
"FORWARD --in-interface %s --out-interface %s --jump ACCEPT",
ip_iface,
ip_iface);
add_share_rule(req, "filter", "FORWARD --out-interface %s --jump REJECT", ip_iface);
add_share_rule(req, "filter", "FORWARD --in-interface %s --jump REJECT", ip_iface);
add_share_rule(req,
"filter",
"INPUT --in-interface %s --protocol udp --destination-port 67 --jump ACCEPT",
ip_iface);
add_share_rule(req,
"filter",
"INPUT --in-interface %s --protocol tcp --destination-port 67 --jump ACCEPT",
ip_iface);
add_share_rule(req,
"filter",
"INPUT --in-interface %s --protocol udp --destination-port 53 --jump ACCEPT",
ip_iface);
add_share_rule(req,
"filter",
"INPUT --in-interface %s --protocol tcp --destination-port 53 --jump ACCEPT",
ip_iface);
share_rules = nm_utils_share_rules_new();
nm_act_request_set_shared(req, TRUE);
nm_utils_share_rules_add_rule_v(
share_rules,
"nat",
"POSTROUTING --source %s/%s ! --destination %s/%s --jump MASQUERADE",
str_addr,
str_mask,
str_addr,
str_mask);
nm_utils_share_rules_add_rule_v(
share_rules,
"filter",
"FORWARD --destination %s/%s --out-interface %s --match state --state "
"ESTABLISHED,RELATED --jump ACCEPT",
str_addr,
str_mask,
ip_iface);
nm_utils_share_rules_add_rule_v(share_rules,
"filter",
"FORWARD --source %s/%s --in-interface %s --jump ACCEPT",
str_addr,
str_mask,
ip_iface);
nm_utils_share_rules_add_rule_v(share_rules,
"filter",
"FORWARD --in-interface %s --out-interface %s --jump ACCEPT",
ip_iface,
ip_iface);
nm_utils_share_rules_add_rule_v(share_rules,
"filter",
"FORWARD --out-interface %s --jump REJECT",
ip_iface);
nm_utils_share_rules_add_rule_v(share_rules,
"filter",
"FORWARD --in-interface %s --jump REJECT",
ip_iface);
nm_utils_share_rules_add_rule_v(
share_rules,
"filter",
"INPUT --in-interface %s --protocol udp --destination-port 67 --jump ACCEPT",
ip_iface);
nm_utils_share_rules_add_rule_v(
share_rules,
"filter",
"INPUT --in-interface %s --protocol tcp --destination-port 67 --jump ACCEPT",
ip_iface);
nm_utils_share_rules_add_rule_v(
share_rules,
"filter",
"INPUT --in-interface %s --protocol udp --destination-port 53 --jump ACCEPT",
ip_iface);
nm_utils_share_rules_add_rule_v(
share_rules,
"filter",
"INPUT --in-interface %s --protocol tcp --destination-port 53 --jump ACCEPT",
ip_iface);
nm_utils_share_rules_apply(share_rules, TRUE);
nm_act_request_set_shared(req, share_rules);
conn = nm_act_request_get_applied_connection(req);
s_con = nm_connection_get_setting_connection(conn);
@@ -11721,7 +11729,7 @@ start_sharing(NMDevice *self, NMIP4Config *config, GError **error)
"could not start dnsmasq due to %s",
local->message);
g_error_free(local);
nm_act_request_set_shared(req, FALSE);
nm_act_request_set_shared(req, NULL);
return FALSE;
}

120
src/nm-act-request.c
View File

@@ -22,14 +22,8 @@
#include "nm-libnm-core-intern/nm-auth-subject.h"
typedef struct {
char *table;
char *rule;
} ShareRule;
typedef struct {
CList call_ids_lst_head;
gboolean shared;
GSList * share_rules;
CList call_ids_lst_head;
NMUtilsShareRules *share_rules;
} NMActRequestPrivate;
struct _NMActRequest {
@@ -254,109 +248,32 @@ nm_act_request_clear_secrets(NMActRequest *self)
/*****************************************************************************/
static void
clear_share_rules(NMActRequest *req)
{
NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE(req);
GSList * iter;
for (iter = priv->share_rules; iter; iter = g_slist_next(iter)) {
ShareRule *rule = (ShareRule *) iter->data;
g_free(rule->table);
g_free(rule->rule);
g_free(rule);
}
g_slist_free(priv->share_rules);
priv->share_rules = NULL;
}
void
nm_act_request_set_shared(NMActRequest *req, gboolean shared)
{
NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE(req);
GSList * list, *iter;
g_return_if_fail(NM_IS_ACT_REQUEST(req));
NM_ACT_REQUEST_GET_PRIVATE(req)->shared = shared;
/* Tear the rules down in reverse order when sharing is stopped */
list = g_slist_copy(priv->share_rules);
if (!shared)
list = g_slist_reverse(list);
/* Send the rules to iptables */
for (iter = list; iter; iter = g_slist_next(iter)) {
ShareRule * rule = (ShareRule *) iter->data;
char * envp[1] = {NULL};
gs_strfreev char **argv = NULL;
gs_free char * cmd = NULL;
cmd = g_strdup_printf("%s --table %s %s %s",
IPTABLES_PATH,
rule->table,
shared ? "--insert" : "--delete",
rule->rule);
if (!cmd)
continue;
argv = g_strsplit(cmd, " ", 0);
if (argv && argv[0]) {
int status;
GError *error = NULL;
nm_log_info(LOGD_SHARING, "Executing: %s", cmd);
if (!g_spawn_sync("/",
argv,
envp,
G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
NULL,
NULL,
NULL,
NULL,
&status,
&error)) {
nm_log_warn(LOGD_SHARING, "Error executing command: %s", error->message);
g_clear_error(&error);
} else if (WEXITSTATUS(status)) {
nm_log_warn(LOGD_SHARING,
"** Command returned exit status %d.",
WEXITSTATUS(status));
}
}
}
g_slist_free(list);
/* Clear the share rule list when sharing is stopped */
if (!shared)
clear_share_rules(req);
}
gboolean
NMUtilsShareRules *
nm_act_request_get_shared(NMActRequest *req)
{
g_return_val_if_fail(NM_IS_ACT_REQUEST(req), FALSE);
return NM_ACT_REQUEST_GET_PRIVATE(req)->shared;
return NM_ACT_REQUEST_GET_PRIVATE(req)->share_rules;
}
void
nm_act_request_add_share_rule(NMActRequest *req, const char *table, const char *table_rule)
nm_act_request_set_shared(NMActRequest *req, NMUtilsShareRules *rules)
{
NMActRequestPrivate *priv = NM_ACT_REQUEST_GET_PRIVATE(req);
ShareRule * rule;
g_return_if_fail(NM_IS_ACT_REQUEST(req));
g_return_if_fail(table != NULL);
g_return_if_fail(table_rule != NULL);
rule = g_malloc0(sizeof(ShareRule));
rule->table = g_strdup(table);
rule->rule = g_strdup(table_rule);
priv->share_rules = g_slist_prepend(priv->share_rules, rule);
if (priv->share_rules == rules)
return;
if (priv->share_rules) {
nm_utils_share_rules_apply(priv->share_rules, FALSE);
priv->share_rules = NULL;
}
if (rules) {
priv->share_rules = rules;
nm_utils_share_rules_apply(priv->share_rules, TRUE);
}
}
/*****************************************************************************/
@@ -589,10 +506,9 @@ dispose(GObject *object)
c_list_for_each_entry_safe (call_id, call_id_safe, &priv->call_ids_lst_head, call_ids_lst)
_do_cancel_secrets(self, call_id, TRUE);
/* Clear any share rules */
if (priv->share_rules) {
nm_act_request_set_shared(NM_ACT_REQUEST(object), FALSE);
clear_share_rules(NM_ACT_REQUEST(object));
nm_utils_share_rules_apply(priv->share_rules, FALSE);
nm_clear_pointer(&priv->share_rules, nm_utils_share_rules_free);
}
G_OBJECT_CLASS(nm_act_request_parent_class)->dispose(object);

10
src/nm-act-request.h
View File

@@ -36,11 +36,15 @@ NMSettingsConnection *nm_act_request_get_settings_connection(NMActRequest *req);
NMConnection *nm_act_request_get_applied_connection(NMActRequest *req);
gboolean nm_act_request_get_shared(NMActRequest *req);
/*****************************************************************************/
void nm_act_request_set_shared(NMActRequest *req, gboolean shared);
struct _NMUtilsShareRules;
void nm_act_request_add_share_rule(NMActRequest *req, const char *table, const char *rule);
struct _NMUtilsShareRules *nm_act_request_get_shared(NMActRequest *req);
void nm_act_request_set_shared(NMActRequest *req, struct _NMUtilsShareRules *rules);
/*****************************************************************************/
/* Secrets handling */