From e2df6c7503e6d54e9228a21da6205227b1c86b90 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Feb 2021 16:52:45 +0100 Subject: [PATCH 01/29] release: bump version to 1.31.0 (development) --- configure.ac | 4 ++-- meson.build | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 98c225939..63bc6831c 100644 --- a/configure.ac +++ b/configure.ac @@ -7,8 +7,8 @@ dnl - add corresponding NM_VERSION_x_y_z macros in dnl "shared/nm-version-macros.h.in" dnl - update number in meson.build m4_define([nm_major_version], [1]) -m4_define([nm_minor_version], [29]) -m4_define([nm_micro_version], [90]) +m4_define([nm_minor_version], [31]) +m4_define([nm_micro_version], [0]) m4_define([nm_version], [nm_major_version.nm_minor_version.nm_micro_version]) diff --git a/meson.build b/meson.build index 594fa63cf..0ea059241 100644 --- a/meson.build +++ b/meson.build @@ -6,7 +6,7 @@ project( # - add corresponding NM_VERSION_x_y_z macros in # "shared/nm-version-macros.h.in" # - update number in configure.ac - version: '1.29.90', + version: '1.31.0', license: 'GPL2+', default_options: [ 'buildtype=debugoptimized', From 2e334f54b27f91f40c3aa8bdba3254e2284d30bd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Feb 2021 11:30:18 +0100 Subject: [PATCH 02/29] service: don't give CAP_DAC_OVERRIDE capability to NetworkManager https://bugzilla.redhat.com/show_bug.cgi?id=1921826 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/742 --- data/NetworkManager.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/NetworkManager.service.in b/data/NetworkManager.service.in index 91ebd9a36..382cdee82 100644 --- a/data/NetworkManager.service.in +++ b/data/NetworkManager.service.in @@ -14,7 +14,7 @@ ExecStart=@sbindir@/NetworkManager --no-daemon Restart=on-failure # NM doesn't want systemd to kill its children for it KillMode=process -CapabilityBoundingSet=CAP_NET_ADMIN CAP_DAC_OVERRIDE CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETGID CAP_SETUID CAP_SYS_MODULE CAP_AUDIT_WRITE CAP_KILL CAP_SYS_CHROOT +CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETGID CAP_SETUID CAP_SYS_MODULE CAP_AUDIT_WRITE CAP_KILL CAP_SYS_CHROOT ProtectSystem=true ProtectHome=read-only From 801c41a11c2cd37dc1271c026edc0a3292cc69b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2021 10:17:41 +0100 Subject: [PATCH 03/29] build: make path to polkit-agent-helper-1 binary configurable Add new configure option to set the path to "polkit-agent-helper-1". The path cannot be obtained from pkg-config and `pkg-config --variable=prefix polkit-agent-1` is not good enough. On Fedora, the path is "/usr/lib/polkit-1/polkit-agent-helper-1". On Debian Buster, the path is "/usr/lib/policykit-1/polkit-agent-helper-1" On Debian Sid, the path is "/usr/libexec/polkit-agent-helper-1" (but currently it is also symlinked from "/usr/lib/policykit-1/polkit-agent-helper-1". --- NEWS | 1 + clients/common/nm-polkit-listener.c | 2 +- config.h.meson | 6 +++--- configure.ac | 28 +++++++++++++++++++--------- meson.build | 17 ++++++++++++----- meson_options.txt | 2 +- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/NEWS b/NEWS index 5ba853309..094680045 100644 --- a/NEWS +++ b/NEWS @@ -28,6 +28,7 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! * initrd: support new ip method "link6" for IPv6 link-local only. * ci: use ci-templates for containers on gitlab-ci. * ci: test build against Alpine Linux on gitlab-ci. +* build: new configure option to set path to "polkit-agent-helper-1". * Many bugfixes and improvements. ============================================= diff --git a/clients/common/nm-polkit-listener.c b/clients/common/nm-polkit-listener.c index db21abfc9..357e1efb8 100644 --- a/clients/common/nm-polkit-listener.c +++ b/clients/common/nm-polkit-listener.c @@ -501,7 +501,7 @@ begin_authentication(AuthRequest *request) { int fd_flags; const char *helper_argv[] = { - POLKIT_PACKAGE_PREFIX "/lib/polkit-1/polkit-agent-helper-1", + POLKIT_AGENT_HELPER_1_PATH, request->username, NULL, }; diff --git a/config.h.meson b/config.h.meson index bb5458aa6..a911dbe25 100644 --- a/config.h.meson +++ b/config.h.meson @@ -109,6 +109,9 @@ /* Define to the full name and version of this package. */ #mesondefine PACKAGE_STRING +/* path to polkit-agent-helper-1 binary */ +#mesondefine POLKIT_AGENT_HELPER_1_PATH + /* Path to resolvconf */ #mesondefine RESOLVCONF_PATH @@ -218,9 +221,6 @@ /* Define if you have oFono support (experimental) */ #mesondefine WITH_OFONO -/* Define the polkit agent package prefix */ -#mesondefine POLKIT_PACKAGE_PREFIX - /* Define if you have PPP support */ #mesondefine WITH_PPP diff --git a/configure.ac b/configure.ac index 63bc6831c..2bf5e4e06 100644 --- a/configure.ac +++ b/configure.ac @@ -642,16 +642,25 @@ fi AC_DEFINE_UNQUOTED(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT, "$enable_polkit", [The default value of the auth-polkit configuration option]) AC_SUBST(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT, "$enable_polkit") -PKG_CHECK_MODULES(POLKIT, [polkit-agent-1 >= 0.97], [have_pk_agent=yes],[have_pk_agent=no]) -if test x"$have_pk_agent" = x"no"; then - POLKIT_PACKAGE_PREFIX="/usr" -else - POLKIT_PACKAGE_PREFIX=`$PKG_CONFIG --variable=prefix polkit-agent-1` +AC_ARG_WITH([polkit-agent-helper-1-path], + AS_HELP_STRING([--with-polkit-agent-helper-1-path=PATH], + [Path name to the polkit-agent-helper-1 binary from polkit]), + POLKIT_AGENT_HELPER_1_PATH="$withval", + POLKIT_AGENT_HELPER_1_PATH="") +if test -z "$POLKIT_AGENT_HELPER_1_PATH" ; then + for p in /usr/libexec/polkit-agent-helper-1 \ + /usr/lib/polkit-1/polkit-agent-helper-1 \ + /usr/lib/policykit-1/polkit-agent-helper-1 ; do + if test -f "$p" ; then + POLKIT_AGENT_HELPER_1_PATH="$p" + break + fi + done fi -AC_DEFINE_UNQUOTED([POLKIT_PACKAGE_PREFIX], - ["$POLKIT_PACKAGE_PREFIX"], - [polkit-agent package prefix]) - +test -z "$POLKIT_AGENT_HELPER_1_PATH" && POLKIT_AGENT_HELPER_1_PATH=/usr/lib/polkit-1/polkit-agent-helper-1 +AC_DEFINE_UNQUOTED([POLKIT_AGENT_HELPER_1_PATH], + ["$POLKIT_AGENT_HELPER_1_PATH"], + [path to polkit-agent-helper-1 binary]) AC_ARG_ENABLE(modify-system, AS_HELP_STRING([--enable-modify-system], [Allow users to modify system connections])) if test "${enable_modify_system}" = "yes"; then @@ -1332,6 +1341,7 @@ if test "${enable_modify_system}" = "yes"; then else echo " policykit: main.auth-polkit=${enable_polkit} (restrictive modify.system)" fi +echo " polkit-agent-helper-1: $POLKIT_AGENT_HELPER_1_PATH" echo " selinux: $have_selinux" echo " systemd-journald: $have_systemd_journal (default: logging.backend=${nm_config_logging_backend_default})" echo " hostname persist: ${hostname_persist}" diff --git a/meson.build b/meson.build index 0ea059241..9d3d08c4f 100644 --- a/meson.build +++ b/meson.build @@ -514,12 +514,18 @@ config_h.set_quoted('NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT', config_auth_polkit_def enable_modify_system = get_option('modify_system') -polkit_agent_dep = dependency('polkit-agent-1', version: '>= 0.97', required : false) -if polkit_agent_dep.found() - config_h.set_quoted('POLKIT_PACKAGE_PREFIX', polkit_agent_dep.get_pkgconfig_variable('prefix')) -else - config_h.set_quoted('POLKIT_PACKAGE_PREFIX', '/usr') +polkit_agent_helper_1_path = get_option('polkit_agent_helper_1_path') +foreach p : [ '/usr/libexec/polkit-agent-helper-1', + '/usr/lib/polkit-1/polkit-agent-helper-1', + '/usr/lib/policykit-1/polkit-agent-helper-1' ] + if polkit_agent_helper_1_path == '' and run_command('test', '-f', p).returncode() == 0 + polkit_agent_helper_1_path = p + endif +endforeach +if polkit_agent_helper_1_path == '' + polkit_agent_helper_1_path = '/usr/lib/polkit-1/polkit-agent-helper-1' endif +config_h.set_quoted('POLKIT_AGENT_HELPER_1_PATH', polkit_agent_helper_1_path) crypto = get_option('crypto') @@ -1016,6 +1022,7 @@ if enable_polkit output += ' modify.system)' endif output += '\n' +output += ' polkit-agent-helper-1: ' + polkit_agent_helper_1_path + '\n' output += ' selinux: ' + enable_selinux.to_string() + '\n' output += ' systemd-journald: ' + enable_systemd_journal.to_string() + ' (default: logging.backend=' + config_logging_backend_default + ')\n' output += ' hostname persist: ' + hostname_persist + '\n' diff --git a/meson_options.txt b/meson_options.txt index d4f62829f..71de3206e 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -16,7 +16,7 @@ option('suspend_resume', type: 'combo', choices: ['upower', 'systemd', 'elogind' option('polkit', type: 'boolean', value: true, description: 'User auth-polkit configuration option.') option('config_auth_polkit_default', type: 'combo', choices: ['default', 'true', 'false', 'root-only'], value: 'default', description: 'Default value for configuration main.auth-polkit.') option('modify_system', type: 'boolean', value: false, description: 'Allow users to modify system connections') -option('polkit_agent', type: 'boolean', value: false, description: 'enable polkit agent for clients') +option('polkit_agent_helper_1_path', type: 'string', value: '', description: 'Path name to the polkit-agent-helper-1 binary from polkit') option('selinux', type: 'boolean', value: true, description: 'Build with SELinux') option('systemd_journal', type: 'boolean', value: true, description: 'Use systemd journal for logging') option('config_logging_backend_default', type: 'combo', choices: ['default', 'syslog', 'journal'], value: 'default', description: 'Default value for logging.backend') From 4d66d6c7a195b9d57613d5f47741b5e470b3f2b2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2021 13:06:13 +0100 Subject: [PATCH 04/29] Revert "service: don't give CAP_DAC_OVERRIDE capability to NetworkManager" Well, that was short. Seems we need CAP_DAC_OVERRIDE at least for the OVS plugin. The OVS socket is srwxr-x---. 1 openvswitch openvswitch 0 Xxx xx xx:xx /run/openvswitch/db.sock and without CAP_DAC_OVERRIDE, NetworkManager cannot talk to OVS. We should fix that differently by adding a nm-sudo D-Bus service that can hand a file descriptor to NetworkManager. This reverts commit 2e334f54b27f91f40c3aa8bdba3254e2284d30bd. --- data/NetworkManager.service.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/data/NetworkManager.service.in b/data/NetworkManager.service.in index 382cdee82..6aaaed78b 100644 --- a/data/NetworkManager.service.in +++ b/data/NetworkManager.service.in @@ -14,7 +14,9 @@ ExecStart=@sbindir@/NetworkManager --no-daemon Restart=on-failure # NM doesn't want systemd to kill its children for it KillMode=process -CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETGID CAP_SETUID CAP_SYS_MODULE CAP_AUDIT_WRITE CAP_KILL CAP_SYS_CHROOT + +# CAP_DAC_OVERRIDE: required to open /run/openvswitch/db.sock socket. +CapabilityBoundingSet=CAP_NET_ADMIN CAP_DAC_OVERRIDE CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETGID CAP_SETUID CAP_SYS_MODULE CAP_AUDIT_WRITE CAP_KILL CAP_SYS_CHROOT ProtectSystem=true ProtectHome=read-only From e23bafe5d5d1afc187a1466f6acb111db3804c60 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2021 11:39:31 +0100 Subject: [PATCH 05/29] contrib/rpm: revert building "--with test" for RHEL 9 "--with test" does two things: (1) it enables "-Werror" compiler option. We always enable all compiler warnings we care about, but this option makes all warnings fatal. Compiler warnings depend on compiler version and build options. It's hard to build without any compiler warnings, in particular for *future* compiler versions which we don't know yet. It is desirable that a SRPM from yesterday can also be build tomorrow. (2) it fails build if any unit tests fail. We always run all unit tests, but "--with test" makes it fatal. Again, we have many unit tests that interact with the system (that is, make system calls, like creating IP addresses or write files). It is surprisingly hard to get them pass 100% on all the systems we care. For example, on copr a test setup randomly fails during ifr.ifr_flags = IFF_TAP | IFF_NO_PI; nm_utils_ifname_cpy(ifr.ifr_name, TEST_IFNAME); r = ioctl(fd, TUNSETIFF, &ifr); It's not clear why, nor is it at all clear that there is a bug in NetworkManager. Making tests fatal basically means that a build on copr infrastructure fails with a probability from a few percent. Enough to be seriously annoying. Note that on copr we actually build "--with test", because we want to catch these issues. Likewise for our CI builds we explicitly specify "--with test". In general, we build with various build configurations (compiler warnings) and run unit tests on a source package many times. Starting on the developer machine (`make check`), gitlab-ci, copr builds, NetworkManager-ci. If you build an SRPM with such sources, a failure of the unit tests is much more likely a glitch than an actual issue. This is about changing the default if you build a Fedora/RHEL package. That is with the Fedora/RHEL packages that are build in koji/brew. Well, at least usually. In practice, we don't build frequently on non x64_86 archs, so what I said there is less true. But the package build is not there to replace CI/testing. The package build is there to get a (mostly) working binary. Note that RHEL packages anyway go through rpmdiff too, and rpmdiff parses the build log and complain if `make check` fails. This reverts commit e68e5c0a4c36ab6fe7cf4793f77ca741179690ce. --- contrib/fedora/rpm/NetworkManager.spec | 4 ---- 1 file changed, 4 deletions(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index f9bf4802c..15fd16def 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -55,12 +55,8 @@ %if "x__BCOND_DEFAULT_TEST__" == "x1" || "x__BCOND_DEFAULT_TEST__" == "x0" %global bcond_default_test __BCOND_DEFAULT_TEST__ %else -%if 0%{?rhel} >= 9 -%global bcond_default_test 1 -%else %global bcond_default_test 0 %endif -%endif %bcond_with meson %bcond_without adsl From 5ccb8ce17a161fa7e926a1644a4c1aadbf74c002 Mon Sep 17 00:00:00 2001 From: Andrew Zaborowski Date: Fri, 12 Feb 2021 11:04:26 +0100 Subject: [PATCH 06/29] iwd: Fix the leaks in get_agent_request_network_path Don't request new copies of strings from g_variant_get() to avoid leaking memory as pointed out by Thomas Haller. Fixes: dc0e31fb7014 ('iwd: Add the wifi.iwd.autoconnect setting') --- src/core/devices/wifi/nm-device-iwd.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/core/devices/wifi/nm-device-iwd.c b/src/core/devices/wifi/nm-device-iwd.c index f0de90d3f..95ade44b5 100644 --- a/src/core/devices/wifi/nm-device-iwd.c +++ b/src/core/devices/wifi/nm-device-iwd.c @@ -1282,15 +1282,13 @@ get_agent_request_network_path(GDBusMethodInvocation *invocation) const char *network_path = NULL; if (nm_streq(method_name, "RequestPassphrase")) - g_variant_get(params, "(o)", &network_path); + g_variant_get(params, "(&o)", &network_path); else if (nm_streq(method_name, "RequestPrivateKeyPassphrase")) - g_variant_get(params, "(o)", &network_path); + g_variant_get(params, "(&o)", &network_path); else if (nm_streq(method_name, "RequestUserNameAndPassword")) - g_variant_get(params, "(o)", &network_path); - else if (nm_streq(method_name, "RequestUserPassword")) { - const char *user; - g_variant_get(params, "(os)", &network_path, &user); - } + g_variant_get(params, "(&o)", &network_path); + else if (nm_streq(method_name, "RequestUserPassword")) + g_variant_get(params, "(&os)", &network_path, NULL); return network_path; } From 6da5fc59d860913a278f488f0873ce68f038de2b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 08:47:16 +0100 Subject: [PATCH 07/29] libnm: log PID in LIBNM_CLIENT_DEBUG debug logging --- libnm/nm-libnm-utils.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libnm/nm-libnm-utils.c b/libnm/nm-libnm-utils.c index 1fb6a47c3..1000e04d1 100644 --- a/libnm/nm-libnm-utils.c +++ b/libnm/nm-libnm-utils.c @@ -46,6 +46,7 @@ _nml_dbus_log(NMLDBusLogLevel level, gboolean use_stdout, const char *fmt, ...) va_list args; const char * prefix = ""; gint64 ts; + pid_t pid; /* we only call _nml_dbus_log() after nml_dbus_log_enabled(), which already does * an atomic access to the variable. Since the value is only initialized once and @@ -89,14 +90,18 @@ _nml_dbus_log(NMLDBusLogLevel level, gboolean use_stdout, const char *fmt, ...) ts = nm_utils_clock_gettime_nsec(CLOCK_BOOTTIME); + pid = getpid(); + if (use_stdout) { - g_print("libnm-dbus: %s[%" G_GINT64_FORMAT ".%05" G_GINT64_FORMAT "] %s\n", + g_print("libnm-dbus[%lld]: %s[%" G_GINT64_FORMAT ".%05" G_GINT64_FORMAT "] %s\n", + (long long) pid, prefix, ts / NM_UTILS_NSEC_PER_SEC, (ts / (NM_UTILS_NSEC_PER_SEC / 10000)) % 10000, msg); } else { - g_printerr("libnm-dbus: %s[%" G_GINT64_FORMAT ".%05" G_GINT64_FORMAT "] %s\n", + g_printerr("libnm-dbus[%lld]: %s[%" G_GINT64_FORMAT ".%05" G_GINT64_FORMAT "] %s\n", + (long long) pid, prefix, ts / NM_UTILS_NSEC_PER_SEC, (ts / (NM_UTILS_NSEC_PER_SEC / 10000)) % 10000, From 1f9622358a691aa97b72b3df118feac7970769c4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 09:11:42 +0100 Subject: [PATCH 08/29] libnm: avoid assertion failure in _dbus_handle_properties_changed() for logging no properties --- libnm/nm-client.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libnm/nm-client.c b/libnm/nm-client.c index 2c7751965..ec79b40ef 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -2930,11 +2930,15 @@ _dbus_handle_properties_changed(NMClient * self, gs_free char *ss = NULL; NML_NMCLIENT_LOG_T(self, - "[%s]: %s: properties changed for interface %s { %s }", + "[%s]: %s: properties changed for interface %s %s%s%s", object_path, log_context, interface_name, - (ss = g_variant_print(changed_properties, TRUE))); + NM_PRINT_FMT_QUOTED(changed_properties, + "{ ", + (ss = g_variant_print(changed_properties, TRUE)), + " }", + "(no changed properties)")); } if (inout_dbobj) { From e1e9abdf041b4cc95fb1936b75ced7669f3d7867 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 09:17:07 +0100 Subject: [PATCH 09/29] libnm: fix tracking object state in NMClient cache NMClient has a NMLDBusObject instance for each D-Bus object that it sees. This object can be in different states, like that we already saw it on D-Bus or that it is only referred to by another property. Due to a bug, we would wrongly not update the state and trigger an assertion. Reproduce with python-dbusmock (commit e89e28bf1bc0254a1eb71b71cf68ef7a97d11e5b) by running `pytest -v -s tests/test_networkmanager.py -k test_one_wifi_with_accesspoints`. With LIBNM_CLIENT_DEBUG we get: >>> libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: properties-changed: properties changed for interface org.freedesktop.NetworkManager.Device { {'ActiveConnection': } } libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: properties-changed: set property org.freedesktop.NetworkManager.Device.ActiveConnection libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x01 linked libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x01 consumed >>> libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: set D-Bus object state watched-only libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x02 linked libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager]: changed-type 0x02 linked libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x02 consumed >>> libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: property ActiveConnection references /org/freedesktop/NetworkManager/ActiveConnection/0 but object is not present on D-Bus libnm-dbus[96085]: [6464.06459] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager]: changed-type 0x02 consumed libnm-dbus[96085]: [6464.06460] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: properties-changed: properties changed for interface org.freedesktop.NetworkManager.Device { {'State': } } libnm-dbus[96085]: [6464.06460] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: properties-changed: set property org.freedesktop.NetworkManager.Device.State libnm-dbus[96085]: [6464.06460] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x01 linked libnm-dbus[96085]: [6464.06460] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x01 consumed libnm-dbus[96085]: [6464.06460] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x02 linked libnm-dbus[96085]: [6464.06460] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager]: changed-type 0x02 linked libnm-dbus[96085]: [6464.06461] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x02 consumed libnm-dbus[96085]: [6464.06461] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager]: changed-type 0x02 consumed libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: properties-changed: properties changed for interface org.freedesktop.NetworkManager.Device { {'StateReason': <(uint32 100, uint32 0)>} } libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: properties-changed: set property org.freedesktop.NetworkManager.Device.StateReason libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x01 linked libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x01 consumed libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x02 linked libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager]: changed-type 0x02 linked libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/Devices/mock_WiFi2]: changed-type 0x02 consumed libnm-dbus[96085]: [6464.06462] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager]: changed-type 0x02 consumed >>> libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: properties changed for interface org.freedesktop.NetworkManager.Connection.Active { {'Devices': <[objectpath '/org/freedesktop/NetworkManager/Devices/mock_WiFi2']>, 'Default6': , 'Default': , 'Type': <'802-11-wireless'>, 'Vpn': , 'Connection': , 'Master': , 'SpecificObject': , 'Uuid': <'72757a57-8cb6-4052-a18f-4e2be4ba27d9'>, 'State': , 'Id': <'AP_3'>} } >>> here we lack "set D-Bus object state on-dbus" libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Devices libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Default6 libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Default libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Type libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Vpn libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Connection libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Master libnm-dbus[96085]: [6464.06465] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.SpecificObject libnm-dbus[96085]: [6464.06466] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Uuid libnm-dbus[96085]: [6464.06466] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.State libnm-dbus[96085]: [6464.06466] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: interfaces-added: set property org.freedesktop.NetworkManager.Connection.Active.Id libnm-dbus[96085]: [6464.06466] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: changed-type 0x01 linked libnm-dbus[96085]: [6464.06466] nmclient[c9bf1eaa1f4b6c99]: [/org/freedesktop/NetworkManager/ActiveConnection/0]: changed-type 0x01 consumed Bail out! libnm:ERROR:libnm/nm-client.c:2863:_dbus_handle_obj_changed_dbus: assertion failed: (dbobj->obj_state >= NML_DBUS_OBJ_STATE_ON_DBUS) Backtrace: #3 0x00007f0bd11173bf in g_assertion_message_expr (domain=domain@entry=0x7f0bd1576018 "libnm", file=file@entry=0x7f0bd1576006 "libnm/nm-client.c", line=line@entry=2863, func=func@entry=0x7f0bd157f1b0 <__func__.170> "_dbus_handle_obj_changed_dbus", expr=expr@entry=0x7f0bd157cba0 "dbobj->obj_state >= NML_DBUS_OBJ_STATE_ON_DBUS") at ../glib/gtestutils.c:2963 #4 0x00007f0bd14959dd in _dbus_handle_obj_changed_dbus (self=self@entry=0x5612d4f5a130, log_context=) at libnm/nm-client.c:2863 #5 0x00007f0bd1495c29 in _dbus_handle_changes (self=self@entry=0x5612d4f5a130, log_context=, allow_init_start_check_complete=allow_init_start_check_complete@entry=1) at libnm/nm-client.c:2909 #6 0x00007f0bd1497e56 in _dbus_managed_objects_changed_cb (connection=, sender_name=, arg_object_path=, interface_name=, signal_name=, parameters=0x7f0bb800d720, user_data=0x5612d4f5a130) at libnm/nm-client.c:3172 #7 0x00007f0bd132a8df in emit_signal_instance_in_idle_cb (data=data@entry=0x7f0bb8003700) at ../gio/gdbusconnection.c:3789 #8 0x00007f0bd10f1b5b in g_idle_dispatch (source=source@entry=0x7f0bb8012260, callback=0x7f0bd132a860 , user_data=0x7f0bb8003700) at ../glib/gmain.c:5836 #9 0x00007f0bd10f2a9f in g_main_dispatch (context=0x5612d4f4b630) at ../glib/gmain.c:3325 #10 g_main_context_dispatch (context=0x5612d4f4b630) at ../glib/gmain.c:4043 #11 0x00007f0bd1144a98 in g_main_context_iterate.constprop.0 (context=0x5612d4f4b630, block=block@entry=1, dispatch=dispatch@entry=1, self=) at ../glib/gmain.c:4119 #12 0x00007f0bd10f2163 in g_main_loop_run (loop=0x5612d4f4b720) at ../glib/gmain.c:4317 #13 0x00005612d44b6543 in main (argc=7, argv=0x7fff4414f1d8) at clients/cli/nmcli.c:1036 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=982613 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/662 Fixes: ce0e898fb476 ('libnm: refactor caching of D-Bus objects in NMClient') --- libnm/nm-client.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libnm/nm-client.c b/libnm/nm-client.c index ec79b40ef..92ad5ef2b 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -2950,9 +2950,12 @@ _dbus_handle_properties_changed(NMClient * self, dbobj = _dbobjs_dbobj_get_r(self, dbus_path); } - if (dbobj) + if (dbobj) { + nm_assert(dbobj->obj_state >= NML_DBUS_OBJ_STATE_WATCHED_ONLY); db_iface_data = nml_dbus_object_iface_data_get(dbobj, interface_name, allow_add_iface); - else if (allow_add_iface) { + if (db_iface_data && dbobj->obj_state == NML_DBUS_OBJ_STATE_WATCHED_ONLY) + nml_dbus_object_set_obj_state(dbobj, NML_DBUS_OBJ_STATE_ON_DBUS, self); + } else if (allow_add_iface) { dbobj = _dbobjs_dbobj_create(self, g_steal_pointer(&dbus_path)); nml_dbus_object_set_obj_state(dbobj, NML_DBUS_OBJ_STATE_ON_DBUS, self); db_iface_data = nml_dbus_object_iface_data_get(dbobj, interface_name, TRUE); From 3ceec9c6ac399b849fcabc52cf58e9001a6b8684 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 09:59:53 +0100 Subject: [PATCH 10/29] libnm: add assertion in _dbus_handle_properties_changed() --- libnm/nm-client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libnm/nm-client.c b/libnm/nm-client.c index 92ad5ef2b..f1b259700 100644 --- a/libnm/nm-client.c +++ b/libnm/nm-client.c @@ -2959,6 +2959,7 @@ _dbus_handle_properties_changed(NMClient * self, dbobj = _dbobjs_dbobj_create(self, g_steal_pointer(&dbus_path)); nml_dbus_object_set_obj_state(dbobj, NML_DBUS_OBJ_STATE_ON_DBUS, self); db_iface_data = nml_dbus_object_iface_data_get(dbobj, interface_name, TRUE); + nm_assert(db_iface_data); } NM_SET_OUT(inout_dbobj, dbobj); From ac1c66eb343693cb63b8624b4be6088e1583670e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 10:04:52 +0100 Subject: [PATCH 11/29] shared: refactor nm_assert() for NMRefString - also check consistency of the string. - disable more expensive check unless running with NM_MORE_ASSERTS>10. --- shared/nm-glib-aux/nm-ref-string.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/shared/nm-glib-aux/nm-ref-string.c b/shared/nm-glib-aux/nm-ref-string.c index 902f1c802..1084c47f8 100644 --- a/shared/nm-glib-aux/nm-ref-string.c +++ b/shared/nm-glib-aux/nm-ref-string.c @@ -49,20 +49,28 @@ _ref_string_equal(gconstpointer pa, gconstpointer pb) static void _ASSERT(const RefString *rstr0) { -#if NM_MORE_ASSERTS int r; nm_assert(rstr0); - G_LOCK(gl_lock); - r = g_atomic_int_get(&rstr0->ref_count); + if (NM_MORE_ASSERTS > 0) { + r = g_atomic_int_get(&rstr0->ref_count); + nm_assert(r > 0); + nm_assert(r < G_MAXINT); + } - nm_assert(r > 0); - nm_assert(r < G_MAXINT); + nm_assert(rstr0->r.str == rstr0->str_data); + nm_assert(rstr0->r.str[rstr0->r.len] == '\0'); - nm_assert(rstr0 == g_hash_table_lookup(gl_hash, rstr0)); - G_UNLOCK(gl_lock); -#endif + if (NM_MORE_ASSERTS > 10) { + G_LOCK(gl_lock); + r = g_atomic_int_get(&rstr0->ref_count); + nm_assert(r > 0); + nm_assert(r < G_MAXINT); + + nm_assert(rstr0 == g_hash_table_lookup(gl_hash, rstr0)); + G_UNLOCK(gl_lock); + } } /** From f591aa41c6acb9c890e57c822058b5e47b8aeca6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 13:49:19 +0100 Subject: [PATCH 12/29] tests: add "/bin:/sbin" to "$PATH" for unit tests We call `tc` from iproute2, which commonly is at "/sbin/tc". That might not be in the "$PATH" of a regular user, and consequently we fail to run the test. Work around that by always adding "/bin" and "/sbin" to the $PATH. --- shared/nm-utils/nm-test-utils.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index 6b41c11ee..d51f972ed 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -624,6 +624,23 @@ __nmtst_init(int * argc, g_setenv("G_MESSAGES_DEBUG", "all", TRUE); } + /* "tc" is in /sbin, which might not be in $PATH of a regular user. Unconditionally + * add "/bin" and "/sbin" to $PATH for all tests. */ + { + static char *path_new; + const char * path_old; + + g_assert(!path_new); + + path_old = g_getenv("PATH"); + path_new = g_strjoin("", + path_old ?: "", + (nm_str_is_empty(path_old) ? "" : ":"), + "/bin:/sbin", + NULL); + g_setenv("PATH", path_new, TRUE); + } + /* Delay messages until we setup logging. */ for (i = 0; i < debug_messages->len; i++) __NMTST_LOG(g_message, "%s", g_array_index(debug_messages, const char *, i)); From ecdbb1ab8458b7a373038a2abd8cabd56e664ceb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 20:36:53 +0100 Subject: [PATCH 13/29] platform/tests: skip tests if "unshare(CLONE_NEWNET|CLONE_NEWNS)" fails Inside a podman container (without `--priviledged`) we don't have permissions for "unshare(CLONE_NEWNET|CLONE_NEWNS)". It's not useful to fail tests in environments where they cannot run. Skip them. --- src/core/platform/tests/test-common.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/core/platform/tests/test-common.c b/src/core/platform/tests/test-common.c index 87e5329a9..4a117d59d 100644 --- a/src/core/platform/tests/test-common.c +++ b/src/core/platform/tests/test-common.c @@ -2563,7 +2563,20 @@ main(int argc, char **argv) if (unshare(CLONE_NEWNET | CLONE_NEWNS) != 0) { errsv = errno; - g_error("unshare(CLONE_NEWNET|CLONE_NEWNS) failed with %s (%d)", + if (errsv == EPERM) { +#ifdef REQUIRE_ROOT_TESTS + g_print("Fail test: unshare(CLONE_NEWNET|CLONE_NEWNS) failed with %s (%d)\n", + nm_strerror_native(errsv), + errsv); + return EXIT_FAILURE; +#else + g_print("Skipping test: unshare(CLONE_NEWNET|CLONE_NEWNS) failed with %s (%d)\n", + nm_strerror_native(errsv), + errsv); + return g_test_run(); +#endif + } + g_error("Fail test: unshare(CLONE_NEWNET|CLONE_NEWNS) failed with %s (%d)", nm_strerror_native(errsv), errsv); } From f9636080ace7d7834083dcb1a3300e08368ee4eb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 20:45:21 +0100 Subject: [PATCH 14/29] platform: reorder code in _netns_stack_get_impl() We should always register the GArray stack with pthread for cleanup the thread local storage. Do that first, before creating the NMPNetns instance at the bottom of the stack. --- shared/nm-platform/nmp-netns.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/shared/nm-platform/nmp-netns.c b/shared/nm-platform/nmp-netns.c index c7cb617b7..0828f4abd 100644 --- a/shared/nm-platform/nmp-netns.c +++ b/shared/nm-platform/nmp-netns.c @@ -158,9 +158,18 @@ _netns_stack_get_impl(void) g_array_set_clear_func(s, _netns_stack_clear_cb); _netns_stack = s; + /* register a destructor function to cleanup the array. If we fail + * to do so, we will leak NMPNetns instances (and their file descriptor) when the + * thread exits. */ + if (pthread_key_create(&key, (void (*)(void *)) g_array_unref) != 0) + _LOGE(NULL, "failure to initialize thread-local storage"); + else if (pthread_setspecific(key, s) != 0) + _LOGE(NULL, "failure to set thread-local storage"); + /* at the bottom of the stack we must try to create a netns instance * that we never pop. It's the base to which we need to return. */ netns = _netns_new(&error); + if (!netns) { _LOGE(NULL, "failed to create initial netns: %s", error->message); return s; @@ -169,14 +178,6 @@ _netns_stack_get_impl(void) /* we leak this instance inside the stack. */ _stack_push(s, netns, _CLONE_NS_ALL); - /* finally, register a destructor function to cleanup the array. If we fail - * to do so, we will leak NMPNetns instances (and their file descriptor) when the - * thread exits. */ - if (pthread_key_create(&key, (void (*)(void *)) g_array_unref) != 0) - _LOGE(NULL, "failure to initialize thread-local storage"); - else if (pthread_setspecific(key, s) != 0) - _LOGE(NULL, "failure to set thread-local storage"); - return s; } From 0213300dcee438a0b48a3d3a6da3112286ad43d1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 15 Feb 2021 20:52:55 +0100 Subject: [PATCH 15/29] platform: downgrade severity of message for failure to create NMPNetns Under restricted permissions (like inside a podman container) opening "/proc/self/ns/net" fails with Permission denied. Consequently we cannot create our bottom NMPNetns instance. That is mostly fine, however we would log an error message with severity . Note that test "src/core/platform/tests/test-platform-general" asserts that no and messages get logged. Hence, the test will fail. That is undesirable. Downgrade the message to so that the test passes. Also, it's not clear that this error message is useful here. Being unable to open a netns fd is fine and not necessarily an error condition. --- shared/nm-platform/nmp-netns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/nm-platform/nmp-netns.c b/shared/nm-platform/nmp-netns.c index 0828f4abd..f97339a75 100644 --- a/shared/nm-platform/nmp-netns.c +++ b/shared/nm-platform/nmp-netns.c @@ -171,7 +171,7 @@ _netns_stack_get_impl(void) netns = _netns_new(&error); if (!netns) { - _LOGE(NULL, "failed to create initial netns: %s", error->message); + _LOGD(NULL, "failed to create initial netns: %s", error->message); return s; } From 8c04f72e36e7803a3ca114cca57fc420e9f81719 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Feb 2021 11:09:32 +0100 Subject: [PATCH 16/29] contrib/rpm: update URL for NetworkManager in RPM package The previous URL http://www.gnome.org/projects/NetworkManager/ now redirects to https://wiki.gnome.org/Apps, which isn't very useful. Instead, link to our NetworkManager page. The page is still sparsely populated, but we should improve that. --- contrib/fedora/rpm/NetworkManager.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 15fd16def..ce6cb4139 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -176,7 +176,7 @@ Version: %{rpm_version} Release: %{release_version}%{?snap}%{?dist} Group: System Environment/Base License: GPLv2+ and LGPLv2+ -URL: http://www.gnome.org/projects/NetworkManager/ +URL: https://networkmanager.dev/ #Source: https://download.gnome.org/sources/NetworkManager/%{real_version_major}/%{name}-%{real_version}.tar.xz Source: __SOURCE1__ From b3909c36f693d32591cdbaaa0b03b5846599ff56 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Feb 2021 11:09:32 +0100 Subject: [PATCH 17/29] policy: update vendor-URL for NetworkManager in polkit policy The previous URL http://www.gnome.org/projects/NetworkManager/ now redirects to https://wiki.gnome.org/Apps, which isn't very useful. Instead, link to our NetworkManager page. The page is still sparsely populated, but we should improve that. --- data/org.freedesktop.NetworkManager.policy.in.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/org.freedesktop.NetworkManager.policy.in.in b/data/org.freedesktop.NetworkManager.policy.in.in index 8b6ea5155..8d46dac73 100644 --- a/data/org.freedesktop.NetworkManager.policy.in.in +++ b/data/org.freedesktop.NetworkManager.policy.in.in @@ -6,7 +6,7 @@ NetworkManager - http://www.gnome.org/projects/NetworkManager + https://networkmanager.dev/ nm-icon From 7bf2ddf73f8d8c2d882c4932e88b76eef337cd0a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Feb 2021 15:45:44 +0100 Subject: [PATCH 18/29] platform: ensure NM_SOCK_ADDR_UNION_INIT_UNSPEC() fully initializes union In C, initialization of a union does not define that excess memory is initialized. Ensure that, by initializing the largest member of the NMSockAddrUnion union. --- src/core/platform/nmp-object.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/platform/nmp-object.h b/src/core/platform/nmp-object.h index dc2cc86b6..19f6bcd7b 100644 --- a/src/core/platform/nmp-object.h +++ b/src/core/platform/nmp-object.h @@ -34,10 +34,14 @@ typedef union { struct sockaddr_in6 in6; } NMSockAddrUnion; +G_STATIC_ASSERT(sizeof(NMSockAddrUnion) == sizeof(((NMSockAddrUnion *) NULL)->in6)); + +/* we initialize the largest union member, to ensure that all fields are initialized. */ + #define NM_SOCK_ADDR_UNION_INIT_UNSPEC \ { \ - .sa = { \ - .sa_family = AF_UNSPEC, \ + .in6 = { \ + .sin6_family = AF_UNSPEC, \ }, \ } From 98348ee5396dde5756fbb82ebf16b90790b6b32d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 13 Feb 2021 15:47:31 +0100 Subject: [PATCH 19/29] wireguard: prefer last resolved IP from resolving endpoint from DNS We periodically re-resolve the DNS name for entpoints. Since WireGuard has no concept of being connected, we want to eventually pick up if the DNS name resolves to a different IP address. However, on resolution failure, we will never clear the endpoint we already have. Thus, resolving names can only give a better endpoint, not remove an IP address entirely. DNS names might do Round-Robin load distribution and the name of the endpoint might resolve to multiple IP addresses. Improve to stick to the IP address that we already have -- provided that the IP address is still among the new resolution result. Otherwise, we continue to pick the first IP address that was resolved. --- src/core/devices/nm-device-wireguard.c | 42 ++++++++++++++++---------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/core/devices/nm-device-wireguard.c b/src/core/devices/nm-device-wireguard.c index fd057ded9..5bee09e6f 100644 --- a/src/core/devices/nm-device-wireguard.c +++ b/src/core/devices/nm-device-wireguard.c @@ -729,7 +729,7 @@ _peers_resolve_cb(GObject *source_object, GAsyncResult *res, gpointer user_data) PeerData * peer_data; gs_free_error GError *resolv_error = NULL; GList * list; - gboolean changed = FALSE; + gboolean changed; NMSockAddrUnion sockaddr; gint64 retry_in_msec; char s_sockaddr[100]; @@ -775,36 +775,49 @@ _peers_resolve_cb(GObject *source_object, GAsyncResult *res, gpointer user_data) } sockaddr = (NMSockAddrUnion) NM_SOCK_ADDR_UNION_INIT_UNSPEC; + changed = FALSE; if (!resolv_error) { GList *iter; for (iter = list; iter; iter = iter->next) { - GInetAddress *a = iter->data; - GSocketFamily f = g_inet_address_get_family(a); + GInetAddress * a = iter->data; + NMSockAddrUnion sockaddr_tmp; + NMSockAddrUnion *s; - if (f == G_SOCKET_FAMILY_IPV4) { + s = sockaddr.sa.sa_family == AF_UNSPEC ? &sockaddr : &sockaddr_tmp; + + switch (g_inet_address_get_family(a)) { + case G_SOCKET_FAMILY_IPV4: nm_assert(g_inet_address_get_native_size(a) == sizeof(struct in_addr)); - sockaddr.in = (struct sockaddr_in){ + s->in = (struct sockaddr_in){ .sin_family = AF_INET, .sin_port = htons(nm_sock_addr_endpoint_get_port( _nm_wireguard_peer_get_endpoint(peer_data->peer))), }; - memcpy(&sockaddr.in.sin_addr, g_inet_address_to_bytes(a), sizeof(struct in_addr)); + memcpy(&s->in.sin_addr, g_inet_address_to_bytes(a), sizeof(struct in_addr)); break; - } - if (f == G_SOCKET_FAMILY_IPV6) { + case G_SOCKET_FAMILY_IPV6: nm_assert(g_inet_address_get_native_size(a) == sizeof(struct in6_addr)); - sockaddr.in6 = (struct sockaddr_in6){ + s->in6 = (struct sockaddr_in6){ .sin6_family = AF_INET6, .sin6_port = htons(nm_sock_addr_endpoint_get_port( _nm_wireguard_peer_get_endpoint(peer_data->peer))), .sin6_scope_id = 0, .sin6_flowinfo = 0, }; - memcpy(&sockaddr.in6.sin6_addr, - g_inet_address_to_bytes(a), - sizeof(struct in6_addr)); + memcpy(&s->in6.sin6_addr, g_inet_address_to_bytes(a), sizeof(struct in6_addr)); + break; + default: + continue; + } + + changed = TRUE; + if (peer_data->ep_resolv.sockaddr.sa.sa_family == AF_UNSPEC) + break; + + if (nm_sock_addr_union_cmp(&peer_data->ep_resolv.sockaddr, &sockaddr) == 0) { + changed = FALSE; break; } } @@ -819,11 +832,8 @@ _peers_resolve_cb(GObject *source_object, GAsyncResult *res, gpointer user_data) * a possibly good IP address, since WireGuard supports automatic roaming * anyway. Either the IP address is still good (and we would wrongly * reject it), or it isn't -- in which case it does not hurt much. */ - } else { - if (nm_sock_addr_union_cmp(&peer_data->ep_resolv.sockaddr, &sockaddr) != 0) - changed = TRUE; + } else if (changed) peer_data->ep_resolv.sockaddr = sockaddr; - } if (resolv_error || peer_data->ep_resolv.sockaddr.sa.sa_family == AF_UNSPEC) { /* while it technically did not fail, something is probably odd. Retry frequently to From d9968b133b32fbfbc5e726a8fc96e38cc044c831 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Feb 2021 21:33:30 +0100 Subject: [PATCH 20/29] build: rename build option "--with-polkit-agent-helper-1{-path,}" Suggested-by: Michael Biebl --- configure.ac | 9 ++++++--- meson.build | 5 ++++- meson_options.txt | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 2bf5e4e06..39aeea557 100644 --- a/configure.ac +++ b/configure.ac @@ -642,9 +642,9 @@ fi AC_DEFINE_UNQUOTED(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT, "$enable_polkit", [The default value of the auth-polkit configuration option]) AC_SUBST(NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT_TEXT, "$enable_polkit") -AC_ARG_WITH([polkit-agent-helper-1-path], - AS_HELP_STRING([--with-polkit-agent-helper-1-path=PATH], - [Path name to the polkit-agent-helper-1 binary from polkit]), +AC_ARG_WITH([polkit-agent-helper-1], + AS_HELP_STRING([--with-polkit-agent-helper-1=/path/to/polkit-agent-helper-1], + [Path to the polkit-agent-helper-1 binary from polkit]), POLKIT_AGENT_HELPER_1_PATH="$withval", POLKIT_AGENT_HELPER_1_PATH="") if test -z "$POLKIT_AGENT_HELPER_1_PATH" ; then @@ -658,6 +658,9 @@ if test -z "$POLKIT_AGENT_HELPER_1_PATH" ; then done fi test -z "$POLKIT_AGENT_HELPER_1_PATH" && POLKIT_AGENT_HELPER_1_PATH=/usr/lib/polkit-1/polkit-agent-helper-1 +if test "$POLKIT_AGENT_HELPER_1_PATH" = "${POLKIT_AGENT_HELPER_1_PATH#/}" ; then + AC_MSG_ERROR(["polkit_agent_helper_1 must be an absolute path, but is '$POLKIT_AGENT_HELPER_1_PATH'"]) +fi AC_DEFINE_UNQUOTED([POLKIT_AGENT_HELPER_1_PATH], ["$POLKIT_AGENT_HELPER_1_PATH"], [path to polkit-agent-helper-1 binary]) diff --git a/meson.build b/meson.build index 9d3d08c4f..44b4e2e4b 100644 --- a/meson.build +++ b/meson.build @@ -514,7 +514,7 @@ config_h.set_quoted('NM_CONFIG_DEFAULT_MAIN_AUTH_POLKIT', config_auth_polkit_def enable_modify_system = get_option('modify_system') -polkit_agent_helper_1_path = get_option('polkit_agent_helper_1_path') +polkit_agent_helper_1_path = get_option('polkit_agent_helper_1') foreach p : [ '/usr/libexec/polkit-agent-helper-1', '/usr/lib/polkit-1/polkit-agent-helper-1', '/usr/lib/policykit-1/polkit-agent-helper-1' ] @@ -525,6 +525,9 @@ endforeach if polkit_agent_helper_1_path == '' polkit_agent_helper_1_path = '/usr/lib/polkit-1/polkit-agent-helper-1' endif +if polkit_agent_helper_1_path[0] != '/' + error('polkit_agent_helper_1 must be an absolute path, but is ' + polkit_agent_helper_1_path) +endif config_h.set_quoted('POLKIT_AGENT_HELPER_1_PATH', polkit_agent_helper_1_path) diff --git a/meson_options.txt b/meson_options.txt index 71de3206e..5100ed71f 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -16,7 +16,7 @@ option('suspend_resume', type: 'combo', choices: ['upower', 'systemd', 'elogind' option('polkit', type: 'boolean', value: true, description: 'User auth-polkit configuration option.') option('config_auth_polkit_default', type: 'combo', choices: ['default', 'true', 'false', 'root-only'], value: 'default', description: 'Default value for configuration main.auth-polkit.') option('modify_system', type: 'boolean', value: false, description: 'Allow users to modify system connections') -option('polkit_agent_helper_1_path', type: 'string', value: '', description: 'Path name to the polkit-agent-helper-1 binary from polkit') +option('polkit_agent_helper_1', type: 'string', value: '', description: 'Path name to the polkit-agent-helper-1 binary from polkit') option('selinux', type: 'boolean', value: true, description: 'Build with SELinux') option('systemd_journal', type: 'boolean', value: true, description: 'Use systemd journal for logging') option('config_logging_backend_default', type: 'combo', choices: ['default', 'syslog', 'journal'], value: 'default', description: 'Default value for logging.backend') From e258410c877ee081a19f38653d20d884d3923d04 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 16 Feb 2021 21:23:37 +0100 Subject: [PATCH 21/29] initrd: cleanup parsing DNS in reader_parse_ip() --- src/core/initrd/nmi-cmdline-reader.c | 37 +++++++++++----------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/core/initrd/nmi-cmdline-reader.c b/src/core/initrd/nmi-cmdline-reader.c index d95b0f9be..c83473d14 100644 --- a/src/core/initrd/nmi-cmdline-reader.c +++ b/src/core/initrd/nmi-cmdline-reader.c @@ -408,10 +408,12 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) int client_ip_family = AF_UNSPEC; int client_ip_prefix = -1; const char * dns[2] = { - 0, + NULL, + NULL, }; int dns_addr_family[2] = { - 0, + AF_UNSPEC, + AF_UNSPEC, }; int i; GError *error = NULL; @@ -469,11 +471,15 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) tmp = get_word(&argument, ':'); dns_addr_family[0] = get_ip_address_family(tmp, FALSE); if (dns_addr_family[0] != AF_UNSPEC) { - dns[0] = tmp; - dns[1] = get_word(&argument, ':'); - dns_addr_family[1] = get_ip_address_family(dns[1], FALSE); - if (*argument) - _LOGW(LOGD_CORE, "Ignoring extra: '%s'.", argument); + dns[0] = tmp; + dns[1] = get_word(&argument, ':'); + if (dns[1]) { + dns_addr_family[1] = get_ip_address_family(dns[1], FALSE); + if (dns_addr_family[1] == AF_UNSPEC) + _LOGW(LOGD_CORE, "Ignoring invalid DNS server: '%s'.", dns[1]); + if (*argument) + _LOGW(LOGD_CORE, "Ignoring extra: '%s'.", argument); + } } else { mtu = tmp; macaddr = argument; @@ -683,21 +689,8 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) for (i = 0; i < 2; i++) { if (dns_addr_family[i] == AF_UNSPEC) break; - if (nm_utils_ipaddr_is_valid(dns_addr_family[i], dns[i])) { - switch (dns_addr_family[i]) { - case AF_INET: - nm_setting_ip_config_add_dns(s_ip4, dns[i]); - break; - case AF_INET6: - nm_setting_ip_config_add_dns(s_ip6, dns[i]); - break; - default: - _LOGW(LOGD_CORE, "Unknown address family: %s", dns[i]); - break; - } - } else { - _LOGW(LOGD_CORE, "Invalid name server: %s", dns[i]); - } + nm_assert(nm_utils_ipaddr_is_valid(dns_addr_family[i], dns[i])); + nm_setting_ip_config_add_dns(NM_IS_IPv4(dns_addr_family[i]) ? s_ip4 : s_ip6, dns[i]); } if (mtu && *mtu) From 7e126fe898f130f53f3e5cb2f87eca2169978b4d Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 12 Feb 2021 09:50:48 +0100 Subject: [PATCH 22/29] initrd: set autoconnect-retries=1 and increase default DHCP timeout By default a connection is retried 4 times before it is blocked from autoconnecting. This means that if a user specifies an explicit DHCP timeout in the initrd command line, NM will wait up to 4 times more. Instead, set the "connection.autoconnect-retries" property of connections always to 1, so that NM only waits for the time specified. Before this commit a default DHCP connection would take at most (45 x 4) seconds. Since the multiplier is now only 1, also increase the DHCP timeout to have a total time of (90 x 1) seconds, which is the half than before. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/559 --- src/core/initrd/nmi-cmdline-reader.c | 6 ++- src/core/initrd/tests/test-cmdline-reader.c | 59 +++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/core/initrd/nmi-cmdline-reader.c b/src/core/initrd/nmi-cmdline-reader.c index c83473d14..4fa97cc21 100644 --- a/src/core/initrd/nmi-cmdline-reader.c +++ b/src/core/initrd/nmi-cmdline-reader.c @@ -53,6 +53,7 @@ reader_new(void) g_hash_table_new_full(nm_direct_hash, NULL, g_object_unref, NULL), .vlan_parents = g_ptr_array_new_with_free_func(g_free), .array = g_ptr_array_new(), + .dhcp_timeout = 90, }; return reader; @@ -147,6 +148,8 @@ reader_create_connection(Reader * reader, type_name, NM_SETTING_CONNECTION_MULTI_CONNECT, multi_connect, + NM_SETTING_CONNECTION_AUTOCONNECT_RETRIES, + 1, NULL); if (nm_streq0(type_name, NM_SETTING_INFINIBAND_SETTING_NAME)) { @@ -1075,7 +1078,8 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, else if (nm_streq(tag, "rd.peerdns")) reader->ignore_auto_dns = !_nm_utils_ascii_str_to_bool(argument, TRUE); else if (nm_streq(tag, "rd.net.timeout.dhcp")) { - reader->dhcp_timeout = _nm_utils_ascii_str_to_int64(argument, 10, 0, G_MAXINT32, 0); + reader->dhcp_timeout = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); } else if (nm_streq(tag, "rd.net.dhcp.vendor-class")) { if (nm_utils_validate_dhcp4_vendor_class_id(argument, NULL)) nm_utils_strdup_reset(&reader->dhcp4_vci, argument); diff --git a/src/core/initrd/tests/test-cmdline-reader.c b/src/core/initrd/tests/test-cmdline-reader.c index cb65cd33b..6f3dd1a50 100644 --- a/src/core/initrd/tests/test-cmdline-reader.c +++ b/src/core/initrd/tests/test-cmdline-reader.c @@ -229,11 +229,64 @@ test_dhcp_with_mtu(void) } } +static void +test_dhcp_timeout(void) +{ + struct { + const char *const *cmdline; + int timeout; + } data[] = { + {NM_MAKE_STRV("ip=dhcp"), 90}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=0"), 90}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=foobar"), 90}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=42"), 42}, + }; + guint i; + + for (i = 0; i < G_N_ELEMENTS(data); i++) { + gs_unref_object NMConnection *connection = NULL; + NMSettingConnection * s_con; + NMSettingIPConfig * s_ip4; + NMSettingIPConfig * s_ip6; + + connection = _parse_con(data[i].cmdline, "default_connection"); + + s_con = nm_connection_get_setting_connection(connection); + g_assert(s_con); + g_assert_cmpstr(nm_setting_connection_get_connection_type(s_con), + ==, + NM_SETTING_WIRED_SETTING_NAME); + g_assert_cmpstr(nm_setting_connection_get_id(s_con), ==, "Wired Connection"); + g_assert_cmpint(nm_setting_connection_get_timestamp(s_con), ==, 0); + g_assert_cmpint(nm_setting_connection_get_multi_connect(s_con), + ==, + NM_CONNECTION_MULTI_CONNECT_MULTIPLE); + g_assert_cmpint(nm_setting_connection_get_wait_device_timeout(s_con), ==, -1); + g_assert_cmpint(nm_setting_connection_get_autoconnect_retries(s_con), ==, 1); + g_assert(nm_setting_connection_get_autoconnect(s_con)); + + s_ip4 = nm_connection_get_setting_ip4_config(connection); + g_assert(s_ip4); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), + ==, + NM_SETTING_IP4_CONFIG_METHOD_AUTO); + g_assert_cmpint(nm_setting_ip_config_get_dhcp_timeout(s_ip4), ==, data[i].timeout); + + s_ip6 = nm_connection_get_setting_ip6_config(connection); + g_assert(s_ip6); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip6), + ==, + NM_SETTING_IP6_CONFIG_METHOD_AUTO); + g_assert_cmpint(nm_setting_ip_config_get_dhcp_timeout(s_ip6), ==, data[i].timeout); + } +} + static void test_if_auto_with_mtu(void) { const char *const *ARGV = NM_MAKE_STRV("ip=eth0:auto:1666"); gs_unref_object NMConnection *connection = NULL; + NMSettingConnection * s_con; NMSettingWired * s_wired; NMSettingIPConfig * s_ip4; NMSettingIPConfig * s_ip6; @@ -242,6 +295,10 @@ test_if_auto_with_mtu(void) g_assert_cmpstr(nm_connection_get_id(connection), ==, "eth0"); + s_con = nm_connection_get_setting_connection(connection); + g_assert(s_con); + g_assert_cmpint(nm_setting_connection_get_autoconnect_retries(s_con), ==, 1); + s_wired = nm_connection_get_setting_wired(connection); g_assert(s_wired); g_assert_cmpint(nm_setting_wired_get_mtu(s_wired), ==, 1666); @@ -250,6 +307,7 @@ test_if_auto_with_mtu(void) g_assert(s_ip4); g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_AUTO); g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip4)); + g_assert_cmpint(nm_setting_ip_config_get_dhcp_timeout(s_ip4), ==, 90); s_ip6 = nm_connection_get_setting_ip6_config(connection); g_assert(s_ip6); @@ -2074,6 +2132,7 @@ main(int argc, char **argv) g_test_add_func("/initrd/cmdline/auto", test_auto); g_test_add_func("/initrd/cmdline/dhcp_with_hostname", test_dhcp_with_hostname); g_test_add_func("/initrd/cmdline/dhcp_with_mtu", test_dhcp_with_mtu); + g_test_add_func("/initrd/cmdline/dhcp_timeout", test_dhcp_timeout); g_test_add_func("/initrd/cmdline/if_auto_with_mtu", test_if_auto_with_mtu); g_test_add_func("/initrd/cmdline/if_dhcp6", test_if_dhcp6); g_test_add_func("/initrd/cmdline/if_auto_with_mtu_and_mac", test_if_auto_with_mtu_and_mac); From 97833237bf38347c75022eb380208d99e1df9d5f Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 12 Feb 2021 09:53:30 +0100 Subject: [PATCH 23/29] initrd: accept 'infinity' as argument to rd.net.timeout.dhcp --- src/core/initrd/nmi-cmdline-reader.c | 8 ++++++-- src/core/initrd/tests/test-cmdline-reader.c | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/initrd/nmi-cmdline-reader.c b/src/core/initrd/nmi-cmdline-reader.c index 4fa97cc21..8d1c27890 100644 --- a/src/core/initrd/nmi-cmdline-reader.c +++ b/src/core/initrd/nmi-cmdline-reader.c @@ -1078,8 +1078,12 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, else if (nm_streq(tag, "rd.peerdns")) reader->ignore_auto_dns = !_nm_utils_ascii_str_to_bool(argument, TRUE); else if (nm_streq(tag, "rd.net.timeout.dhcp")) { - reader->dhcp_timeout = - _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); + if (nm_streq0(argument, "infinity")) { + reader->dhcp_timeout = G_MAXINT32; + } else { + reader->dhcp_timeout = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); + } } else if (nm_streq(tag, "rd.net.dhcp.vendor-class")) { if (nm_utils_validate_dhcp4_vendor_class_id(argument, NULL)) nm_utils_strdup_reset(&reader->dhcp4_vci, argument); diff --git a/src/core/initrd/tests/test-cmdline-reader.c b/src/core/initrd/tests/test-cmdline-reader.c index 6f3dd1a50..6795e32d3 100644 --- a/src/core/initrd/tests/test-cmdline-reader.c +++ b/src/core/initrd/tests/test-cmdline-reader.c @@ -240,6 +240,7 @@ test_dhcp_timeout(void) {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=0"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=foobar"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=42"), 42}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=infinity"), G_MAXINT32}, }; guint i; From 099ce63888013e82c0f369b02a8f27a0b31813a6 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 12 Feb 2021 09:56:36 +0100 Subject: [PATCH 24/29] initrd: support the rd.net.dhcp.retry argument Since we always set autoconnect-retries=1, use the value of rd.net.dhcp.retry as a multiplier for the DHCP timeout. --- src/core/initrd/nmi-cmdline-reader.c | 14 ++++++++++---- src/core/initrd/tests/test-cmdline-reader.c | 4 ++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/core/initrd/nmi-cmdline-reader.c b/src/core/initrd/nmi-cmdline-reader.c index 8d1c27890..1a95e4f7d 100644 --- a/src/core/initrd/nmi-cmdline-reader.c +++ b/src/core/initrd/nmi-cmdline-reader.c @@ -53,7 +53,6 @@ reader_new(void) g_hash_table_new_full(nm_direct_hash, NULL, g_object_unref, NULL), .vlan_parents = g_ptr_array_new_with_free_func(g_free), .array = g_ptr_array_new(), - .dhcp_timeout = 90, }; return reader; @@ -1061,6 +1060,8 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, gs_unref_ptrarray GPtrArray *routes = NULL; gs_unref_ptrarray GPtrArray *znets = NULL; int i; + guint64 dhcp_timeout = 90; + guint64 dhcp_num_tries = 1; reader = reader_new(); @@ -1079,11 +1080,14 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, reader->ignore_auto_dns = !_nm_utils_ascii_str_to_bool(argument, TRUE); else if (nm_streq(tag, "rd.net.timeout.dhcp")) { if (nm_streq0(argument, "infinity")) { - reader->dhcp_timeout = G_MAXINT32; + dhcp_timeout = G_MAXINT32; } else { - reader->dhcp_timeout = - _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, reader->dhcp_timeout); + dhcp_timeout = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, dhcp_timeout); } + } else if (nm_streq(tag, "rd.net.dhcp.retry")) { + dhcp_num_tries = + _nm_utils_ascii_str_to_int64(argument, 10, 1, G_MAXINT32, dhcp_num_tries); } else if (nm_streq(tag, "rd.net.dhcp.vendor-class")) { if (nm_utils_validate_dhcp4_vendor_class_id(argument, NULL)) nm_utils_strdup_reset(&reader->dhcp4_vci, argument); @@ -1093,6 +1097,8 @@ nmi_cmdline_reader_parse(const char * sysfs_dir, } } + reader->dhcp_timeout = NM_CLAMP(dhcp_timeout * dhcp_num_tries, 1, G_MAXINT32); + for (i = 0; argv[i]; i++) { gs_free char *argument_clone = NULL; char * argument; diff --git a/src/core/initrd/tests/test-cmdline-reader.c b/src/core/initrd/tests/test-cmdline-reader.c index 6795e32d3..33fb22d36 100644 --- a/src/core/initrd/tests/test-cmdline-reader.c +++ b/src/core/initrd/tests/test-cmdline-reader.c @@ -240,7 +240,11 @@ test_dhcp_timeout(void) {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=0"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=foobar"), 90}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=42"), 42}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.dhcp.retry=2"), 180}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.dhcp.retry=3", "rd.net.timeout.dhcp=40"), 120}, {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=infinity"), G_MAXINT32}, + {NM_MAKE_STRV("ip=dhcp", "rd.net.timeout.dhcp=infinity", "rd.net.dhcp.retry=100"), + G_MAXINT32}, }; guint i; From b941686b5ab9536a38c10f14301ae71d21a215d3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Feb 2021 14:24:11 +0100 Subject: [PATCH 25/29] build: dist "libnm/nm-enum-types.[ch].template" files --- Makefile.am | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile.am b/Makefile.am index f2fb9288e..9279672c1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1649,6 +1649,9 @@ DISTCLEANFILES += \ libnm/libnm.pc EXTRA_DIST += \ + libnm/nm-enum-types.c.template \ + libnm/nm-enum-types.h.template \ + \ libnm/libnm.pc.in \ libnm/libnm.ver From 6b784064601d95c128abf2ad0a060c433bd8b648 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Sat, 30 Jan 2021 10:27:57 -0500 Subject: [PATCH 26/29] docs: improve manual page about ipv4.addresses Signed-off-by: Wen Liang https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/738 --- .gitignore | 1 + Makefile.am | 7 +++++- .../generate-docs-nm-settings-nmcli.xml.in | 2 +- clients/common/meson.build | 23 ++++++++++++------- clients/common/settings-docs.h.in | 2 +- libnm-core/nm-setting-ip4-config.c | 7 ++++++ 6 files changed, 31 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 3524f7082..2b49984ac 100644 --- a/.gitignore +++ b/.gitignore @@ -72,6 +72,7 @@ test-*.trs /clients/cloud-setup/nm-cloud-setup /clients/cloud-setup/nm-cloud-setup.service /clients/cloud-setup/tests/test-cloud-setup-general +/clients/common/settings-docs-input.xml /clients/common/settings-docs.h /clients/common/tests/test-clients-common /clients/common/tests/test-libnm-core-aux diff --git a/Makefile.am b/Makefile.am index 9279672c1..9e35298b3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1731,6 +1731,9 @@ libnm/nm-settings-docs-gir.xml: tools/generate-docs-nm-settings-docs-gir.py libn man/nm-settings-docs-nmcli.xml: clients/cli/generate-docs-nm-settings-nmcli.xml libnm/nm-property-infos-nmcli.xml libnm/nm-settings-docs-gir.xml tools/generate-docs-nm-settings-docs-merge.py man/common.ent $(AM_V_GEN) "$(PYTHON)" $(srcdir)/tools/generate-docs-nm-settings-docs-merge.py --only-from-first $@ $(wordlist 1,3,$^) +clients/common/settings-docs-input.xml: libnm/nm-property-infos-nmcli.xml libnm/nm-settings-docs-gir.xml tools/generate-docs-nm-settings-docs-merge.py + $(AM_V_GEN) "$(PYTHON)" $(srcdir)/tools/generate-docs-nm-settings-docs-merge.py $@ $(wordlist 1,2,$^) + man/nm-settings-docs-%.xml: libnm/nm-property-infos-%.xml libnm/nm-settings-docs-gir.xml tools/generate-docs-nm-settings-docs-merge.py man/common.ent $(AM_V_GEN) "$(PYTHON)" $(srcdir)/tools/generate-docs-nm-settings-docs-merge.py $@ $(wordlist 1,2,$^) @@ -4554,7 +4557,7 @@ $(clients_common_libnmc_base_la_OBJECTS): clients/common/.dirstamp clients_common_settings_doc_h = clients/common/settings-docs.h if BUILD_DOCS -$(clients_common_settings_doc_h): clients/common/settings-docs.xsl libnm/nm-settings-docs-gir.xml clients/common/.dirstamp +$(clients_common_settings_doc_h): clients/common/settings-docs.xsl clients/common/settings-docs-input.xml clients/common/.dirstamp $(AM_V_GEN) $(XSLTPROC) --output $@ $< $(word 2,$^) DISTCLEANFILES += $(clients_common_settings_doc_h) check-local-settings-docs: $(clients_common_settings_doc_h) @@ -5475,6 +5478,8 @@ CLEANFILES += \ \ $(NULL) +CLEANFILES += clients/common/settings-docs-input.xml + ############################################################################### include Makefile.examples diff --git a/clients/cli/generate-docs-nm-settings-nmcli.xml.in b/clients/cli/generate-docs-nm-settings-nmcli.xml.in index 1044ae0d3..5bae8ff9e 100644 --- a/clients/cli/generate-docs-nm-settings-nmcli.xml.in +++ b/clients/cli/generate-docs-nm-settings-nmcli.xml.in @@ -642,7 +642,7 @@ description="DNS servers priority. The relative priority for DNS servers specified by this setting. A lower numerical value is better (higher priority). Negative values have the special effect of excluding other configurations with a greater numerical priority value; so in presence of at least one negative priority, only DNS servers from connections with the lowest priority value will be used. To avoid all DNS leaks, set the priority of the profile that should be used to the most negative value of all active connections profiles. Zero selects a globally configured default value. If the latter is missing or zero too, it defaults to 50 for VPNs (including WireGuard) and 100 for other connections. Note that the priority is to order DNS settings for multiple active connections. It does not disambiguate multiple DNS servers within the same connection profile. When multiple devices have configurations with the same priority, VPNs will be considered first, then devices with the best (lowest metric) default route and then all other devices. When using dns=default, servers with higher priority will be on top of resolv.conf. To prioritize a given server over another one within the same connection, just specify them in the desired order. Note that commonly the resolver tries name servers in /etc/resolv.conf in the order listed, proceeding with the next server in the list on failure. See for example the "rotate" option of the dns-options setting. If there are any negative DNS priorities, then only name servers from the devices with that lowest priority will be considered. When using a DNS resolver that supports Conditional Forwarding or Split DNS (with dns=dnsmasq or dns=systemd-resolved settings), each connection is used to query domains in its search list. The search domains determine which name servers to ask, and the DNS priority is used to prioritize name servers based on the domain. Queries for domains not present in any search list are routed through connections having the '~.' special wildcard domain, which is added automatically to connections with the default route (or can be added manually). When multiple connections specify the same domain, the one with the best priority (lowest numerical value) wins. If a sub domain is configured on another interface it will be accepted regardless the priority, unless parent domain on the other interface has a negative priority, which causes the sub domain to be shadowed. With Split DNS one can avoid undesired DNS leaks by properly configuring DNS priorities and the search domains, so that only name servers of the desired interface are configured." /> + description="A list of IPv4 addresses and their prefix length. Multiple addresses can be separated by comma. For example "192.168.1.5/24, 10.1.0.5/24"." /> diff --git a/clients/common/meson.build b/clients/common/meson.build index bf591b539..6b91f36ff 100644 --- a/clients/common/meson.build +++ b/clients/common/meson.build @@ -29,17 +29,24 @@ libnmc_base_dep = declare_dependency( ) if enable_docs + settings_docs_input_xml = custom_target( + 'settings-docs-input.xml', + input: [nm_settings_docs_xml_gir, nm_property_infos_xml['nmcli']], + output: 'settings-docs-input.xml', + command: [ + python.path(), + join_paths(meson.source_root(), 'tools', 'generate-docs-nm-settings-docs-merge.py'), + '@OUTPUT@', + nm_property_infos_xml['nmcli'], + nm_settings_docs_xml_gir, + ], + ) + settings_docs_source = custom_target( 'settings-docs.h', - input: nm_settings_docs_xml_gir, + input: settings_docs_input_xml, output: 'settings-docs.h', - command: [ - xsltproc, - '--output', - '@OUTPUT@', - join_paths(meson.current_source_dir(), 'settings-docs.xsl'), - '@INPUT@', - ], + command: [xsltproc, '--output', '@OUTPUT@', join_paths(meson.current_source_dir(), 'settings-docs.xsl'), '@INPUT@'], ) test( diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index 2c275a99c..9cd2609d2 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -225,7 +225,7 @@ #define DESCRIBE_DOC_NM_SETTING_IP_TUNNEL_REMOTE N_("The remote endpoint of the tunnel; the value must contain an IPv4 or IPv6 address.") #define DESCRIBE_DOC_NM_SETTING_IP_TUNNEL_TOS N_("The type of service (IPv4) or traffic class (IPv6) field to be set on tunneled packets.") #define DESCRIBE_DOC_NM_SETTING_IP_TUNNEL_TTL N_("The TTL to assign to tunneled packets. 0 is a special value meaning that packets inherit the TTL value.") -#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_ADDRESSES N_("Array of IP addresses.") +#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_ADDRESSES N_("A list of IPv4 addresses and their prefix length. Multiple addresses can be separated by comma. For example \"192.168.1.5/24, 10.1.0.5/24\".") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DAD_TIMEOUT N_("Timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. A zero value means that no duplicate address detection is performed, -1 means the default value (either configuration ipvx.dad-timeout override or zero). A value greater than zero is a timeout in milliseconds. The property is currently implemented only for IPv4.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_CLIENT_ID N_("A string sent to the DHCP server to identify the local machine which the DHCP server may use to customize the DHCP lease and options. When the property is a hex string ('aa:bb:cc') it is interpreted as a binary client ID, in which case the first byte is assumed to be the 'type' field as per RFC 2132 section 9.14 and the remaining bytes may be an hardware address (e.g. '01:xx:xx:xx:xx:xx:xx' where 1 is the Ethernet ARP type and the rest is a MAC address). If the property is not a hex string it is considered as a non-hardware-address client ID and the 'type' field is set to 0. The special values \"mac\" and \"perm-mac\" are supported, which use the current or permanent MAC address of the device to generate a client identifier with type ethernet (01). Currently, these options only work for ethernet type of links. The special value \"ipv6-duid\" uses the DUID from \"ipv6.dhcp-duid\" property as an RFC4361-compliant client identifier. As IAID it uses \"ipv4.dhcp-iaid\" and falls back to \"ipv6.dhcp-iaid\" if unset. The special value \"duid\" generates a RFC4361-compliant client identifier based on \"ipv4.dhcp-iaid\" and uses a DUID generated by hashing /etc/machine-id. The special value \"stable\" is supported to generate a type 0 client identifier based on the stable-id (see connection.stable-id) and a per-host key. If you set the stable-id, you may want to include the \"${DEVICE}\" or \"${MAC}\" specifier to get a per-device key. If unset, a globally configured default is used. If still unset, the default depends on the DHCP plugin.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_FQDN N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified FQDN will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-hostname\" are mutually exclusive and cannot be set at the same time.") diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c index 20fbc017b..4e5ff0f47 100644 --- a/libnm-core/nm-setting-ip4-config.c +++ b/libnm-core/nm-setting-ip4-config.c @@ -946,6 +946,13 @@ nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) * for that subnet. * ---end--- */ + /* ---nmcli--- + * property: addresses + * format: a comma separated list of addresses + * description: A list of IPv4 addresses and their prefix length. Multiple addresses + * can be separated by comma. For example "192.168.1.5/24, 10.1.0.5/24". + * ---end--- + */ _nm_properties_override_gobj( properties_override, g_object_class_find_property(G_OBJECT_CLASS(setting_class), NM_SETTING_IP_CONFIG_ADDRESSES), From 39c3eacb7d3a5f5ad13c299045216ccbaace1858 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Feb 2021 17:40:24 +0100 Subject: [PATCH 27/29] platform/tests: relax check for signals in test_ip6_route() /route/ip6: NMPlatformSignalAssert: ../src/core/platform/tests/test-route.c:449, test_ip6_route(): failure to accept signal one time: 'ip6-route-changed-added' ifindex 0 (2 times received) --- src/core/platform/tests/test-route.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/platform/tests/test-route.c b/src/core/platform/tests/test-route.c index 47587865b..645bb5b18 100644 --- a/src/core/platform/tests/test-route.c +++ b/src/core/platform/tests/test-route.c @@ -446,7 +446,7 @@ test_ip6_route(void) metric, mss); g_assert(nmtstp_ip6_route_get(NM_PLATFORM_GET, ifindex, &network, plen, metric, NULL, 0)); - accept_signal(route_added); + accept_signals(route_added, 1, 2); /* Add route again */ nmtstp_ip6_route_add(NM_PLATFORM_GET, From 04290879b73e569632bf1904120b7b3f641430b3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Feb 2021 17:50:29 +0100 Subject: [PATCH 28/29] release: fix pattern in release.sh script for checking branch name --- contrib/fedora/rpm/release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/fedora/rpm/release.sh b/contrib/fedora/rpm/release.sh index b91dfe541..c07a8c284 100755 --- a/contrib/fedora/rpm/release.sh +++ b/contrib/fedora/rpm/release.sh @@ -266,7 +266,7 @@ if [ "$CUR_BRANCH" = master ]; then number_is_odd "${VERSION_ARR[1]}" || die "Unexpected version number on master. Should be an odd development version" [ "$RELEASE_MODE" = devel -o "$RELEASE_MODE" = rc1 -o "$RELEASE_MODE" = major-post ] || die "Unexpected branch name \"$CUR_BRANCH\" for \"$RELEASE_MODE\"" else - re='^nm-[0-9][1-9]*-[0-9][1-9]*$' + re='^nm-[0-9]+-[0-9]+$' [[ "$CUR_BRANCH" =~ $re ]] || die "Unexpected current branch $CUR_BRANCH. Should be master or nm-?-??" if number_is_odd "${VERSION_ARR[1]}"; then # we are on a release candiate branch. From 1a1606186c2a835f738d408155378eba4e5e4169 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 18 Feb 2021 18:20:51 +0100 Subject: [PATCH 29/29] NEWS: update --- NEWS | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 094680045..4d54d111b 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,6 @@ ============================================= -NetworkManager-1.30 -Overview of changes since NetworkManager-1.28 +NetworkManager-1.32 +Overview of changes since NetworkManager-1.30 ============================================= This is a snapshot of NetworkManager 1.30 development series. @@ -8,6 +8,11 @@ The API is subject to change and not guaranteed to be compatible with the later release. USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! +============================================= +NetworkManager-1.30 +Overview of changes since NetworkManager-1.28 +============================================= + * Increase timeout of NetworkManager-wait-online.service to 60 seconds. * Add "ipv4.dhcp-client-id=ipv6-duid" option for RFC4361. * The dhcpcd plugin now requires a minimum version of dhcpcd-9.3.3 with @@ -23,13 +28,15 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! * libnm: nm_setting_bond_add_option() no longer validates the option that is set. Instead, use nm_connection_verify() to validate the profile. -* libnm: add support for reading/writing keyfile format. +* libnm: add support for reading/writing keyfile format. This required to + relicense previously GPL-2.0+ code as LGPL-2.1+ with the agreement of the + copyright holders. * initrd: support for rd.net.timeout.carrier option. * initrd: support new ip method "link6" for IPv6 link-local only. * ci: use ci-templates for containers on gitlab-ci. * ci: test build against Alpine Linux on gitlab-ci. * build: new configure option to set path to "polkit-agent-helper-1". -* Many bugfixes and improvements. +* Many bugfixes, improvements and translation updates. ============================================= NetworkManager-1.28