fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2025-12-25 21:10:08 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

libnm: automatically clear secret string for direct string properties

Browse source 
Let's sprinkle some snake ointment.

This is questionable, because we copy secrets all over the place where
we their deallocation (and clearing) is not in our control. For example,
the GValue setter/getter copies the string (but does not clean the
secret). Also, when converting the property to a GVariant, we won't
clear it. So this does not catch a lot of cases.

Still, if we can with relative ease avoid leaking the string at some
places, do it.
This commit is contained in:
Thomas Haller 2022-01-02 10:44:02 +01:00
parent 171287d94b
commit 16bf47f8ca
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
2 changed files with 39 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
src/libnm-core-impl/nm-setting.c
View file

@ -8,11 +8,12 @@
#include "nm-setting.h"
#include "nm-setting-private.h"
#include "nm-utils.h"
#include "libnm-core-intern/nm-core-internal.h"
#include "nm-utils-private.h"
#include "libnm-glib-aux/nm-secret-utils.h"
#include "nm-property-compare.h"
#include "nm-setting-private.h"
#include "nm-utils-private.h"
#include "nm-utils.h"
/**
* SECTION:nm-setting
@ -672,9 +673,13 @@ _property_direct_set_string(const NMSettInfoProperty *property_info, char **dst,
goto out_take;
}
if (NM_FLAGS_HAS(property_info->param_spec->flags, NM_SETTING_PARAM_SECRET))
return nm_strdup_reset_secret(dst, src);
return nm_strdup_reset(dst, src);
out_take:
nm_assert(!NM_FLAGS_HAS(property_info->param_spec->flags, NM_SETTING_PARAM_SECRET));
return nm_strdup_reset_take(dst, s);
}
@ -1039,7 +1044,10 @@ _finalize_direct(NMSetting *setting)
char **p_val =
_nm_setting_get_private(setting, sett_info, property_info->direct_offset);
nm_clear_g_free(p_val);
if (NM_FLAGS_HAS(property_info->param_spec->flags, NM_SETTING_PARAM_SECRET))
nm_clear_pointer(p_val, nm_free_secret);
else
nm_clear_g_free(p_val);
break;
}
case NM_VALUE_TYPE_BYTES:
@ -1458,6 +1466,7 @@ _nm_setting_property_from_dbus_fcn_direct(_NM_SETT_INFO_PROP_FROM_DBUS_FCN_ARGS
gs_free char *v_free = NULL;
char **p_val;
const char *v;
gboolean changed;
if (g_variant_is_of_type(value, G_VARIANT_TYPE_STRING)) {
v = g_variant_get_string(value, NULL);
@ -1472,9 +1481,14 @@ _nm_setting_property_from_dbus_fcn_direct(_NM_SETT_INFO_PROP_FROM_DBUS_FCN_ARGS
}
p_val = _nm_setting_get_private(setting, sett_info, property_info->direct_offset);
if (!_property_direct_set_string(property_info, p_val, v))
goto out_unchanged;
changed = _property_direct_set_string(property_info, p_val, v);
if (NM_FLAGS_HAS(property_info->param_spec->flags, NM_SETTING_PARAM_SECRET))
nm_clear_pointer(&v_free, nm_free_secret);
if (!changed)
goto out_unchanged;
goto out_notify;
}
case NM_VALUE_TYPE_BYTES:

23
src/libnm-core-impl/tests/test-setting.c
View file

@ -4430,6 +4430,7 @@ test_setting_metadata(void)
GArray *property_types_data;
guint prop_idx_val;
gboolean can_set_including_default = FALSE;
int n_special_options;
g_assert(sip->name);
@ -4579,10 +4580,24 @@ test_setting_metadata(void)
g_assert(sip->property_type->direct_type == NM_VALUE_TYPE_STRING);
}
g_assert(((sip->direct_set_string_mac_address_len != 0)
+ (!!sip->direct_set_string_strip) + (!!sip->direct_set_string_ascii_strdown)
+ (sip->direct_set_string_ip_address_addr_family != 0))
<= 1);
n_special_options = (sip->direct_set_string_mac_address_len != 0)
+ (!!sip->direct_set_string_strip)
+ (!!sip->direct_set_string_ascii_strdown)
+ (sip->direct_set_string_ip_address_addr_family != 0);
/* currently, we have no cases where special options are mixed. There is no problem to support
* that, but as it's not needed, don't do it for now. */
g_assert_cmpint(n_special_options, <=, 1);
if (n_special_options > 0) {
/* currently, special options are only relevant for string properties. */
g_assert(sip->property_type->direct_type == NM_VALUE_TYPE_STRING);
}
if (sip->param_spec && NM_FLAGS_HAS(sip->param_spec->flags, NM_SETTING_PARAM_SECRET)) {
/* Currently, special options are not supported for secrets. */
g_assert_cmpint(n_special_options, ==, 0);
}
if (!sip->property_type->to_dbus_fcn) {
/* it's allowed to have no to_dbus_fcn(), to ignore a property. But such