From e85cc46d0b36cdba50fe8411cc93d55a49ebfccf Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 23 Sep 2025 17:06:39 +0200 Subject: [PATCH] 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. --- NEWS | 3 + 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, 137 insertions(+), 60 deletions(-) diff --git a/NEWS b/NEWS index c93798c62b..26d42c8e52 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,9 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! polkit permissions to allow non-admin users to create system-wide connection. That configuration is discouraged because it can be used to bypass filesystem permissions. +* 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.56 diff --git a/src/core/devices/nm-device-ethernet.c b/src/core/devices/nm-device-ethernet.c index 5396914e82..11f691ded9 100644 --- a/src/core/devices/nm-device-ethernet.c +++ b/src/core/devices/nm-device-ethernet.c @@ -629,10 +629,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 5d67081c77..eb39cb2ab0 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 50a0dd7b5f..9ca4cdd3af 100644 --- a/src/core/devices/wifi/nm-device-wifi.c +++ b/src/core/devices/wifi/nm-device-wifi.c @@ -3019,7 +3019,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) @@ -3095,6 +3096,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 635174bdf0..fd360e7238 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);