From a99ac7ccd84c22c1a30c29bb4f1ce50d211d6aaa Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 22 Jul 2021 18:31:35 +0200 Subject: [PATCH 1/6] glib-aux: add nm_g_idle_add() g_idle_add() is discouraged, and the checkpatch.pl script warns about it. Sometimes there is a legitimate use of it, when you want to always schedule an idle action (without intent to cancel or track it). That makes more sense for g_idle_add() than it does for g_timeout_add(), because a timeout really should be tracked and cancelled if necessary. Add a wrapper to rename the legitimate uses. This way, we can avoid the checkpatch.pl warnings, and can grep for the remaining illegitimate uses. --- contrib/scripts/checkpatch.pl | 2 +- src/libnm-glib-aux/nm-shared-utils.h | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/contrib/scripts/checkpatch.pl b/contrib/scripts/checkpatch.pl index c535fd0a93..d1bc6c69c2 100755 --- a/contrib/scripts/checkpatch.pl +++ b/contrib/scripts/checkpatch.pl @@ -191,7 +191,7 @@ complain ("This gtk-doc annotation looks wrong") if $line =~ /\*.*\( *(transfer- complain ("Prefer nm_assert() or g_return*() to g_assert*()") if $line =~ /g_assert/ and (not $filename =~ /\/tests\//) and (not $filename =~ /\/nm-test-/); complain ("Use gs_free_error with GError variables") if $line =~ /\bgs_free\b +GError *\*/; complain ("Don't use strcmp/g_strcmp0 unless you need to sort. Consider nm_streq()/nm_streq0(),NM_IN_STRSET() for testing equality") if $line =~ /\b(strcmp|g_strcmp0)\b/; -complain ("Don't use API that uses the numeric source id. Instead, use GSource and API like nm_g_idle_add_source(), nm_clear_g_source_inst(), etc.") if $line =~ /\b(g_idle_add|g_idle_add_full|g_timeout_add|g_timeout_add_seconds|g_source_remove|nm_clear_g_source)\b/; +complain ("Don't use API that uses the numeric source id. Instead, use GSource and API like nm_g_idle_add(), nm_g_idle_add_source(), nm_clear_g_source_inst(), etc.") if $line =~ /\b(g_idle_add|g_idle_add_full|g_timeout_add|g_timeout_add_seconds|g_source_remove|nm_clear_g_source)\b/; #complain ("Use spaces instead of tabs") if $line =~ /\t/; # Further on we process stuff without comments. diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index e8c282d8a2..8d12f05ef5 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -1725,6 +1725,20 @@ nm_g_source_attach(GSource *source, GMainContext *context) return source; } +static inline void +nm_g_idle_add(GSourceFunc func, gpointer user_data) +{ + /* g_idle_add() is discouraged because it relies on the guint source ids. + * + * Usually, you would want to use nm_g_idle_add_source() which returns a GSource* + * instance. + * + * However, if you don't care to ever call g_source_remove() on the source id, then + * g_idle_add() is fine. But our checkpatch script would complain about it. nm_g_idle_add(), + * is the solution because it intentionally ignores the guint source id. */ + g_idle_add(func, user_data); +} + static inline GSource * nm_g_idle_add_source(GSourceFunc func, gpointer user_data) { From f57679dd93caf45d7faca59a1c7cfa8fd3b6e4d3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jul 2021 21:47:32 +0200 Subject: [PATCH 2/6] all: use nm_g_idle_add() instead of g_idle_add() g_idle_add() is discouraged, because we shouldn't use guint source IDs. --- src/core/nm-core-utils.c | 2 +- src/core/nm-dbus-manager.c | 2 +- src/core/nm-dbus-object.c | 2 +- src/nmtui/nmtui.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index 972d40b081..ec4d724454 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -458,7 +458,7 @@ _kc_invoke_callback(pid_t pid, data->sync.success = success; data->sync.child_status = child_status; - g_idle_add(_kc_invoke_callback_idle, data); + nm_g_idle_add(_kc_invoke_callback_idle, data); } /* nm_utils_kill_child_async: diff --git a/src/core/nm-dbus-manager.c b/src/core/nm-dbus-manager.c index c47d4dbfaf..323d0a19f1 100644 --- a/src/core/nm-dbus-manager.c +++ b/src/core/nm-dbus-manager.c @@ -262,7 +262,7 @@ private_server_closed_connection(GDBusConnection *conn, /* Delay the close of connection to ensure that D-Bus signals * are handled */ - g_idle_add(close_connection_in_idle, info); + nm_g_idle_add(close_connection_in_idle, info); } static gboolean diff --git a/src/core/nm-dbus-object.c b/src/core/nm-dbus-object.c index 0414973929..95077626f6 100644 --- a/src/core/nm-dbus-object.c +++ b/src/core/nm-dbus-object.c @@ -178,7 +178,7 @@ nm_dbus_object_unexport_on_idle(gpointer /* (NMDBusObject *) */ self_take) nm_shutdown_wait_obj_register_object(self, "unexport-dbus-obj-on-idle"); /* pass on ownership. */ - g_idle_add(_unexport_on_idle_cb, g_steal_pointer(&self)); + nm_g_idle_add(_unexport_on_idle_cb, g_steal_pointer(&self)); } /*****************************************************************************/ diff --git a/src/nmtui/nmtui.c b/src/nmtui/nmtui.c index 5edf70e589..897e09112a 100644 --- a/src/nmtui/nmtui.c +++ b/src/nmtui/nmtui.c @@ -283,7 +283,7 @@ main(int argc, char **argv) startup_data.argc = argc; startup_data.argv = argv; - g_idle_add(idle_run_subprogram, &startup_data); + nm_g_idle_add(idle_run_subprogram, &startup_data); loop = g_main_loop_new(NULL, FALSE); g_main_loop_run(loop); g_main_loop_unref(loop); From 641f4473b7cf221b00c38dcce76cd3ba6496c763 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Jul 2021 21:21:18 +0200 Subject: [PATCH 3/6] build: fix calling test "check-local-devices-ovs" to check OVS device plugin --- Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile.am b/Makefile.am index ed08230973..659dc054f1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4113,6 +4113,8 @@ check-local-devices-ovs: src/core/devices/ovs/libnm-device-plugin-ovs.la $(srcdir)/tools/check-exports.sh $(builddir)/src/core/devices/ovs/.libs/libnm-device-plugin-ovs.so "$(srcdir)/linker-script-devices.ver" $(call check_so_symbols,$(builddir)/src/core/devices/ovs/.libs/libnm-device-plugin-ovs.so) +check_local += check-local-devices-ovs + endif EXTRA_DIST += \ From 684f2acffea3ed5704330bff05b87acbf371ccdd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Jul 2021 22:18:32 +0200 Subject: [PATCH 4/6] build: add way to keep unused symbols when linking NetworkManager NetworkManager (and NetworkManager-all-sym) must not only contain symbols that are used by itself. Also the device and settings plugin are dlopen'd by NetworkManager and use symobls form the binary. That means, if a symbols is only used by a plugin, then we must make sure that the linker keeps it in the binary. Add a mechanism for that. --- Makefile.am | 8 ++++++++ src/core/meson.build | 14 ++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 659dc054f1..af327de094 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2656,6 +2656,12 @@ $(src_core_libNetworkManagerTest_la_OBJECTS): $(src_libnm_core_public_mkenums_h) ############################################################################### +# NetworkManager binary also must contain symbols that are not used by the binary +# itself, but by the plugins (that are dlopened). We need to explicitly include +# them during linking. +networkmanager_undefined_symbols = \ + $(NULL) + noinst_PROGRAMS += src/core/NetworkManager-all-sym src_core_NetworkManager_all_sym_CPPFLAGS = $(src_core_cppflags) @@ -2670,6 +2676,7 @@ src_core_NetworkManager_all_sym_LDADD = \ src_core_NetworkManager_all_sym_LDFLAGS = \ -rdynamic \ + $(networkmanager_undefined_symbols:%=-u %) \ $(SANITIZER_EXEC_LDFLAGS) \ $(NULL) @@ -2696,6 +2703,7 @@ src_core_NetworkManager_LDADD = \ src_core_NetworkManager_LDFLAGS = \ -rdynamic \ -Wl,--version-script="src/core/NetworkManager.ver" \ + $(networkmanager_undefined_symbols:%=-u %) \ $(SANITIZER_EXEC_LDFLAGS) \ $(NULL) diff --git a/src/core/meson.build b/src/core/meson.build index f4d7af6407..1ce641d6d8 100644 --- a/src/core/meson.build +++ b/src/core/meson.build @@ -270,6 +270,14 @@ endif subdir('devices') subdir('settings/plugins') +# NetworkManager binary also must contain symbols that are not used by the binary +# itself, but by the plugins (that are dlopened). We need to explicitly include +# them during linking. +networkmanager_undefined_symbols_args = [] +foreach s: [] + networkmanager_undefined_symbols_args += ['-u', s] +endforeach + # NetworkManager binary # libNetworkManager.a, as built by meson doesn't contain all symbols @@ -284,7 +292,9 @@ NetworkManager_all_sym = executable( nm_deps, libudev_dep, ], - link_args: '-Wl,--no-gc-sections', + link_args: [ + '-Wl,--no-gc-sections', + ] + networkmanager_undefined_symbols_args, link_whole: [ libNetworkManager, libNetworkManagerBase, @@ -345,7 +355,7 @@ NetworkManager = executable( link_args: [ '-rdynamic', '-Wl,--version-script,@0@'.format(ver_script.full_path()), - ], + ] + networkmanager_undefined_symbols_args, link_depends: ver_script, install: true, install_dir: nm_sbindir, From f137b32d31174e5226f4ab7d3657c90851e50000 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 18 Jul 2021 08:53:43 +0200 Subject: [PATCH 5/6] sudo: introduce nm-sudo D-Bus service NetworkManager runs as root and has lots of capabilities. We want to reduce the attach surface by dropping capabilities, but there is a genuine need to do certain things. For example, we currently require dac_override capability, to open the unix socket of ovsdb. Most users wouldn't use OVS, so we should find a way to not require that dac_override capability. The solution is to have a separate, D-Bus activate service (nm-sudo), which has the capability to open and provide the file descriptor. For authentication, we only rely on D-Bus. We watch the name owner of NetworkManager, and only accept requests from that service. We trust D-Bus to get it right a request from that name owner is really coming from NetworkManager. If we couldn't trust that, how could PolicyKit or any authentication via D-Bus work? For testing, the user can set NM_SUDO_NO_AUTH_FOR_TESTING=1. https://bugzilla.redhat.com/show_bug.cgi?id=1921826 --- .gitignore | 4 + Makefile.am | 62 +- contrib/fedora/rpm/NetworkManager.spec | 8 +- data/meson.build | 1 + data/nm-sudo.service.in | 66 ++ po/POTFILES.skip | 1 + src/core/meson.build | 1 + src/core/nm-sudo-call.c | 5 + src/core/nm-sudo-call.h | 6 + src/libnm-base/meson.build | 1 + src/libnm-base/nm-sudo-utils.c | 5 + src/libnm-base/nm-sudo-utils.h | 14 + src/meson.build | 1 + src/nm-sudo/meson.build | 36 ++ src/nm-sudo/nm-sudo.c | 600 ++++++++++++++++++ src/nm-sudo/nm-sudo.conf | 13 + .../org.freedesktop.nm.sudo.service.in | 5 + 17 files changed, 826 insertions(+), 3 deletions(-) create mode 100644 data/nm-sudo.service.in create mode 100644 src/core/nm-sudo-call.c create mode 100644 src/core/nm-sudo-call.h create mode 100644 src/libnm-base/nm-sudo-utils.c create mode 100644 src/libnm-base/nm-sudo-utils.h create mode 100644 src/nm-sudo/meson.build create mode 100644 src/nm-sudo/nm-sudo.c create mode 100644 src/nm-sudo/nm-sudo.conf create mode 100644 src/nm-sudo/org.freedesktop.nm.sudo.service.in diff --git a/.gitignore b/.gitignore index 041f681f00..b62e3aa7de 100644 --- a/.gitignore +++ b/.gitignore @@ -68,6 +68,9 @@ test-*.trs /src/nm-dispatcher/org.freedesktop.nm_dispatcher.service /src/nm-dispatcher/tests/test-dispatcher-envp +/src/nm-sudo/nm-sudo +/src/nm-sudo/org.freedesktop.nm.sudo.service + /data/NetworkManager.service /data/NetworkManager-wait-online.service /data/NetworkManager-dispatcher.service @@ -75,6 +78,7 @@ test-*.trs /data/server.conf /data/org.freedesktop.NetworkManager.policy /data/org.freedesktop.NetworkManager.policy.in +/data/nm-sudo.service /docs/api/version.xml /docs/api/settings-spec.html diff --git a/Makefile.am b/Makefile.am index af327de094..7a6f56199a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -504,6 +504,8 @@ src_libnm_base_libnm_base_la_SOURCES = \ src/libnm-base/nm-ethtool-utils-base.h \ src/libnm-base/nm-net-aux.c \ src/libnm-base/nm-net-aux.h \ + src/libnm-base/nm-sudo-utils.c \ + src/libnm-base/nm-sudo-utils.h \ $(NULL) src_libnm_base_libnm_base_la_LDFLAGS = \ @@ -2584,8 +2586,10 @@ src_core_libNetworkManager_la_SOURCES = \ src/core/nm-policy.h \ src/core/nm-rfkill-manager.c \ src/core/nm-rfkill-manager.h \ - src/core/nm-session-monitor.h \ src/core/nm-session-monitor.c \ + src/core/nm-session-monitor.h \ + src/core/nm-sudo-call.c \ + src/core/nm-sudo-call.h \ src/core/nm-keep-alive.c \ src/core/nm-keep-alive.h \ src/core/nm-sleep-monitor.c \ @@ -4617,6 +4621,56 @@ EXTRA_DIST += \ src/nm-dispatcher/tests/meson.build \ $(NULL) +############################################################################### +# src/nm-sudo +############################################################################### + +libexec_PROGRAMS += src/nm-sudo/nm-sudo + +src_nm_sudo_nm_sudo_SOURCES = \ + src/nm-sudo/nm-sudo.c \ + $(NULL) + +src_nm_sudo_nm_sudo_CPPFLAGS = \ + $(dflt_cppflags) \ + -I$(builddir)/src/libnm-core-public \ + -I$(srcdir)/src/libnm-core-public \ + -I$(builddir)/src/libnm-client-public \ + -I$(srcdir)/src/libnm-client-public \ + -I$(srcdir)/src \ + -I$(builddir)/src \ + $(GLIB_CFLAGS) \ + $(NULL) + +src_nm_sudo_nm_sudo_LDFLAGS = \ + -Wl,--version-script="$(srcdir)/linker-script-binary.ver" \ + $(SANITIZER_EXEC_LDFLAGS) \ + $(NULL) + +src_nm_sudo_nm_sudo_LDADD = \ + src/libnm-base/libnm-base.la \ + src/libnm-glib-aux/libnm-glib-aux.la \ + src/libnm-std-aux/libnm-std-aux.la \ + src/c-siphash/libc-siphash.la \ + $(GLIB_LIBS) \ + $(NULL) + +src/nm-sudo/org.freedesktop.nm.sudo.service: $(srcdir)/src/nm-sudo/org.freedesktop.nm.sudo.service.in + @sed \ + -e 's|@libexecdir[@]|$(libexecdir)|g' \ + $< >$@ + +dbusactivation_DATA += src/nm-sudo/org.freedesktop.nm.sudo.service +CLEANFILES += src/nm-sudo/org.freedesktop.nm.sudo.service + +dbusservice_DATA += src/nm-sudo/nm-sudo.conf + +EXTRA_DIST += \ + src/nm-sudo/nm-sudo.conf \ + src/nm-sudo/org.freedesktop.nm.sudo.service.in \ + src/nm-sudo/meson.build \ + $(NULL) + ############################################################################### # src/nm-daemon-helper ############################################################################### @@ -5299,6 +5353,7 @@ systemdsystemunit_DATA += \ data/NetworkManager.service \ data/NetworkManager-wait-online.service \ data/NetworkManager-dispatcher.service \ + data/nm-sudo.service \ $(NULL) data/NetworkManager.service: $(srcdir)/data/NetworkManager.service.in @@ -5315,6 +5370,9 @@ endif data/NetworkManager-dispatcher.service: $(srcdir)/data/NetworkManager-dispatcher.service.in $(AM_V_GEN) $(data_edit) $< >$@ +data/nm-sudo.service: $(srcdir)/data/nm-sudo.service.in + $(AM_V_GEN) $(data_edit) $< >$@ + endif examples_DATA += data/server.conf @@ -5344,6 +5402,7 @@ EXTRA_DIST += \ data/NetworkManager-wait-online-systemd-pre200.service.in \ data/NetworkManager-wait-online.service.in \ data/NetworkManager.service.in \ + data/nm-sudo.service.in \ data/meson.build \ data/nm-shared.xml \ data/server.conf.in \ @@ -5353,6 +5412,7 @@ CLEANFILES += \ data/NetworkManager-dispatcher.service \ data/NetworkManager-wait-online.service \ data/NetworkManager.service \ + data/nm-sudo.service \ data/server.conf \ $(NULL) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index f8374519db..9a213cac73 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -40,7 +40,7 @@ %global real_version_major %(printf '%s' '%{real_version}' | sed -n 's/^\\([1-9][0-9]*\\.[0-9][0-9]*\\)\\.[0-9][0-9]*$/\\1/p') -%global systemd_units NetworkManager.service NetworkManager-wait-online.service NetworkManager-dispatcher.service +%global systemd_units NetworkManager.service NetworkManager-wait-online.service NetworkManager-dispatcher.service nm-sudo.service %global systemd_units_cloud_setup nm-cloud-setup.service nm-cloud-setup.timer @@ -940,7 +940,7 @@ if [ $1 -eq 0 ]; then /usr/sbin/update-alternatives --remove ifup %{_libexecdir}/nm-ifup >/dev/null 2>&1 || : fi -%systemd_preun NetworkManager-wait-online.service NetworkManager-dispatcher.service +%systemd_preun NetworkManager-wait-online.service NetworkManager-dispatcher.service nm-sudo.service %if %{with nm_cloud_setup} @@ -974,6 +974,7 @@ fi %files %{dbus_sys_dir}/org.freedesktop.NetworkManager.conf %{dbus_sys_dir}/nm-dispatcher.conf +%{dbus_sys_dir}/nm-sudo.conf %{dbus_sys_dir}/nm-ifcfg-rh.conf %{_sbindir}/%{name} %{_bindir}/nmcli @@ -999,6 +1000,7 @@ fi %{_libexecdir}/nm-iface-helper %{_libexecdir}/nm-initrd-generator %{_libexecdir}/nm-daemon-helper +%{_libexecdir}/nm-sudo %dir %{_libdir}/%{name} %dir %{nmplugindir} %{nmplugindir}/libnm-settings-plugin*.so @@ -1022,6 +1024,7 @@ fi %dir %{_localstatedir}/lib/NetworkManager %dir %{_sysconfdir}/sysconfig/network-scripts %{_datadir}/dbus-1/system-services/org.freedesktop.nm_dispatcher.service +%{_datadir}/dbus-1/system-services/org.freedesktop.nm.sudo.service %{_datadir}/polkit-1/actions/*.policy %{_prefix}/lib/udev/rules.d/*.rules %if %{with firewalld_zone} @@ -1031,6 +1034,7 @@ fi %{systemd_dir}/NetworkManager.service %{systemd_dir}/NetworkManager-wait-online.service %{systemd_dir}/NetworkManager-dispatcher.service +%{systemd_dir}/nm-sudo.service %dir %{_datadir}/doc/NetworkManager/examples %{_datadir}/doc/NetworkManager/examples/server.conf %doc NEWS AUTHORS README CONTRIBUTING.md TODO diff --git a/data/meson.build b/data/meson.build index 64a1372b41..edb6418d0e 100644 --- a/data/meson.build +++ b/data/meson.build @@ -11,6 +11,7 @@ if install_systemdunitdir services = [ 'NetworkManager-dispatcher.service.in', 'NetworkManager.service.in', + 'nm-sudo.service.in', ] if have_systemd_200 diff --git a/data/nm-sudo.service.in b/data/nm-sudo.service.in new file mode 100644 index 0000000000..9cd30dbfdf --- /dev/null +++ b/data/nm-sudo.service.in @@ -0,0 +1,66 @@ +[Unit] +Description=Network Manager Sudo Helper +# +# nm-sudo exists for privilege separation. It allows to run NetworkManager +# without certain capabilities, and ask nm-sudo for special operations +# where more privileges are required. +# +# While nm-sudo has privileges that NetworkManager has not, it does not +# mean that itself should run totally unconstrained. On the contrary, it +# also should only have permissions it requires. +# +# nm-sudo rejects all requests that come from any other than the name +# owner of "org.freedesktop.NetworkManager" (that is, NetworkManager process +# itself). It is thus only an implementation detail and provides no public +# API to the user. + +[Service] +Type=dbus +BusName=org.freedesktop.nm.sudo +ExecStart=@libexecdir@/nm-sudo + +# Environment=NM_SUDO_NO_AUTH_FOR_TESTING=0 +# Environment=NM_SUDO_IDLE_TIMEOUT=10 +# Environment=NM_SUDO_LOG=TRACE +# Environment=G_DEBUG=fatal-warnings +# Environment=G_DBUS_DEBUG=all + +[Install] +Alias=dbus-org.freedesktop.nm.sudo.service + +[Service] +# Restrict: +AmbientCapabilities= +CapabilityBoundingSet= +PrivateDevices=true +PrivateMounts=true +PrivateNetwork=true +PrivateTmp=true +ProtectClock=true +ProtectControlGroups=true +ProtectHome=true +ProtectHostname=true +ProtectKernelLogs=true +ProtectKernelModules=true +ProtectKernelTunables=true +ProtectSystem=strict +RestrictAddressFamilies= +RestrictNamespaces=true +SystemCallFilter=~@clock +SystemCallFilter=~@cpu-emulation +SystemCallFilter=~@debug +SystemCallFilter=~@module +SystemCallFilter=~@mount +SystemCallFilter=~@obsolete +SystemCallFilter=~@privileged +SystemCallFilter=~@raw-io +SystemCallFilter=~@reboot +SystemCallFilter=~@swap +NoNewPrivileges=true +SupplementaryGroups= + +# Grant: +CapabilityBoundingSet=CAP_DAC_OVERRIDE +PrivateUsers=no +RestrictAddressFamilies=AF_UNIX +SystemCallFilter=@resources diff --git a/po/POTFILES.skip b/po/POTFILES.skip index 692aa74950..3f70738f8b 100644 --- a/po/POTFILES.skip +++ b/po/POTFILES.skip @@ -1,6 +1,7 @@ contrib/fedora/rpm/ data/NetworkManager-wait-online.service.in data/NetworkManager.service.in +data/nm-sudo.service.in data/org.freedesktop.NetworkManager.policy.in examples/python/NetworkManager.py examples/python/systray/eggtrayicon.c diff --git a/src/core/meson.build b/src/core/meson.build index 1ce641d6d8..fc94def205 100644 --- a/src/core/meson.build +++ b/src/core/meson.build @@ -172,6 +172,7 @@ libNetworkManager = static_library( 'nm-rfkill-manager.c', 'nm-session-monitor.c', 'nm-sleep-monitor.c', + 'nm-sudo-call.c', ), dependencies: nm_deps, link_with: [ diff --git a/src/core/nm-sudo-call.c b/src/core/nm-sudo-call.c new file mode 100644 index 0000000000..deeab52cb2 --- /dev/null +++ b/src/core/nm-sudo-call.c @@ -0,0 +1,5 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "src/core/nm-default-daemon.h" + +#include "nm-sudo-call.h" diff --git a/src/core/nm-sudo-call.h b/src/core/nm-sudo-call.h new file mode 100644 index 0000000000..74b0a28cbb --- /dev/null +++ b/src/core/nm-sudo-call.h @@ -0,0 +1,6 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#ifndef __NM_SUDO_CALL_H__ +#define __NM_SUDO_CALL_H__ + +#endif /* __NM_SUDO_CALL_H__ */ diff --git a/src/libnm-base/meson.build b/src/libnm-base/meson.build index d7e7a1b85f..3cd554d269 100644 --- a/src/libnm-base/meson.build +++ b/src/libnm-base/meson.build @@ -5,6 +5,7 @@ libnm_base = static_library( sources: files( 'nm-ethtool-base.c', 'nm-net-aux.c', + 'nm-sudo-utils.c', ), include_directories: [ src_inc, diff --git a/src/libnm-base/nm-sudo-utils.c b/src/libnm-base/nm-sudo-utils.c new file mode 100644 index 0000000000..6ae1e78cd7 --- /dev/null +++ b/src/libnm-base/nm-sudo-utils.c @@ -0,0 +1,5 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "libnm-glib-aux/nm-default-glib-i18n-lib.h" + +#include "nm-sudo-utils.h" diff --git a/src/libnm-base/nm-sudo-utils.h b/src/libnm-base/nm-sudo-utils.h new file mode 100644 index 0000000000..cffea48dbc --- /dev/null +++ b/src/libnm-base/nm-sudo-utils.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#ifndef __NM_SUDO_UTILS_H__ +#define __NM_SUDO_UTILS_H__ + +/*****************************************************************************/ + +#define NM_SUDO_DBUS_BUS_NAME "org.freedesktop.nm.sudo" +#define NM_SUDO_DBUS_OBJECT_PATH "/org/freedesktop/nm/sudo" +#define NM_SUDO_DBUS_IFACE_NAME "org.freedesktop.nm.sudo" + +/*****************************************************************************/ + +#endif /* __NM_SUDO_UTILS_H__ */ diff --git a/src/meson.build b/src/meson.build index 39bfe7ef78..4751a29684 100644 --- a/src/meson.build +++ b/src/meson.build @@ -93,6 +93,7 @@ if enable_nmtui endif subdir('nmcli') subdir('nm-dispatcher') +subdir('nm-sudo') subdir('nm-daemon-helper') subdir('nm-online') if enable_nmtui diff --git a/src/nm-sudo/meson.build b/src/nm-sudo/meson.build new file mode 100644 index 0000000000..875ce3d515 --- /dev/null +++ b/src/nm-sudo/meson.build @@ -0,0 +1,36 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later + +configure_file( + input: 'org.freedesktop.nm.sudo.service.in', + output: '@BASENAME@', + install_dir: dbus_system_bus_services_dir, + configuration: data_conf, +) + +install_data( + 'nm-sudo.conf', + install_dir: dbus_conf_dir, +) + +executable( + 'nm-sudo', + 'nm-sudo.c', + include_directories : [ + src_inc, + top_inc, + ], + dependencies: [ + glib_dep, + ], + link_with: [ + libnm_base, + libnm_log_null, + libnm_glib_aux, + libnm_std_aux, + libc_siphash, + ], + link_args: ldflags_linker_script_binary, + link_depends: linker_script_binary, + install: true, + install_dir: nm_libexecdir, +) diff --git a/src/nm-sudo/nm-sudo.c b/src/nm-sudo/nm-sudo.c new file mode 100644 index 0000000000..5be29d5a96 --- /dev/null +++ b/src/nm-sudo/nm-sudo.c @@ -0,0 +1,600 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "libnm-glib-aux/nm-default-glib-i18n-prog.h" + +#include "c-list/src/c-list.h" +#include "libnm-glib-aux/nm-logging-base.h" +#include "libnm-glib-aux/nm-shared-utils.h" +#include "libnm-glib-aux/nm-time-utils.h" +#include "libnm-glib-aux/nm-dbus-aux.h" +#include "libnm-base/nm-sudo-utils.h" + +/* nm-sudo doesn't link with libnm-core nor libnm-base, but these headers + * can be used independently. */ +#include "libnm-core-public/nm-dbus-interface.h" + +/*****************************************************************************/ + +#define IDLE_TIMEOUT_MSEC 2000 +#define IDLE_TIMEOUT_INFINITY G_MAXINT32 + +/*****************************************************************************/ + +/* Serves only the purpose to mark environment variables that are honored by + * the application. You can search for this macro, and find what options are supported. */ +#define _ENV(var) ("" var "") + +/*****************************************************************************/ + +typedef struct _GlobalData GlobalData; + +typedef struct { + CList pending_jobs_lst; + GlobalData *gl; +} PendingJobData; + +struct _GlobalData { + GCancellable * quit_cancellable; + GDBusConnection *dbus_connection; + GSource * source_sigterm; + + CList pending_jobs_lst_head; + + GSource *source_idle_timeout; + char * name_owner; + guint name_owner_changed_id; + guint service_regist_id; + gint64 start_timestamp_msec; + guint32 timeout_msec; + bool name_owner_initialized; + bool service_registered; + + /* This is controlled by $NM_SUDO_NO_AUTH_FOR_TESTING. It disables authentication + * of the request, so it is ONLY for testing. */ + bool no_auth_for_testing; + + bool is_shutting_down_quitting; + bool is_shutting_down_timeout; + bool is_shutting_down_cleanup; +}; + +/*****************************************************************************/ + +static void _pending_job_register_object(GlobalData *gl, GObject *obj); + +/*****************************************************************************/ + +#define _nm_log(level, ...) _nm_log_simple_printf((level), __VA_ARGS__); + +#define _NMLOG(level, ...) \ + G_STMT_START \ + { \ + const NMLogLevel _level = (level); \ + \ + if (_nm_logging_enabled(_level)) { \ + _nm_log(_level, __VA_ARGS__); \ + } \ + } \ + G_STMT_END + +/*****************************************************************************/ + +static void +_handle_ping(GlobalData *gl, GDBusMethodInvocation *invocation, const char *arg) +{ + gs_free char *msg = NULL; + gint64 running_msec; + + running_msec = nm_utils_clock_gettime_msec(CLOCK_BOOTTIME) - gl->start_timestamp_msec; + + msg = g_strdup_printf("pid=%lu, unique-name=%s, nm-name-owner=%s, since=%ld.%03d%s, pong=%s", + (unsigned long) getpid(), + g_dbus_connection_get_unique_name(gl->dbus_connection), + gl->name_owner ?: "(none)", + running_msec / 1000, + (int) (running_msec % 1000), + gl->no_auth_for_testing ? ", no-auth-for-testing" : "", + arg); + g_dbus_method_invocation_return_value(invocation, g_variant_new("(s)", msg)); +} + +/*****************************************************************************/ + +static gboolean +_signal_callback_term(gpointer user_data) +{ + GlobalData *gl = user_data; + + _LOGD("sigterm received (%s)", + c_list_is_empty(&gl->pending_jobs_lst_head) ? "quit mainloop" : "cancel operations"); + + gl->is_shutting_down_quitting = TRUE; + g_cancellable_cancel(gl->quit_cancellable); + return G_SOURCE_CONTINUE; +} + +/*****************************************************************************/ + +typedef struct { + GDBusConnection **p_dbus_connection; + GError ** p_error; +} BusGetData; + +static void +_bus_get_cb(GObject *source, GAsyncResult *result, gpointer user_data) +{ + BusGetData *data = user_data; + + *data->p_dbus_connection = g_bus_get_finish(result, data->p_error); +} + +static GDBusConnection * +_bus_get(GCancellable *cancellable, int *out_exit_code) +{ + gs_free_error GError *error = NULL; + gs_unref_object GDBusConnection *dbus_connection = NULL; + BusGetData data = { + .p_dbus_connection = &dbus_connection, + .p_error = &error, + }; + + g_bus_get(G_BUS_TYPE_SYSTEM, cancellable, _bus_get_cb, &data); + + while (!dbus_connection && !error) + g_main_context_iteration(NULL, TRUE); + + if (!dbus_connection) { + gboolean was_cancelled = nm_utils_error_is_cancelled(error); + + NM_SET_OUT(out_exit_code, was_cancelled ? EXIT_SUCCESS : EXIT_FAILURE); + if (!was_cancelled) + _LOGE("dbus: failure to get D-Bus connection: %s", error->message); + return NULL; + } + + /* On bus-disconnect, GDBus will raise(SIGTERM), which we handle like a + * regular request to quit. */ + g_dbus_connection_set_exit_on_close(dbus_connection, TRUE); + + _LOGD("dbus: unique name: %s", g_dbus_connection_get_unique_name(dbus_connection)); + + return g_steal_pointer(&dbus_connection); +} + +/*****************************************************************************/ + +static void +_name_owner_changed_cb(GDBusConnection *connection, + const char * sender_name, + const char * object_path, + const char * interface_name, + const char * signal_name, + GVariant * parameters, + gpointer user_data) +{ + GlobalData *gl = user_data; + const char *new_owner; + + if (!gl->name_owner_initialized) + return; + + if (!g_variant_is_of_type(parameters, G_VARIANT_TYPE("(sss)"))) + return; + + g_variant_get(parameters, "(&s&s&s)", NULL, NULL, &new_owner); + new_owner = nm_str_not_empty(new_owner); + + _LOGD("%s name-owner changed: %s -> %s", + NM_DBUS_SERVICE, + gl->name_owner ?: "(null)", + new_owner ?: "(null)"); + + nm_utils_strdup_reset(&gl->name_owner, new_owner); +} + +typedef struct { + GlobalData *gl; + char ** p_name_owner; + gboolean is_cancelled; +} BusFindNMNameOwnerData; + +static void +_bus_find_nm_nameowner_cb(const char *name_owner, GError *error, gpointer user_data) +{ + BusFindNMNameOwnerData *data = user_data; + + *data->p_name_owner = nm_strdup_not_empty(name_owner); + data->is_cancelled = nm_utils_error_is_cancelled(error); + data->gl->name_owner_initialized = TRUE; +} + +static gboolean +_bus_find_nm_nameowner(GlobalData *gl) +{ + BusFindNMNameOwnerData data; + guint name_owner_changed_id; + gs_free char * name_owner = NULL; + + name_owner_changed_id = + nm_dbus_connection_signal_subscribe_name_owner_changed(gl->dbus_connection, + NM_DBUS_SERVICE, + _name_owner_changed_cb, + gl, + NULL); + + data = (BusFindNMNameOwnerData){ + .gl = gl, + .is_cancelled = FALSE, + .p_name_owner = &name_owner, + }; + nm_dbus_connection_call_get_name_owner(gl->dbus_connection, + NM_DBUS_SERVICE, + 10000, + gl->quit_cancellable, + _bus_find_nm_nameowner_cb, + &data); + while (!gl->name_owner_initialized) + g_main_context_iteration(NULL, TRUE); + + if (data.is_cancelled) { + g_dbus_connection_signal_unsubscribe(gl->dbus_connection, name_owner_changed_id); + return FALSE; + } + + gl->name_owner_changed_id = name_owner_changed_id; + gl->name_owner = g_steal_pointer(&name_owner); + return TRUE; +} + +/*****************************************************************************/ + +static void +_bus_method_call(GDBusConnection * connection, + const char * sender, + const char * object_path, + const char * interface_name, + const char * method_name, + GVariant * parameters, + GDBusMethodInvocation *invocation, + gpointer user_data) +{ + GlobalData *gl = user_data; + const char *arg_s; + + nm_assert(nm_streq(object_path, NM_SUDO_DBUS_OBJECT_PATH)); + nm_assert(nm_streq(interface_name, NM_SUDO_DBUS_IFACE_NAME)); + + if (!gl->no_auth_for_testing && !nm_streq0(sender, gl->name_owner)) { + _LOGT("dbus: request sender=%s, %s%s, ACCESS DENIED", + sender, + method_name, + g_variant_get_type_string(parameters)); + g_dbus_method_invocation_return_error(invocation, + G_DBUS_ERROR, + G_DBUS_ERROR_ACCESS_DENIED, + "Access denied"); + return; + } + + _pending_job_register_object(gl, G_OBJECT(invocation)); + + _LOGT("dbus: request sender=%s, %s%s", + sender, + method_name, + g_variant_get_type_string(parameters)); + + if (nm_streq(method_name, "Ping")) { + g_variant_get(parameters, "(&s)", &arg_s); + _handle_ping(gl, invocation, arg_s); + } else + nm_assert_not_reached(); +} + +static GDBusInterfaceInfo *const interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO( + NM_SUDO_DBUS_IFACE_NAME, + .methods = NM_DEFINE_GDBUS_METHOD_INFOS( + NM_DEFINE_GDBUS_METHOD_INFO( + "Ping", + .in_args = NM_DEFINE_GDBUS_ARG_INFOS(NM_DEFINE_GDBUS_ARG_INFO("arg", "s"), ), + .out_args = NM_DEFINE_GDBUS_ARG_INFOS(NM_DEFINE_GDBUS_ARG_INFO("arg", "s"), ), ), ), ); + +typedef struct { + GlobalData *gl; + gboolean is_waiting; +} BusRegisterServiceRequestNameData; + +static void +_bus_register_service_request_name_cb(GObject *source, GAsyncResult *res, gpointer user_data) +{ + BusRegisterServiceRequestNameData *data = user_data; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *ret = NULL; + gboolean success = FALSE; + + ret = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error); + + if (nm_utils_error_is_cancelled(error)) + goto out; + + if (error) + _LOGE("d-bus: failed to request name %s: %s", NM_SUDO_DBUS_BUS_NAME, error->message); + else { + guint32 ret_val; + + g_variant_get(ret, "(u)", &ret_val); + if (ret_val != DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) { + _LOGW("dbus: request name for %s failed to take name (response %u)", + NM_SUDO_DBUS_BUS_NAME, + ret_val); + } else { + _LOGD("dbus: request name for %s succeeded", NM_SUDO_DBUS_BUS_NAME); + success = TRUE; + } + } + +out: + if (success) + data->gl->service_registered = TRUE; + data->is_waiting = FALSE; +} + +static void +_bus_register_service(GlobalData *gl) +{ + static const GDBusInterfaceVTable interface_vtable = { + .method_call = _bus_method_call, + }; + gs_free_error GError * error = NULL; + BusRegisterServiceRequestNameData data; + + nm_assert(!gl->service_registered); + + gl->service_regist_id = + g_dbus_connection_register_object(gl->dbus_connection, + NM_SUDO_DBUS_OBJECT_PATH, + interface_info, + NM_UNCONST_PTR(GDBusInterfaceVTable, &interface_vtable), + gl, + NULL, + &error); + if (gl->service_regist_id == 0) { + _LOGE("dbus: error registering object %s: %s", NM_SUDO_DBUS_OBJECT_PATH, error->message); + return; + } + + _LOGD("dbus: object %s registered", NM_SUDO_DBUS_OBJECT_PATH); + + data = (BusRegisterServiceRequestNameData){ + .gl = gl, + .is_waiting = TRUE, + }; + + g_dbus_connection_call( + gl->dbus_connection, + DBUS_SERVICE_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "RequestName", + g_variant_new("(su)", + NM_SUDO_DBUS_BUS_NAME, + (guint) (DBUS_NAME_FLAG_ALLOW_REPLACEMENT | DBUS_NAME_FLAG_REPLACE_EXISTING)), + G_VARIANT_TYPE("(u)"), + G_DBUS_CALL_FLAGS_NONE, + -1, + gl->quit_cancellable, + _bus_register_service_request_name_cb, + &data); + + /* Note that with D-Bus activation, the first request will already hit us before RequestName + * completes. */ + + while (data.is_waiting) + g_main_context_iteration(NULL, TRUE); +} + +/*****************************************************************************/ + +static gboolean +_idle_timeout_cb(gpointer user_data) +{ + GlobalData *gl = user_data; + + _LOGT("idle-timeout: expired"); + gl->is_shutting_down_timeout = TRUE; + return G_SOURCE_CONTINUE; +} + +static void +_idle_timeout_restart(GlobalData *gl) +{ + nm_clear_g_source_inst(&gl->source_idle_timeout); + + if (gl->is_shutting_down_quitting) + return; + + if (gl->is_shutting_down_cleanup) + return; + + if (!c_list_is_empty(&gl->pending_jobs_lst_head)) + return; + + if (gl->timeout_msec == IDLE_TIMEOUT_INFINITY) + return; + + nm_assert(gl->timeout_msec < G_MAXINT32); + G_STATIC_ASSERT_EXPR(G_MAXINT32 < G_MAXUINT); + + _LOGT("idle-timeout: start (%u msec)", gl->timeout_msec); + gl->source_idle_timeout = nm_g_timeout_add_source(gl->timeout_msec, _idle_timeout_cb, gl); +} + +/*****************************************************************************/ + +static gboolean +_pending_job_register_object_release_on_idle_cb(gpointer data) +{ + PendingJobData *idle_data = data; + GlobalData * gl = idle_data->gl; + + c_list_unlink_stale(&idle_data->pending_jobs_lst); + nm_g_slice_free(idle_data); + + _idle_timeout_restart(gl); + return G_SOURCE_REMOVE; +} + +static void +_pending_job_register_object_weak_cb(gpointer data, GObject *where_the_object_was) +{ + /* The object might be destroyed on another thread. We need + * to sync with the main GMainContext by scheduling an idle action + * there. */ + nm_g_idle_add(_pending_job_register_object_release_on_idle_cb, data); +} + +static void +_pending_job_register_object(GlobalData *gl, GObject *obj) +{ + PendingJobData *idle_data; + + /* if we just hit the timeout, we can ignore it. */ + gl->is_shutting_down_timeout = FALSE; + + if (nm_clear_g_source_inst(&gl->source_idle_timeout)) + _LOGT("idle-timeout: suspend timeout for pending request"); + + idle_data = g_slice_new(PendingJobData); + + idle_data->gl = gl; + c_list_link_tail(&gl->pending_jobs_lst_head, &idle_data->pending_jobs_lst); + + g_object_weak_ref(obj, _pending_job_register_object_weak_cb, idle_data); +} + +/*****************************************************************************/ + +static void +_initial_setup(GlobalData *gl) +{ + gl->no_auth_for_testing = + _nm_utils_ascii_str_to_int64(g_getenv(_ENV("NM_SUDO_NO_AUTH_FOR_TESTING")), 0, 0, 1, 0); + gl->timeout_msec = _nm_utils_ascii_str_to_int64(g_getenv(_ENV("NM_SUDO_IDLE_TIMEOUT_MSEC")), + 0, + 0, + G_MAXINT32, + IDLE_TIMEOUT_MSEC); + + gl->quit_cancellable = g_cancellable_new(); + + signal(SIGPIPE, SIG_IGN); + gl->source_sigterm = nm_g_unix_signal_add_source(SIGTERM, _signal_callback_term, gl); +} + +int +main(int argc, char **argv) +{ + GlobalData _gl = { + .quit_cancellable = NULL, + .pending_jobs_lst_head = C_LIST_INIT(_gl.pending_jobs_lst_head), + }; + GlobalData *const gl = &_gl; + int exit_code; + int r = 0; + + _nm_logging_enabled_init(g_getenv(_ENV("NM_SUDO_LOG"))); + + gl->start_timestamp_msec = nm_utils_clock_gettime_msec(CLOCK_BOOTTIME); + + _LOGD("starting nm-sudo (%s)", NM_DIST_VERSION); + + _initial_setup(gl); + + if (gl->no_auth_for_testing) { + _LOGW("WARNING: running in debug mode without authentication " + "(NM_SUDO_NO_AUTH_FOR_TESTING). "); + } + + if (gl->timeout_msec != IDLE_TIMEOUT_INFINITY) + _LOGT("idle-timeout: %u msec", gl->timeout_msec); + else + _LOGT("idle-timeout: disabled"); + + gl->dbus_connection = _bus_get(gl->quit_cancellable, &r); + if (!gl->dbus_connection) { + exit_code = r; + goto done; + } + + if (!_bus_find_nm_nameowner(gl)) { + /* abort due to cancellation. That is success. */ + exit_code = EXIT_SUCCESS; + goto done; + } + _LOGD("%s name-owner: %s", NM_DBUS_SERVICE, gl->name_owner ?: "(null)"); + + _idle_timeout_restart(gl); + + exit_code = EXIT_SUCCESS; + + _bus_register_service(gl); + if (!gl->service_registered) { + /* We failed to RequestName, but due to D-Bus activation we + * might have a pending request still (on the unique name). + * Process it below. + * + * Let's fake a shutdown signal, and still process the request below. */ + exit_code = EXIT_FAILURE; + gl->is_shutting_down_quitting = TRUE; + } + + while (TRUE) { + if (!c_list_is_empty(&gl->pending_jobs_lst_head)) { + /* we must first reply to all requests. No matter what. */ + } else { + if (gl->is_shutting_down_quitting || gl->is_shutting_down_timeout) { + /* we either hit the idle timeout or received SIGTERM. Note that + * if we received an idle-timeout and the very moment afterwards + * a new request, then _bus_method_call() will clear gl->is_shutting_down_timeout + * (via _pending_job_register_object()). */ + break; + } + } + + g_main_context_iteration(NULL, TRUE); + } + +done: + gl->is_shutting_down_cleanup = TRUE; + _LOGD("exiting..."); + + nm_assert(c_list_is_empty(&gl->pending_jobs_lst_head)); + + if (gl->service_regist_id != 0) { + g_dbus_connection_unregister_object(gl->dbus_connection, + nm_steal_int(&gl->service_regist_id)); + } + if (gl->name_owner_changed_id != 0) { + g_dbus_connection_signal_unsubscribe(gl->dbus_connection, + nm_steal_int(&gl->name_owner_changed_id)); + } + nm_clear_g_cancellable(&gl->quit_cancellable); + nm_clear_g_source_inst(&gl->source_sigterm); + nm_clear_g_source_inst(&gl->source_idle_timeout); + nm_clear_g_free(&gl->name_owner); + + while (g_main_context_iteration(NULL, FALSE)) { + ; + } + + if (gl->dbus_connection) { + g_dbus_connection_flush_sync(gl->dbus_connection, NULL, NULL); + g_clear_object(&gl->dbus_connection); + + while (g_main_context_iteration(NULL, FALSE)) { + ; + } + } + + _LOGD("exit (%d)", exit_code); + return exit_code; +} diff --git a/src/nm-sudo/nm-sudo.conf b/src/nm-sudo/nm-sudo.conf new file mode 100644 index 0000000000..922c62314a --- /dev/null +++ b/src/nm-sudo/nm-sudo.conf @@ -0,0 +1,13 @@ + + + + + + + + + + + diff --git a/src/nm-sudo/org.freedesktop.nm.sudo.service.in b/src/nm-sudo/org.freedesktop.nm.sudo.service.in new file mode 100644 index 0000000000..43d29de14d --- /dev/null +++ b/src/nm-sudo/org.freedesktop.nm.sudo.service.in @@ -0,0 +1,5 @@ +[D-BUS Service] +Name=org.freedesktop.nm.sudo +Exec=@libexecdir@/nm-sudo +User=root +SystemdService=dbus-org.freedesktop.nm.sudo.service From de5dddccbe81eba866226e95435ed7f90c7d1b98 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 21 Jul 2021 16:31:48 +0200 Subject: [PATCH 6/6] core: get file descriptor to ovsdb unix socket from nm-sudo To talk to ovsdb, we use the unix socket at /var/run/openvswitch/db.sock. But that socket is owned by another user and NetworkManager would need dac_override capability to open it. We want to drop dac_override, but we still need to talk to ovsdb. Add a GetFD() method to nm-sudo. We still first try to open the socket directly. Maybe it just works. Note that SELinux may block passing file descriptors from nm-sudo. If it doesn't work for you, test with SELinux permissive mode and wait for an SELinux update. --- Makefile.am | 2 + src/core/devices/ovs/nm-ovsdb.c | 101 +++++++++++++++++++++----------- src/core/meson.build | 5 +- src/core/nm-sudo-call.c | 94 +++++++++++++++++++++++++++++ src/core/nm-sudo-call.h | 9 +++ src/libnm-base/nm-sudo-utils.c | 53 +++++++++++++++++ src/libnm-base/nm-sudo-utils.h | 9 +++ src/nm-sudo/nm-sudo.c | 33 ++++++++++- 8 files changed, 271 insertions(+), 35 deletions(-) diff --git a/Makefile.am b/Makefile.am index 7a6f56199a..8f658b52c0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2664,6 +2664,8 @@ $(src_core_libNetworkManagerTest_la_OBJECTS): $(src_libnm_core_public_mkenums_h) # itself, but by the plugins (that are dlopened). We need to explicitly include # them during linking. networkmanager_undefined_symbols = \ + nm_sudo_call_get_fd \ + nm_sudo_utils_open_fd \ $(NULL) noinst_PROGRAMS += src/core/NetworkManager-all-sym diff --git a/src/core/devices/ovs/nm-ovsdb.c b/src/core/devices/ovs/nm-ovsdb.c index 4f72e9fa9d..58455ca719 100644 --- a/src/core/devices/ovs/nm-ovsdb.c +++ b/src/core/devices/ovs/nm-ovsdb.c @@ -17,6 +17,7 @@ #include "devices/nm-device.h" #include "nm-manager.h" #include "nm-setting-ovs-external-ids.h" +#include "nm-sudo-call.h" /*****************************************************************************/ @@ -118,9 +119,8 @@ enum { static guint signals[LAST_SIGNAL] = {0}; typedef struct { - GSocketClient * client; GSocketConnection *conn; - GCancellable * cancellable; + GCancellable * conn_cancellable; char buf[4096]; /* Input buffer */ size_t bufp; /* Last decoded byte in the input buffer. */ GString * input; /* JSON stream waiting for decoding. */ @@ -2223,7 +2223,7 @@ ovsdb_disconnect(NMOvsdb *self, gboolean retry, gboolean is_disposing) nm_assert(!retry || !is_disposing); - if (!priv->client) + if (!priv->conn && !priv->conn_cancellable) return; _LOGD("disconnecting from ovsdb, retry %d", retry); @@ -2250,10 +2250,9 @@ ovsdb_disconnect(NMOvsdb *self, gboolean retry, gboolean is_disposing) priv->bufp = 0; g_string_truncate(priv->input, 0); g_string_truncate(priv->output, 0); - g_clear_object(&priv->client); g_clear_object(&priv->conn); nm_clear_g_free(&priv->db_uuid); - nm_clear_g_cancellable(&priv->cancellable); + nm_clear_g_cancellable(&priv->conn_cancellable); if (retry) ovsdb_try_connect(self); @@ -2348,32 +2347,77 @@ _monitor_bridges_cb(NMOvsdb *self, json_t *result, GError *error, gpointer user_ } static void -_client_connect_cb(GObject *source_object, GAsyncResult *res, gpointer user_data) +_ovsdb_connect_complete_with_fd(NMOvsdb *self, int fd_take) { - GSocketClient * client = G_SOCKET_CLIENT(source_object); - NMOvsdb * self = NM_OVSDB(user_data); - NMOvsdbPrivate * priv; - GError * error = NULL; - GSocketConnection *conn; - - conn = g_socket_client_connect_finish(client, res, &error); - if (conn == NULL) { - if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) - _LOGI("%s", error->message); + NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE(self); + gs_unref_object GSocket *socket = NULL; + gs_free_error GError *error = NULL; + socket = g_socket_new_from_fd(nm_steal_fd(&fd_take), &error); + if (!socket) { + _LOGT("connect: failure to open socket for new FD: %s", error->message); ovsdb_disconnect(self, FALSE, FALSE); - g_clear_error(&error); return; } - priv = NM_OVSDB_GET_PRIVATE(self); - priv->conn = conn; - g_clear_object(&priv->cancellable); + priv->conn = g_socket_connection_factory_create_connection(socket); + g_clear_object(&priv->conn_cancellable); ovsdb_read(self); ovsdb_next_command(self); } +static void +_ovsdb_connect_sudo_cb(int fd_take, GError *error, gpointer user_data) +{ + nm_auto_close int fd = fd_take; + NMOvsdb * self; + + if (nm_utils_error_is_cancelled(error)) + return; + + self = user_data; + + if (error) { + _LOGT("connect: failure to get FD from nm-sudo: %s", error->message); + ovsdb_disconnect(self, FALSE, FALSE); + return; + } + + _LOGT("connect: connected successfully with FD from nm-sudo"); + _ovsdb_connect_complete_with_fd(self, nm_steal_fd(&fd)); +} + +static void +_ovsdb_connect_idle(gpointer user_data, GCancellable *cancellable) +{ + NMOvsdb * self; + NMOvsdbPrivate * priv; + nm_auto_close int fd = -1; + gs_free_error GError *error = NULL; + + if (g_cancellable_is_cancelled(cancellable)) + return; + + self = user_data; + priv = NM_OVSDB_GET_PRIVATE(self); + + fd = nm_sudo_utils_open_fd(NM_SUDO_GET_FD_TYPE_OVSDB_SOCKET, &error); + if (fd < 0) { + _LOGT("connect: opening %s failed (\"%s\"). Retry with nm-sudo", + NM_OVSDB_SOCKET, + error->message); + nm_sudo_call_get_fd(NM_SUDO_GET_FD_TYPE_OVSDB_SOCKET, + priv->conn_cancellable, + _ovsdb_connect_sudo_cb, + self); + return; + } + + _LOGT("connect: opening %s succeeded", NM_OVSDB_SOCKET); + _ovsdb_connect_complete_with_fd(self, nm_steal_fd(&fd)); +} + /** * ovsdb_try_connect: * @@ -2385,22 +2429,13 @@ static void ovsdb_try_connect(NMOvsdb *self) { NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE(self); - GSocketAddress *addr; - if (priv->client) + if (priv->conn || priv->conn_cancellable) return; - /* TODO: This should probably be made configurable via NetworkManager.conf */ - addr = g_unix_socket_address_new(RUNSTATEDIR "/openvswitch/db.sock"); - - priv->client = g_socket_client_new(); - priv->cancellable = g_cancellable_new(); - g_socket_client_connect_async(priv->client, - G_SOCKET_CONNECTABLE(addr), - priv->cancellable, - _client_connect_cb, - self); - g_object_unref(addr); + _LOGT("connect: start connecting socket %s on idle", NM_OVSDB_SOCKET); + priv->conn_cancellable = g_cancellable_new(); + nm_utils_invoke_on_idle(priv->conn_cancellable, _ovsdb_connect_idle, self); /* Queue a monitor call before any other command, ensuring that we have an up * to date view of existing bridged that we need for add and remove ops. */ diff --git a/src/core/meson.build b/src/core/meson.build index fc94def205..f935ed1606 100644 --- a/src/core/meson.build +++ b/src/core/meson.build @@ -275,7 +275,10 @@ subdir('settings/plugins') # itself, but by the plugins (that are dlopened). We need to explicitly include # them during linking. networkmanager_undefined_symbols_args = [] -foreach s: [] +foreach s: [ + 'nm_sudo_call_get_fd', + 'nm_sudo_utils_open_fd', + ] networkmanager_undefined_symbols_args += ['-u', s] endforeach diff --git a/src/core/nm-sudo-call.c b/src/core/nm-sudo-call.c index deeab52cb2..6f9ca4f138 100644 --- a/src/core/nm-sudo-call.c +++ b/src/core/nm-sudo-call.c @@ -3,3 +3,97 @@ #include "src/core/nm-default-daemon.h" #include "nm-sudo-call.h" + +#include + +#include "nm-dbus-manager.h" + +/*****************************************************************************/ + +static void +_nm_sudo_call_get_fd_cb(GObject *source, GAsyncResult *res, gpointer user_data) +{ + NMSudoCallGetFDCallback callback; + gpointer callback_data; + gs_unref_variant GVariant *ret = NULL; + gs_free_error GError *error = NULL; + gs_unref_object GUnixFDList *fd_list = NULL; + gs_free int * fd_arr = NULL; + + nm_utils_user_data_unpack(user_data, &callback, &callback_data); + + ret = g_dbus_connection_call_with_unix_fd_list_finish(G_DBUS_CONNECTION(source), + &fd_list, + res, + &error); + + if (error) { + callback(-1, error, callback_data); + return; + } + + if (!fd_list || g_unix_fd_list_get_length(fd_list) != 1) { + nm_utils_error_set(&error, + NM_UTILS_ERROR_UNKNOWN, + "Unexpectedly not one FD is returned by nm-sudo GetFD()"); + callback(-1, error, callback_data); + return; + } + + fd_arr = g_unix_fd_list_steal_fds(fd_list, NULL); + + /* we transfer ownership of the file descriptor! */ + callback(fd_arr[0], NULL, callback_data); +} + +static gboolean +_nm_sudo_call_get_fd_fail_on_idle(gpointer user_data) +{ + gs_unref_object GCancellable *cancellable = NULL; + NMSudoCallGetFDCallback callback; + gpointer callback_data; + gs_free_error GError *error = NULL; + + nm_utils_user_data_unpack(user_data, &cancellable, &callback, &callback_data); + + if (!g_cancellable_set_error_if_cancelled(cancellable, &error)) + nm_utils_error_set(&error, NM_UTILS_ERROR_UNKNOWN, "Cannot talk to nm-sudo without D-Bus"); + + callback(-1, error, callback_data); + return G_SOURCE_REMOVE; +} + +void +nm_sudo_call_get_fd(NMSudoGetFDType fd_type, + GCancellable * cancellable, + NMSudoCallGetFDCallback callback, + gpointer user_data) +{ + GDBusConnection *dbus_connection; + + nm_assert(NM_IN_SET(fd_type, NM_SUDO_GET_FD_TYPE_OVSDB_SOCKET)); + nm_assert(!cancellable || G_IS_CANCELLABLE(cancellable)); + nm_assert(callback); + + dbus_connection = NM_MAIN_DBUS_CONNECTION_GET; + + if (!dbus_connection) { + nm_g_idle_add(_nm_sudo_call_get_fd_fail_on_idle, + nm_utils_user_data_pack(g_object_ref(cancellable), callback, user_data)); + return; + } + + g_dbus_connection_call_with_unix_fd_list(dbus_connection, + NM_SUDO_DBUS_BUS_NAME, + NM_SUDO_DBUS_OBJECT_PATH, + NM_SUDO_DBUS_IFACE_NAME, + "GetFD", + g_variant_new("(u)", fd_type), + G_VARIANT_TYPE("()"), + G_DBUS_CALL_FLAGS_NONE, + 10000, + NULL, + cancellable, + _nm_sudo_call_get_fd_cb, + nm_utils_user_data_pack(callback, user_data)); +} diff --git a/src/core/nm-sudo-call.h b/src/core/nm-sudo-call.h index 74b0a28cbb..741ae19b5b 100644 --- a/src/core/nm-sudo-call.h +++ b/src/core/nm-sudo-call.h @@ -3,4 +3,13 @@ #ifndef __NM_SUDO_CALL_H__ #define __NM_SUDO_CALL_H__ +#include "libnm-base/nm-sudo-utils.h" + +typedef void (*NMSudoCallGetFDCallback)(int fd_take, GError *error, gpointer user_data); + +void nm_sudo_call_get_fd(NMSudoGetFDType fd_type, + GCancellable * cancellable, + NMSudoCallGetFDCallback callback, + gpointer user_data); + #endif /* __NM_SUDO_CALL_H__ */ diff --git a/src/libnm-base/nm-sudo-utils.c b/src/libnm-base/nm-sudo-utils.c index 6ae1e78cd7..18bd0051bd 100644 --- a/src/libnm-base/nm-sudo-utils.c +++ b/src/libnm-base/nm-sudo-utils.c @@ -3,3 +3,56 @@ #include "libnm-glib-aux/nm-default-glib-i18n-lib.h" #include "nm-sudo-utils.h" + +#include +#include + +/*****************************************************************************/ + +int +nm_sudo_utils_open_fd(NMSudoGetFDType fd_type, GError **error) +{ + nm_auto_close int fd = -1; + int r; + int errsv; + + switch (fd_type) { + case NM_SUDO_GET_FD_TYPE_OVSDB_SOCKET: + { + struct sockaddr_un sock; + + memset(&sock, 0, sizeof(sock)); + sock.sun_family = AF_UNIX; + G_STATIC_ASSERT_EXPR(NM_STRLEN(NM_OVSDB_SOCKET) + 1 < sizeof(sock.sun_path)); + if (g_strlcpy(sock.sun_path, NM_OVSDB_SOCKET, sizeof(sock.sun_path)) + >= sizeof(sock.sun_path)) + nm_assert_not_reached(); + + fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); + if (fd < 0) { + errsv = NM_ERRNO_NATIVE(errno); + g_set_error(error, G_IO_ERROR, g_io_error_from_errno(errsv), "error creating socket"); + return -errsv; + } + + r = connect(fd, + (const struct sockaddr *) &sock, + G_STRUCT_OFFSET(struct sockaddr_un, sun_path) + NM_STRLEN(NM_OVSDB_SOCKET) + 1); + if (r != 0) { + errsv = NM_ERRNO_NATIVE(errno); + g_set_error(error, + G_IO_ERROR, + g_io_error_from_errno(errsv), + "error connecting socket (%s)", + nm_strerror_native(errsv)); + return -errsv; + } + + return nm_steal_fd(&fd); + } + case NM_SUDO_GET_FD_TYPE_NONE: + default: + nm_utils_error_set(error, NM_UTILS_ERROR_UNKNOWN, "invalid fd_type"); + return -EINVAL; + } +} diff --git a/src/libnm-base/nm-sudo-utils.h b/src/libnm-base/nm-sudo-utils.h index cffea48dbc..01597fe467 100644 --- a/src/libnm-base/nm-sudo-utils.h +++ b/src/libnm-base/nm-sudo-utils.h @@ -11,4 +11,13 @@ /*****************************************************************************/ +#define NM_OVSDB_SOCKET RUNSTATEDIR "/openvswitch/db.sock" + +typedef enum { + NM_SUDO_GET_FD_TYPE_NONE = 0, + NM_SUDO_GET_FD_TYPE_OVSDB_SOCKET = 1, +} NMSudoGetFDType; + +int nm_sudo_utils_open_fd(NMSudoGetFDType fd_type, GError **error); + #endif /* __NM_SUDO_UTILS_H__ */ diff --git a/src/nm-sudo/nm-sudo.c b/src/nm-sudo/nm-sudo.c index 5be29d5a96..7dd22b1c69 100644 --- a/src/nm-sudo/nm-sudo.c +++ b/src/nm-sudo/nm-sudo.c @@ -2,6 +2,8 @@ #include "libnm-glib-aux/nm-default-glib-i18n-prog.h" +#include + #include "c-list/src/c-list.h" #include "libnm-glib-aux/nm-logging-base.h" #include "libnm-glib-aux/nm-shared-utils.h" @@ -98,6 +100,28 @@ _handle_ping(GlobalData *gl, GDBusMethodInvocation *invocation, const char *arg) g_dbus_method_invocation_return_value(invocation, g_variant_new("(s)", msg)); } +static void +_handle_get_fd(GlobalData *gl, GDBusMethodInvocation *invocation, guint32 fd_type) +{ + nm_auto_close int fd = -1; + gs_unref_object GUnixFDList *fd_list = NULL; + gs_free_error GError *error = NULL; + + if (fd_type != (NMSudoGetFDType) fd_type) + fd_type = NM_SUDO_GET_FD_TYPE_NONE; + + fd = nm_sudo_utils_open_fd(fd_type, &error); + if (fd < 0) { + g_dbus_method_invocation_take_error(invocation, g_steal_pointer(&error)); + return; + } + + fd_list = g_unix_fd_list_new_from_array(&fd, 1); + nm_steal_fd(&fd); + + g_dbus_method_invocation_return_value_with_unix_fd_list(invocation, NULL, fd_list); +} + /*****************************************************************************/ static gboolean @@ -260,6 +284,7 @@ _bus_method_call(GDBusConnection * connection, { GlobalData *gl = user_data; const char *arg_s; + guint32 arg_u; nm_assert(nm_streq(object_path, NM_SUDO_DBUS_OBJECT_PATH)); nm_assert(nm_streq(interface_name, NM_SUDO_DBUS_IFACE_NAME)); @@ -286,6 +311,9 @@ _bus_method_call(GDBusConnection * connection, if (nm_streq(method_name, "Ping")) { g_variant_get(parameters, "(&s)", &arg_s); _handle_ping(gl, invocation, arg_s); + } else if (nm_streq(method_name, "GetFD")) { + g_variant_get(parameters, "(u)", &arg_u); + _handle_get_fd(gl, invocation, arg_u); } else nm_assert_not_reached(); } @@ -296,7 +324,10 @@ static GDBusInterfaceInfo *const interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO NM_DEFINE_GDBUS_METHOD_INFO( "Ping", .in_args = NM_DEFINE_GDBUS_ARG_INFOS(NM_DEFINE_GDBUS_ARG_INFO("arg", "s"), ), - .out_args = NM_DEFINE_GDBUS_ARG_INFOS(NM_DEFINE_GDBUS_ARG_INFO("arg", "s"), ), ), ), ); + .out_args = NM_DEFINE_GDBUS_ARG_INFOS(NM_DEFINE_GDBUS_ARG_INFO("arg", "s"), ), ), + NM_DEFINE_GDBUS_METHOD_INFO("GetFD", + .in_args = NM_DEFINE_GDBUS_ARG_INFOS( + NM_DEFINE_GDBUS_ARG_INFO("fd_type", "u"), ), ), ), ); typedef struct { GlobalData *gl;