From abdf3385d6881d98ffa4311e4d34a74c1b558020 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 10 Oct 2025 11:44:55 +0200 Subject: [PATCH 01/14] libnm-core, core: add permission helpers Add utility functions to get the number of users and the first user from the connection.permissions property of a connection. (cherry picked from commit 59543620dcf7bb3e4b1316536f0330ab4a752e3e) (cherry picked from commit 2fc662cc712e9e1a1992cbbc187f257f57476e53) --- src/core/nm-core-utils.c | 11 ++++++ src/core/nm-core-utils.h | 4 ++ src/libnm-core-impl/nm-setting-connection.c | 41 +++++++++++++++++++++ src/libnm-core-intern/nm-core-internal.h | 5 +++ 4 files changed, 61 insertions(+) diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index d3c2eb21ca..7560d98b09 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -5474,3 +5474,14 @@ nm_utils_shorten_hostname(const char *hostname, char **shortened) *shortened = g_steal_pointer(&s); return TRUE; } + +const char * +nm_utils_get_connection_first_permissions_user(NMConnection *connection) +{ + NMSettingConnection *s_con; + + s_con = nm_connection_get_setting_connection(connection); + nm_assert(s_con); + + return _nm_setting_connection_get_first_permissions_user(s_con); +} diff --git a/src/core/nm-core-utils.h b/src/core/nm-core-utils.h index fdfed5f65d..a647eab1b9 100644 --- a/src/core/nm-core-utils.h +++ b/src/core/nm-core-utils.h @@ -490,4 +490,8 @@ uid_t nm_utils_get_nm_uid(void); gid_t nm_utils_get_nm_gid(void); +/*****************************************************************************/ + +const char *nm_utils_get_connection_first_permissions_user(NMConnection *connection); + #endif /* __NM_CORE_UTILS_H__ */ diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index 4829dcdcd9..ec3a1fed48 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -431,6 +431,47 @@ nm_setting_connection_permissions_user_allowed_by_uid(NMSettingConnection *setti return _permissions_user_allowed(setting, NULL, uid); } +guint +_nm_setting_connection_get_num_permissions_users(NMSettingConnection *setting) +{ + NMSettingConnectionPrivate *priv; + guint i; + guint count = 0; + + nm_assert(NM_IS_SETTING_CONNECTION(setting)); + priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting); + + for (i = 0; priv->permissions && i < priv->permissions->len; i++) { + const Permission *permission = &nm_g_array_index(priv->permissions, Permission, i); + + if (permission->ptype == PERM_TYPE_USER) { + count++; + } + } + + return count; +} + +const char * +_nm_setting_connection_get_first_permissions_user(NMSettingConnection *setting) +{ + NMSettingConnectionPrivate *priv; + guint i; + + nm_assert(NM_IS_SETTING_CONNECTION(setting)); + priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting); + + for (i = 0; priv->permissions && i < priv->permissions->len; i++) { + const Permission *permission = &nm_g_array_index(priv->permissions, Permission, i); + + if (permission->ptype == PERM_TYPE_USER) { + return permission->item; + } + } + + return NULL; +} + /** * nm_setting_connection_add_permission: * @setting: the #NMSettingConnection diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index 8b23dd44cc..6991185d39 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -1187,4 +1187,9 @@ gboolean nm_connection_need_secrets_for_rerequest(NMConnection *connection); const GPtrArray *_nm_setting_ovs_port_get_trunks_arr(NMSettingOvsPort *self); +/*****************************************************************************/ + +guint _nm_setting_connection_get_num_permissions_users(NMSettingConnection *setting); +const char *_nm_setting_connection_get_first_permissions_user(NMSettingConnection *setting); + #endif From d1776c539465c81da6143c1470cedb72312e1561 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 2 Oct 2025 16:29:35 +0200 Subject: [PATCH 02/14] helpers: move helper programs to the same directory Create a new 'nm-helpers' directory for all the helper programs, to avoid having too many subdirs in the src directory. (cherry picked from commit 3d76d12eee88b667d1a385b861c54fcdd4e490ed) (cherry picked from commit afa6fc951b4a19b55d76fb365446a5cf8896a1d3) --- src/meson.build | 3 +-- src/nm-daemon-helper/README.md | 11 ---------- src/nm-daemon-helper/meson.build | 15 -------------- src/{nm-priv-helper => nm-helpers}/README.md | 20 ++++++++++++++++++- .../meson.build | 20 +++++++++++++++++++ .../nm-daemon-helper.c | 0 .../nm-priv-helper.c | 0 .../nm-priv-helper.conf | 0 .../org.freedesktop.nm_priv_helper.service.in | 0 9 files changed, 40 insertions(+), 29 deletions(-) delete mode 100644 src/nm-daemon-helper/README.md delete mode 100644 src/nm-daemon-helper/meson.build rename src/{nm-priv-helper => nm-helpers}/README.md (64%) rename src/{nm-priv-helper => nm-helpers}/meson.build (67%) rename src/{nm-daemon-helper => nm-helpers}/nm-daemon-helper.c (100%) rename src/{nm-priv-helper => nm-helpers}/nm-priv-helper.c (100%) rename src/{nm-priv-helper => nm-helpers}/nm-priv-helper.conf (100%) rename src/{nm-priv-helper => nm-helpers}/org.freedesktop.nm_priv_helper.service.in (100%) diff --git a/src/meson.build b/src/meson.build index 1a0319828b..0cf93bfdde 100644 --- a/src/meson.build +++ b/src/meson.build @@ -103,8 +103,7 @@ if enable_nmtui endif subdir('nmcli') subdir('nm-dispatcher') -subdir('nm-priv-helper') -subdir('nm-daemon-helper') +subdir('nm-helpers') subdir('nm-online') if enable_nmtui subdir('nmtui') diff --git a/src/nm-daemon-helper/README.md b/src/nm-daemon-helper/README.md deleted file mode 100644 index 695f533553..0000000000 --- a/src/nm-daemon-helper/README.md +++ /dev/null @@ -1,11 +0,0 @@ -nm-daemon-helper -================ - -A internal helper application that is spawned by NetworkManager -to perform certain actions. - -Currently all it does is doing a reverse DNS lookup, which -cannot be done by NetworkManager because the operation requires -to reconfigure the libc resolver (which is a process-wide operation). - -This is not directly useful to the user. diff --git a/src/nm-daemon-helper/meson.build b/src/nm-daemon-helper/meson.build deleted file mode 100644 index da0d6571e1..0000000000 --- a/src/nm-daemon-helper/meson.build +++ /dev/null @@ -1,15 +0,0 @@ -executable( - 'nm-daemon-helper', - 'nm-daemon-helper.c', - include_directories : [ - src_inc, - top_inc, - ], - link_with: [ - libnm_std_aux, - ], - link_args: ldflags_linker_script_binary, - link_depends: linker_script_binary, - install: true, - install_dir: nm_libexecdir, -) diff --git a/src/nm-priv-helper/README.md b/src/nm-helpers/README.md similarity index 64% rename from src/nm-priv-helper/README.md rename to src/nm-helpers/README.md index 576da7a70b..ccaa5bf5cd 100644 --- a/src/nm-priv-helper/README.md +++ b/src/nm-helpers/README.md @@ -1,5 +1,23 @@ +nm-helpers +========== + +This directory contains stand-alone helper programs used by various +components. + +nm-daemon-helper +---------------- + +A internal helper application that is spawned by NetworkManager +to perform certain actions. + +Currently all it does is doing a reverse DNS lookup, which +cannot be done by NetworkManager because the operation requires +to reconfigure the libc resolver (which is a process-wide operation). + +This is not directly useful to the user. + nm-priv-helper -============== +-------------- This is a D-Bus activatable, exit-on-idle service, which provides an internal API to NetworkManager daemon. diff --git a/src/nm-priv-helper/meson.build b/src/nm-helpers/meson.build similarity index 67% rename from src/nm-priv-helper/meson.build rename to src/nm-helpers/meson.build index 6141e0e207..5f330cbc94 100644 --- a/src/nm-priv-helper/meson.build +++ b/src/nm-helpers/meson.build @@ -1,5 +1,25 @@ # SPDX-License-Identifier: LGPL-2.1-or-later +# nm-daemon-helper + +executable( + 'nm-daemon-helper', + 'nm-daemon-helper.c', + include_directories : [ + src_inc, + top_inc, + ], + link_with: [ + libnm_std_aux, + ], + link_args: ldflags_linker_script_binary, + link_depends: linker_script_binary, + install: true, + install_dir: nm_libexecdir, +) + +# nm-priv-helper + configure_file( input: 'org.freedesktop.nm_priv_helper.service.in', output: '@BASENAME@', diff --git a/src/nm-daemon-helper/nm-daemon-helper.c b/src/nm-helpers/nm-daemon-helper.c similarity index 100% rename from src/nm-daemon-helper/nm-daemon-helper.c rename to src/nm-helpers/nm-daemon-helper.c diff --git a/src/nm-priv-helper/nm-priv-helper.c b/src/nm-helpers/nm-priv-helper.c similarity index 100% rename from src/nm-priv-helper/nm-priv-helper.c rename to src/nm-helpers/nm-priv-helper.c diff --git a/src/nm-priv-helper/nm-priv-helper.conf b/src/nm-helpers/nm-priv-helper.conf similarity index 100% rename from src/nm-priv-helper/nm-priv-helper.conf rename to src/nm-helpers/nm-priv-helper.conf diff --git a/src/nm-priv-helper/org.freedesktop.nm_priv_helper.service.in b/src/nm-helpers/org.freedesktop.nm_priv_helper.service.in similarity index 100% rename from src/nm-priv-helper/org.freedesktop.nm_priv_helper.service.in rename to src/nm-helpers/org.freedesktop.nm_priv_helper.service.in From 310887be7123ace3c0517c15e5552761f0201d5b Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 29 Sep 2025 09:52:51 +0200 Subject: [PATCH 03/14] daemon-helper: add read-file-as-user Add a new command to read the content of a file after switching to the given user. This command can be used to enforce Unix filesystem permissions when accessing a file on behalf of a user. (cherry picked from commit 285457a5f8284f21387753d7f245e3f51ce29248) (cherry picked from commit 022b992846712ecff6454524eb3934ff0800cf54) --- src/libnm-std-aux/nm-std-utils.c | 114 +++++++++++++++++++++++++++++- src/libnm-std-aux/nm-std-utils.h | 4 ++ src/nm-helpers/README.md | 11 +-- src/nm-helpers/nm-daemon-helper.c | 33 +++++++++ 4 files changed, 156 insertions(+), 6 deletions(-) diff --git a/src/libnm-std-aux/nm-std-utils.c b/src/libnm-std-aux/nm-std-utils.c index 8bc109f2e2..6c909dfab3 100644 --- a/src/libnm-std-aux/nm-std-utils.c +++ b/src/libnm-std-aux/nm-std-utils.c @@ -4,10 +4,14 @@ #include "nm-std-utils.h" -#include #include +#include +#include #include #include +#include +#include +#include /*****************************************************************************/ @@ -95,6 +99,114 @@ out_huge: /*****************************************************************************/ +bool +nm_utils_set_effective_user(const char *user, char *errbuf, size_t errbuf_len) +{ + struct passwd *pwentry; + int errsv; + char error[1024]; + + errno = 0; + pwentry = getpwnam(user); + if (!pwentry) { + errsv = errno; + if (errsv == 0) { + snprintf(errbuf, errbuf_len, "user not found"); + } else { + snprintf(errbuf, + errbuf_len, + "error getting user entry: %d (%s)\n", + errsv, + strerror_r(errsv, error, sizeof(error))); + } + return false; + } + + if (setgid(pwentry->pw_gid) != 0) { + errsv = errno; + snprintf(errbuf, + errbuf_len, + "failed to change group to %u: %d (%s)\n", + pwentry->pw_gid, + errsv, + strerror_r(errsv, error, sizeof(error))); + return false; + } + + if (initgroups(user, pwentry->pw_gid) != 0) { + errsv = errno; + snprintf(errbuf, + errbuf_len, + "failed to reset supplementary group list to %u: %d (%s)\n", + pwentry->pw_gid, + errsv, + strerror_r(errsv, error, sizeof(error))); + return false; + } + + if (setuid(pwentry->pw_uid) != 0) { + errsv = errno; + snprintf(errbuf, + errbuf_len, + "failed to change user to %u: %d (%s)\n", + pwentry->pw_uid, + errsv, + strerror_r(errsv, error, sizeof(error))); + return false; + } + + return true; +} + +/*****************************************************************************/ + +bool +nm_utils_read_file_to_stdout(const char *filename, char *errbuf, size_t errbuf_len) +{ + nm_auto_close int fd = -1; + char buffer[4096]; + char error[1024]; + ssize_t bytes_read; + int errsv; + + fd = open(filename, O_RDONLY); + if (fd == -1) { + errsv = errno; + snprintf(errbuf, + errbuf_len, + "error opening the file: %d (%s)", + errsv, + strerror_r(errsv, error, sizeof(error))); + return false; + } + + while ((bytes_read = read(fd, buffer, sizeof(buffer))) > 0) { + if (fwrite(buffer, 1, bytes_read, stdout) != (size_t) bytes_read) { + errsv = errno; + snprintf(errbuf, + errbuf_len, + "error writing to stdout: %d (%s)", + errsv, + strerror_r(errsv, error, sizeof(error))); + return false; + } + } + + if (bytes_read < 0) { + errsv = errno; + snprintf(errbuf, + errbuf_len, + "error reading the file: %d (%s)", + errsv, + strerror_r(errsv, error, sizeof(error))); + return false; + } + + return true; +} + +/*****************************************************************************/ + /** * _nm_strerror_r: * @errsv: the errno passed to strerror_r() diff --git a/src/libnm-std-aux/nm-std-utils.h b/src/libnm-std-aux/nm-std-utils.h index 015444c93d..04c61c30f5 100644 --- a/src/libnm-std-aux/nm-std-utils.h +++ b/src/libnm-std-aux/nm-std-utils.h @@ -37,4 +37,8 @@ size_t nm_utils_get_next_realloc_size(bool true_realloc, size_t requested); const char *_nm_strerror_r(int errsv, char *buf, size_t buf_size); +bool nm_utils_set_effective_user(const char *user, char *errbuf, size_t errbuf_size); + +bool nm_utils_read_file_to_stdout(const char *filename, char *errbuf, size_t errbuf_len); + #endif /* __NM_STD_UTILS_H__ */ diff --git a/src/nm-helpers/README.md b/src/nm-helpers/README.md index ccaa5bf5cd..ab0ea02444 100644 --- a/src/nm-helpers/README.md +++ b/src/nm-helpers/README.md @@ -7,12 +7,13 @@ components. nm-daemon-helper ---------------- -A internal helper application that is spawned by NetworkManager -to perform certain actions. +A internal helper application that is spawned by NetworkManager to +perform certain actions which can't be done in the daemon. -Currently all it does is doing a reverse DNS lookup, which -cannot be done by NetworkManager because the operation requires -to reconfigure the libc resolver (which is a process-wide operation). +Currently it's used to do a reverse DNS lookup after reconfiguring the +libc resolver (which is a process-wide operation), and to read files +on behalf of unprivileged users (which requires a seteuid that affects +all the threads of the process). This is not directly useful to the user. diff --git a/src/nm-helpers/nm-daemon-helper.c b/src/nm-helpers/nm-daemon-helper.c index 32be93a4ef..759993a451 100644 --- a/src/nm-helpers/nm-daemon-helper.c +++ b/src/nm-helpers/nm-daemon-helper.c @@ -137,6 +137,37 @@ cmd_resolve_address(void) return RETURN_ERROR; } +static int +cmd_read_file_as_user(void) +{ + nm_auto_free char *user = NULL; + nm_auto_free char *filename = NULL; + char error[1024]; + + user = read_arg(); + if (!user) + return RETURN_INVALID_ARGS; + + filename = read_arg(); + if (!filename) + return RETURN_INVALID_ARGS; + + if (more_args()) + return RETURN_INVALID_ARGS; + + if (!nm_utils_set_effective_user(user, error, sizeof(error))) { + fprintf(stderr, "Failed to set effective user '%s': %s", user, error); + return RETURN_ERROR; + } + + if (!nm_utils_read_file_to_stdout(filename, error, sizeof(error))) { + fprintf(stderr, "Failed to read file '%s' as user '%s': %s", filename, user, error); + return RETURN_ERROR; + } + + return RETURN_SUCCESS; +} + int main(int argc, char **argv) { @@ -150,6 +181,8 @@ main(int argc, char **argv) return cmd_version(); if (nm_streq(cmd, "resolve-address")) return cmd_resolve_address(); + if (nm_streq(cmd, "read-file-as-user")) + return cmd_read_file_as_user(); return RETURN_INVALID_CMD; } From 4f3597448dd774729b74810de262f7fcffb6d2f9 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 16 Sep 2025 16:56:00 +0200 Subject: [PATCH 04/14] supplicant: remove blobs before adding new ones When connecting, we add the blobs to the Interface object of the supplicant. Those blobs are not removed on disconnect and so when we try to add blobs with the same id, the supplicant returns an error. Make sure we start from a clean slate on each connection attempt, by deleting all existing blobs. Probably we should also delete the added blobs on disconnect, but that's left for a future improvement. (cherry picked from commit 0093bbd9507df3b16eaa08cd3a6b799b678c7599) (cherry picked from commit ce3ebf6d3e5c511f872872bf51bee7f08db6f045) --- src/core/supplicant/nm-supplicant-interface.c | 191 +++++++++++++++--- 1 file changed, 159 insertions(+), 32 deletions(-) diff --git a/src/core/supplicant/nm-supplicant-interface.c b/src/core/supplicant/nm-supplicant-interface.c index 514b7c0d39..ef72447a5f 100644 --- a/src/core/supplicant/nm-supplicant-interface.c +++ b/src/core/supplicant/nm-supplicant-interface.c @@ -46,6 +46,7 @@ typedef struct { gpointer user_data; guint fail_on_idle_id; guint blobs_left; + guint remove_blobs_left; guint calls_left; struct _AddNetworkData *add_network_data; } AssocData; @@ -2264,6 +2265,7 @@ assoc_add_blob_cb(GObject *source, GAsyncResult *result, gpointer user_data) return; } + nm_assert(priv->assoc_data->blobs_left > 0); priv->assoc_data->blobs_left--; _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: blob added (%u left)", NM_HASH_OBFUSCATE_PTR(priv->assoc_data), @@ -2272,6 +2274,148 @@ assoc_add_blob_cb(GObject *source, GAsyncResult *result, gpointer user_data) assoc_call_select_network(self); } +static void +assoc_add_blobs(NMSupplicantInterface *self) +{ + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE(self); + GHashTable *blobs; + GHashTableIter iter; + const char *blob_name; + GBytes *blob_data; + + blobs = nm_supplicant_config_get_blobs(priv->assoc_data->cfg); + priv->assoc_data->blobs_left = nm_g_hash_table_size(blobs); + + _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: need to add %u blobs", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + priv->assoc_data->blobs_left); + + if (priv->assoc_data->blobs_left == 0) { + assoc_call_select_network(self); + return; + } + + g_hash_table_iter_init(&iter, blobs); + while (g_hash_table_iter_next(&iter, (gpointer) &blob_name, (gpointer) &blob_data)) { + _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: adding blob '%s'", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + blob_name); + _dbus_connection_call( + self, + NM_WPAS_DBUS_IFACE_INTERFACE, + "AddBlob", + g_variant_new("(s@ay)", blob_name, nm_g_bytes_to_variant_ay(blob_data)), + G_VARIANT_TYPE("()"), + G_DBUS_CALL_FLAGS_NONE, + DBUS_TIMEOUT_MSEC, + priv->assoc_data->cancellable, + assoc_add_blob_cb, + self); + } +} + +static void +assoc_remove_blob_cb(GObject *source, GAsyncResult *result, gpointer user_data) +{ + NMSupplicantInterface *self; + NMSupplicantInterfacePrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *res = NULL; + + res = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), result, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + self = NM_SUPPLICANT_INTERFACE(user_data); + priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE(self); + + /* We don't consider a failure fatal. The new association might be able + * to proceed even with the existing blobs, if they don't conflict with new + * ones. */ + + nm_assert(priv->assoc_data->remove_blobs_left > 0); + priv->assoc_data->remove_blobs_left--; + + if (error) { + g_dbus_error_strip_remote_error(error); + _LOGD("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: failed to delete blob: %s", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + error->message); + } else { + _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: blob removed (%u left)", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + priv->assoc_data->remove_blobs_left); + } + + if (priv->assoc_data->remove_blobs_left == 0) + assoc_add_blobs(self); +} + +static void +assoc_get_blobs_cb(GObject *source, GAsyncResult *result, gpointer user_data) +{ + NMSupplicantInterface *self; + NMSupplicantInterfacePrivate *priv; + gs_free_error GError *error = NULL; + gs_unref_variant GVariant *res = NULL; + gs_unref_variant GVariant *value = NULL; + GVariantIter iter; + const char *blob_name; + GVariant *blob_data; + + res = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), result, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + self = NM_SUPPLICANT_INTERFACE(user_data); + priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE(self); + + if (error) { + _LOGD("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: failed to get blob list: %s", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + error->message); + assoc_add_blobs(self); + return; + } + + g_variant_get(res, "(v)", &value); + + /* While the "Blobs" property is documented as type "as", it is actually "a{say}" */ + if (!value || !g_variant_is_of_type(value, G_VARIANT_TYPE("a{say}"))) { + _LOGD("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: failed to get blob list: wrong return type %s", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + value ? g_variant_get_type_string(value) : "NULL"); + assoc_add_blobs(self); + return; + } + + g_variant_iter_init(&iter, value); + priv->assoc_data->remove_blobs_left = g_variant_iter_n_children(&iter); + _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: need to delete %u blobs", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + priv->assoc_data->remove_blobs_left); + + if (priv->assoc_data->remove_blobs_left == 0) { + assoc_add_blobs(self); + } else { + while (g_variant_iter_loop(&iter, "{&s@ay}", &blob_name, &blob_data)) { + _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: removing blob '%s'", + NM_HASH_OBFUSCATE_PTR(priv->assoc_data), + blob_name); + _dbus_connection_call(self, + NM_WPAS_DBUS_IFACE_INTERFACE, + "RemoveBlob", + g_variant_new("(s)", blob_name), + G_VARIANT_TYPE("()"), + G_DBUS_CALL_FLAGS_NONE, + DBUS_TIMEOUT_MSEC, + priv->assoc_data->cancellable, + assoc_remove_blob_cb, + self); + } + } +} + static void assoc_add_network_cb(GObject *source, GAsyncResult *result, gpointer user_data) { @@ -2279,12 +2423,8 @@ assoc_add_network_cb(GObject *source, GAsyncResult *result, gpointer user_data) AssocData *assoc_data; NMSupplicantInterface *self; NMSupplicantInterfacePrivate *priv; - gs_unref_variant GVariant *res = NULL; - gs_free_error GError *error = NULL; - GHashTable *blobs; - GHashTableIter iter; - const char *blob_name; - GBytes *blob_data; + gs_unref_variant GVariant *res = NULL; + gs_free_error GError *error = NULL; nm_auto_ref_string NMRefString *name_owner = NULL; nm_auto_ref_string NMRefString *object_path = NULL; @@ -2336,34 +2476,21 @@ assoc_add_network_cb(GObject *source, GAsyncResult *result, gpointer user_data) nm_assert(!priv->net_path); g_variant_get(res, "(o)", &priv->net_path); - /* Send blobs first; otherwise jump to selecting the network */ - blobs = nm_supplicant_config_get_blobs(priv->assoc_data->cfg); - priv->assoc_data->blobs_left = blobs ? g_hash_table_size(blobs) : 0u; - - _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: network added (%s) (%u blobs left)", + _LOGT("assoc[" NM_HASH_OBFUSCATE_PTR_FMT "]: network added (%s)", NM_HASH_OBFUSCATE_PTR(priv->assoc_data), - priv->net_path, - priv->assoc_data->blobs_left); + priv->net_path); - if (priv->assoc_data->blobs_left == 0) { - assoc_call_select_network(self); - return; - } - - g_hash_table_iter_init(&iter, blobs); - while (g_hash_table_iter_next(&iter, (gpointer) &blob_name, (gpointer) &blob_data)) { - _dbus_connection_call( - self, - NM_WPAS_DBUS_IFACE_INTERFACE, - "AddBlob", - g_variant_new("(s@ay)", blob_name, nm_g_bytes_to_variant_ay(blob_data)), - G_VARIANT_TYPE("()"), - G_DBUS_CALL_FLAGS_NONE, - DBUS_TIMEOUT_MSEC, - priv->assoc_data->cancellable, - assoc_add_blob_cb, - self); - } + /* Delete any existing blobs before adding new ones */ + _dbus_connection_call(self, + DBUS_INTERFACE_PROPERTIES, + "Get", + g_variant_new("(ss)", NM_WPAS_DBUS_IFACE_INTERFACE, "Blobs"), + G_VARIANT_TYPE("(v)"), + G_DBUS_CALL_FLAGS_NONE, + DBUS_TIMEOUT_MSEC, + priv->assoc_data->cancellable, + assoc_get_blobs_cb, + self); } static void From 7acf70dfb912ca65e1daaf669ad5f4ed1faadbcb Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 16 Sep 2025 16:57:19 +0200 Subject: [PATCH 05/14] core: support returning binary output from the daemon helper The full output of the daemon helper is added to a NMStrBuf, without interpreting it as a string (that is, without stopping at the first NUL character). However, when we retrieve the content from the NMStrBuf we assume it's a string. This is fine for certain commands that expect a string output, but it's not for other commands as the read-file-as-user one. Add a new argument to nm_utils_spawn_helper() to specify whether the output is binary or not. Also have different finish functions depending on the return type. (cherry picked from commit 1d90d50fc6e8c167581c6831c2511bc4148f234b) (cherry picked from commit 59df5fc93fc30c9b8c9ceca3c42b173f831f53f7) --- src/core/devices/nm-device-utils.c | 3 ++- src/core/nm-core-utils.c | 39 ++++++++++++++++++++++++++---- src/core/nm-core-utils.h | 4 ++- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/core/devices/nm-device-utils.c b/src/core/devices/nm-device-utils.c index a78499fb22..3e86e2e8cb 100644 --- a/src/core/devices/nm-device-utils.c +++ b/src/core/devices/nm-device-utils.c @@ -237,7 +237,7 @@ resolve_addr_helper_cb(GObject *source, GAsyncResult *result, gpointer user_data gs_free_error GError *error = NULL; gs_free char *output = NULL; - output = nm_utils_spawn_helper_finish(result, &error); + output = nm_utils_spawn_helper_finish_string(result, &error); if (nm_utils_error_is_cancelled(error)) return; @@ -276,6 +276,7 @@ resolve_addr_spawn_helper(ResolveAddrInfo *info, ResolveAddrService services) nm_inet_ntop(info->addr_family, &info->address, addr_str); _LOG2D(info, "start lookup via nm-daemon-helper using services: %s", str); nm_utils_spawn_helper(NM_MAKE_STRV("resolve-address", addr_str, str), + FALSE, g_task_get_cancellable(info->task), resolve_addr_helper_cb, info); diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index 7560d98b09..447bbd530c 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -5011,6 +5011,7 @@ typedef struct { int child_stdin; int child_stdout; int child_stderr; + gboolean binary_output; GSource *input_source; GSource *output_source; GSource *error_source; @@ -5090,9 +5091,17 @@ helper_complete(HelperInfo *info, GError *error) } nm_clear_g_cancellable_disconnect(g_task_get_cancellable(info->task), &info->cancellable_id); - g_task_return_pointer(info->task, - nm_str_buf_finalize(&info->in_buffer, NULL) ?: g_new0(char, 1), - g_free); + + if (info->binary_output) { + g_task_return_pointer( + info->task, + g_bytes_new(nm_str_buf_get_str_unsafe(&info->in_buffer), info->in_buffer.len), + (GDestroyNotify) (g_bytes_unref)); + } else { + g_task_return_pointer(info->task, + nm_str_buf_finalize(&info->in_buffer, NULL) ?: g_new0(char, 1), + g_free); + } helper_info_free(info); } @@ -5235,6 +5244,7 @@ helper_cancelled(GObject *object, gpointer user_data) void nm_utils_spawn_helper(const char *const *args, + gboolean binary_output, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer cb_data) @@ -5250,9 +5260,14 @@ nm_utils_spawn_helper(const char *const *args, info = g_new(HelperInfo, 1); *info = (HelperInfo) { - .task = nm_g_task_new(NULL, cancellable, nm_utils_spawn_helper, callback, cb_data), + .task = nm_g_task_new(NULL, cancellable, nm_utils_spawn_helper, callback, cb_data), + .binary_output = binary_output, }; + /* Store if the caller requested binary output so that we can check later + * that the right result function is called. */ + g_task_set_task_data(info->task, GINT_TO_POINTER(binary_output), NULL); + if (!g_spawn_async_with_pipes("/", (char **) NM_MAKE_STRV(LIBEXECDIR "/nm-daemon-helper"), (char **) NM_MAKE_STRV(), @@ -5363,11 +5378,25 @@ nm_utils_spawn_helper(const char *const *args, } char * -nm_utils_spawn_helper_finish(GAsyncResult *result, GError **error) +nm_utils_spawn_helper_finish_string(GAsyncResult *result, GError **error) { GTask *task = G_TASK(result); nm_assert(nm_g_task_is_valid(result, NULL, nm_utils_spawn_helper)); + /* Check binary_output */ + nm_assert(GPOINTER_TO_INT(g_task_get_task_data(task)) == FALSE); + + return g_task_propagate_pointer(task, error); +} + +GBytes * +nm_utils_spawn_helper_finish_binary(GAsyncResult *result, GError **error) +{ + GTask *task = G_TASK(result); + + nm_assert(nm_g_task_is_valid(result, NULL, nm_utils_spawn_helper)); + /* Check binary_output */ + nm_assert(GPOINTER_TO_INT(g_task_get_task_data(task)) == TRUE); return g_task_propagate_pointer(task, error); } diff --git a/src/core/nm-core-utils.h b/src/core/nm-core-utils.h index a647eab1b9..b6a7dd4e52 100644 --- a/src/core/nm-core-utils.h +++ b/src/core/nm-core-utils.h @@ -478,11 +478,13 @@ guint8 nm_wifi_utils_level_to_quality(int val); /*****************************************************************************/ void nm_utils_spawn_helper(const char *const *args, + gboolean binary_output, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer cb_data); -char *nm_utils_spawn_helper_finish(GAsyncResult *result, GError **error); +char *nm_utils_spawn_helper_finish_string(GAsyncResult *result, GError **error); +GBytes *nm_utils_spawn_helper_finish_binary(GAsyncResult *result, GError **error); /*****************************************************************************/ From b7926872e154fb508691de8aa8885de86b96456f Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 16 Sep 2025 16:58:31 +0200 Subject: [PATCH 06/14] supplicant: rename variables Rename uid to to blob_id, and con_uid to con_uuid. (cherry picked from commit 586f7700b8ad6b4b4cffdb4cdb2bed2e4726ef5c) (cherry picked from commit a17f51fe156ed63882d5dc49e594e23913f883fd) --- src/core/supplicant/nm-supplicant-config.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/supplicant/nm-supplicant-config.c b/src/core/supplicant/nm-supplicant-config.c index 9e06639510..faf48b3be8 100644 --- a/src/core/supplicant/nm-supplicant-config.c +++ b/src/core/supplicant/nm-supplicant-config.c @@ -258,19 +258,19 @@ static gboolean nm_supplicant_config_add_blob_for_connection(NMSupplicantConfig *self, GBytes *field, const char *name, - const char *con_uid, + const char *con_uuid, GError **error) { if (field && g_bytes_get_size(field)) { - gs_free char *uid = NULL; + gs_free char *blob_id = NULL; char *p; - uid = g_strdup_printf("%s-%s", con_uid, name); - for (p = uid; *p; p++) { + blob_id = g_strdup_printf("%s-%s", con_uuid, name); + for (p = blob_id; *p; p++) { if (*p == '/') *p = '-'; } - if (!nm_supplicant_config_add_blob(self, name, field, uid, error)) + if (!nm_supplicant_config_add_blob(self, name, field, blob_id, error)) return FALSE; } return TRUE; From 399d7be7712b268c630f9a62b27b1b7cf904c307 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 23 Sep 2025 17:00:53 +0200 Subject: [PATCH 07/14] core: add functions to read private files of connections Add function nm_utils_read_private_files(). It can be used to read a list of paths as the given user. It spawns the daemon-helper to read each path and returns asynchronously a hash table containing the files content. Also add nm_utils_get_connection_private_files_paths() to return a list of file paths referenced in a connection. The function currently returns only 802.1x file paths for certificates and keys. (cherry picked from commit de4eb64253d493364d676b509f63f2e8d1810061) (cherry picked from commit 9432822f3460975520aeba4b3367108d5322b28a) --- src/core/nm-core-utils.c | 199 +++++++++++++++++++++++++++++++++++++++ src/core/nm-core-utils.h | 11 +++ 2 files changed, 210 insertions(+) diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index 447bbd530c..4531fdac04 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -5514,3 +5514,202 @@ nm_utils_get_connection_first_permissions_user(NMConnection *connection) return _nm_setting_connection_get_first_permissions_user(s_con); } + +/*****************************************************************************/ + +static void +get_8021x_private_files(NMConnection *connection, GPtrArray *files) +{ + const struct { + NMSetting8021xCKScheme (*get_scheme_func)(NMSetting8021x *); + const char *(*get_path_func)(NMSetting8021x *); + } funcs[] = { + {nm_setting_802_1x_get_ca_cert_scheme, nm_setting_802_1x_get_ca_cert_path}, + {nm_setting_802_1x_get_client_cert_scheme, nm_setting_802_1x_get_client_cert_path}, + {nm_setting_802_1x_get_private_key_scheme, nm_setting_802_1x_get_private_key_path}, + {nm_setting_802_1x_get_phase2_ca_cert_scheme, nm_setting_802_1x_get_phase2_ca_cert_path}, + {nm_setting_802_1x_get_phase2_client_cert_scheme, + nm_setting_802_1x_get_phase2_client_cert_path}, + {nm_setting_802_1x_get_phase2_private_key_scheme, + nm_setting_802_1x_get_phase2_private_key_path}, + }; + NMSetting8021x *s_8021x; + const char *path; + guint i; + + s_8021x = nm_connection_get_setting_802_1x(connection); + if (!s_8021x) + return; + + for (i = 0; i < G_N_ELEMENTS(funcs); i++) { + if (funcs[i].get_scheme_func(s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH) { + path = funcs[i].get_path_func(s_8021x); + if (path) { + g_ptr_array_add(files, (gpointer) path); + } + } + } +} + +const char ** +nm_utils_get_connection_private_files_paths(NMConnection *connection) +{ + GPtrArray *files; + + files = g_ptr_array_new(); + get_8021x_private_files(connection, files); + g_ptr_array_add(files, NULL); + + return (const char **) g_ptr_array_free(files, files->len == 1); +} + +typedef struct _ReadInfo ReadInfo; + +typedef struct { + char *path; + ReadInfo *read_info; +} FileInfo; + +struct _ReadInfo { + GTask *task; + GHashTable *table; + GPtrArray *file_infos; /* of FileInfo */ + GError *first_error; + guint num_pending; +}; + +static void +read_file_helper_cb(GObject *source, GAsyncResult *result, gpointer user_data) +{ + FileInfo *file_info = user_data; + ReadInfo *read_info = file_info->read_info; + gs_unref_bytes GBytes *output = NULL; + gs_free_error GError *error = NULL; + + output = nm_utils_spawn_helper_finish_binary(result, &error); + + nm_assert(read_info->num_pending > 0); + read_info->num_pending--; + + if (nm_utils_error_is_cancelled(error)) { + /* nop */ + } else if (error) { + nm_log_dbg(LOGD_CORE, + "read-private-files: failed to read file '%s': %s", + file_info->path, + error->message); + if (!read_info->first_error) { + /* @error just says "helper process exited with status X". + * Return a more human-friendly one. */ + read_info->first_error = g_error_new(NM_UTILS_ERROR, + NM_UTILS_ERROR_UNKNOWN, + "error reading file '%s'", + file_info->path); + } + } else { + nm_log_dbg(LOGD_SUPPLICANT, + "read-private-files: successfully read file '%s'", + file_info->path); + + /* Store the file contents in the hash table */ + if (!read_info->table) { + read_info->table = g_hash_table_new_full(nm_str_hash, + g_str_equal, + g_free, + (GDestroyNotify) g_bytes_unref); + } + g_hash_table_insert(read_info->table, + g_steal_pointer(&file_info->path), + g_steal_pointer(&output)); + } + + g_clear_pointer(&file_info->path, g_free); + + /* If all operations are completed, return */ + if (read_info->num_pending == 0) { + if (read_info->first_error) { + g_task_return_error(read_info->task, g_steal_pointer(&read_info->first_error)); + } else { + g_task_return_pointer(read_info->task, + g_steal_pointer(&read_info->table), + (GDestroyNotify) g_hash_table_unref); + } + + if (read_info->table) + g_hash_table_unref(read_info->table); + if (read_info->file_infos) + g_ptr_array_unref(read_info->file_infos); + + g_object_unref(read_info->task); + g_free(read_info); + } +} + +/** + * nm_utils_read_private_files: + * @paths: array of file paths to be read + * @user: name of the user to impersonate when reading the files + * @cancellable: cancellable to cancel the operation + * @callback: callback to invoke on completion + * @cb_data: data for @callback + * + * Reads the given list of files @paths on behalf of user @user. Invokes + * @callback asynchronously on completion. The callback must use + * nm_utils_read_private_files_finish() to obtain the result. + */ +void +nm_utils_read_private_files(const char *const *paths, + const char *user, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer cb_data) +{ + ReadInfo *read_info; + FileInfo *file_info; + guint i; + + g_return_if_fail(paths && paths[0]); + g_return_if_fail(cancellable); + g_return_if_fail(callback); + g_return_if_fail(cb_data); + + read_info = g_new(ReadInfo, 1); + *read_info = (ReadInfo) { + .task = nm_g_task_new(NULL, cancellable, nm_utils_read_private_files, callback, cb_data), + .file_infos = g_ptr_array_new_with_free_func(g_free), + }; + + for (i = 0; paths[i]; i++) { + file_info = g_new(FileInfo, 1); + *file_info = (FileInfo) { + .path = g_strdup(paths[i]), + .read_info = read_info, + }; + g_ptr_array_add(read_info->file_infos, file_info); + read_info->num_pending++; + + nm_utils_spawn_helper(NM_MAKE_STRV("read-file-as-user", user, paths[i]), + TRUE, + cancellable, + read_file_helper_cb, + file_info); + } +} + +/** + * nm_utils_read_private_files_finish: + * @result: the GAsyncResult + * @error: on return, the error + * + * Returns the files read by nm_utils_read_private_files(). The return value + * is a hash table {char * -> GBytes *}. Free it with g_hash_table_unref(). + */ +GHashTable * +nm_utils_read_private_files_finish(GAsyncResult *result, GError **error) +{ + GTask *task = G_TASK(result); + + nm_assert(nm_g_task_is_valid(result, NULL, nm_utils_read_private_files)); + + return g_task_propagate_pointer(task, error); +} diff --git a/src/core/nm-core-utils.h b/src/core/nm-core-utils.h index b6a7dd4e52..224018c6d0 100644 --- a/src/core/nm-core-utils.h +++ b/src/core/nm-core-utils.h @@ -496,4 +496,15 @@ gid_t nm_utils_get_nm_gid(void); const char *nm_utils_get_connection_first_permissions_user(NMConnection *connection); +/*****************************************************************************/ + +const char **nm_utils_get_connection_private_files_paths(NMConnection *connection); + +void nm_utils_read_private_files(const char *const *paths, + const char *user, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer cb_data); +GHashTable *nm_utils_read_private_files_finish(GAsyncResult *result, GError **error); + #endif /* __NM_CORE_UTILS_H__ */ From b8f8731636ff1de6a2e44b17e95ae27320e9dc4e Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 23 Sep 2025 17:04:02 +0200 Subject: [PATCH 08/14] device: read private files in stage2 During stage2 (prepare) of an activation, check if the connection is private and if it contains any certificate/key path. If so, start reading the files and delay stage2. Once done, store the files' content into priv->private_files.table and continue the activation. (cherry picked from commit 98e6dbdf21e5b165bae498ab2a29bb14f331ccd1) (cherry picked from commit a417df34847ae7cd1eb0d77af8b70beb6619cfbe) --- src/core/devices/nm-device-private.h | 2 + src/core/devices/nm-device.c | 127 ++++++++++++++++++++++++++- 2 files changed, 128 insertions(+), 1 deletion(-) diff --git a/src/core/devices/nm-device-private.h b/src/core/devices/nm-device-private.h index 2f73a01bac..2568d9f1a1 100644 --- a/src/core/devices/nm-device-private.h +++ b/src/core/devices/nm-device-private.h @@ -179,4 +179,6 @@ void nm_device_auth_request(NMDevice *self, void nm_device_link_properties_set(NMDevice *self, gboolean reapply); +GHashTable *nm_device_get_private_files(NMDevice *self); + #endif /* NM_DEVICE_PRIVATE_H */ diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 6bba982af6..b0a3f7806f 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -335,6 +335,12 @@ typedef struct { int addr_family; } HostnameResolver; +typedef enum { + PRIVATE_FILES_STATE_UNKNOWN = 0, + PRIVATE_FILES_STATE_READING, + PRIVATE_FILES_STATE_DONE, +} PrivateFilesState; + /*****************************************************************************/ enum { @@ -771,6 +777,13 @@ typedef struct _NMDevicePrivate { guint64 rx_bytes; } stats; + struct { + GHashTable *table; + GCancellable *cancellable; + char *user; + PrivateFilesState state; + } private_files; + bool mtu_force_set_done : 1; bool needs_ip6_subnet : 1; @@ -10807,6 +10820,49 @@ tc_commit(NMDevice *self) return TRUE; } +static void +read_private_files_cb(GObject *source_object, GAsyncResult *result, gpointer data) +{ + gs_unref_hashtable GHashTable *table = NULL; + gs_free_error GError *error = NULL; + NMDevice *self; + NMDevicePrivate *priv; + + table = nm_utils_read_private_files_finish(result, &error); + if (nm_utils_error_is_cancelled(error)) + return; + + self = NM_DEVICE(data); + priv = NM_DEVICE_GET_PRIVATE(self); + + if (error) { + NMConnection *connection = nm_device_get_applied_connection(self); + + _LOGW(LOGD_DEVICE, + "could not read files for private connection %s owned by user '%s': %s", + connection ? nm_connection_get_uuid(connection) : NULL, + priv->private_files.user, + error->message); + nm_device_state_changed(self, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_CONFIG_FAILED); + return; + } + + _LOGD(LOGD_DEVICE, "private files successfully read"); + + priv->private_files.state = PRIVATE_FILES_STATE_DONE; + priv->private_files.table = g_steal_pointer(&table); + g_clear_pointer(&priv->private_files.user, g_free); + g_clear_object(&priv->private_files.cancellable); + + nm_device_activate_schedule_stage2_device_config(self, FALSE); +} + +GHashTable * +nm_device_get_private_files(NMDevice *self) +{ + return NM_DEVICE_GET_PRIVATE(self)->private_files.table; +} + /* * activate_stage2_device_config * @@ -10819,6 +10875,7 @@ activate_stage2_device_config(NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); NMDeviceClass *klass = NM_DEVICE_GET_CLASS(self); + NMConnection *applied; NMActStageReturn ret; NMSettingWired *s_wired; gboolean no_firmware = FALSE; @@ -10827,6 +10884,68 @@ activate_stage2_device_config(NMDevice *self) nm_device_state_changed(self, NM_DEVICE_STATE_CONFIG, NM_DEVICE_STATE_REASON_NONE); + applied = nm_device_get_applied_connection(self); + + /* If the connection is private (owned by a specific user), we need to + * verify that the user has permission to access any files specified in + * the connection, such as certificates and keys. We do that by calling + * nm_utils_read_private_files() and saving the file contents in a hash + * table that can be accessed later during the activation. It is important + * to never access the files again to avoid TOCTOU bugs. + */ + switch (priv->private_files.state) { + case PRIVATE_FILES_STATE_UNKNOWN: + { + gs_free const char **paths = NULL; + NMSettingConnection *s_con; + const char *user; + + s_con = nm_connection_get_setting_connection(applied); + nm_assert(s_con); + user = _nm_setting_connection_get_first_permissions_user(s_con); + + priv->private_files.user = g_strdup(user); + if (!priv->private_files.user) { + priv->private_files.state = PRIVATE_FILES_STATE_DONE; + break; + } + + paths = nm_utils_get_connection_private_files_paths(applied); + if (!paths) { + priv->private_files.state = PRIVATE_FILES_STATE_DONE; + break; + } + + if (_nm_setting_connection_get_num_permissions_users(s_con) > 1) { + _LOGW(LOGD_DEVICE, + "private connections with multiple users are not allowed to reference " + "certificates and keys on the filesystem. Specify only one user in the " + "connection.permissions property."); + nm_device_state_changed(self, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_CONFIG_FAILED); + return; + } + + priv->private_files.state = PRIVATE_FILES_STATE_READING; + priv->private_files.cancellable = g_cancellable_new(); + + _LOGD(LOGD_DEVICE, "reading private files"); + nm_utils_read_private_files(paths, + priv->private_files.user, + priv->private_files.cancellable, + read_private_files_cb, + self); + return; + } + case PRIVATE_FILES_STATE_READING: + /* wait */ + return; + case PRIVATE_FILES_STATE_DONE: + /* proceed */ + break; + } + if (!nm_device_managed_type_is_external(self)) { _ethtool_state_set(self); nm_device_link_properties_set(self, FALSE); @@ -10843,7 +10962,7 @@ activate_stage2_device_config(NMDevice *self) priv->tc_committed = TRUE; } - nm_routing_rules_sync(nm_device_get_applied_connection(self), + nm_routing_rules_sync(applied, NM_TERNARY_TRUE, klass->get_extra_rules, self, @@ -17154,6 +17273,12 @@ nm_device_cleanup(NMDevice *self, NMDeviceStateReason reason, CleanupType cleanu if (klass->deactivate) klass->deactivate(self); + /* Clean up private files */ + nm_clear_g_cancellable(&priv->private_files.cancellable); + g_clear_pointer(&priv->private_files.table, g_hash_table_unref); + g_clear_pointer(&priv->private_files.user, g_free); + priv->private_files.state = PRIVATE_FILES_STATE_UNKNOWN; + ifindex = nm_device_get_ip_ifindex(self); if (cleanup_type == CLEANUP_TYPE_DECONFIGURE) { From f08ee617b99a25c7c3d4d820fbe2539f9b78e986 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 23 Sep 2025 17:06:39 +0200 Subject: [PATCH 09/14] core: pass certificates as blobs to supplicant for private connections In case of private connections, the device has already read the certificates and keys content from disk, validating that the owner of the connection has access to them. Pass those files as blobs to the supplicant so that it doesn't have to read them again from the filesystem, creating the opportunity for TOCTOU bugs. (cherry picked from commit 36ea70c0993cb48d3155c2de6d6c8e48a2b08c60) (cherry picked from commit aac5b80fcad34489e737b6eb1c5389bd32169d23) --- NEWS | 9 + src/core/devices/nm-device-ethernet.c | 11 +- src/core/devices/nm-device-macsec.c | 11 +- src/core/devices/wifi/nm-device-wifi.c | 4 +- src/core/supplicant/nm-supplicant-config.c | 160 ++++++++++++------ src/core/supplicant/nm-supplicant-config.h | 4 +- .../supplicant/tests/test-supplicant-config.c | 4 +- 7 files changed, 143 insertions(+), 60 deletions(-) diff --git a/NEWS b/NEWS index cb32853c3d..d611f669b2 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,12 @@ +=============================================== +NetworkManager-1.54.3 +Overview of changes since NetworkManager-1.54.2 +=============================================== + +* For private connections (the ones that specify a user in the + "connection.permissions" property), verify that the user can access + the 802.1X certificates and keys set in the connection. + =============================================== NetworkManager-1.54.2 Overview of changes since NetworkManager-1.54.1 diff --git a/src/core/devices/nm-device-ethernet.c b/src/core/devices/nm-device-ethernet.c index 4034fdaad6..db1245b330 100644 --- a/src/core/devices/nm-device-ethernet.c +++ b/src/core/devices/nm-device-ethernet.c @@ -630,10 +630,17 @@ build_supplicant_config(NMDeviceEthernet *self, GError **error) mtu = nm_platform_link_get_mtu(nm_device_get_platform(NM_DEVICE(self)), nm_device_get_ifindex(NM_DEVICE(self))); - config = nm_supplicant_config_new(NM_SUPPL_CAP_MASK_NONE); + config = nm_supplicant_config_new(NM_SUPPL_CAP_MASK_NONE, + nm_utils_get_connection_first_permissions_user(connection)); security = nm_connection_get_setting_802_1x(connection); - if (!nm_supplicant_config_add_setting_8021x(config, security, con_uuid, mtu, TRUE, error)) { + if (!nm_supplicant_config_add_setting_8021x(config, + security, + con_uuid, + mtu, + TRUE, + nm_device_get_private_files(NM_DEVICE(self)), + error)) { g_prefix_error(error, "802-1x-setting: "); g_clear_object(&config); } diff --git a/src/core/devices/nm-device-macsec.c b/src/core/devices/nm-device-macsec.c index 2ff1eeb30a..1659ea0527 100644 --- a/src/core/devices/nm-device-macsec.c +++ b/src/core/devices/nm-device-macsec.c @@ -201,7 +201,8 @@ build_supplicant_config(NMDeviceMacsec *self, GError **error) mtu = nm_platform_link_get_mtu(nm_device_get_platform(NM_DEVICE(self)), nm_device_get_ifindex(NM_DEVICE(self))); - config = nm_supplicant_config_new(NM_SUPPL_CAP_MASK_NONE); + config = nm_supplicant_config_new(NM_SUPPL_CAP_MASK_NONE, + nm_utils_get_connection_first_permissions_user(connection)); s_macsec = nm_device_get_applied_setting(NM_DEVICE(self), NM_TYPE_SETTING_MACSEC); @@ -227,7 +228,13 @@ build_supplicant_config(NMDeviceMacsec *self, GError **error) if (nm_setting_macsec_get_mode(s_macsec) == NM_SETTING_MACSEC_MODE_EAP) { s_8021x = nm_connection_get_setting_802_1x(connection); - if (!nm_supplicant_config_add_setting_8021x(config, s_8021x, con_uuid, mtu, TRUE, error)) { + if (!nm_supplicant_config_add_setting_8021x(config, + s_8021x, + con_uuid, + mtu, + TRUE, + nm_device_get_private_files(NM_DEVICE(self)), + error)) { g_prefix_error(error, "802-1x-setting: "); return NULL; } diff --git a/src/core/devices/wifi/nm-device-wifi.c b/src/core/devices/wifi/nm-device-wifi.c index b890b11052..148caa11d6 100644 --- a/src/core/devices/wifi/nm-device-wifi.c +++ b/src/core/devices/wifi/nm-device-wifi.c @@ -2946,7 +2946,8 @@ build_supplicant_config(NMDeviceWifi *self, s_wireless = nm_connection_get_setting_wireless(connection); g_return_val_if_fail(s_wireless != NULL, NULL); - config = nm_supplicant_config_new(nm_supplicant_interface_get_capabilities(priv->sup_iface)); + config = nm_supplicant_config_new(nm_supplicant_interface_get_capabilities(priv->sup_iface), + nm_utils_get_connection_first_permissions_user(connection)); /* Warn if AP mode may not be supported */ if (nm_streq0(nm_setting_wireless_get_mode(s_wireless), NM_SETTING_WIRELESS_MODE_AP) @@ -3022,6 +3023,7 @@ build_supplicant_config(NMDeviceWifi *self, mtu, pmf, fils, + nm_device_get_private_files(NM_DEVICE(self)), error)) { g_prefix_error(error, "802-11-wireless-security: "); goto error; diff --git a/src/core/supplicant/nm-supplicant-config.c b/src/core/supplicant/nm-supplicant-config.c index faf48b3be8..56cc832b57 100644 --- a/src/core/supplicant/nm-supplicant-config.c +++ b/src/core/supplicant/nm-supplicant-config.c @@ -30,6 +30,7 @@ typedef struct { typedef struct { GHashTable *config; GHashTable *blobs; + char *private_user; NMSupplCapMask capabilities; guint32 ap_scan; bool fast_required : 1; @@ -60,7 +61,7 @@ _get_capability(NMSupplicantConfigPrivate *priv, NMSupplCapType type) } NMSupplicantConfig * -nm_supplicant_config_new(NMSupplCapMask capabilities) +nm_supplicant_config_new(NMSupplCapMask capabilities, const char *private_user) { NMSupplicantConfigPrivate *priv; NMSupplicantConfig *self; @@ -69,6 +70,7 @@ nm_supplicant_config_new(NMSupplCapMask capabilities) priv = NM_SUPPLICANT_CONFIG_GET_PRIVATE(self); priv->capabilities = capabilities; + priv->private_user = g_strdup(private_user); return self; } @@ -283,6 +285,7 @@ nm_supplicant_config_finalize(GObject *object) g_hash_table_destroy(priv->config); nm_clear_pointer(&priv->blobs, g_hash_table_destroy); + nm_clear_pointer(&priv->private_user, g_free); G_OBJECT_CLASS(nm_supplicant_config_parent_class)->finalize(object); } @@ -930,6 +933,7 @@ nm_supplicant_config_add_setting_wireless_security(NMSupplicantConfig guint32 mtu, NMSettingWirelessSecurityPmf pmf, NMSettingWirelessSecurityFils fils, + GHashTable *files, GError **error) { NMSupplicantConfigPrivate *priv = NM_SUPPLICANT_CONFIG_GET_PRIVATE(self); @@ -1284,6 +1288,7 @@ nm_supplicant_config_add_setting_wireless_security(NMSupplicantConfig con_uuid, mtu, FALSE, + files, error)) return FALSE; } @@ -1365,6 +1370,7 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, const char *con_uuid, guint32 mtu, gboolean wired, + GHashTable *files, GError **error) { NMSupplicantConfigPrivate *priv; @@ -1594,24 +1600,21 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, } /* CA certificate */ + path = NULL; + bytes = NULL; if (ca_cert_override) { - if (!add_string_val(self, ca_cert_override, "ca_cert", FALSE, NULL, error)) - return FALSE; + /* This is a build-time-configured system-wide file path, no need to pass + * it as a blob */ + path = ca_cert_override; } else { switch (nm_setting_802_1x_get_ca_cert_scheme(setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: bytes = nm_setting_802_1x_get_ca_cert_blob(setting); - if (!nm_supplicant_config_add_blob_for_connection(self, - bytes, - "ca_cert", - con_uuid, - error)) - return FALSE; break; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_ca_cert_path(setting); - if (!add_string_val(self, path, "ca_cert", FALSE, NULL, error)) - return FALSE; + if (priv->private_user) + bytes = nm_g_hash_table_lookup(files, path); break; case NM_SETTING_802_1X_CK_SCHEME_PKCS11: if (!add_pkcs11_uri_with_pin(self, @@ -1627,26 +1630,32 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, break; } } + if (bytes) { + if (!nm_supplicant_config_add_blob_for_connection(self, bytes, "ca_cert", con_uuid, error)) + return FALSE; + } else if (path) { + /* Private connections cannot use paths other than the system CA store */ + g_return_val_if_fail(ca_cert_override || !priv->private_user, FALSE); + if (!add_string_val(self, path, "ca_cert", FALSE, NULL, error)) + return FALSE; + } /* Phase 2 CA certificate */ + path = NULL; + bytes = NULL; if (ca_cert_override) { - if (!add_string_val(self, ca_cert_override, "ca_cert2", FALSE, NULL, error)) - return FALSE; + /* This is a build-time-configured system-wide file path, no need to pass + * it as a blob */ + path = ca_cert_override; } else { switch (nm_setting_802_1x_get_phase2_ca_cert_scheme(setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: bytes = nm_setting_802_1x_get_phase2_ca_cert_blob(setting); - if (!nm_supplicant_config_add_blob_for_connection(self, - bytes, - "ca_cert2", - con_uuid, - error)) - return FALSE; break; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_phase2_ca_cert_path(setting); - if (!add_string_val(self, path, "ca_cert2", FALSE, NULL, error)) - return FALSE; + if (priv->private_user) + bytes = nm_g_hash_table_lookup(files, path); break; case NM_SETTING_802_1X_CK_SCHEME_PKCS11: if (!add_pkcs11_uri_with_pin( @@ -1663,6 +1672,15 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, break; } } + if (bytes) { + if (!nm_supplicant_config_add_blob_for_connection(self, bytes, "ca_cert2", con_uuid, error)) + return FALSE; + } else if (path) { + /* Private connections cannot use paths other than the system CA store */ + g_return_val_if_fail(ca_cert_override || !priv->private_user, FALSE); + if (!add_string_val(self, path, "ca_cert2", FALSE, NULL, error)) + return FALSE; + } /* Subject match */ value = nm_setting_802_1x_get_subject_match(setting); @@ -1714,21 +1732,17 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, /* Private key */ added = FALSE; + path = NULL; + bytes = NULL; switch (nm_setting_802_1x_get_private_key_scheme(setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: bytes = nm_setting_802_1x_get_private_key_blob(setting); - if (!nm_supplicant_config_add_blob_for_connection(self, - bytes, - "private_key", - con_uuid, - error)) - return FALSE; added = TRUE; break; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_private_key_path(setting); - if (!add_string_val(self, path, "private_key", FALSE, NULL, error)) - return FALSE; + if (priv->private_user) + bytes = nm_g_hash_table_lookup(files, path); added = TRUE; break; case NM_SETTING_802_1X_CK_SCHEME_PKCS11: @@ -1745,6 +1759,19 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, default: break; } + if (bytes) { + if (!nm_supplicant_config_add_blob_for_connection(self, + bytes, + "private_key", + con_uuid, + error)) + return FALSE; + } else if (path) { + /* Private connections cannot use paths */ + g_return_val_if_fail(!priv->private_user, FALSE); + if (!add_string_val(self, path, "private_key", FALSE, NULL, error)) + return FALSE; + } if (added) { NMSetting8021xCKFormat format; @@ -1768,20 +1795,16 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, /* Only add the client cert if the private key is not PKCS#12, as * wpa_supplicant configuration directs us to do. */ + path = NULL; + bytes = NULL; switch (nm_setting_802_1x_get_client_cert_scheme(setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: bytes = nm_setting_802_1x_get_client_cert_blob(setting); - if (!nm_supplicant_config_add_blob_for_connection(self, - bytes, - "client_cert", - con_uuid, - error)) - return FALSE; break; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_client_cert_path(setting); - if (!add_string_val(self, path, "client_cert", FALSE, NULL, error)) - return FALSE; + if (priv->private_user) + bytes = nm_g_hash_table_lookup(files, path); break; case NM_SETTING_802_1X_CK_SCHEME_PKCS11: if (!add_pkcs11_uri_with_pin( @@ -1797,26 +1820,35 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, default: break; } + if (bytes) { + if (!nm_supplicant_config_add_blob_for_connection(self, + bytes, + "client_cert", + con_uuid, + error)) + return FALSE; + } else if (path) { + /* Private connections cannot use paths */ + g_return_val_if_fail(!priv->private_user, FALSE); + if (!add_string_val(self, path, "client_cert", FALSE, NULL, error)) + return FALSE; + } } } /* Phase 2 private key */ added = FALSE; + path = NULL; + bytes = NULL; switch (nm_setting_802_1x_get_phase2_private_key_scheme(setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: bytes = nm_setting_802_1x_get_phase2_private_key_blob(setting); - if (!nm_supplicant_config_add_blob_for_connection(self, - bytes, - "private_key2", - con_uuid, - error)) - return FALSE; added = TRUE; break; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_phase2_private_key_path(setting); - if (!add_string_val(self, path, "private_key2", FALSE, NULL, error)) - return FALSE; + if (priv->private_user) + bytes = nm_g_hash_table_lookup(files, path); added = TRUE; break; case NM_SETTING_802_1X_CK_SCHEME_PKCS11: @@ -1834,6 +1866,19 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, default: break; } + if (bytes) { + if (!nm_supplicant_config_add_blob_for_connection(self, + bytes, + "private_key2", + con_uuid, + error)) + return FALSE; + } else if (path) { + /* Private connections cannot use paths */ + g_return_val_if_fail(!priv->private_user, FALSE); + if (!add_string_val(self, path, "private_key2", FALSE, NULL, error)) + return FALSE; + } if (added) { NMSetting8021xCKFormat format; @@ -1857,20 +1902,16 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, /* Only add the client cert if the private key is not PKCS#12, as * wpa_supplicant configuration directs us to do. */ + path = NULL; + bytes = NULL; switch (nm_setting_802_1x_get_phase2_client_cert_scheme(setting)) { case NM_SETTING_802_1X_CK_SCHEME_BLOB: bytes = nm_setting_802_1x_get_phase2_client_cert_blob(setting); - if (!nm_supplicant_config_add_blob_for_connection(self, - bytes, - "client_cert2", - con_uuid, - error)) - return FALSE; break; case NM_SETTING_802_1X_CK_SCHEME_PATH: path = nm_setting_802_1x_get_phase2_client_cert_path(setting); - if (!add_string_val(self, path, "client_cert2", FALSE, NULL, error)) - return FALSE; + if (priv->private_user) + bytes = nm_g_hash_table_lookup(files, path); break; case NM_SETTING_802_1X_CK_SCHEME_PKCS11: if (!add_pkcs11_uri_with_pin( @@ -1886,6 +1927,19 @@ nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, default: break; } + if (bytes) { + if (!nm_supplicant_config_add_blob_for_connection(self, + bytes, + "client_cert2", + con_uuid, + error)) + return FALSE; + } else if (path) { + /* Private connections cannot use paths */ + g_return_val_if_fail(!priv->private_user, FALSE); + if (!add_string_val(self, path, "client_cert2", FALSE, NULL, error)) + return FALSE; + } } } diff --git a/src/core/supplicant/nm-supplicant-config.h b/src/core/supplicant/nm-supplicant-config.h index c52b756e78..96460b86c7 100644 --- a/src/core/supplicant/nm-supplicant-config.h +++ b/src/core/supplicant/nm-supplicant-config.h @@ -29,7 +29,7 @@ typedef struct _NMSupplicantConfigClass NMSupplicantConfigClass; GType nm_supplicant_config_get_type(void); -NMSupplicantConfig *nm_supplicant_config_new(NMSupplCapMask capabilities); +NMSupplicantConfig *nm_supplicant_config_new(NMSupplCapMask capabilities, const char *private_user); guint32 nm_supplicant_config_get_ap_scan(NMSupplicantConfig *self); @@ -57,6 +57,7 @@ gboolean nm_supplicant_config_add_setting_wireless_security(NMSupplicantConfig guint32 mtu, NMSettingWirelessSecurityPmf pmf, NMSettingWirelessSecurityFils fils, + GHashTable *files, GError **error); gboolean nm_supplicant_config_add_no_security(NMSupplicantConfig *self, GError **error); @@ -66,6 +67,7 @@ gboolean nm_supplicant_config_add_setting_8021x(NMSupplicantConfig *self, const char *con_uuid, guint32 mtu, gboolean wired, + GHashTable *files, GError **error); gboolean nm_supplicant_config_add_setting_macsec(NMSupplicantConfig *self, diff --git a/src/core/supplicant/tests/test-supplicant-config.c b/src/core/supplicant/tests/test-supplicant-config.c index 1ca5b26e56..416fe0054f 100644 --- a/src/core/supplicant/tests/test-supplicant-config.c +++ b/src/core/supplicant/tests/test-supplicant-config.c @@ -98,7 +98,8 @@ build_supplicant_config(NMConnection *connection, NMSetting8021x *s_8021x; gboolean success; - config = nm_supplicant_config_new(capabilities); + config = nm_supplicant_config_new(capabilities, + nm_utils_get_connection_first_permissions_user(connection)); s_wifi = nm_connection_get_setting_wireless(connection); g_assert(s_wifi); @@ -120,6 +121,7 @@ build_supplicant_config(NMConnection *connection, mtu, pmf, fils, + NULL, &error); } else { success = nm_supplicant_config_add_no_security(config, &error); From 9bc4d626809ed534a49dd5e436c4079eccf60b0a Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 27 Oct 2025 17:40:14 +0100 Subject: [PATCH 10/14] core,libnm-core: introduce property flag for certificate and keys If we add a new property in the future and it references a certificate or key stored on disk, we need to also implement the logic to verify the access to the file for private connections. Add a new property flag NM_SETTING_PARAM_CERT_KEY_FILE to existing certificate and key properties, so that it's easier to see that they need special treatment. Also add some assertions to verify that the properties with the flag are handled properly. While at it, move the enumeration of private-files to the settings. (cherry picked from commit acbfae5e051af8647e32d14ccc6be05419dcca77) (cherry picked from commit e3c27f2a22b75c98c300c5ba6249193b9047eaaf) --- src/core/nm-core-utils.c | 46 +++-------- src/libnm-core-impl/nm-setting-8021x.c | 97 ++++++++++++++++++++++-- src/libnm-core-impl/nm-setting-private.h | 17 ++++- src/libnm-core-impl/nm-setting.c | 29 +++++++ src/libnm-core-intern/nm-core-internal.h | 2 + 5 files changed, 143 insertions(+), 48 deletions(-) diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index 4531fdac04..4a7f760d79 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -5517,47 +5517,19 @@ nm_utils_get_connection_first_permissions_user(NMConnection *connection) /*****************************************************************************/ -static void -get_8021x_private_files(NMConnection *connection, GPtrArray *files) -{ - const struct { - NMSetting8021xCKScheme (*get_scheme_func)(NMSetting8021x *); - const char *(*get_path_func)(NMSetting8021x *); - } funcs[] = { - {nm_setting_802_1x_get_ca_cert_scheme, nm_setting_802_1x_get_ca_cert_path}, - {nm_setting_802_1x_get_client_cert_scheme, nm_setting_802_1x_get_client_cert_path}, - {nm_setting_802_1x_get_private_key_scheme, nm_setting_802_1x_get_private_key_path}, - {nm_setting_802_1x_get_phase2_ca_cert_scheme, nm_setting_802_1x_get_phase2_ca_cert_path}, - {nm_setting_802_1x_get_phase2_client_cert_scheme, - nm_setting_802_1x_get_phase2_client_cert_path}, - {nm_setting_802_1x_get_phase2_private_key_scheme, - nm_setting_802_1x_get_phase2_private_key_path}, - }; - NMSetting8021x *s_8021x; - const char *path; - guint i; - - s_8021x = nm_connection_get_setting_802_1x(connection); - if (!s_8021x) - return; - - for (i = 0; i < G_N_ELEMENTS(funcs); i++) { - if (funcs[i].get_scheme_func(s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH) { - path = funcs[i].get_path_func(s_8021x); - if (path) { - g_ptr_array_add(files, (gpointer) path); - } - } - } -} - const char ** nm_utils_get_connection_private_files_paths(NMConnection *connection) { - GPtrArray *files; + GPtrArray *files; + gs_free NMSetting **settings = NULL; + guint num_settings; + guint i; - files = g_ptr_array_new(); - get_8021x_private_files(connection, files); + files = g_ptr_array_new(); + settings = nm_connection_get_settings(connection, &num_settings); + for (i = 0; i < num_settings; i++) { + _nm_setting_get_private_files(settings[i], files); + } g_ptr_array_add(files, NULL); return (const char **) g_ptr_array_free(files, files->len == 1); diff --git a/src/libnm-core-impl/nm-setting-8021x.c b/src/libnm-core-impl/nm-setting-8021x.c index 4ea6072966..f933380333 100644 --- a/src/libnm-core-impl/nm-setting-8021x.c +++ b/src/libnm-core-impl/nm-setting-8021x.c @@ -3133,6 +3133,86 @@ need_secrets(NMSetting *setting, gboolean check_rerequest) /*****************************************************************************/ +static void +get_private_files(NMSetting *setting, GPtrArray *files) +{ + const struct { + const char *property; + NMSetting8021xCKScheme (*get_scheme_func)(NMSetting8021x *); + const char *(*get_path_func)(NMSetting8021x *); + } cert_props[] = { + {NM_SETTING_802_1X_CA_CERT, + nm_setting_802_1x_get_ca_cert_scheme, + nm_setting_802_1x_get_ca_cert_path}, + {NM_SETTING_802_1X_CLIENT_CERT, + nm_setting_802_1x_get_client_cert_scheme, + nm_setting_802_1x_get_client_cert_path}, + {NM_SETTING_802_1X_PRIVATE_KEY, + nm_setting_802_1x_get_private_key_scheme, + nm_setting_802_1x_get_private_key_path}, + {NM_SETTING_802_1X_PHASE2_CA_CERT, + nm_setting_802_1x_get_phase2_ca_cert_scheme, + nm_setting_802_1x_get_phase2_ca_cert_path}, + {NM_SETTING_802_1X_PHASE2_CLIENT_CERT, + nm_setting_802_1x_get_phase2_client_cert_scheme, + nm_setting_802_1x_get_phase2_client_cert_path}, + {NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, + nm_setting_802_1x_get_phase2_private_key_scheme, + nm_setting_802_1x_get_phase2_private_key_path}, + }; + NMSetting8021x *s_8021x = NM_SETTING_802_1X(setting); + const char *path; + guint i; + + if (NM_MORE_ASSERT_ONCE(5)) { + GObjectClass *klass; + gs_free GParamSpec **properties = NULL; + guint n_properties; + gboolean found; + guint j; + + /* Check that all the properties in the setting with flag CERT_KEY_FILE + * are listed in the table, and vice versa. */ + + klass = G_OBJECT_GET_CLASS(setting); + + properties = g_object_class_list_properties(klass, &n_properties); + for (i = 0; i < n_properties; i++) { + if (!(properties[i]->flags & NM_SETTING_PARAM_CERT_KEY_FILE)) + continue; + + found = FALSE; + for (j = 0; j < G_N_ELEMENTS(cert_props); j++) { + if (nm_streq0(properties[i]->name, cert_props[j].property)) { + found = TRUE; + break; + } + } + + nm_assert(found); + } + + for (i = 0; i < G_N_ELEMENTS(cert_props); i++) { + GParamSpec *prop; + + prop = g_object_class_find_property(klass, cert_props[i].property); + nm_assert(prop); + nm_assert(prop->flags & NM_SETTING_PARAM_CERT_KEY_FILE); + } + } + + for (i = 0; i < G_N_ELEMENTS(cert_props); i++) { + if (cert_props[i].get_scheme_func(s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH) { + path = cert_props[i].get_path_func(s_8021x); + if (path) { + g_ptr_array_add(files, (gpointer) path); + } + } + } +} + +/*****************************************************************************/ + static void get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) { @@ -3223,8 +3303,9 @@ nm_setting_802_1x_class_init(NMSetting8021xClass *klass) object_class->set_property = set_property; object_class->finalize = finalize; - setting_class->verify = verify; - setting_class->need_secrets = need_secrets; + setting_class->verify = verify; + setting_class->need_secrets = need_secrets; + setting_class->get_private_files = get_private_files; /** * NMSetting8021x:eap: @@ -3359,7 +3440,7 @@ nm_setting_802_1x_class_init(NMSetting8021xClass *klass) obj_properties, NM_SETTING_802_1X_CA_CERT, PROP_CA_CERT, - NM_SETTING_PARAM_NONE, + NM_SETTING_PARAM_CERT_KEY_FILE, NMSetting8021xPrivate, ca_cert); @@ -3556,7 +3637,7 @@ nm_setting_802_1x_class_init(NMSetting8021xClass *klass) obj_properties, NM_SETTING_802_1X_CLIENT_CERT, PROP_CLIENT_CERT, - NM_SETTING_PARAM_NONE, + NM_SETTING_PARAM_CERT_KEY_FILE, NMSetting8021xPrivate, client_cert); @@ -3803,7 +3884,7 @@ nm_setting_802_1x_class_init(NMSetting8021xClass *klass) obj_properties, NM_SETTING_802_1X_PHASE2_CA_CERT, PROP_PHASE2_CA_CERT, - NM_SETTING_PARAM_NONE, + NM_SETTING_PARAM_CERT_KEY_FILE, NMSetting8021xPrivate, phase2_ca_cert); @@ -4006,7 +4087,7 @@ nm_setting_802_1x_class_init(NMSetting8021xClass *klass) obj_properties, NM_SETTING_802_1X_PHASE2_CLIENT_CERT, PROP_PHASE2_CLIENT_CERT, - NM_SETTING_PARAM_NONE, + NM_SETTING_PARAM_CERT_KEY_FILE, NMSetting8021xPrivate, phase2_client_cert); @@ -4175,7 +4256,7 @@ nm_setting_802_1x_class_init(NMSetting8021xClass *klass) obj_properties, NM_SETTING_802_1X_PRIVATE_KEY, PROP_PRIVATE_KEY, - NM_SETTING_PARAM_NONE, + NM_SETTING_PARAM_CERT_KEY_FILE, NMSetting8021xPrivate, private_key); @@ -4276,7 +4357,7 @@ nm_setting_802_1x_class_init(NMSetting8021xClass *klass) obj_properties, NM_SETTING_802_1X_PHASE2_PRIVATE_KEY, PROP_PHASE2_PRIVATE_KEY, - NM_SETTING_PARAM_NONE, + NM_SETTING_PARAM_CERT_KEY_FILE, NMSetting8021xPrivate, phase2_private_key); diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 8ee770f471..61f9678936 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -154,6 +154,11 @@ struct _NMSettingClass { guint /* NMSettingParseFlags */ parse_flags, GError **error); + /* returns a list of certificate/key files referenced in the connection. + * When the connection is private, we need to verify that the owner of + * the connection has access to them. */ + void (*get_private_files)(NMSetting *setting, GPtrArray *files); + const struct _NMMetaSettingInfo *setting_info; }; @@ -334,6 +339,11 @@ struct _NMRange { */ #define NM_SETTING_PARAM_TO_DBUS_IGNORE_FLAGS (1 << (7 + G_PARAM_USER_SHIFT)) +/* The property can refer to a certificate or key stored on disk. As such, + * special care is needed when accessing the file for private connections. + */ +#define NM_SETTING_PARAM_CERT_KEY_FILE (1 << (8 + G_PARAM_USER_SHIFT)) + extern const NMSettInfoPropertType nm_sett_info_propert_type_setting_name; extern const NMSettInfoPropertType nm_sett_info_propert_type_deprecated_interface_name; extern const NMSettInfoPropertType nm_sett_info_propert_type_deprecated_ignore_i; @@ -859,9 +869,10 @@ _nm_properties_override(GArray *properties_override, const NMSettInfoProperty *p { \ GParamSpec *_param_spec; \ \ - G_STATIC_ASSERT(!NM_FLAGS_ANY((param_flags), \ - ~(NM_SETTING_PARAM_SECRET | NM_SETTING_PARAM_INFERRABLE \ - | NM_SETTING_PARAM_FUZZY_IGNORE))); \ + G_STATIC_ASSERT( \ + !NM_FLAGS_ANY((param_flags), \ + ~(NM_SETTING_PARAM_SECRET | NM_SETTING_PARAM_INFERRABLE \ + | NM_SETTING_PARAM_FUZZY_IGNORE | NM_SETTING_PARAM_CERT_KEY_FILE))); \ \ _param_spec = g_param_spec_boxed("" prop_name "", \ "", \ diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index 98424c76fc..295eabe7a6 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -2262,6 +2262,34 @@ init_from_dbus(NMSetting *setting, return TRUE; } +static void +get_private_files(NMSetting *setting, GPtrArray *files) +{ + if (NM_MORE_ASSERTS) { + GParamSpec **properties; + guint n_properties; + int i; + + properties = g_object_class_list_properties(G_OBJECT_GET_CLASS(setting), &n_properties); + for (i = 0; i < n_properties; i++) { + if (properties[i]->flags & NM_SETTING_PARAM_CERT_KEY_FILE) { + /* Certificates and keys needs special handling, see setting 802.1X */ + nm_assert_not_reached(); + } + } + g_free(properties); + } +} + +void +_nm_setting_get_private_files(NMSetting *setting, GPtrArray *files) +{ + g_return_if_fail(NM_IS_SETTING(setting)); + g_return_if_fail(files); + + NM_SETTING_GET_CLASS(setting)->get_private_files(setting, files); +} + /** * nm_setting_get_dbus_property_type: * @setting: an #NMSetting @@ -4672,6 +4700,7 @@ nm_setting_class_init(NMSettingClass *setting_class) setting_class->enumerate_values = enumerate_values; setting_class->aggregate = aggregate; setting_class->init_from_dbus = init_from_dbus; + setting_class->get_private_files = get_private_files; /** * NMSetting:name: diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index 6991185d39..b8df4d2e9d 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -1192,4 +1192,6 @@ const GPtrArray *_nm_setting_ovs_port_get_trunks_arr(NMSettingOvsPort *self); guint _nm_setting_connection_get_num_permissions_users(NMSettingConnection *setting); const char *_nm_setting_connection_get_first_permissions_user(NMSettingConnection *setting); +void _nm_setting_get_private_files(NMSetting *setting, GPtrArray *files); + #endif From ce4ec4ac809ac8be0ae150fe7954bcf056c802ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 10 Dec 2025 10:09:55 +0100 Subject: [PATCH 11/14] libnm: introduce NM_VERSION_1_54_3 --- src/libnm-core-public/nm-version-macros.h.in | 1 + src/libnm-core-public/nm-version.h | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/src/libnm-core-public/nm-version-macros.h.in b/src/libnm-core-public/nm-version-macros.h.in index 4bdf6716c6..c4b211c628 100644 --- a/src/libnm-core-public/nm-version-macros.h.in +++ b/src/libnm-core-public/nm-version-macros.h.in @@ -79,6 +79,7 @@ #define NM_VERSION_1_52 (NM_ENCODE_VERSION(1, 52, 0)) #define NM_VERSION_1_54 (NM_ENCODE_VERSION(1, 54, 0)) #define NM_VERSION_1_54_2 (NM_ENCODE_VERSION(1, 54, 2)) +#define NM_VERSION_1_54_3 (NM_ENCODE_VERSION(1, 54, 3)) /* For releases, NM_API_VERSION is equal to NM_VERSION. * diff --git a/src/libnm-core-public/nm-version.h b/src/libnm-core-public/nm-version.h index f8f49df2f7..26ede6543c 100644 --- a/src/libnm-core-public/nm-version.h +++ b/src/libnm-core-public/nm-version.h @@ -445,6 +445,12 @@ #define NM_AVAILABLE_IN_1_54_2 #endif +#if NM_VERSION_MAX_ALLOWED < NM_VERSION_1_54_3 +#define NM_AVAILABLE_IN_1_54_3 G_UNAVAILABLE(1, 54.3) +#else +#define NM_AVAILABLE_IN_1_54_3 +#endif + /* * Synchronous API for calling D-Bus in libnm is deprecated. See * https://networkmanager.dev/docs/libnm/latest/usage.html#sync-api From 15346f1a4f150f1ad16d5f447ecb21b727daa4c2 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 10 Oct 2025 15:08:34 +0200 Subject: [PATCH 12/14] vpn: add nm_vpn_plugin_info_supports_safe_private_file_access() The new API indicates that the VPN plugin supports reading files (certificates, keys) of private connections in a safe way (i.e. checking user permissions), or that it doesn't need to read any file from disk. (cherry picked from commit 10db4baeb6d3eef76cf036b2f342ab61caa29764) (cherry picked from commit 8437e14758d1d70de2c01b43685f47101967b3e5) --- src/libnm-client-impl/libnm.ver | 5 +++++ src/libnm-client-impl/tests/test-gir.py | 1 + src/libnm-core-impl/nm-vpn-plugin-info.c | 23 ++++++++++++++++++++++ src/libnm-core-public/nm-vpn-plugin-info.h | 2 ++ 4 files changed, 31 insertions(+) diff --git a/src/libnm-client-impl/libnm.ver b/src/libnm-client-impl/libnm.ver index 040d68292b..9c79ee587a 100644 --- a/src/libnm-client-impl/libnm.ver +++ b/src/libnm-client-impl/libnm.ver @@ -2083,3 +2083,8 @@ global: nm_setting_hsr_get_protocol_version; nm_setting_hsr_protocol_version_get_type; } libnm_1_54_0; + +libnm_1_54_3 { +global: + nm_vpn_plugin_info_supports_safe_private_file_access; +} libnm_1_54_2; diff --git a/src/libnm-client-impl/tests/test-gir.py b/src/libnm-client-impl/tests/test-gir.py index 554080fa8b..af36c7ac31 100755 --- a/src/libnm-client-impl/tests/test-gir.py +++ b/src/libnm-client-impl/tests/test-gir.py @@ -93,6 +93,7 @@ def syms_from_ver(verfile): # hardcode it. c_syms["nm_ethtool_optname_is_feature"] = "1.20" c_syms["nm_setting_bond_port_get_prio"] = "1.44" + c_syms["nm_vpn_plugin_info_supports_safe_private_file_access"] = "1.56" return c_syms diff --git a/src/libnm-core-impl/nm-vpn-plugin-info.c b/src/libnm-core-impl/nm-vpn-plugin-info.c index 223d8ab33b..04b0fb164e 100644 --- a/src/libnm-core-impl/nm-vpn-plugin-info.c +++ b/src/libnm-core-impl/nm-vpn-plugin-info.c @@ -913,6 +913,29 @@ nm_vpn_plugin_info_supports_multiple(NMVpnPluginInfo *self) return _nm_utils_ascii_str_to_bool(s, FALSE); } +/** + * nm_vpn_plugin_info_supports_safe_private_file_access: + * @self: plugin info instance + * + * Returns: %TRUE if the service supports reading files (certificates, keys) of + * private connections in a safe way (i.e. checking user permissions), or + if the service doesn't need to read any file from disk. + * + * Since: 1.56, 1.54.3 + */ +gboolean +nm_vpn_plugin_info_supports_safe_private_file_access(NMVpnPluginInfo *self) +{ + const char *s; + + g_return_val_if_fail(NM_IS_VPN_PLUGIN_INFO(self), FALSE); + + s = nm_vpn_plugin_info_lookup_property(self, + NM_VPN_PLUGIN_INFO_KF_GROUP_CONNECTION, + "supports-safe-private-file-access"); + return _nm_utils_ascii_str_to_bool(s, FALSE); +} + /** * nm_vpn_plugin_info_get_aliases: * @self: plugin info instance diff --git a/src/libnm-core-public/nm-vpn-plugin-info.h b/src/libnm-core-public/nm-vpn-plugin-info.h index c045daa03d..a6396862cd 100644 --- a/src/libnm-core-public/nm-vpn-plugin-info.h +++ b/src/libnm-core-public/nm-vpn-plugin-info.h @@ -64,6 +64,8 @@ NM_AVAILABLE_IN_1_4 gboolean nm_vpn_plugin_info_supports_hints(NMVpnPluginInfo *self); NM_AVAILABLE_IN_1_42 gboolean nm_vpn_plugin_info_supports_multiple(NMVpnPluginInfo *self); +NM_AVAILABLE_IN_1_54_3 +gboolean nm_vpn_plugin_info_supports_safe_private_file_access(NMVpnPluginInfo *self); NM_AVAILABLE_IN_1_4 const char *const *nm_vpn_plugin_info_get_aliases(NMVpnPluginInfo *self); NM_AVAILABLE_IN_1_2 From 4587832735653ff27eae7afb7c4e3541d6cfc271 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 26 Sep 2025 21:04:04 +0200 Subject: [PATCH 13/14] libnm: add function to copy a certificate or key as user Add a new public function nm_utils_copy_cert_as_user() to libnm. It reads a certificate or key file on behalf of the given user and writes it to a directory in /run/NetworkManager. It is useful for VPN plugins that run as root and need to verify that the user owning the connection (the one listed in the connection.permissions property) can access the file. (cherry picked from commit 1a52bbe7c9dcabc066d8930dfd7b7cfe74dabf78) (cherry picked from commit 3d85bace3dcd8aaf9773db90fa412a7cdc131e4b) --- NEWS | 2 + contrib/fedora/rpm/NetworkManager.spec | 1 + src/libnm-client-impl/libnm.ver | 1 + src/libnm-client-impl/tests/meson.build | 16 +- .../tests/test-copy-cert-as-user.c | 32 +++ src/libnm-client-impl/tests/test-gir.py | 1 + src/libnm-core-impl/nm-utils.c | 255 ++++++++++++++++++ src/libnm-core-public/nm-utils.h | 3 + src/nm-helpers/README.md | 8 + src/nm-helpers/meson.build | 20 ++ src/nm-helpers/nm-libnm-helper.c | 45 ++++ 11 files changed, 378 insertions(+), 6 deletions(-) create mode 100644 src/libnm-client-impl/tests/test-copy-cert-as-user.c create mode 100644 src/nm-helpers/nm-libnm-helper.c diff --git a/NEWS b/NEWS index d611f669b2..339ff25314 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ Overview of changes since NetworkManager-1.54.2 * For private connections (the ones that specify a user in the "connection.permissions" property), verify that the user can access the 802.1X certificates and keys set in the connection. +* Introduce a libnm function that can be used by VPN plugins to check + user permissions on certificate and keys. =============================================== NetworkManager-1.54.2 diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index 004cbbb396..29bf80505f 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -896,6 +896,7 @@ fi %{_libexecdir}/nm-dispatcher %{_libexecdir}/nm-initrd-generator %{_libexecdir}/nm-daemon-helper +%{_libexecdir}/nm-libnm-helper %{_libexecdir}/nm-priv-helper %dir %{_libdir}/%{name} %dir %{nmplugindir} diff --git a/src/libnm-client-impl/libnm.ver b/src/libnm-client-impl/libnm.ver index 9c79ee587a..bd66e61d8a 100644 --- a/src/libnm-client-impl/libnm.ver +++ b/src/libnm-client-impl/libnm.ver @@ -2086,5 +2086,6 @@ global: libnm_1_54_3 { global: + nm_utils_copy_cert_as_user; nm_vpn_plugin_info_supports_safe_private_file_access; } libnm_1_54_2; diff --git a/src/libnm-client-impl/tests/meson.build b/src/libnm-client-impl/tests/meson.build index 42e9883e77..500504db6e 100644 --- a/src/libnm-client-impl/tests/meson.build +++ b/src/libnm-client-impl/tests/meson.build @@ -5,6 +5,7 @@ test_units = [ 'test-nm-client', 'test-remote-settings-client', 'test-secret-agent', + 'test-copy-cert-as-user' ] foreach test_unit: test_units @@ -37,12 +38,15 @@ foreach test_unit: test_units ], ) - test( - 'src/libnm-client-impl/tests/' + test_unit, - test_script, - timeout: 90, - args: test_args + [exe.full_path()], - ) + # test-copy-cert-as-user is a manual test, don't run it automatically + if test_unit != 'test-copy-cert-as-user' + test( + 'src/libnm-client-impl/tests/' + test_unit, + test_script, + timeout: 90, + args: test_args + [exe.full_path()], + ) + endif endforeach if enable_introspection diff --git a/src/libnm-client-impl/tests/test-copy-cert-as-user.c b/src/libnm-client-impl/tests/test-copy-cert-as-user.c new file mode 100644 index 0000000000..b2ef9de67d --- /dev/null +++ b/src/libnm-client-impl/tests/test-copy-cert-as-user.c @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +/* + * This is a program to manually test the + * nm_utils_copy_cert_as_user() libnm function. + */ + +#include "libnm-client-impl/nm-default-libnm.h" + +#include "nm-utils.h" + +int +main(int argc, char **argv) +{ + gs_free_error GError *error = NULL; + gs_free char *filename = NULL; + + if (argc != 3) { + g_printerr("Usage: %s \n", argv[0]); + return 1; + } + + filename = nm_utils_copy_cert_as_user(argv[1], argv[2], &error); + if (!filename) { + g_printerr("Error: %s\n", error->message); + return 1; + } + + g_print("%s\n", filename); + + return 0; +} diff --git a/src/libnm-client-impl/tests/test-gir.py b/src/libnm-client-impl/tests/test-gir.py index af36c7ac31..81518e37c6 100755 --- a/src/libnm-client-impl/tests/test-gir.py +++ b/src/libnm-client-impl/tests/test-gir.py @@ -93,6 +93,7 @@ def syms_from_ver(verfile): # hardcode it. c_syms["nm_ethtool_optname_is_feature"] = "1.20" c_syms["nm_setting_bond_port_get_prio"] = "1.44" + c_syms["nm_utils_copy_cert_as_user"] = "1.56" c_syms["nm_vpn_plugin_info_supports_safe_private_file_access"] = "1.56" return c_syms diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index 6d5df98ccc..9a78e9471c 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -17,6 +17,7 @@ #include #include +#include "libnm-glib-aux/nm-io-utils.h" #include "libnm-glib-aux/nm-uuid.h" #include "libnm-glib-aux/nm-json-aux.h" #include "libnm-glib-aux/nm-str-buf.h" @@ -6195,3 +6196,257 @@ nm_utils_ensure_gtypes(void) for (meta_type = 0; meta_type < _NM_META_SETTING_TYPE_NUM; meta_type++) nm_meta_setting_infos[meta_type].get_setting_gtype(); } + +/*****************************************************************************/ + +typedef struct { + GPid pid; + GSource *child_watch_source; + GMainLoop *loop; + GError *error; + + int child_stdout; + int child_stderr; + + GSource *output_source; + GSource *error_source; + + NMStrBuf output_buffer; + NMStrBuf error_buffer; +} HelperInfo; + +static void +helper_complete(HelperInfo *info, GError *error_take) +{ + if (error_take) { + if (!info->error) + info->error = error_take; + else + g_error_free(error_take); + } + + if (info->output_source || info->error_source || info->pid != -1) { + /* Wait that the pipe is closed and process has terminated */ + return; + } + + if (info->error && info->error_buffer.len > 0) { + /* Prefer the message from stderr as it's more informative */ + g_error_free(info->error); + info->error = g_error_new(NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_FAILED, + "%s", + nm_str_buf_get_str(&info->error_buffer)); + } + + g_main_loop_quit(info->loop); +} + +static gboolean +helper_have_err_data(int fd, GIOCondition condition, gpointer user_data) +{ + HelperInfo *info = user_data; + gssize n_read; + GError *error = NULL; + + n_read = nm_utils_fd_read(fd, &info->error_buffer); + + if (n_read > 0) + return G_SOURCE_CONTINUE; + + nm_clear_g_source_inst(&info->error_source); + nm_clear_fd(&info->child_stderr); + + if (n_read < 0) { + error = g_error_new(NM_UTILS_ERROR, + NM_UTILS_ERROR_UNKNOWN, + "read from process returned %d (%s)", + (int) -n_read, + nm_strerror_native((int) -n_read)); + } + + helper_complete(info, error); + return G_SOURCE_CONTINUE; +} + +static gboolean +helper_have_data(int fd, GIOCondition condition, gpointer user_data) +{ + HelperInfo *info = user_data; + gssize n_read; + GError *error = NULL; + + n_read = nm_utils_fd_read(fd, &info->output_buffer); + + if (n_read > 0) + return G_SOURCE_CONTINUE; + + nm_clear_g_source_inst(&info->output_source); + nm_clear_fd(&info->child_stdout); + + if (n_read < 0) { + error = g_error_new(NM_UTILS_ERROR, + NM_UTILS_ERROR_UNKNOWN, + "read from process returned %d (%s)", + (int) -n_read, + nm_strerror_native((int) -n_read)); + } + + helper_complete(info, error); + return G_SOURCE_CONTINUE; +} + +static void +helper_child_terminated(GPid pid, int status, gpointer user_data) +{ + HelperInfo *info = user_data; + gs_free char *status_desc = NULL; + GError *error = NULL; + + info->pid = -1; + nm_clear_g_source_inst(&info->child_watch_source); + + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { + if (!status_desc) + status_desc = nm_utils_get_process_exit_status_desc(status); + error = + g_error_new(NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, "helper process %s", status_desc); + } + + helper_complete(info, error); +} + +#define RUN_CERT_DIR NMRUNDIR "/cert" + +/** + * nm_utils_copy_cert_as_user: + * @filename: the file name of the certificate or key to copy + * @user: the user to impersonate when reading the file + * @error: (nullable): return location for a #GError, or %NULL + * + * Reads @filename on behalf of user @user and writes the + * content to a new file in /run/NetworkManager/cert/. + * The new file has permission 600 and is owned by root. + * + * This function is useful for VPN plugins that run as root and need + * to verify that the user owning the connection (the one listed in the + * connection.permissions property) can access the file. + * + * Returns: (transfer full): the name of the new temporary file. Or %NULL + * if an error occurred, including when the given user can't access the + * file. + * + * Since: 1.56, 1.54.3 + */ +char * +nm_utils_copy_cert_as_user(const char *filename, const char *user, GError **error) +{ + gs_unref_bytes GBytes *bytes = NULL; + char dst_path[] = RUN_CERT_DIR "/XXXXXX"; + HelperInfo info = { + .child_stdout = -1, + .child_stderr = -1, + }; + GMainContext *context; + int fd = -1; + + g_return_val_if_fail(filename, NULL); + g_return_val_if_fail(user, NULL); + g_return_val_if_fail(!error || !*error, NULL); + + if (geteuid() != 0) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("This function needs to be called by root")); + return NULL; + } + + if (!g_spawn_async_with_pipes( + "/", + (char **) + NM_MAKE_STRV(LIBEXECDIR "/nm-libnm-helper", "read-file-as-user", filename, user), + (char **) NM_MAKE_STRV(), + G_SPAWN_CLOEXEC_PIPES | G_SPAWN_DO_NOT_REAP_CHILD, + NULL, + NULL, + &info.pid, + NULL, + &info.child_stdout, + &info.child_stderr, + error)) { + return NULL; + } + + context = g_main_context_new(); + info.loop = g_main_loop_new(context, FALSE); + + /* Watch process */ + info.child_watch_source = nm_g_child_watch_source_new(info.pid, + G_PRIORITY_DEFAULT, + helper_child_terminated, + &info, + NULL); + g_source_attach(info.child_watch_source, context); + + /* Watch stdout */ + info.output_buffer = NM_STR_BUF_INIT(0, FALSE); + info.output_source = nm_g_unix_fd_source_new(info.child_stdout, + G_IO_IN | G_IO_ERR | G_IO_HUP, + G_PRIORITY_DEFAULT, + helper_have_data, + &info, + NULL); + g_source_attach(info.output_source, context); + + /* Watch stderr */ + info.error_buffer = NM_STR_BUF_INIT(0, FALSE); + info.error_source = nm_g_unix_fd_source_new(info.child_stderr, + G_IO_IN | G_IO_ERR | G_IO_HUP, + G_PRIORITY_DEFAULT, + helper_have_err_data, + &info, + NULL); + g_source_attach(info.error_source, context); + + /* Wait termination */ + g_main_loop_run(info.loop); + g_clear_pointer(&info.loop, g_main_loop_unref); + g_clear_pointer(&context, g_main_context_unref); + + if (info.error) { + nm_str_buf_destroy(&info.output_buffer); + nm_str_buf_destroy(&info.error_buffer); + g_propagate_error(error, g_steal_pointer(&info.error)); + return NULL; + } + + /* Write the data to a new file */ + + bytes = g_bytes_new(nm_str_buf_get_str_unsafe(&info.output_buffer), info.output_buffer.len); + nm_str_buf_destroy(&info.output_buffer); + nm_str_buf_destroy(&info.error_buffer); + + mkdir(RUN_CERT_DIR, 0600); + fd = mkstemp(dst_path); + if (fd < 0) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("Failure creating the temporary file")); + return NULL; + } + nm_close(fd); + + if (!nm_utils_file_set_contents(dst_path, + g_bytes_get_data(bytes, NULL), + g_bytes_get_size(bytes), + 0600, + NULL, + NULL, + error)) { + return NULL; + } + + return g_strdup(dst_path); +} diff --git a/src/libnm-core-public/nm-utils.h b/src/libnm-core-public/nm-utils.h index 35ef580db7..dd7977a1c6 100644 --- a/src/libnm-core-public/nm-utils.h +++ b/src/libnm-core-public/nm-utils.h @@ -261,6 +261,9 @@ nm_utils_base64secret_decode(const char *base64_key, gsize required_key_len, gui NM_AVAILABLE_IN_1_42 void nm_utils_ensure_gtypes(void); +NM_AVAILABLE_IN_1_54_3 +char *nm_utils_copy_cert_as_user(const char *filename, const char *user, GError **error); + G_END_DECLS #endif /* __NM_UTILS_H__ */ diff --git a/src/nm-helpers/README.md b/src/nm-helpers/README.md index ab0ea02444..66a9429221 100644 --- a/src/nm-helpers/README.md +++ b/src/nm-helpers/README.md @@ -17,6 +17,14 @@ all the threads of the process). This is not directly useful to the user. +nm-libnm-helper +--------------- + +A internal helper application that is spawned by libnm to perform +certain actions without impacting the calling process. + +This is not directly useful to the user. + nm-priv-helper -------------- diff --git a/src/nm-helpers/meson.build b/src/nm-helpers/meson.build index 5f330cbc94..7c148079d2 100644 --- a/src/nm-helpers/meson.build +++ b/src/nm-helpers/meson.build @@ -18,6 +18,26 @@ executable( install_dir: nm_libexecdir, ) +# nm-libnm-helper + +executable( + 'nm-libnm-helper', + ['nm-libnm-helper.c'], + include_directories : [ + src_inc, + top_inc, + ], + dependencies: [ + glib_dep, + ], + link_with: [ + libnm_glib_aux, + libnm_std_aux, + ], + install: true, + install_dir: nm_libexecdir, +) + # nm-priv-helper configure_file( diff --git a/src/nm-helpers/nm-libnm-helper.c b/src/nm-helpers/nm-libnm-helper.c new file mode 100644 index 0000000000..bd0ba67d94 --- /dev/null +++ b/src/nm-helpers/nm-libnm-helper.c @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "libnm-std-aux/nm-default-std.h" + +#include + +enum { + RETURN_SUCCESS = 0, + RETURN_INVALID_CMD = 1, + RETURN_INVALID_ARGS = 2, + RETURN_ERROR = 3, +}; + +static int +read_file_as_user(const char *filename, const char *user) +{ + char error[1024]; + + if (!nm_utils_set_effective_user(user, error, sizeof(error))) { + fprintf(stderr, "Failed to set effective user '%s': %s", user, error); + return RETURN_ERROR; + } + + if (!nm_utils_read_file_to_stdout(filename, error, sizeof(error))) { + fprintf(stderr, "Failed to read file '%s' as user '%s': %s", filename, user, error); + return RETURN_ERROR; + } + + return RETURN_SUCCESS; +} + +int +main(int argc, char **argv) +{ + if (argc <= 1) + return RETURN_INVALID_CMD; + + if (nm_streq(argv[1], "read-file-as-user")) { + if (argc != 4) + return RETURN_INVALID_ARGS; + return read_file_as_user(argv[2], argv[3]); + } + + return RETURN_INVALID_CMD; +} \ No newline at end of file From 4393b3768631b3fdabdb90a755b16fcda9a0bfd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Fri, 12 Dec 2025 12:00:31 +0100 Subject: [PATCH 14/14] nm-version: set API_VERSION with MICRO+1 (temporary) In the past, stable branches used odd micro numbers as development micro version. Because of that, NM_API_VERSION was defined with MICRO+1 so we don't get warnings during development. As we stopped using odd micro=devel it is wrong to set MICRO+1 on odd releases. Final users of 1.52.3 has NM_API_VERSION 1.52.4. However, during development we need to have MICRO+1. For example, if we are working on top of 1.52.3 towards the next 1.52.4, we define new symbols with NM_AVAILABLE_IN_1_52_4. Because of that, we get compilation failures until we finally bump to 1.52.4, just before the release. The CI remains red until then, potentially missing many bugs. For now, just set MICRO+1 all the time. It is wrong, but it was wrong half of the time anyway, and at least we'll have a green CI until we implement a definitive solution. (cherry picked from commit 13bfa44cebf504e88e2ac00ab85145119263d8fe) --- src/libnm-core-public/nm-version-macros.h.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libnm-core-public/nm-version-macros.h.in b/src/libnm-core-public/nm-version-macros.h.in index c4b211c628..9771944de0 100644 --- a/src/libnm-core-public/nm-version-macros.h.in +++ b/src/libnm-core-public/nm-version-macros.h.in @@ -88,10 +88,10 @@ * version, you are already using the future API, even if * it is not yet released. Hence, the currently used API * version is the future one. */ -#define NM_API_VERSION \ - (((NM_MINOR_VERSION % 2) == 1) \ - ? NM_ENCODE_VERSION (NM_MAJOR_VERSION, NM_MINOR_VERSION + 1, 0 ) \ - : NM_ENCODE_VERSION (NM_MAJOR_VERSION, NM_MINOR_VERSION , ((NM_MICRO_VERSION + 1) / 2) * 2)) +#define NM_API_VERSION \ + (((NM_MINOR_VERSION % 2) == 1) \ + ? NM_ENCODE_VERSION(NM_MAJOR_VERSION, NM_MINOR_VERSION + 1, 0) \ + : NM_ENCODE_VERSION(NM_MAJOR_VERSION, NM_MINOR_VERSION, NM_MICRO_VERSION + 1)) /* deprecated. */ #define NM_VERSION_CUR_STABLE NM_API_VERSION