core: refactor tracking of shared-rules to use NMUtilsShareRules

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.

src/devices/nm-device.c

@@ -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,
+    share_rules = nm_utils_share_rules_new();
+
+    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);
-    add_share_rule(req,
+    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);
-    add_share_rule(req,
+    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);
-    add_share_rule(req,
+    nm_utils_share_rules_add_rule_v(share_rules,
                                     "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);
+    nm_utils_share_rules_add_rule_v(share_rules,
-    add_share_rule(req, "filter", "FORWARD --in-interface %s --jump REJECT", ip_iface);
+                                    "filter",
-    add_share_rule(req,
+                                    "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);
-    add_share_rule(req,
+    nm_utils_share_rules_add_rule_v(
+        share_rules,
         "filter",
         "INPUT --in-interface %s --protocol tcp --destination-port 67 --jump ACCEPT",
         ip_iface);
-    add_share_rule(req,
+    nm_utils_share_rules_add_rule_v(
+        share_rules,
         "filter",
         "INPUT --in-interface %s --protocol udp --destination-port 53 --jump ACCEPT",
         ip_iface);
-    add_share_rule(req,
+    nm_utils_share_rules_add_rule_v(
+        share_rules,
         "filter",
         "INPUT --in-interface %s --protocol tcp --destination-port 53 --jump ACCEPT",
         ip_iface);
 
-    nm_act_request_set_shared(req, TRUE);
+    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;
     }
 

src/nm-act-request.c

@@ -21,15 +21,9 @@
 #include "settings/nm-settings-connection.h"
 #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;
+    NMUtilsShareRules *share_rules;
-    GSList * share_rules;
 } NMActRequestPrivate;
 
 struct _NMActRequest {
@@ -254,109 +248,32 @@ nm_act_request_clear_secrets(NMActRequest *self)
 
 /*****************************************************************************/
 
-static void
+NMUtilsShareRules *
-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
 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));
+    if (priv->share_rules == rules)
-    rule->table       = g_strdup(table);
+        return;
-    rule->rule        = g_strdup(table_rule);
+
-    priv->share_rules = g_slist_prepend(priv->share_rules, rule);
+    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);
+        nm_utils_share_rules_apply(priv->share_rules, FALSE);
-        clear_share_rules(NM_ACT_REQUEST(object));
+        nm_clear_pointer(&priv->share_rules, nm_utils_share_rules_free);
     }
 
     G_OBJECT_CLASS(nm_act_request_parent_class)->dispose(object);

src/nm-act-request.h

@@ -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 */