From 78cc70feb33c9721cfce0f614d3bed656698056b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Aug 2021 13:31:07 +0200 Subject: [PATCH 1/5] firewalld: prefix firewalld logging messages with "firewalld" It seems more apt than "firewall: ...". (cherry picked from commit b55f95abfa02e9199e93a4598d1175abe21812a3) --- src/core/nm-firewalld-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/nm-firewalld-manager.c b/src/core/nm-firewalld-manager.c index c562d97856..1078529a42 100644 --- a/src/core/nm-firewalld-manager.c +++ b/src/core/nm-firewalld-manager.c @@ -105,7 +105,7 @@ _ops_type_to_string(OpsType ops_type) } #define _NMLOG_DOMAIN LOGD_FIREWALL -#define _NMLOG_PREFIX_NAME "firewall" +#define _NMLOG_PREFIX_NAME "firewalld" #define _NMLOG(level, call_id, ...) \ G_STMT_START \ { \ From 500c66551a098837338a7e6321690aa104408d2f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Aug 2021 12:57:43 +0200 Subject: [PATCH 2/5] firewalld: track current name_owner in NMFirewalldManager Not only track whether we have a name-owner, but also which. (cherry picked from commit 9debc3d028f8eca84ae402b9bf920ce6448c6f60) --- src/core/nm-firewalld-manager.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/core/nm-firewalld-manager.c b/src/core/nm-firewalld-manager.c index 1078529a42..3c80b45772 100644 --- a/src/core/nm-firewalld-manager.c +++ b/src/core/nm-firewalld-manager.c @@ -30,10 +30,11 @@ typedef struct { CList pending_calls; + char *name_owner; + guint name_owner_changed_id; bool dbus_inited : 1; - bool running : 1; } NMFirewalldManagerPrivate; struct _NMFirewalldManager { @@ -154,7 +155,7 @@ _get_running(NMFirewalldManagerPrivate *priv) * service is indeed running. That is the time when we queue the * requests, and they will be started once the get-name-owner call * returns. */ - return priv->running || (priv->dbus_connection && !priv->dbus_inited); + return priv->name_owner || (priv->dbus_connection && !priv->dbus_inited); } gboolean @@ -315,7 +316,7 @@ _handle_dbus_start(NMFirewalldManager *self, NMFirewalldManagerCallId *call_id) GVariant * arg; nm_assert(call_id); - nm_assert(priv->running); + nm_assert(priv->name_owner); nm_assert(!call_id->is_idle); nm_assert(c_list_contains(&priv->pending_calls, &call_id->lst)); @@ -378,10 +379,10 @@ _start_request(NMFirewalldManager * self, iface, NM_PRINT_FMT_QUOTED(zone, "\"", zone, "\"", "default"), call_id->is_idle ? " (not running, simulate success)" - : (!priv->running ? " (waiting to initialize)" : "")); + : (!priv->name_owner ? " (waiting to initialize)" : "")); if (!call_id->is_idle) { - if (priv->running) + if (priv->name_owner) _handle_dbus_start(self, call_id); if (!call_id->callback) { /* if the user did not provide a callback, the call_id is useless. @@ -463,6 +464,7 @@ name_owner_changed(NMFirewalldManager *self, const char *owner) gboolean was_running; gboolean now_running; gboolean just_initied; + gboolean name_owner_changed; owner = nm_str_not_empty(owner); @@ -474,8 +476,8 @@ name_owner_changed(NMFirewalldManager *self, const char *owner) was_running = _get_running(priv); just_initied = !priv->dbus_inited; - priv->dbus_inited = TRUE; - priv->running = !!owner; + priv->dbus_inited = TRUE; + name_owner_changed = nm_strdup_reset(&priv->name_owner, owner); now_running = _get_running(priv); @@ -495,7 +497,7 @@ name_owner_changed(NMFirewalldManager *self, const char *owner) nm_assert(!call_id->is_idle); nm_assert(call_id->dbus.arg); - if (priv->running) { + if (priv->name_owner) { _LOGD(call_id, "initalizing: make D-Bus call"); _handle_dbus_start(self, call_id); } else { @@ -511,7 +513,7 @@ name_owner_changed(NMFirewalldManager *self, const char *owner) } } - if (was_running != now_running) + if (was_running != now_running || name_owner_changed) g_signal_emit(self, signals[STATE_CHANGED], 0, FALSE); } @@ -541,7 +543,7 @@ get_name_owner_cb(const char *name_owner, GError *error, gpointer user_data) NMFirewalldManager * self; NMFirewalldManagerPrivate *priv; - if (!name_owner && g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + if (nm_utils_error_is_cancelled(error)) return; self = user_data; From 17312aa25c55881817829781e6b2cb3ba62d9725 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Aug 2021 12:58:32 +0200 Subject: [PATCH 3/5] firewalld: make D-Bus calls against unique name for firewalld service As we keep track of the current name owner, use its unique name for the D-Bus requests. We also track when the name owner changes, so at the point when we make the D-Bus call, the current name owner was still running. We should talk to it directly. If at the same time, firewalld restarts, we go through our usual tracking of the name owner and will retry -- but always talking to the unique name. (cherry picked from commit 3d949f98e4725be32a93df3730e2d2b2250f185d) --- src/core/nm-firewalld-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/nm-firewalld-manager.c b/src/core/nm-firewalld-manager.c index 3c80b45772..7d0b2bea30 100644 --- a/src/core/nm-firewalld-manager.c +++ b/src/core/nm-firewalld-manager.c @@ -342,7 +342,7 @@ _handle_dbus_start(NMFirewalldManager *self, NMFirewalldManagerCallId *call_id) call_id->dbus.cancellable = g_cancellable_new(); g_dbus_connection_call(priv->dbus_connection, - FIREWALL_DBUS_SERVICE, + priv->name_owner, FIREWALL_DBUS_PATH, FIREWALL_DBUS_INTERFACE_ZONE, dbus_method, From 424535093c3206b5bd70da3f2ce5756d2baaf53b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Aug 2021 13:15:34 +0200 Subject: [PATCH 4/5] firewalld: fix initialized_now argument for NMFirewalldManager's "state-changed" signal (cherry picked from commit b2ed02dda984801160ad3fe1d0a1f7973e93c9cc) --- src/core/nm-firewalld-manager.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/nm-firewalld-manager.c b/src/core/nm-firewalld-manager.c index 7d0b2bea30..ec6fcca5b2 100644 --- a/src/core/nm-firewalld-manager.c +++ b/src/core/nm-firewalld-manager.c @@ -147,6 +147,14 @@ _ops_type_to_string(OpsType ops_type) /*****************************************************************************/ +static void +_signal_emit_state_changed(NMFirewalldManager *self, gboolean initialized_now) +{ + g_signal_emit(self, signals[STATE_CHANGED], 0, initialized_now); +} + +/*****************************************************************************/ + static gboolean _get_running(NMFirewalldManagerPrivate *priv) { @@ -514,7 +522,7 @@ name_owner_changed(NMFirewalldManager *self, const char *owner) } if (was_running != now_running || name_owner_changed) - g_signal_emit(self, signals[STATE_CHANGED], 0, FALSE); + _signal_emit_state_changed(self, just_initied); } static void From d409a3c2301497b2d77dd737f27e495f6a087783 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 2 Aug 2021 09:14:10 +0200 Subject: [PATCH 5/5] firewalld: listen to Reloaded signal and reconfigure firewall zones During reload, firewalld drops the current runtime configuration. NetworkManager should listen to that, and reconfigure the zones that it cares about. (cherry picked from commit 0f100abd851bf36769adaded9b079a925b97a7c6) --- src/core/nm-firewalld-manager.c | 48 ++++++++++++++++++++++++++++----- src/core/nm-firewalld-manager.h | 6 +++++ src/core/nm-policy.c | 13 ++++----- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/core/nm-firewalld-manager.c b/src/core/nm-firewalld-manager.c index ec6fcca5b2..43ab278ad8 100644 --- a/src/core/nm-firewalld-manager.c +++ b/src/core/nm-firewalld-manager.c @@ -15,6 +15,7 @@ #define FIREWALL_DBUS_SERVICE "org.fedoraproject.FirewallD1" #define FIREWALL_DBUS_PATH "/org/fedoraproject/FirewallD1" +#define FIREWALL_DBUS_INTERFACE "org.fedoraproject.FirewallD1" #define FIREWALL_DBUS_INTERFACE_ZONE "org.fedoraproject.FirewallD1.zone" /*****************************************************************************/ @@ -32,6 +33,7 @@ typedef struct { char *name_owner; + guint reloaded_id; guint name_owner_changed_id; bool dbus_inited : 1; @@ -148,9 +150,9 @@ _ops_type_to_string(OpsType ops_type) /*****************************************************************************/ static void -_signal_emit_state_changed(NMFirewalldManager *self, gboolean initialized_now) +_signal_emit_state_changed(NMFirewalldManager *self, NMFirewalldManagerStateChangedType signal_type) { - g_signal_emit(self, signals[STATE_CHANGED], 0, initialized_now); + g_signal_emit(self, signals[STATE_CHANGED], 0, (int) signal_type); } /*****************************************************************************/ @@ -521,8 +523,30 @@ name_owner_changed(NMFirewalldManager *self, const char *owner) } } - if (was_running != now_running || name_owner_changed) - _signal_emit_state_changed(self, just_initied); + if (just_initied) + _signal_emit_state_changed(self, NM_FIREWALLD_MANAGER_STATE_CHANGED_TYPE_INITIALIZED); + else if (was_running != now_running || name_owner_changed) + _signal_emit_state_changed(self, + NM_FIREWALLD_MANAGER_STATE_CHANGED_TYPE_NAME_OWNER_CHANGED); +} + +static void +reloaded_cb(GDBusConnection *connection, + const char * sender_name, + const char * object_path, + const char * interface_name, + const char * signal_name, + GVariant * parameters, + gpointer user_data) +{ + NMFirewalldManager * self = user_data; + NMFirewalldManagerPrivate *priv = NM_FIREWALLD_MANAGER_GET_PRIVATE(self); + + if (!nm_streq0(sender_name, priv->name_owner)) + return; + + _LOGT(NULL, "reloaded signal received"); + _signal_emit_state_changed(self, NM_FIREWALLD_MANAGER_STATE_CHANGED_TYPE_RELOADED); } static void @@ -578,6 +602,17 @@ nm_firewalld_manager_init(NMFirewalldManager *self) return; } + priv->reloaded_id = g_dbus_connection_signal_subscribe(priv->dbus_connection, + FIREWALL_DBUS_SERVICE, + FIREWALL_DBUS_INTERFACE, + "Reloaded", + FIREWALL_DBUS_PATH, + NULL, + G_DBUS_SIGNAL_FLAGS_NONE, + reloaded_cb, + self, + NULL); + priv->name_owner_changed_id = nm_dbus_connection_signal_subscribe_name_owner_changed(priv->dbus_connection, FIREWALL_DBUS_SERVICE, @@ -604,6 +639,7 @@ dispose(GObject *object) * we don't expect pending operations at this point. */ nm_assert(c_list_is_empty(&priv->pending_calls)); + nm_clear_g_dbus_connection_signal(priv->dbus_connection, &priv->reloaded_id); nm_clear_g_dbus_connection_signal(priv->dbus_connection, &priv->name_owner_changed_id); nm_clear_g_cancellable(&priv->get_name_owner_cancellable); @@ -626,8 +662,8 @@ nm_firewalld_manager_class_init(NMFirewalldManagerClass *klass) 0, NULL, NULL, - g_cclosure_marshal_VOID__BOOLEAN, + g_cclosure_marshal_VOID__INT, G_TYPE_NONE, 1, - G_TYPE_BOOLEAN /* initialized_now */); + G_TYPE_INT /* signal-type */); } diff --git a/src/core/nm-firewalld-manager.h b/src/core/nm-firewalld-manager.h index febb9bac48..1f76bebaa4 100644 --- a/src/core/nm-firewalld-manager.h +++ b/src/core/nm-firewalld-manager.h @@ -19,6 +19,12 @@ #define NM_FIREWALLD_MANAGER_STATE_CHANGED "state-changed" +typedef enum { + NM_FIREWALLD_MANAGER_STATE_CHANGED_TYPE_INITIALIZED, + NM_FIREWALLD_MANAGER_STATE_CHANGED_TYPE_NAME_OWNER_CHANGED, + NM_FIREWALLD_MANAGER_STATE_CHANGED_TYPE_RELOADED, +} NMFirewalldManagerStateChangedType; + typedef struct _NMFirewalldManagerCallId NMFirewalldManagerCallId; typedef struct _NMFirewalldManager NMFirewalldManager; diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index e4914acfb5..e147e5047c 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -2519,14 +2519,15 @@ connection_added(NMSettings *settings, NMSettingsConnection *connection, gpointe } static void -firewall_state_changed(NMFirewalldManager *manager, gboolean initialized_now, gpointer user_data) +firewall_state_changed(NMFirewalldManager *manager, int signal_type_i, gpointer user_data) { - NMPolicy * self = (NMPolicy *) user_data; - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); - const CList * tmp_lst; - NMDevice * device; + const NMFirewalldManagerStateChangedType signal_type = signal_type_i; + NMPolicy * self = user_data; + NMPolicyPrivate * priv = NM_POLICY_GET_PRIVATE(self); + const CList * tmp_lst; + NMDevice * device; - if (initialized_now) { + if (signal_type == NM_FIREWALLD_MANAGER_STATE_CHANGED_TYPE_INITIALIZED) { /* the firewall manager was initializing, but all requests * so fare were queued and are already sent. No need to * re-update the firewall zone of the devices. */