From 75950829755d516512f4e2d92b2c8582a7fe2bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20S=C3=BCtter?= Date: Mon, 21 Oct 2024 09:41:27 +0200 Subject: [PATCH] networkmanager: use PATH to search for binaries instead of hard-coding store paths --- .../services/networking/networkmanager.nix | 8 + nixos/tests/networking/networkmanager.nix | 32 ++++ .../networking/networkmanager/default.nix | 28 ++- .../networking/networkmanager/env-paths.patch | 171 ++++++++++++++++++ .../networking/networkmanager/fix-paths.patch | 58 ------ 5 files changed, 222 insertions(+), 75 deletions(-) create mode 100644 pkgs/tools/networking/networkmanager/env-paths.patch diff --git a/nixos/modules/services/networking/networkmanager.nix b/nixos/modules/services/networking/networkmanager.nix index dedd53e345fe5..2b82c14584bef 100644 --- a/nixos/modules/services/networking/networkmanager.nix +++ b/nixos/modules/services/networking/networkmanager.nix @@ -117,6 +117,7 @@ let packages = [ pkgs.modemmanager pkgs.networkmanager + pkgs.openconnect ] ++ cfg.plugins ++ lib.optionals (!delegateWireless && !enableIwd) [ @@ -578,6 +579,10 @@ in wantedBy = [ "network.target" ]; restartTriggers = [ configFile ]; + # NetworkManager has been patched to search for needed binaries in its PATH. + # nftables, dnsmasq, dhcpcd and iputils are required for standard functionality. + path = [ pkgs.openconnect pkgs.nftables pkgs.dnsmasq pkgs.dhcpcd pkgs.iputils ]; + aliases = [ "dbus-org.freedesktop.NetworkManager.service" ]; serviceConfig = { @@ -609,6 +614,9 @@ in wantedBy = [ "multi-user.target" ]; before = [ "network-online.target" ]; after = [ "NetworkManager.service" ]; + # This might be needed if some ensured profile needs to enable an + # openconnect VPN connection. + path = [ pkgs.openconnect ]; script = let path = id: "/run/NetworkManager/system-connections/${id}.nmconnection"; in '' diff --git a/nixos/tests/networking/networkmanager.nix b/nixos/tests/networking/networkmanager.nix index bd989408df8a1..777c31267e727 100644 --- a/nixos/tests/networking/networkmanager.nix +++ b/nixos/tests/networking/networkmanager.nix @@ -162,6 +162,38 @@ let client.wait_until_succeeds("ping -c 1 fd00:1234:5678:1::23") ''; }; + shared = { + name = "shared"; + nodes = { + inherit router; + client = clientConfig { + networking.networkmanager.logLevel = "DEBUG"; + networking.networkmanager.ensureProfiles.profiles.shared = { + connection = { + interface-name = "eth0"; + type = "ethernet"; + id = "shared"; + }; + ipv4.method = "shared"; + ipv4.addresses = "192.168.21.42/24"; + }; + }; + }; + testScript = '' + start_all() + router.systemctl("start network-online.target") + router.wait_for_unit("network-online.target") + client.wait_for_unit("NetworkManager.service") + with subtest("Test shared connection type and make sure the binaries are found."): + # nftables binary is found: + client.wait_until_succeeds("journalctl -u NetworkManager | grep -q 'trying to find nftables (nft) backend.'") + client.wait_until_succeeds("journalctl -u NetworkManager | grep -q 'nft.*: communicate with nft'") + # dhcpcd binary is found: + client.wait_until_succeeds("journalctl -u NetworkManager | grep -q \"enabled DHCP client \'dhcpcd\'\"") + # dnsmasq binary is found: + client.wait_until_succeeds("journalctl -u NetworkManager | grep -q 'dnsmasq-manager: dnsmasq started with pid'") + ''; + }; }; in lib.mapAttrs (lib.const (attrs: makeTest (attrs // { name = "${attrs.name}-Networking-NetworkManager"; diff --git a/pkgs/tools/networking/networkmanager/default.nix b/pkgs/tools/networking/networkmanager/default.nix index fbc7ec0857b58..482a43bab982d 100644 --- a/pkgs/tools/networking/networkmanager/default.nix +++ b/pkgs/tools/networking/networkmanager/default.nix @@ -10,13 +10,9 @@ , polkit , gnutls , ppp -, dhcpcd -, iptables -, nftables , python3 , vala , libgcrypt -, dnsmasq , bluez5 , readline , libselinux @@ -30,7 +26,6 @@ , libsoup , ethtool , gnused -, iputils , kmod , jansson , elfutils @@ -40,7 +35,6 @@ , docbook_xml_dtd_412 , docbook_xml_dtd_42 , docbook_xml_dtd_43 -, openconnect , curl , meson , mesonEmulatorHook @@ -53,6 +47,7 @@ , systemd , udev , withSystemd ? lib.meta.availableOn stdenv.hostPlatform systemd +, withModemManager ? true }: let @@ -93,12 +88,8 @@ stdenv.mkDerivation rec { # Features # Allow using iwd when configured to do so "-Diwd=true" - "-Dpppd=${ppp}/bin/pppd" - "-Diptables=${iptables}/bin/iptables" - "-Dnft=${nftables}/bin/nft" - "-Dmodem_manager=true" + (lib.mesonBool "modem_manager" withModemManager) "-Dnmtui=true" - "-Ddnsmasq=${dnsmasq}/bin/dnsmasq" "-Dqt=false" # Handlers @@ -107,7 +98,6 @@ stdenv.mkDerivation rec { # DHCP clients # ISC DHCP client has reached it's end of life, so stop using it "-Ddhclient=no" - "-Ddhcpcd=${dhcpcd}/bin/dhcpcd" "-Ddhcpcanon=no" # Miscellaneous @@ -124,10 +114,14 @@ stdenv.mkDerivation rec { patches = [ (substituteAll { src = ./fix-paths.patch; - inherit iputils openconnect ethtool gnused systemd; + inherit ethtool gnused; inherit runtimeShell; }) + # Enables searching for needed binaries in PATH instead of hard-coding + # binary locations at runtime. + ./env-paths.patch + # Meson does not support using different directories during build and # for installation like Autotools did with flags passed to make install. ./fix-install-paths.patch @@ -146,15 +140,15 @@ stdenv.mkDerivation rec { ppp libndp curl - mobile-broadband-provider-info - bluez5 - dnsmasq - modemmanager readline newt libsoup jansson dbus # used to get directory paths with pkg-config during configuration + ] ++ lib.optionals withModemManager [ + mobile-broadband-provider-info + bluez5 + modemmanager ]; propagatedBuildInputs = [ gnutls libgcrypt ]; diff --git a/pkgs/tools/networking/networkmanager/env-paths.patch b/pkgs/tools/networking/networkmanager/env-paths.patch new file mode 100644 index 0000000000000..2fad837705700 --- /dev/null +++ b/pkgs/tools/networking/networkmanager/env-paths.patch @@ -0,0 +1,171 @@ +diff --git a/src/core/nm-firewall-utils.c b/src/core/nm-firewall-utils.c +index b6b2b0d721..bc1ad8996e 100644 +--- a/src/core/nm-firewall-utils.c ++++ b/src/core/nm-firewall-utils.c +@@ -13,12 +13,14 @@ + #include "libnm-platform/nm-platform.h" + + #include "nm-config.h" ++#include "nm-utils.h" + #include "NetworkManagerUtils.h" + + /*****************************************************************************/ + +-static const struct { ++static struct { + const char *name; ++ const char *binary; + const char *path; + } FirewallBackends[] = { + [NM_FIREWALL_BACKEND_NONE - 1] = +@@ -28,11 +30,13 @@ static const struct { + [NM_FIREWALL_BACKEND_NFTABLES - 1] = + { + .name = "nftables", ++ .binary = "nft", + .path = NFT_PATH, + }, + [NM_FIREWALL_BACKEND_IPTABLES - 1] = + { + .name = "iptables", ++ .binary = "iptables", + .path = IPTABLES_PATH, + }, + }; +@@ -213,7 +217,7 @@ _share_iptables_call_v(const char *const *argv) + } + + #define _share_iptables_call(...) \ +- _share_iptables_call_v(NM_MAKE_STRV("" IPTABLES_PATH "", "--wait", "2", __VA_ARGS__)) ++ _share_iptables_call_v(NM_MAKE_STRV(FirewallBackends[NM_FIREWALL_BACKEND_IPTABLES-1].path, "--wait", "2", __VA_ARGS__)) + + static gboolean + _share_iptables_chain_op(const char *table, const char *chain, const char *op) +@@ -598,7 +602,7 @@ nm_firewall_nft_call(GBytes *stdin_buf, + g_subprocess_launcher_set_environ(subprocess_launcher, NM_STRV_EMPTY()); + + call_data->subprocess = g_subprocess_launcher_spawnv(subprocess_launcher, +- NM_MAKE_STRV(NFT_PATH, "-f", "-"), ++ NM_MAKE_STRV(FirewallBackends[NM_FIREWALL_BACKEND_NFTABLES-1].path, "-f", "-"), + &error); + + if (!call_data->subprocess) { +@@ -1013,9 +1017,9 @@ nm_firewall_config_apply_sync(NMFirewallConfig *self, gboolean up) + static NMFirewallBackend + _firewall_backend_detect(void) + { +- if (g_file_test(NFT_PATH, G_FILE_TEST_IS_EXECUTABLE)) ++ if (nm_utils_find_helper(FirewallBackends[NM_FIREWALL_BACKEND_NFTABLES - 1].binary, NULL, NULL) != NULL) + return NM_FIREWALL_BACKEND_NFTABLES; +- if (g_file_test(IPTABLES_PATH, G_FILE_TEST_IS_EXECUTABLE)) ++ if (nm_utils_find_helper(FirewallBackends[NM_FIREWALL_BACKEND_IPTABLES - 1].binary, NULL, NULL) != NULL) + return NM_FIREWALL_BACKEND_IPTABLES; + + return NM_FIREWALL_BACKEND_NFTABLES; +@@ -1030,9 +1034,10 @@ nm_firewall_utils_get_backend(void) + again: + b = g_atomic_int_get(&backend); + if (b == NM_FIREWALL_BACKEND_UNKNOWN) { +- gs_free char *conf_value = NULL; +- gboolean detect; +- int i; ++ gs_free char *conf_value = NULL; ++ gboolean detect; ++ const char *path_value = NULL; ++ int i; + + conf_value = + nm_config_data_get_value(NM_CONFIG_GET_DATA_ORIG, +@@ -1053,6 +1058,19 @@ again: + if (detect) + b = _firewall_backend_detect(); + ++ nm_log_dbg(LOGD_SHARING, "firewall: trying to find %s (%s) backend.", ++ FirewallBackends[b - 1].name, FirewallBackends[b - 1].binary); ++ ++ path_value = nm_utils_find_helper(FirewallBackends[b - 1].binary, ++ NULL, ++ NULL); ++ ++ if (path_value == NULL) { ++ g_atomic_int_set(&backend, NM_FIREWALL_BACKEND_NONE); ++ } else { ++ FirewallBackends[b - 1].path = path_value; ++ } ++ + nm_assert(NM_IN_SET(b, + NM_FIREWALL_BACKEND_NONE, + NM_FIREWALL_BACKEND_IPTABLES, +@@ -1062,18 +1080,12 @@ again: + goto again; + + nm_log_dbg(LOGD_SHARING, +- "firewall: use %s backend%s%s%s%s%s%s%s", ++ "firewall: use %s backend%s%s%s", + FirewallBackends[b - 1].name, + NM_PRINT_FMT_QUOTED(FirewallBackends[b - 1].path, + " (", + FirewallBackends[b - 1].path, + ")", +- ""), +- detect ? " (detected)" : "", +- NM_PRINT_FMT_QUOTED(detect && conf_value, +- " (invalid setting \"", +- conf_value, +- "\")", + "")); + } + +diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c +index 06a96318ab..cda89ba96a 100644 +--- a/src/libnm-core-impl/nm-utils.c ++++ b/src/libnm-core-impl/nm-utils.c +@@ -3647,10 +3647,31 @@ nm_utils_file_search_in_paths(const char *progname, + gpointer user_data, + GError **error) + { ++ const char *path_env = NULL; ++ gs_free const char **env_paths = NULL; ++ gs_free const char **all_paths = NULL; ++ size_t paths_len; ++ size_t env_paths_len; ++ guint i; ++ + g_return_val_if_fail(!error || !*error, NULL); + g_return_val_if_fail(progname && progname[0] && !strchr(progname, '/'), NULL); + g_return_val_if_fail(file_test_flags || predicate, NULL); + ++ path_env = g_getenv("PATH"); ++ env_paths = nm_strsplit_set_full(path_env, ":", NM_STRSPLIT_SET_FLAGS_STRSTRIP); ++ ++ paths_len = g_strv_length((char **) paths); ++ env_paths_len = g_strv_length((char **) env_paths); ++ ++ all_paths = g_new(const char *, (env_paths_len + paths_len + 1)); ++ ++ for (i = 0; i < env_paths_len; i++) ++ all_paths[i] = env_paths[i]; ++ for (i = 0; i < paths_len; i++) ++ all_paths[env_paths_len + i] = paths[i]; ++ all_paths[env_paths_len + i] = NULL; ++ + /* Only consider @try_first if it is a valid, absolute path. This makes + * it simpler to pass in a path from configure checks. */ + if (try_first && try_first[0] == '/' +@@ -3658,12 +3679,13 @@ nm_utils_file_search_in_paths(const char *progname, + && (!predicate || predicate(try_first, user_data))) + return g_intern_string(try_first); + +- if (paths && paths[0]) { ++ if (all_paths && all_paths[0]) { + nm_auto_str_buf NMStrBuf strbuf = + NM_STR_BUF_INIT(NM_UTILS_GET_NEXT_REALLOC_SIZE_104, FALSE); ++ const char **ptr = all_paths; + +- for (; *paths; paths++) { +- const char *path = *paths; ++ for (; *ptr; ptr++) { ++ const char *path = *ptr; + const char *s; + + if (!path[0]) diff --git a/pkgs/tools/networking/networkmanager/fix-paths.patch b/pkgs/tools/networking/networkmanager/fix-paths.patch index ecdb60ceeb82f..37123ad80a527 100644 --- a/pkgs/tools/networking/networkmanager/fix-paths.patch +++ b/pkgs/tools/networking/networkmanager/fix-paths.patch @@ -10,27 +10,6 @@ index 148acade5c..6395fbfbe5 100644 +PROGRAM="@runtimeShell@ -c '@ethtool@/bin/ethtool -i $$1 |@gnused@/bin/sed -n s/^driver:\ //p' -- $env{INTERFACE}", ENV{ID_NET_DRIVER}="%c" LABEL="nm_drivers_end" -diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c -index f3441508ab..7cde8d7d39 100644 ---- a/src/core/devices/nm-device.c -+++ b/src/core/devices/nm-device.c -@@ -14839,14 +14839,14 @@ nm_device_start_ip_check(NMDevice *self) - gw = nm_l3_config_data_get_best_default_route(l3cd, AF_INET); - if (gw) { - nm_inet4_ntop(NMP_OBJECT_CAST_IP4_ROUTE(gw)->gateway, buf); -- ping_binary = nm_utils_find_helper("ping", "/usr/bin/ping", NULL); -+ ping_binary = "@iputils@/bin/ping"; - log_domain = LOGD_IP4; - } - } else if (priv->ip_data_6.state == NM_DEVICE_IP_STATE_READY) { - gw = nm_l3_config_data_get_best_default_route(l3cd, AF_INET6); - if (gw) { - nm_inet6_ntop(&NMP_OBJECT_CAST_IP6_ROUTE(gw)->gateway, buf); -- ping_binary = nm_utils_find_helper("ping6", "/usr/bin/ping6", NULL); -+ ping_binary = "@iputils@/bin/ping"; - log_domain = LOGD_IP6; - } - } diff --git a/src/libnm-client-impl/meson.build b/src/libnm-client-impl/meson.build index 3dd2338a82..de75cc040b 100644 --- a/src/libnm-client-impl/meson.build @@ -51,43 +30,6 @@ index 3dd2338a82..de75cc040b 100644 gen_gir_cmd, '--lib-path', meson.current_build_dir(), '--gir', libnm_gir[0], -diff --git a/src/libnmc-base/nm-vpn-helpers.c b/src/libnmc-base/nm-vpn-helpers.c -index cbe76f5f1c..8515f94994 100644 ---- a/src/libnmc-base/nm-vpn-helpers.c -+++ b/src/libnmc-base/nm-vpn-helpers.c -@@ -284,15 +284,6 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, - const char *const *iter; - const char *path; - const char *opt; -- const char *const DEFAULT_PATHS[] = { -- "/sbin/", -- "/usr/sbin/", -- "/usr/local/sbin/", -- "/bin/", -- "/usr/bin/", -- "/usr/local/bin/", -- NULL, -- }; - const char *oc_argv[(12 + 2 * G_N_ELEMENTS(oc_property_args))]; - const char *gw; - int port; -@@ -311,15 +302,7 @@ nm_vpn_openconnect_authenticate_helper(NMSettingVpn *s_vpn, GPtrArray *secrets, - - port = extract_url_port(gw); - -- path = nm_utils_file_search_in_paths("openconnect", -- "/usr/sbin/openconnect", -- DEFAULT_PATHS, -- G_FILE_TEST_IS_EXECUTABLE, -- NULL, -- NULL, -- error); -- if (!path) -- return FALSE; -+ path = "@openconnect@/bin/openconnect"; - - oc_argv[oc_argc++] = path; - oc_argv[oc_argc++] = "--authenticate"; diff --git a/src/libnmc-setting/meson.build b/src/libnmc-setting/meson.build index 4d5079dfb3..5a15447fde 100644 --- a/src/libnmc-setting/meson.build