Skip to content

Commit

Permalink
networkmanager: use PATH to search for binaries instead of hard-codin…
Browse files Browse the repository at this point in the history
…g store paths
  • Loading branch information
0xf09f95b4 committed Oct 27, 2024
1 parent b080a92 commit 7595082
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 75 deletions.
8 changes: 8 additions & 0 deletions nixos/modules/services/networking/networkmanager.nix
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ let
packages = [
pkgs.modemmanager
pkgs.networkmanager
pkgs.openconnect
]
++ cfg.plugins
++ lib.optionals (!delegateWireless && !enableIwd) [
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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 ''
Expand Down
32 changes: 32 additions & 0 deletions nixos/tests/networking/networkmanager.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
28 changes: 11 additions & 17 deletions pkgs/tools/networking/networkmanager/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@
, polkit
, gnutls
, ppp
, dhcpcd
, iptables
, nftables
, python3
, vala
, libgcrypt
, dnsmasq
, bluez5
, readline
, libselinux
Expand All @@ -30,7 +26,6 @@
, libsoup
, ethtool
, gnused
, iputils
, kmod
, jansson
, elfutils
Expand All @@ -40,7 +35,6 @@
, docbook_xml_dtd_412
, docbook_xml_dtd_42
, docbook_xml_dtd_43
, openconnect
, curl
, meson
, mesonEmulatorHook
Expand All @@ -53,6 +47,7 @@
, systemd
, udev
, withSystemd ? lib.meta.availableOn stdenv.hostPlatform systemd
, withModemManager ? true
}:

let
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 ];
Expand Down
171 changes: 171 additions & 0 deletions pkgs/tools/networking/networkmanager/env-paths.patch
Original file line number Diff line number Diff line change
@@ -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])
Loading

0 comments on commit 7595082

Please sign in to comment.