system/NetworkManager
system/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager synced 2024-10-14 12:05:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

libnm: cleanup handling of "connection.permissions" and improve validation

Browse source 
Previously, both nm_setting_connection_add_permission() and the GObject
property setter would merely assert that the provided values are valid
(and otherwise don't do anything). That is bad for handling errors.

For example, we use the property setter to initialize the setting from
keyfile and GVariant (D-Bus). That means, if a user provides an invalid
permissions value, we would emit a g_critical() assertion failure, but
otherwise ignore the configuration. What we instead need to do is to
accept the value, and afterwards fail verification. That way, a proper error
message can be generated.

  $ mcli connection add type ethernet autoconnect no ifname bogus con-name x connection.permissions 'bogus:'

  (process:429514): libnm-CRITICAL **: 12:12:00.359: permission_new: assertion 'strchr (uname, ':') == NULL' failed

  (process:429514): libnm-CRITICAL **: 12:12:00.359: nm_setting_connection_add_permission: assertion 'p != NULL' failed
  Connection 'x' (2802d117-f84e-44d9-925b-bfe26fd85da1) successfully added.
  $ $  nmcli -f connection.permissions connection show x
  connection.permissions:                 --

While at it, also don't track the permissions in a GSList. Tracking one
permission in a GSList requires 3 allocations (one for the user string,
one for the Permission struct, and one for the GSList struct). Instead,
use a GArray. That is still not great, because GArray cannot be embedded
inside NMSettingConnectionPrivate, so tracking one permission also
requires 3 allocations (which is really a fault of GArray). So, GArray
is not better in the common case where there is only one permissions. But even
in the worst case (only one entry), GArray is no worse than GSList.

Also change the API of nm_setting_connection_add_permission().
Previously, the function would assert that the arguments are in
a certain form (strcmp (ptype, "user") == 0), but still document
the such behaviors like regular operation ("[returns] %FALSE if @ptype
or @pitem was invalid"). Don't assert against the function arguments.
Also, if you first set the user to "fo:o", then
nm_setting_connection_add_permission() would accept it -- only at
a later phase, the property setter would assert against such values.
Also, the function would return %FALSE both if the input value was
invalid (an error) and if the value already existed. I think the
function should not treat a duplicate entry like a badly formatted
input.
Now the function does much less asserting of the arguments, but will
return %FALSE only if the values are invalid. And it will silently ignore
duplicate entries.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/636
This commit is contained in:
Thomas Haller 2020-09-24 20:53:13 +02:00
parent 7c6a99eb40
commit 20ebacbea2
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
8 changed files with 353 additions and 253 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
clients/cli/devices.c
View file

@ -3868,7 +3868,10 @@ do_device_wifi_connect(const NMCCommand *cmd, NmCli *nmc, int argc, const char *
/* Connection will only be visible to this user when 'private' is specified */
if (private)
nm_setting_connection_add_permission(s_con, "user", g_get_user_name(), NULL);
nm_setting_connection_add_permission(s_con,
NM_SETTINGS_CONNECTION_PERMISSION_USER,
g_get_user_name() ?: "",
NULL);
}
if (bssid2_arr || hidden) {
s_wifi = (NMSettingWireless *) nm_setting_wireless_new();

34
clients/common/nm-meta-setting-desc.c
View file

@ -2504,12 +2504,14 @@ static gconstpointer _get_fcn_connection_permissions(ARGS_GET_FCN)
for (i = 0; i < n; i++) {
if (!nm_setting_connection_get_permission(s_con, i, &perm_type, &perm_item, NULL))
continue;
if (!nm_streq(perm_type, NM_SETTINGS_CONNECTION_PERMISSION_USER))
continue;
if (!perm)
perm = g_string_new(NULL);
else
g_string_append_c(perm, ',');
g_string_append_printf(perm, "%s:%s", perm_type, perm_item);
g_string_append_printf(perm, NM_SETTINGS_CONNECTION_PERMISSION_USER_PREFIX "%s", perm_item);
}
NM_SET_OUT(out_is_default, !perm);
@ -2591,19 +2593,13 @@ static const char *const *_complete_fcn_connection_type(ARGS_COMPLETE_FCN)
return (const char *const *) (*out_to_free = result);
}
#define PERM_USER_PREFIX "user:"
static const char *
_sanitize_connection_permission_user(const char *perm)
{
if (NM_STR_HAS_PREFIX(perm, PERM_USER_PREFIX))
perm += NM_STRLEN(PERM_USER_PREFIX);
if (perm[0] == '\0')
if (NM_STR_HAS_PREFIX(perm, NM_SETTINGS_CONNECTION_PERMISSION_USER_PREFIX))
perm += NM_STRLEN(NM_SETTINGS_CONNECTION_PERMISSION_USER_PREFIX);
if (!nm_settings_connection_validate_permission_user(perm, -1))
return NULL;
if (!g_utf8_validate(perm, -1, NULL))
return NULL;
return perm;
}
@ -2626,19 +2622,25 @@ static gboolean
_multilist_set_fcn_connection_permissions(NMSetting *setting, const char *item)
{
item = _sanitize_connection_permission_user(item);
nm_setting_connection_add_permission(NM_SETTING_CONNECTION(setting), "user", item, NULL);
if (!item)
return FALSE;
if (!nm_setting_connection_add_permission(NM_SETTING_CONNECTION(setting),
NM_SETTINGS_CONNECTION_PERMISSION_USER,
item,
NULL))
nm_assert_not_reached();
return TRUE;
}
static gboolean
_multilist_remove_by_value_fcn_connection_permissions(NMSetting *setting, const char *item)
{
const char *sanitized;
sanitized = _sanitize_connection_permission_user(item);
item = _sanitize_connection_permission_user(item);
if (!item)
return FALSE;
nm_setting_connection_remove_permission_by_value(NM_SETTING_CONNECTION(setting),
"user",
sanitized ?: item,
NM_SETTINGS_CONNECTION_PERMISSION_USER,
item,
NULL);
return TRUE;
}

29
libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c
View file

@ -314,3 +314,32 @@ nm_utils_validate_dhcp4_vendor_class_id(const char *vci, GError **error)
return TRUE;
}
gboolean
nm_settings_connection_validate_permission_user(const char *item, gssize len)
{
gsize l;
if (!item)
return FALSE;
if (len < 0) {
nm_assert(len == -1);
l = strlen(item);
} else
l = (gsize) len;
if (l == 0)
return FALSE;
if (!g_utf8_validate(item, l, NULL))
return FALSE;
if (l >= 100)
return FALSE;
if (memchr(item, ':', l))
return FALSE;
return TRUE;
}

7
libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h
View file

@ -142,4 +142,11 @@ const char *nm_utils_route_type2str(guint8 val, char *buf, gsize len);
gboolean nm_utils_validate_dhcp4_vendor_class_id(const char *vci, GError **error);
/*****************************************************************************/
#define NM_SETTINGS_CONNECTION_PERMISSION_USER "user"
#define NM_SETTINGS_CONNECTION_PERMISSION_USER_PREFIX "user:"
gboolean nm_settings_connection_validate_permission_user(const char *item, gssize len);
#endif /* __NM_LIBNM_SHARED_UTILS_H__ */

354
libnm-core/nm-setting-connection.c
View file

@ -31,13 +31,14 @@
/*****************************************************************************/
typedef enum {
PERM_TYPE_USER = 0,
typedef enum _nm_packed {
PERM_TYPE_INVALID,
PERM_TYPE_USER,
} PermType;
typedef struct {
guint8 ptype;
char * item;
PermType ptype;
char * item;
} Permission;
NM_GOBJECT_PROPERTIES_DEFINE(NMSettingConnection,
@ -68,7 +69,7 @@ NM_GOBJECT_PROPERTIES_DEFINE(NMSettingConnection,
PROP_MUD_URL, );
typedef struct {
GSList *permissions; /* list of Permission structs */
GArray *permissions;
GSList *secondaries; /* secondary connections to activate with the base connection */
char * id;
char * uuid;
@ -102,83 +103,85 @@ G_DEFINE_TYPE(NMSettingConnection, nm_setting_connection, NM_TYPE_SETTING)
/*****************************************************************************/
#define PERM_USER_PREFIX "user:"
static Permission *
permission_new_from_str(const char *str)
static void
_permission_set_stale(Permission *permission, PermType ptype, char *item_take)
{
Permission *p;
const char *last_colon;
size_t ulen = 0, i;
nm_assert(permission);
nm_assert(NM_IN_SET(ptype, PERM_TYPE_INVALID, PERM_TYPE_USER));
nm_assert(ptype != PERM_TYPE_USER
|| nm_settings_connection_validate_permission_user(item_take, -1));
g_return_val_if_fail(strncmp(str, PERM_USER_PREFIX, strlen(PERM_USER_PREFIX)) == 0, NULL);
str += strlen(PERM_USER_PREFIX);
/* we don't inspect (clear) permission before setting. It takes a
* stale instance. */
*permission = (Permission){
.ptype = ptype,
.item = item_take,
};
}
static void
_permission_clear_stale(Permission *permission)
{
g_free(permission->item);
/* We leave the permission instance with dangling pointers.
* It is "stale". */
}
static gboolean
_permission_set_stale_parse(Permission *permission, const char *str)
{
const char *str0 = str;
const char *last_colon;
gsize ulen;
nm_assert(str);
if (!str)
goto invalid;
if (!NM_STR_HAS_PREFIX(str, NM_SETTINGS_CONNECTION_PERMISSION_USER_PREFIX))
goto invalid;
str += NM_STRLEN(NM_SETTINGS_CONNECTION_PERMISSION_USER_PREFIX);
last_colon = strrchr(str, ':');
if (last_colon) {
/* Ensure that somebody didn't pass "user::" */
g_return_val_if_fail(last_colon > str, NULL);
/* Reject :[detail] for now */
g_return_val_if_fail(*(last_colon + 1) == '\0', NULL);
/* Make sure we don't include detail in the username */
if (last_colon[1] != '\0')
goto invalid;
ulen = last_colon - str;
} else
ulen = strlen(str);
/* Sanity check the length of the username */
g_return_val_if_fail(ulen < 100, NULL);
/* Make sure there's no ':' in the username */
for (i = 0; i < ulen; i++)
g_return_val_if_fail(str[i] != ':', NULL);
/* And the username must be valid UTF-8 */
g_return_val_if_fail(g_utf8_validate(str, -1, NULL) == TRUE, NULL);
if (!nm_settings_connection_validate_permission_user(str, ulen))
goto invalid;
/* Yay, valid... create the new permission */
p = g_slice_new0(Permission);
p->ptype = PERM_TYPE_USER;
if (last_colon) {
p->item = g_malloc(ulen + 1);
memcpy(p->item, str, ulen);
p->item[ulen] = '\0';
} else
p->item = g_strdup(str);
if (permission)
_permission_set_stale(permission, PERM_TYPE_USER, g_strndup(str, ulen));
return TRUE;
return p;
}
static Permission *
permission_new(const char *uname)
{
Permission *p;
g_return_val_if_fail(uname, NULL);
g_return_val_if_fail(uname[0] != '\0', NULL);
g_return_val_if_fail(strchr(uname, ':') == NULL, NULL);
g_return_val_if_fail(g_utf8_validate(uname, -1, NULL) == TRUE, NULL);
/* Yay, valid... create the new permission */
p = g_slice_new0(Permission);
p->ptype = PERM_TYPE_USER;
p->item = g_strdup(uname);
return p;
invalid:
if (permission)
_permission_set_stale(permission, PERM_TYPE_INVALID, g_strdup(str0));
return FALSE;
}
static char *
permission_to_string(Permission *p)
_permission_to_string(Permission *permission)
{
return g_strdup_printf(PERM_USER_PREFIX "%s:", p->item);
}
nm_assert(permission);
static void
permission_free(Permission *p)
{
g_free(p->item);
memset(p, 0, sizeof(*p));
g_slice_free(Permission, p);
switch (permission->ptype) {
case PERM_TYPE_USER:
return g_strdup_printf(NM_SETTINGS_CONNECTION_PERMISSION_USER_PREFIX "%s:",
permission->item);
case PERM_TYPE_INVALID:
nm_assert(permission->item);
return g_strdup(permission->item);
}
nm_assert_not_reached();
return g_strdup(permission->item ?: "");
}
/*****************************************************************************/
@ -279,14 +282,15 @@ nm_setting_connection_get_num_permissions(NMSettingConnection *setting)
{
g_return_val_if_fail(NM_IS_SETTING_CONNECTION(setting), 0);
return g_slist_length(NM_SETTING_CONNECTION_GET_PRIVATE(setting)->permissions);
return nm_g_array_len(NM_SETTING_CONNECTION_GET_PRIVATE(setting)->permissions);
}
/**
* nm_setting_connection_get_permission:
* @setting: the #NMSettingConnection
* @idx: the zero-based index of the permissions entry
* @out_ptype: on return, the permission type (at this time, always "user")
* @out_ptype: on return, the permission type. This is currently always "user",
* unless the entry is invalid, in which case it returns "invalid".
* @out_pitem: on return, the permission item (formatted according to @ptype, see
* #NMSettingConnection:permissions for more detail
* @out_detail: on return, the permission detail (at this time, always %NULL)
@ -304,22 +308,29 @@ nm_setting_connection_get_permission(NMSettingConnection *setting,
const char ** out_detail)
{
NMSettingConnectionPrivate *priv;
Permission * p;
Permission * permission;
g_return_val_if_fail(NM_IS_SETTING_CONNECTION(setting), FALSE);
priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting);
g_return_val_if_fail(idx < g_slist_length(priv->permissions), FALSE);
p = g_slist_nth_data(priv->permissions, idx);
if (out_ptype)
*out_ptype = "user";
if (out_pitem)
*out_pitem = p->item;
if (out_detail)
*out_detail = NULL;
g_return_val_if_fail(idx < nm_g_array_len(priv->permissions), FALSE);
permission = &g_array_index(priv->permissions, Permission, idx);
switch (permission->ptype) {
case PERM_TYPE_USER:
NM_SET_OUT(out_ptype, NM_SETTINGS_CONNECTION_PERMISSION_USER);
NM_SET_OUT(out_pitem, permission->item);
NM_SET_OUT(out_detail, NULL);
return TRUE;
case PERM_TYPE_INVALID:
goto invalid;
}
nm_assert_not_reached();
invalid:
NM_SET_OUT(out_ptype, "invalid");
NM_SET_OUT(out_pitem, permission->item);
NM_SET_OUT(out_detail, NULL);
return TRUE;
}
@ -337,23 +348,22 @@ gboolean
nm_setting_connection_permissions_user_allowed(NMSettingConnection *setting, const char *uname)
{
NMSettingConnectionPrivate *priv;
GSList * iter;
guint i;
g_return_val_if_fail(NM_IS_SETTING_CONNECTION(setting), FALSE);
g_return_val_if_fail(uname != NULL, FALSE);
g_return_val_if_fail(*uname != '\0', FALSE);
priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting);
/* If no permissions, visible to all */
if (priv->permissions == NULL)
if (nm_g_array_len(priv->permissions) == 0) {
/* If no permissions, visible to all */
return TRUE;
}
/* Find the username in the permissions list */
for (iter = priv->permissions; iter; iter = g_slist_next(iter)) {
Permission *p = iter->data;
for (i = 0; i < priv->permissions->len; i++) {
const Permission *permission = &g_array_index(priv->permissions, Permission, i);
if (strcmp(uname, p->item) == 0)
if (permission->ptype == PERM_TYPE_USER && nm_streq(permission->item, uname))
return TRUE;
}
@ -372,8 +382,10 @@ nm_setting_connection_permissions_user_allowed(NMSettingConnection *setting, con
* #NMSettingConnection:permissions: for more details.
*
* Returns: %TRUE if the permission was unique and was successfully added to the
* list, %FALSE if @ptype or @pitem was invalid or it the permission was already
* present in the list
* list, %FALSE if @ptype or @pitem was invalid.
* If the permission was already present in the list, it will not be added
* a second time but %TRUE will be returned. Note that before 1.28, in this
* case %FALSE would be returned.
*/
gboolean
nm_setting_connection_add_permission(NMSettingConnection *setting,
@ -382,30 +394,39 @@ nm_setting_connection_add_permission(NMSettingConnection *setting,
const char * detail)
{
NMSettingConnectionPrivate *priv;
Permission * p;
GSList * iter;
guint i;
g_return_val_if_fail(NM_IS_SETTING_CONNECTION(setting), FALSE);
g_return_val_if_fail(ptype && ptype[0], FALSE);
g_return_val_if_fail(detail == NULL, FALSE);
g_return_val_if_fail(ptype, FALSE);
g_return_val_if_fail(pitem, FALSE);
/* Only "user" for now... */
g_return_val_if_fail(strcmp(ptype, "user") == 0, FALSE);
if (!nm_streq0(ptype, NM_SETTINGS_CONNECTION_PERMISSION_USER))
return FALSE;
if (!nm_settings_connection_validate_permission_user(pitem, -1))
return FALSE;
if (detail)
return FALSE;
priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting);
/* No dupes */
for (iter = priv->permissions; iter; iter = g_slist_next(iter)) {
p = iter->data;
if (strcmp(pitem, p->item) == 0)
return FALSE;
if (!priv->permissions) {
priv->permissions = g_array_sized_new(FALSE, FALSE, sizeof(Permission), 1);
g_array_set_clear_func(priv->permissions, (GDestroyNotify) _permission_clear_stale);
}
p = permission_new(pitem);
g_return_val_if_fail(p != NULL, FALSE);
priv->permissions = g_slist_append(priv->permissions, p);
_notify(setting, PROP_PERMISSIONS);
for (i = 0; i < priv->permissions->len; i++) {
const Permission *permission = &g_array_index(priv->permissions, Permission, i);
if (permission->ptype == PERM_TYPE_USER && nm_streq(permission->item, pitem))
return TRUE;
}
_permission_set_stale(nm_g_array_append_new(priv->permissions, Permission),
PERM_TYPE_USER,
g_strdup(pitem));
_notify(setting, PROP_PERMISSIONS);
return TRUE;
}
@ -420,16 +441,15 @@ void
nm_setting_connection_remove_permission(NMSettingConnection *setting, guint32 idx)
{
NMSettingConnectionPrivate *priv;
GSList * iter;
g_return_if_fail(NM_IS_SETTING_CONNECTION(setting));
priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting);
iter = g_slist_nth(priv->permissions, idx);
g_return_if_fail(iter != NULL);
permission_free((Permission *) iter->data);
priv->permissions = g_slist_delete_link(priv->permissions, iter);
g_return_if_fail(idx < nm_g_array_len(priv->permissions));
g_array_remove_index(priv->permissions, idx);
_notify(setting, PROP_PERMISSIONS);
}
@ -453,25 +473,28 @@ nm_setting_connection_remove_permission_by_value(NMSettingConnection *setting,
const char * detail)
{
NMSettingConnectionPrivate *priv;
Permission * p;
GSList * iter;
guint i;
g_return_val_if_fail(NM_IS_SETTING_CONNECTION(setting), FALSE);
g_return_val_if_fail(ptype && ptype[0], FALSE);
g_return_val_if_fail(detail == NULL, FALSE);
g_return_val_if_fail(pitem != NULL, FALSE);
g_return_val_if_fail(ptype, FALSE);
g_return_val_if_fail(pitem, FALSE);
/* Only "user" for now... */
g_return_val_if_fail(strcmp(ptype, "user") == 0, FALSE);
if (!nm_streq0(ptype, NM_SETTINGS_CONNECTION_PERMISSION_USER))
return FALSE;
if (detail)
return FALSE;
priv = NM_SETTING_CONNECTION_GET_PRIVATE(setting);
for (iter = priv->permissions; iter; iter = g_slist_next(iter)) {
p = iter->data;
if (strcmp(pitem, p->item) == 0) {
permission_free((Permission *) iter->data);
priv->permissions = g_slist_delete_link(priv->permissions, iter);
_notify(setting, PROP_PERMISSIONS);
return TRUE;
if (priv->permissions) {
for (i = 0; i < priv->permissions->len; i++) {
const Permission *permission = &g_array_index(priv->permissions, Permission, i);
if (permission->ptype == PERM_TYPE_USER && nm_streq(permission->item, pitem)) {
g_array_remove_index(priv->permissions, i);
_notify(setting, PROP_PERMISSIONS);
return TRUE;
}
}
}
return FALSE;
@ -1298,6 +1321,27 @@ after_interface_name:
}
}
if (priv->permissions) {
guint i;
for (i = 0; i < priv->permissions->len; i++) {
const Permission *permissions = &g_array_index(priv->permissions, Permission, i);
if (permissions->ptype != PERM_TYPE_USER) {
g_set_error_literal(error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("invalid permissions not in format \"user:$UNAME[:]\""));
g_prefix_error(error,
"%s.%s: ",
nm_setting_get_name(setting),
NM_SETTING_CONNECTION_PERMISSIONS);
return FALSE;
}
nm_assert(nm_settings_connection_validate_permission_user(permissions->item, -1));
}
}
/* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */
if (!priv->uuid) {
@ -1462,40 +1506,6 @@ compare_property(const NMSettInfoSetting *sett_info,
->compare_property(sett_info, property_idx, con_a, set_a, con_b, set_b, flags);
}
static GSList *
perm_strv_to_permlist(char **strv)
{
GSList *list = NULL;
int i;
if (!strv)
return NULL;
for (i = 0; strv[i]; i++) {
Permission *p;
p = permission_new_from_str(strv[i]);
if (p)
list = g_slist_append(list, p);
}
return list;
}
static char **
perm_permlist_to_strv(GSList *permlist)
{
GPtrArray *strings;
GSList * iter;
strings = g_ptr_array_new();
for (iter = permlist; iter; iter = g_slist_next(iter))
g_ptr_array_add(strings, permission_to_string((Permission *) iter->data));
g_ptr_array_add(strings, NULL);
return (char **) g_ptr_array_free(strings, FALSE);
}
/*****************************************************************************/
static void
@ -1521,8 +1531,20 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
g_value_set_string(value, nm_setting_connection_get_connection_type(setting));
break;
case PROP_PERMISSIONS:
g_value_take_boxed(value, perm_permlist_to_strv(priv->permissions));
{
char **strv;
gsize i, l;
l = nm_g_array_len(priv->permissions);
strv = g_new(char *, l + 1u);
for (i = 0; i < l; i++)
strv[i] = _permission_to_string(&g_array_index(priv->permissions, Permission, i));
strv[i] = NULL;
g_value_take_boxed(value, strv);
break;
}
case PROP_AUTOCONNECT:
g_value_set_boolean(value, nm_setting_connection_get_autoconnect(setting));
break;
@ -1613,9 +1635,25 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps
priv->type = g_value_dup_string(value);
break;
case PROP_PERMISSIONS:
g_slist_free_full(priv->permissions, (GDestroyNotify) permission_free);
priv->permissions = perm_strv_to_permlist(g_value_get_boxed(value));
{
const char *const *strv;
guint i;
nm_clear_pointer(&priv->permissions, g_array_unref);
strv = g_value_get_boxed(value);
if (strv && strv[0]) {
priv->permissions =
g_array_sized_new(FALSE, FALSE, sizeof(Permission), NM_PTRARRAY_LEN(strv));
g_array_set_clear_func(priv->permissions, (GDestroyNotify) _permission_clear_stale);
for (i = 0; strv[i]; i++) {
Permission *permission = nm_g_array_append_new(priv->permissions, Permission);
_permission_set_stale_parse(permission, strv[i]);
}
}
break;
}
case PROP_AUTOCONNECT:
priv->autoconnect = g_value_get_boolean(value);
break;
@ -1729,7 +1767,7 @@ finalize(GObject *object)
g_free(priv->master);
g_free(priv->slave_type);
g_free(priv->mud_url);
g_slist_free_full(priv->permissions, (GDestroyNotify) permission_free);
nm_clear_pointer(&priv->permissions, g_array_unref);
g_slist_free_full(priv->secondaries, g_free);
G_OBJECT_CLASS(nm_setting_connection_parent_class)->finalize(object);

117
libnm-core/tests/test-general.c
View file

@ -3208,7 +3208,7 @@ check_permission(NMSettingConnection *s_con, guint32 idx, const char *expected_u
gboolean success;
const char *ptype = NULL, *pitem = NULL, *detail = NULL;
success = nm_setting_connection_get_permission(s_con, 0, &ptype, &pitem, &detail);
success = nm_setting_connection_get_permission(s_con, idx, &ptype, &pitem, &detail);
g_assert(success);
g_assert_cmpstr(ptype, ==, "user");
@ -3233,50 +3233,43 @@ test_setting_connection_permissions_helpers(void)
s_con = NM_SETTING_CONNECTION(nm_setting_connection_new());
/* Ensure a bad [type] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(strcmp(ptype, "user") == 0));
success = nm_setting_connection_add_permission(s_con, "foobar", "blah", NULL);
g_test_assert_expected_messages();
g_assert(!success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
/* Ensure a bad [type] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(ptype && ptype[0]));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(ptype));
success = nm_setting_connection_add_permission(s_con, NULL, "blah", NULL);
g_test_assert_expected_messages();
g_assert(!success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
/* Ensure a bad [item] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(uname));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(p != NULL));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(pitem));
success = nm_setting_connection_add_permission(s_con, "user", NULL, NULL);
g_test_assert_expected_messages();
g_assert(!success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
/* Ensure a bad [item] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(uname[0] != '\0'));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(p != NULL));
success = nm_setting_connection_add_permission(s_con, "user", "", NULL);
g_test_assert_expected_messages();
g_assert(!success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
/* Ensure an [item] with ':' is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(strchr(uname, ':') == NULL));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(p != NULL));
success = nm_setting_connection_add_permission(s_con, "user", "ad:asdf", NULL);
g_test_assert_expected_messages();
g_assert(!success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
/* Ensure a non-UTF-8 [item] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(g_utf8_validate(uname, -1, NULL) == TRUE));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(p != NULL));
success = nm_setting_connection_add_permission(s_con, "user", buf, NULL);
g_test_assert_expected_messages();
g_assert(!success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
/* Ensure a non-NULL [detail] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(detail == NULL));
success = nm_setting_connection_add_permission(s_con, "user", "dafasdf", "asdf");
g_test_assert_expected_messages();
g_assert(!success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
/* Ensure a valid call results in success */
success = nm_setting_connection_add_permission(s_con, "user", TEST_UNAME, NULL);
@ -3337,74 +3330,92 @@ add_permission_property(NMSettingConnection *s_con,
static void
test_setting_connection_permissions_property(void)
{
NMSettingConnection *s_con;
gboolean success;
char buf[9] = {0x61, 0x62, 0x63, 0xff, 0xfe, 0xfd, 0x23, 0x01, 0x00};
gs_unref_object NMSettingConnection *s_con = NULL;
gboolean success;
char buf[9] = {0x61, 0x62, 0x63, 0xff, 0xfe, 0xfd, 0x23, 0x01, 0x00};
s_con = NM_SETTING_CONNECTION(nm_setting_connection_new());
#define _assert_permission_invalid_at_idx(s_con, idx, expected_item) \
G_STMT_START \
{ \
NMSettingConnection *_s_con = (s_con); \
guint _idx = (idx); \
const char * _ptype; \
const char * _pitem; \
const char * _detail; \
const char ** _p_ptype = nmtst_get_rand_bool() ? &_ptype : NULL; \
const char ** _p_pitem = nmtst_get_rand_bool() ? &_pitem : NULL; \
const char ** _p_detail = nmtst_get_rand_bool() ? &_detail : NULL; \
\
g_assert_cmpint(_idx, <, nm_setting_connection_get_num_permissions(_s_con)); \
g_assert( \
nm_setting_connection_get_permission(_s_con, _idx, _p_ptype, _p_pitem, _p_detail)); \
if (_p_ptype) \
g_assert_cmpstr(_ptype, ==, "invalid"); \
if (_p_pitem) { \
const char *_expected_item = (expected_item); \
\
if (!_expected_item) \
g_assert_cmpstr(_pitem, !=, NULL); \
else \
g_assert_cmpstr(_pitem, ==, _expected_item); \
} \
if (_p_detail) \
g_assert_cmpstr(_detail, ==, NULL); \
} \
G_STMT_END
/* Ensure a bad [type] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(
NMTST_G_RETURN_MSG(strncmp(str, PERM_USER_PREFIX, strlen(PERM_USER_PREFIX)) == 0));
add_permission_property(s_con, "foobar", "blah", -1, NULL);
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, "foobar:blah:");
/* Ensure a bad [type] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(
NMTST_G_RETURN_MSG(strncmp(str, PERM_USER_PREFIX, strlen(PERM_USER_PREFIX)) == 0));
add_permission_property(s_con, NULL, "blah", -1, NULL);
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, ":blah:");
/* Ensure a bad [item] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(last_colon > str));
add_permission_property(s_con, "user", NULL, -1, NULL);
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, "user::");
/* Ensure a bad [item] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(last_colon > str));
add_permission_property(s_con, "user", "", -1, NULL);
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, "user::");
/* Ensure an [item] with ':' in the middle is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(str[i] != ':'));
add_permission_property(s_con, "user", "ad:asdf", -1, NULL);
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, "user:ad:asdf:");
/* Ensure an [item] with ':' at the end is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(str[i] != ':'));
add_permission_property(s_con, "user", "adasdfaf:", -1, NULL);
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, "user:adasdfaf::");
/* Ensure a non-UTF-8 [item] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(g_utf8_validate(str, -1, NULL) == TRUE));
add_permission_property(s_con, "user", buf, (int) sizeof(buf), NULL);
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, NULL);
/* Ensure a non-NULL [detail] is rejected */
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(*(last_colon + 1) == '\0'));
add_permission_property(s_con, "user", "dafasdf", -1, "asdf");
g_test_assert_expected_messages();
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
_assert_permission_invalid_at_idx(s_con, 0, "user:dafasdf:asdf");
/* Ensure a valid call results in success */
success = nm_setting_connection_add_permission(s_con, "user", TEST_UNAME, NULL);
g_assert(success);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
check_permission(s_con, 0, TEST_UNAME);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 2);
_assert_permission_invalid_at_idx(s_con, 0, "user:dafasdf:asdf");
check_permission(s_con, 1, TEST_UNAME);
/* Now remove that permission and ensure we have 0 permissions */
nm_setting_connection_remove_permission(s_con, 0);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 0);
g_object_unref(s_con);
g_assert_cmpint(nm_setting_connection_get_num_permissions(s_con), ==, 1);
}
static void
@ -4780,7 +4791,7 @@ test_setting_connection_changed_signal(void)
ASSERT_CHANGED(nm_setting_connection_add_permission(s_con, "user", "billsmith", NULL));
ASSERT_CHANGED(nm_setting_connection_remove_permission(s_con, 0));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(iter != NULL));
NMTST_EXPECT_LIBNM_CRITICAL(NMTST_G_RETURN_MSG(idx < nm_g_array_len(priv->permissions)));
ASSERT_UNCHANGED(nm_setting_connection_remove_permission(s_con, 1));
g_test_assert_expected_messages();

18
src/settings/nm-settings-connection.c
View file

@ -359,10 +359,13 @@ nm_settings_connection_check_visibility(NMSettingsConnection *self,
return TRUE;
for (i = 0; i < num; i++) {
const char *ptype;
const char *user;
uid_t uid;
if (!nm_setting_connection_get_permission(s_con, i, NULL, &user, NULL))
if (!nm_setting_connection_get_permission(s_con, i, &ptype, &user, NULL))
continue;
if (!nm_streq(ptype, NM_SETTINGS_CONNECTION_PERMISSION_USER))
continue;
if (!nm_utils_name_to_uid(user, &uid))
continue;
@ -407,6 +410,8 @@ nm_settings_connection_check_permission(NMSettingsConnection *self, const char *
}
for (i = 0; i < num; i++) {
const char *ptype;
/* For each user get their secret agent and check if that agent has the
* required permission.
*
@ -414,10 +419,13 @@ nm_settings_connection_check_permission(NMSettingsConnection *self, const char *
* name or a PID but if the user isn't running an agent they won't have
* either.
*/
if (nm_setting_connection_get_permission(s_con, i, NULL, &puser, NULL)) {
if (nm_agent_manager_has_agent_with_permission(priv->agent_mgr, puser, permission))
return TRUE;
}
if (!nm_setting_connection_get_permission(s_con, i, &ptype, &puser, NULL))
continue;
if (!nm_streq(ptype, NM_SETTINGS_CONNECTION_PERMISSION_USER))
continue;
if (nm_agent_manager_has_agent_with_permission(priv->agent_mgr, puser, permission))
return TRUE;
}
return FALSE;

42
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
View file

@ -2029,15 +2029,15 @@ write_dcb_setting(NMConnection *connection, shvarFile *ifcfg, GError **error)
static void
write_connection_setting(NMSettingConnection *s_con, shvarFile *ifcfg)
{
guint32 n, i;
GString * str;
const char * master, *master_iface = NULL, *type;
int vint;
gint32 vint32;
NMSettingConnectionMdns mdns;
NMSettingConnectionLlmnr llmnr;
guint32 vuint32;
const char * tmp, *mud_url;
guint32 n, i;
nm_auto_free_gstring GString *str = NULL;
const char * master, *master_iface = NULL, *type;
int vint;
gint32 vint32;
NMSettingConnectionMdns mdns;
NMSettingConnectionLlmnr llmnr;
guint32 vuint32;
const char * tmp, *mud_url;
svSetValueStr(ifcfg, "NAME", nm_setting_connection_get_id(s_con));
svSetValueStr(ifcfg, "UUID", nm_setting_connection_get_uuid(s_con));
@ -2083,22 +2083,25 @@ write_connection_setting(NMSettingConnection *s_con, shvarFile *ifcfg)
/* Permissions */
n = nm_setting_connection_get_num_permissions(s_con);
if (n > 0) {
str = g_string_sized_new(n * 20);
nm_gstring_prepare(&str);
for (i = 0; i < n; i++) {
const char *ptype = NULL;
const char *puser = NULL;
if (!nm_setting_connection_get_permission(s_con, i, &ptype, &puser, NULL))
continue;
if (!nm_streq(ptype, NM_SETTINGS_CONNECTION_PERMISSION_USER))
continue;
/* Items separated by space for consistency with eg
* IPV6ADDR_SECONDARIES and DOMAIN.
*/
if (str->len)
g_string_append_c(str, ' ');
if (nm_setting_connection_get_permission(s_con, i, NULL, &puser, NULL))
g_string_append(str, puser);
g_string_append(str, puser);
}
svSetValueStr(ifcfg, "USERS", str->str);
g_string_free(str, TRUE);
}
svSetValueStr(ifcfg, "ZONE", nm_setting_connection_get_zone(s_con));
@ -2160,22 +2163,21 @@ write_connection_setting(NMSettingConnection *s_con, shvarFile *ifcfg)
/* secondary connection UUIDs */
n = nm_setting_connection_get_num_secondaries(s_con);
if (n > 0) {
str = g_string_sized_new(n * 37);
nm_gstring_prepare(&str);
for (i = 0; i < n; i++) {
const char *uuid;
/* Items separated by space for consistency with eg
* IPV6ADDR_SECONDARIES and DOMAIN.
*/
if (!(uuid = nm_setting_connection_get_secondary(s_con, i)))
continue;
if (str->len)
g_string_append_c(str, ' ');
if ((uuid = nm_setting_connection_get_secondary(s_con, i)) != NULL)
g_string_append(str, uuid);
g_string_append(str, uuid);
}
svSetValueStr(ifcfg, "SECONDARY_UUIDS", str->str);
g_string_free(str, TRUE);
}
vuint32 = nm_setting_connection_get_gateway_ping_timeout(s_con);