From a95b674c396650dea1d9311de357d172aa65f44f Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 15 Nov 2018 19:18:46 +0100 Subject: [PATCH 1/5] build: install dispatcher dirs in /usr The dispatcher looks there for scripts now. This actually doesn't break the RPM build, since it doesn't mind extra empty directories in buildroot. Good. --- Makefile.am | 15 ++++++++------- tools/meson-post-install.sh | 3 +++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Makefile.am b/Makefile.am index 1bba093e25..e86dbcd702 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3891,14 +3891,15 @@ dispatcher/org.freedesktop.nm_dispatcher.service: $(srcdir)/dispatcher/org.freed dbusactivation_DATA += dispatcher/org.freedesktop.nm_dispatcher.service CLEANFILES += dispatcher/org.freedesktop.nm_dispatcher.service - -dispatcherdir=$(sysconfdir)/NetworkManager/dispatcher.d - install-data-hook-dispatcher: - $(mkinstalldirs) -m 0755 $(DESTDIR)$(dispatcherdir) - $(mkinstalldirs) -m 0755 $(DESTDIR)$(dispatcherdir)/pre-down.d - $(mkinstalldirs) -m 0755 $(DESTDIR)$(dispatcherdir)/pre-up.d - $(mkinstalldirs) -m 0755 $(DESTDIR)$(dispatcherdir)/no-wait.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmconfdir)/dispatcher.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmconfdir)/dispatcher.d/pre-down.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmconfdir)/dispatcher.d/pre-up.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmconfdir)/dispatcher.d/no-wait.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmlibdir)/dispatcher.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmlibdir)/dispatcher.d/pre-down.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmlibdir)/dispatcher.d/pre-up.d + $(mkinstalldirs) -m 0755 $(DESTDIR)$(nmlibdir)/dispatcher.d/no-wait.d install_data_hook += install-data-hook-dispatcher diff --git a/tools/meson-post-install.sh b/tools/meson-post-install.sh index 2037c0ceb4..d2474f475d 100755 --- a/tools/meson-post-install.sh +++ b/tools/meson-post-install.sh @@ -27,6 +27,9 @@ for dir in "${pkgconfdir}/conf.d" \ "${pkgconfdir}/dnsmasq.d" \ "${pkgconfdir}/dnsmasq-shared.d" \ "${pkglibdir}/conf.d" \ + "${pkglibdir}/dispatcher.d/no-wait.d" \ + "${pkglibdir}/dispatcher.d/pre-down.d" \ + "${pkglibdir}/dispatcher.d/pre-up.d" \ "${pkglibdir}/VPN"; do mkdir -p "${DESTDIR}${dir}" chmod 0755 "${DESTDIR}${dir}" From 3fc41cd6d5f68a6caa01ed8037d992ed289e5bec Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 20 Nov 2018 16:51:33 +0100 Subject: [PATCH 2/5] src/dispatcher: do not monitor dispatcher scripts The monitors have been in place since the dispatcher has been introduced. They need the daemon to do extra work know where the files are supposed to be. It seems to me the complexity is not worth it. Let's remove them now, making it easier to modify the dispatcher to look for scripts in other places. --- src/nm-dispatcher.c | 149 ++------------------------------------------ 1 file changed, 4 insertions(+), 145 deletions(-) diff --git a/src/nm-dispatcher.c b/src/nm-dispatcher.c index d656edc844..8ac18ed44a 100644 --- a/src/nm-dispatcher.c +++ b/src/nm-dispatcher.c @@ -15,7 +15,7 @@ * with this program; if not, write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. * - * Copyright (C) 2004 - 2017 Red Hat, Inc. + * Copyright (C) 2004 - 2018 Red Hat, Inc. * Copyright (C) 2005 - 2008 Novell, Inc. */ @@ -47,42 +47,6 @@ static GDBusProxy *dispatcher_proxy; static GHashTable *requests = NULL; -typedef struct { - GFileMonitor *monitor; - const char *const description; - const char *const dir; - const guint16 dir_len; - char has_scripts; -} Monitor; - -enum { - MONITOR_INDEX_DEFAULT, - MONITOR_INDEX_PRE_UP, - MONITOR_INDEX_PRE_DOWN, -}; - -static Monitor monitors[3] = { -#define MONITORS_INIT_SET(INDEX, USE, SCRIPT_DIR) [INDEX] = { .dir_len = NM_STRLEN (SCRIPT_DIR), .dir = SCRIPT_DIR, .description = ("" USE), .has_scripts = TRUE } - MONITORS_INIT_SET (MONITOR_INDEX_DEFAULT, "default", NMD_SCRIPT_DIR_DEFAULT), - MONITORS_INIT_SET (MONITOR_INDEX_PRE_UP, "pre-up", NMD_SCRIPT_DIR_PRE_UP), - MONITORS_INIT_SET (MONITOR_INDEX_PRE_DOWN, "pre-down", NMD_SCRIPT_DIR_PRE_DOWN), -}; - -static const Monitor* -_get_monitor_by_action (NMDispatcherAction action) -{ - switch (action) { - case NM_DISPATCHER_ACTION_PRE_UP: - case NM_DISPATCHER_ACTION_VPN_PRE_UP: - return &monitors[MONITOR_INDEX_PRE_UP]; - case NM_DISPATCHER_ACTION_PRE_DOWN: - case NM_DISPATCHER_ACTION_VPN_PRE_DOWN: - return &monitors[MONITOR_INDEX_PRE_DOWN]; - default: - return &monitors[MONITOR_INDEX_DEFAULT]; - } -} - static void dump_proxy_to_props (NMProxyConfig *proxy, GVariantBuilder *builder) { @@ -379,7 +343,6 @@ dispatcher_results_process (guint request_id, NMDispatcherAction action, GVarian { const char *script, *err; guint32 result; - const Monitor *monitor = _get_monitor_by_action (action); g_return_if_fail (results != NULL); @@ -389,31 +352,14 @@ dispatcher_results_process (guint request_id, NMDispatcherAction action, GVarian } while (g_variant_iter_next (results, "(&su&s)", &script, &result, &err)) { - const char *script_validation_msg = ""; - - if (!*script) { - script_validation_msg = " (path is NULL)"; - script = "(unknown)"; - } else if (!strncmp (script, monitor->dir, monitor->dir_len) /* check: prefixed by script directory */ - && script[monitor->dir_len] == '/' && script[monitor->dir_len+1] /* check: with additional "/?" */ - && !strchr (&script[monitor->dir_len+1], '/')) { /* check: and no further '/' */ - /* we expect the script to lie inside monitor->dir. If it does, - * strip the directory name. Otherwise show the full path and a warning. */ - script += monitor->dir_len + 1; - } else - script_validation_msg = " (unexpected path)"; - if (result == DISPATCH_RESULT_SUCCESS) { - _LOGD ("(%u) %s succeeded%s", - request_id, - script, script_validation_msg); + _LOGD ("(%u) %s succeeded", request_id, script); } else { - _LOGW ("(%u) %s failed (%s): %s%s", + _LOGW ("(%u) %s failed (%s): %s", request_id, script, dispatch_result_to_string (result), - err, - script_validation_msg); + err); } } } @@ -474,18 +420,6 @@ action_to_string (NMDispatcherAction action) return action_table[action]; } -static gboolean -dispatcher_idle_cb (gpointer user_data) -{ - DispatchInfo *info = user_data; - - info->idle_id = 0; - if (info->callback) - info->callback (info->request_id, info->user_data); - dispatcher_info_cleanup (info); - return G_SOURCE_REMOVE; -} - static gboolean _dispatcher_call (NMDispatcherAction action, gboolean blocking, @@ -551,21 +485,6 @@ _dispatcher_call (NMDispatcherAction action, : (callback ? " (with callback)" : "")); } - if (!_get_monitor_by_action(action)->has_scripts) { - if (blocking == FALSE && (out_call_id || callback)) { - info = g_malloc0 (sizeof (*info)); - info->action = action; - info->request_id = reqid; - info->callback = callback; - info->user_data = user_data; - info->idle_id = g_idle_add (dispatcher_idle_cb, info); - _LOGD ("(%u) simulate request; no scripts in %s", reqid, _get_monitor_by_action(action)->dir); - } else - _LOGD ("(%u) ignoring request; no scripts in %s", reqid, _get_monitor_by_action(action)->dir); - success = TRUE; - goto done; - } - if (applied_connection) connection_dict = nm_connection_to_dbus (applied_connection, NM_CONNECTION_SERIALIZE_NO_SECRETS); else @@ -698,7 +617,6 @@ _dispatcher_call (NMDispatcherAction action, g_variant_unref (device_dhcp4_props); g_variant_unref (device_dhcp6_props); -done: if (success && info) { /* Track the request in case of cancelation */ g_hash_table_insert (requests, GUINT_TO_POINTER (info->request_id), info); @@ -931,70 +849,11 @@ nm_dispatcher_call_cancel (guint call_id) } } -static void -dispatcher_dir_changed (GFileMonitor *monitor, - GFile *file, - GFile *other_file, - GFileMonitorEvent event_type, - Monitor *item) -{ - const char *name; - char *full_name; - GDir *dir; - GError *error = NULL; - - dir = g_dir_open (item->dir, 0, &error); - if (dir) { - int errsv = 0; - - item->has_scripts = FALSE; - errno = 0; - while (!item->has_scripts - && (name = g_dir_read_name (dir))) { - full_name = g_build_filename (item->dir, name, NULL); - item->has_scripts = g_file_test (full_name, G_FILE_TEST_IS_EXECUTABLE); - g_free (full_name); - } - errsv = errno; - g_dir_close (dir); - if (item->has_scripts) - _LOGD ("%s script directory '%s' has scripts", item->description, item->dir); - else if (errsv == 0) - _LOGD ("%s script directory '%s' has no scripts", item->description, item->dir); - else { - _LOGD ("%s script directory '%s' error reading (%s)", item->description, item->dir, nm_strerror_native (errsv)); - item->has_scripts = TRUE; - } - } else { - if (g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT)) { - _LOGD ("%s script directory '%s' does not exist", item->description, item->dir); - item->has_scripts = FALSE; - } else { - _LOGD ("%s script directory '%s' error (%s)", item->description, item->dir, error->message); - item->has_scripts = TRUE; - } - g_error_free (error); - } - -} - void nm_dispatcher_init (void) { - GFile *file; - guint i; GError *error = NULL; - for (i = 0; i < G_N_ELEMENTS (monitors); i++) { - file = g_file_new_for_path (monitors[i].dir); - monitors[i].monitor = g_file_monitor_directory (file, G_FILE_MONITOR_NONE, NULL, NULL); - if (monitors[i].monitor) { - g_signal_connect (monitors[i].monitor, "changed", G_CALLBACK (dispatcher_dir_changed), &monitors[i]); - dispatcher_dir_changed (monitors[i].monitor, file, NULL, 0, &monitors[i]); - } - g_object_unref (file); - } - dispatcher_proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS, From 35a428f16897e277db81293ae79e9b5ed061c044 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 20 Nov 2018 16:51:46 +0100 Subject: [PATCH 3/5] dispatcher: look for the scripts in /usr/lib as well This makes it possible for packages that ship dispatcher scripts to use the correct location. --- dispatcher/nm-dispatcher.c | 111 +++++++++++++------ shared/nm-libnm-core-aux/nm-dispatcher-api.h | 5 - 2 files changed, 75 insertions(+), 41 deletions(-) diff --git a/dispatcher/nm-dispatcher.c b/dispatcher/nm-dispatcher.c index fe6eaa17ad..231a3f9a83 100644 --- a/dispatcher/nm-dispatcher.c +++ b/dispatcher/nm-dispatcher.c @@ -466,12 +466,6 @@ check_permissions (struct stat *s, const char **out_error_msg) g_return_val_if_fail (out_error_msg != NULL, FALSE); g_return_val_if_fail (*out_error_msg == NULL, FALSE); - /* Only accept regular files */ - if (!S_ISREG (s->st_mode)) { - *out_error_msg = "not a regular file."; - return FALSE; - } - /* Only accept files owned by root */ if (s->st_uid != 0) { *out_error_msg = "not owned by root."; @@ -573,59 +567,104 @@ dispatch_one_script (Request *request) return FALSE; } -static GSList * -find_scripts (const char *str_action) +static int +_compare_basenames (gconstpointer a, gconstpointer b) { - GDir *dir; - const char *filename; - GSList *sorted = NULL; - GError *error = NULL; - const char *dirname; + const char *basename_a = strrchr (a, '/'); + const char *basename_b = strrchr (b, '/'); + int ret; - if ( strcmp (str_action, NMD_ACTION_PRE_UP) == 0 - || strcmp (str_action, NMD_ACTION_VPN_PRE_UP) == 0) - dirname = NMD_SCRIPT_DIR_PRE_UP; - else if ( strcmp (str_action, NMD_ACTION_PRE_DOWN) == 0 - || strcmp (str_action, NMD_ACTION_VPN_PRE_DOWN) == 0) - dirname = NMD_SCRIPT_DIR_PRE_DOWN; - else - dirname = NMD_SCRIPT_DIR_DEFAULT; + nm_assert (basename_a); + nm_assert (basename_b); + + ret = strcmp (++basename_a, ++basename_b); + if (ret) + return ret; + + nm_assert_not_reached (); + return 0; +} + +static void +_find_scripts (GHashTable *scripts, const char *base, const char *subdir) +{ + const char *filename; + gs_free char *dirname = NULL; + GError *error = NULL; + GDir *dir; + + dirname = g_build_filename (base, "dispatcher.d", subdir, NULL); if (!(dir = g_dir_open (dirname, 0, &error))) { g_message ("find-scripts: Failed to open dispatcher directory '%s': %s", dirname, error->message); g_error_free (error); - return NULL; + return; } while ((filename = g_dir_read_name (dir))) { - char *path; - struct stat st; - int err; - const char *err_msg = NULL; - if (!check_filename (filename)) continue; - path = g_build_filename (dirname, filename, NULL); + g_hash_table_insert (scripts, + g_strdup (filename), + g_build_filename (dirname, filename, NULL)); + } + + g_dir_close (dir); +} + +static GSList * +find_scripts (const char *str_action) +{ + gs_unref_hashtable GHashTable *scripts = NULL; + GSList *script_list = NULL; + GHashTableIter iter; + const char *subdir = NULL; + char *path; + char *filename; + + if ( strcmp (str_action, NMD_ACTION_PRE_UP) == 0 + || strcmp (str_action, NMD_ACTION_VPN_PRE_UP) == 0) + subdir = "pre-up.d"; + else if ( strcmp (str_action, NMD_ACTION_PRE_DOWN) == 0 + || strcmp (str_action, NMD_ACTION_VPN_PRE_DOWN) == 0) + subdir = "pre-down.d"; + + scripts = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + + _find_scripts (scripts, NMLIBDIR, subdir); + _find_scripts (scripts, NMCONFDIR, subdir); + + g_hash_table_iter_init (&iter, scripts); + while (g_hash_table_iter_next (&iter, (gpointer *) &filename, (gpointer *) &path)) { + struct stat st; + char *link_target; + int err; + const char *err_msg = NULL; + + link_target = g_file_read_link (path, NULL); + if (g_strcmp0 (link_target, "/dev/null") == 0) { + g_free (link_target); + continue; + } + g_free (link_target); err = stat (path, &st); if (err) g_warning ("find-scripts: Failed to stat '%s': %d", path, err); - else if (S_ISDIR (st.st_mode)) + else if (!S_ISREG (st.st_mode)) ; /* silently skip. */ else if (!check_permissions (&st, &err_msg)) g_warning ("find-scripts: Cannot execute '%s': %s", path, err_msg); else { /* success */ - sorted = g_slist_insert_sorted (sorted, path, (GCompareFunc) g_strcmp0); - path = NULL; + script_list = g_slist_prepend (script_list, g_strdup (path)); + continue; } - g_free (path); } - g_dir_close (dir); - return sorted; + return g_slist_sort (script_list, _compare_basenames); } static gboolean @@ -636,6 +675,7 @@ script_must_wait (const char *path) gs_free char *real = NULL; char *tmp; + link = g_file_read_link (path, NULL); if (link) { if (!g_path_is_absolute (link)) { @@ -648,8 +688,7 @@ script_must_wait (const char *path) dir = g_path_get_dirname (link); real = realpath (dir, NULL); - - if (real && !strcmp (real, NMD_SCRIPT_DIR_NO_WAIT)) + if (real && !g_str_has_suffix (real, "/no-wait.d")) return FALSE; } diff --git a/shared/nm-libnm-core-aux/nm-dispatcher-api.h b/shared/nm-libnm-core-aux/nm-dispatcher-api.h index e6d0d92f85..2da5bd88f1 100644 --- a/shared/nm-libnm-core-aux/nm-dispatcher-api.h +++ b/shared/nm-libnm-core-aux/nm-dispatcher-api.h @@ -21,11 +21,6 @@ #ifndef __NM_DISPACHER_API_H__ #define __NM_DISPACHER_API_H__ -#define NMD_SCRIPT_DIR_DEFAULT NMCONFDIR "/dispatcher.d" -#define NMD_SCRIPT_DIR_PRE_UP NMD_SCRIPT_DIR_DEFAULT "/pre-up.d" -#define NMD_SCRIPT_DIR_PRE_DOWN NMD_SCRIPT_DIR_DEFAULT "/pre-down.d" -#define NMD_SCRIPT_DIR_NO_WAIT NMD_SCRIPT_DIR_DEFAULT "/no-wait.d" - #define NM_DISPATCHER_DBUS_SERVICE "org.freedesktop.nm_dispatcher" #define NM_DISPATCHER_DBUS_INTERFACE "org.freedesktop.nm_dispatcher" #define NM_DISPATCHER_DBUS_PATH "/org/freedesktop/nm_dispatcher" From c4f1fac35d0c57d2034f7954cea05129d1f40659 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 29 Nov 2018 18:00:39 +0100 Subject: [PATCH 4/5] contrib/rpm: (trivial) move some %files around, remove a duplicate Just a cosmetic thing. --- contrib/fedora/rpm/NetworkManager.spec | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index c237866a87..241b8c5ae2 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -824,13 +824,15 @@ fi %{_sbindir}/%{name} %{_bindir}/nmcli %{_datadir}/bash-completion/completions/nmcli -%dir %{_sysconfdir}/%{name}/ +%dir %{_sysconfdir}/%{name} +%dir %{_sysconfdir}/%{name}/conf.d %dir %{_sysconfdir}/%{name}/dispatcher.d %dir %{_sysconfdir}/%{name}/dispatcher.d/pre-down.d %dir %{_sysconfdir}/%{name}/dispatcher.d/pre-up.d %dir %{_sysconfdir}/%{name}/dispatcher.d/no-wait.d %dir %{_sysconfdir}/%{name}/dnsmasq.d %dir %{_sysconfdir}/%{name}/dnsmasq-shared.d +%dir %{_sysconfdir}/%{name}/system-connections %config(noreplace) %{_sysconfdir}/%{name}/NetworkManager.conf %{_bindir}/nm-online %{_libexecdir}/nm-ifup @@ -847,8 +849,6 @@ fi %if %{with nmtui} %exclude %{_mandir}/man1/nmtui* %endif -%dir %{_sysconfdir}/%{name} -%dir %{_sysconfdir}/%{name}/conf.d %dir %{nmlibdir} %dir %{nmlibdir}/conf.d %dir %{nmlibdir}/VPN @@ -857,7 +857,6 @@ fi %{_mandir}/man7/nmcli-examples.7* %{_mandir}/man8/* %dir %{_localstatedir}/lib/NetworkManager -%dir %{_sysconfdir}/NetworkManager/system-connections %dir %{_sysconfdir}/sysconfig/network-scripts %{_datadir}/dbus-1/system-services/org.freedesktop.nm_dispatcher.service %{_datadir}/polkit-1/actions/*.policy From 14eaf6a40b2ccf2b0daedbbaf0730287fd6a4d3b Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 29 Nov 2018 18:01:30 +0100 Subject: [PATCH 5/5] contrib/rpm: provide NetworkManager-dispatcher This is a provide packages that install dispatcher scripts should depend on. It will make it easier to keep track of them and possibly split out the dispatcher into an optional package if not needed. --- contrib/fedora/rpm/NetworkManager.spec | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 241b8c5ae2..39252c1010 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -242,6 +242,8 @@ BuildRequires: libubsan %endif %endif +Provides: %{name}-dispatcher%{?_isa} = %{epoch}:%{version}-%{release} + # NetworkManager uses various parts of systemd-networkd internally, including # DHCP client, IPv4 Link-Local address negotiation or LLDP support. # This provide is essentially here so that NetworkManager shows on Security