From f19c8549054ce50912d556f408bb6f08a261fde7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 24 Oct 2023 16:42:21 +0200 Subject: [PATCH 1/4] glib-aux/tests: add test for nm_utils_uid_to_name() --- .../tests/test-shared-general.c | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/libnm-glib-aux/tests/test-shared-general.c b/src/libnm-glib-aux/tests/test-shared-general.c index 4a2dec0b26..69b1dd18bd 100644 --- a/src/libnm-glib-aux/tests/test-shared-general.c +++ b/src/libnm-glib-aux/tests/test-shared-general.c @@ -5,6 +5,8 @@ #include "libnm-glib-aux/nm-default-glib-i18n-prog.h" +#include + #include "libnm-std-aux/unaligned.h" #include "libnm-glib-aux/nm-random-utils.h" #include "libnm-glib-aux/nm-str-buf.h" @@ -2580,6 +2582,36 @@ test_nm_prioq(void) /*****************************************************************************/ +static const char * +_getpwuid_name(uid_t uid) +{ + static struct passwd *pw; + + pw = getpwuid(uid); + return pw ? nm_str_not_empty(pw->pw_name) : NULL; +} + +static void +test_uid_to_name(void) +{ + int i; + + for (i = 0; i < 20; i++) { + gs_free char *name = NULL; + uid_t uid; + + if (i < 5) + uid = i; + else + uid = nmtst_get_rand_uint32() % 2000u; + + name = nm_utils_uid_to_name(uid); + g_assert_cmpstr(name, ==, _getpwuid_name(uid)); + } +} + +/*****************************************************************************/ + NMTST_DEFINE(); int @@ -2630,6 +2662,7 @@ main(int argc, char **argv) g_test_add_func("/general/test_garray", test_garray); g_test_add_func("/general/test_nm_prioq", test_nm_prioq); g_test_add_func("/general/test_nm_random", test_nm_random); + g_test_add_func("/general/test_uid_to_name", test_uid_to_name); return g_test_run(); } From b2b2823c53b915f409c6f81133a706ee2c9e80f0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Oct 2023 12:11:33 +0200 Subject: [PATCH 2/4] core: avoid getpwuid() unless necessary in permission check Most profiles don't have "connection.permissions" set. Avoid resolving the UID to the name via getpwuid() (in nm_auth_is_subject_in_acl()), until we know that we require it. --- src/core/nm-auth-utils.c | 11 +---- src/libnm-core-impl/nm-setting-connection.c | 55 ++++++++++++++++----- src/libnm-core-intern/nm-core-internal.h | 3 ++ 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/core/nm-auth-utils.c b/src/core/nm-auth-utils.c index 7739f44376..aa2547b3e8 100644 --- a/src/core/nm-auth-utils.c +++ b/src/core/nm-auth-utils.c @@ -8,6 +8,7 @@ #include "nm-auth-utils.h" #include "libnm-glib-aux/nm-c-list.h" +#include "libnm-core-intern/nm-core-internal.h" #include "nm-setting-connection.h" #include "libnm-core-aux-intern/nm-auth-subject.h" #include "nm-auth-manager.h" @@ -603,7 +604,6 @@ gboolean nm_auth_is_subject_in_acl(NMConnection *connection, NMAuthSubject *subject, char **out_error_desc) { NMSettingConnection *s_con; - gs_free char *user = NULL; gulong uid; g_return_val_if_fail(connection, FALSE); @@ -621,13 +621,6 @@ nm_auth_is_subject_in_acl(NMConnection *connection, NMAuthSubject *subject, char if (0 == uid) return TRUE; - user = nm_utils_uid_to_name(uid); - if (!user) { - NM_SET_OUT(out_error_desc, - g_strdup_printf("Could not determine username for uid %lu", uid)); - return FALSE; - } - s_con = nm_connection_get_setting_connection(connection); if (!s_con) { /* This can only happen when called from AddAndActivate, so we know @@ -637,7 +630,7 @@ nm_auth_is_subject_in_acl(NMConnection *connection, NMAuthSubject *subject, char } /* Match the username returned by the session check to a user in the ACL */ - if (!nm_setting_connection_permissions_user_allowed(s_con, user)) { + if (!nm_setting_connection_permissions_user_allowed_by_uid(s_con, uid)) { NM_SET_OUT(out_error_desc, g_strdup_printf("uid %lu has no permission to perform this operation", uid)); return FALSE; diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index 2e787174c6..725626c835 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -355,19 +355,10 @@ invalid: return TRUE; } -/** - * nm_setting_connection_permissions_user_allowed: - * @setting: the #NMSettingConnection - * @uname: the user name to check permissions for - * - * Checks whether the given username is allowed to view/access this connection. - * - * Returns: %TRUE if the requested user is allowed to view this connection, - * %FALSE if the given user is not allowed to view this connection - */ -gboolean -nm_setting_connection_permissions_user_allowed(NMSettingConnection *setting, const char *uname) +static gboolean +_permissions_user_allowed(NMSettingConnection *setting, const char *uname, gulong uid) { + gs_free char *uname_free = NULL; NMSettingConnectionPrivate *priv; guint i; @@ -384,13 +375,51 @@ nm_setting_connection_permissions_user_allowed(NMSettingConnection *setting, con for (i = 0; i < priv->permissions->len; i++) { const Permission *permission = &nm_g_array_index(priv->permissions, Permission, i); - if (permission->ptype == PERM_TYPE_USER && nm_streq(permission->item, uname)) + if (permission->ptype != PERM_TYPE_USER) + continue; + + if (!uname) { + if (uid != G_MAXULONG) + uname_free = nm_utils_uid_to_name(uid); + if (!uname_free) + return FALSE; + uname = uname_free; + } + + if (nm_streq(permission->item, uname)) return TRUE; } return FALSE; } +/** + * nm_setting_connection_permissions_user_allowed: + * @setting: the #NMSettingConnection + * @uname: the user name to check permissions for + * + * Checks whether the given username is allowed to view/access this connection. + * + * Returns: %TRUE if the requested user is allowed to view this connection, + * %FALSE if the given user is not allowed to view this connection + */ +gboolean +nm_setting_connection_permissions_user_allowed(NMSettingConnection *setting, const char *uname) +{ + g_return_val_if_fail(NM_IS_SETTING_CONNECTION(setting), FALSE); + g_return_val_if_fail(uname != NULL, FALSE); + + return _permissions_user_allowed(setting, uname, G_MAXULONG); +} + +gboolean +nm_setting_connection_permissions_user_allowed_by_uid(NMSettingConnection *setting, gulong uid) +{ + g_return_val_if_fail(NM_IS_SETTING_CONNECTION(setting), FALSE); + + return _permissions_user_allowed(setting, NULL, uid); +} + /** * 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 c5a0e8b081..766c7dad1b 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -533,6 +533,9 @@ GPtrArray *_nm_setting_bridge_port_get_vlans(NMSettingBridgePort *setting); GArray *_nm_setting_connection_get_secondaries(NMSettingConnection *setting); +gboolean nm_setting_connection_permissions_user_allowed_by_uid(NMSettingConnection *setting, + gulong uid); + /*****************************************************************************/ NMSettingBluetooth *_nm_connection_get_setting_bluetooth_for_nap(NMConnection *connection); From 5a7d1ec208bc4c5a8dfb99ab4c10487c36482527 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Oct 2023 12:53:31 +0200 Subject: [PATCH 3/4] glib-aux: add nm_getpwuid() helper Calling getpwuid_r() is cumbersome, because it has a separate passwd and string buffer, and you shall retry, when the buffer is too small. Extract nm_getpwuid() for that. This one always allocates a suitable buffer, that the caller can free. This will allow callers to get the full passwd struct. It will also allow callers to avoid the additional strdup() of nm_utils_uid_to_name(), when we don't need a clone of the string. --- src/libnm-glib-aux/nm-shared-utils.c | 77 ++++++++++++++----- src/libnm-glib-aux/nm-shared-utils.h | 6 ++ .../tests/test-shared-general.c | 8 +- 3 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 7e04b690a3..34f5ce4185 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -5771,40 +5771,81 @@ nm_utils_is_specific_hostname(const char *name) /*****************************************************************************/ -/* taken from systemd's uid_to_name(). */ -char * -nm_utils_uid_to_name(uid_t uid) -{ - gs_free char *buf_heap = NULL; - char buf_stack[4096]; - gsize bufsize; - char *buf; +typedef struct { + struct passwd pw; + _nm_alignas(max_align_t) char buf[]; +} GetPwuidData; - bufsize = sizeof(buf_stack); - buf = buf_stack; +/** + * nm_getpwuid: + * uid: the user Id to look up + * + * Calls getpwuid_r() to lookup the passwd entry. See the manual. + * Allocates and returns a suitable buffer. + * + * The returned buffer is likely much large than required. You don't want to + * keep this buffer around for longer than necessary. + * + * Returns: (transfer full): the passwd entry, if found or NULL on error + * or if the entry was not found. + */ +struct passwd * +nm_getpwuid(uid_t uid) +{ + gs_free GetPwuidData *data = NULL; + gsize bufsize; + const gsize OFFSET = G_STRUCT_OFFSET(GetPwuidData, buf); + long int size_max; + + size_max = sysconf(_SC_GETPW_R_SIZE_MAX); + if (size_max > 0 && ((unsigned long int) size_max < G_MAXSIZE - OFFSET)) + bufsize = size_max; + else + bufsize = 4096; for (;;) { - struct passwd pwbuf; struct passwd *pw = NULL; int r; - r = getpwuid_r(uid, &pwbuf, buf, bufsize, &pw); - if (r == 0 && pw) - return nm_strdup_not_empty(pw->pw_name); + if (bufsize >= G_MAXSIZE - OFFSET) + return NULL; + + nm_clear_g_free(&data); + data = g_malloc(OFFSET + bufsize); + + r = getpwuid_r(uid, &data->pw, data->buf, bufsize, &pw); + if (r == 0) { + if (!pw) + return NULL; + nm_assert(pw == (gpointer) data); + return (gpointer) g_steal_pointer(&data); + } if (r != ERANGE) return NULL; if (bufsize > G_MAXSIZE / 2u) return NULL; - bufsize *= 2u; - g_free(buf_heap); - buf_heap = g_malloc(bufsize); - buf = buf_heap; } } +const char * +nm_passwd_name(const struct passwd *pw) +{ + /* Normalize "pw->pw_name" and return it. */ + return pw ? nm_str_not_empty(pw->pw_name) : NULL; +} + +char * +nm_utils_uid_to_name(uid_t uid) +{ + gs_free struct passwd *pw = NULL; + + pw = nm_getpwuid(uid); + return g_strdup(nm_passwd_name(pw)); +} + /* taken from systemd's nss_user_record_by_name() */ gboolean nm_utils_name_to_uid(const char *name, uid_t *out_uid) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index ab2dc013b7..c7278d1acb 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3196,6 +3196,12 @@ gboolean nm_utils_is_localhost(const char *name); gboolean nm_utils_is_specific_hostname(const char *name); +struct passwd; + +struct passwd *nm_getpwuid(uid_t uid); + +const char *nm_passwd_name(const struct passwd *pw); + char *nm_utils_uid_to_name(uid_t uid); gboolean nm_utils_name_to_uid(const char *name, uid_t *out_uid); diff --git a/src/libnm-glib-aux/tests/test-shared-general.c b/src/libnm-glib-aux/tests/test-shared-general.c index 69b1dd18bd..416d26384f 100644 --- a/src/libnm-glib-aux/tests/test-shared-general.c +++ b/src/libnm-glib-aux/tests/test-shared-general.c @@ -2597,8 +2597,9 @@ test_uid_to_name(void) int i; for (i = 0; i < 20; i++) { - gs_free char *name = NULL; - uid_t uid; + gs_free char *name = NULL; + gs_free struct passwd *pw = NULL; + uid_t uid; if (i < 5) uid = i; @@ -2607,6 +2608,9 @@ test_uid_to_name(void) name = nm_utils_uid_to_name(uid); g_assert_cmpstr(name, ==, _getpwuid_name(uid)); + + pw = nm_getpwuid(uid); + g_assert_cmpstr(name, ==, nm_passwd_name(pw)); } } From a7de74018e93b8f595e9d06cabf827714d66afd2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Oct 2023 13:19:26 +0200 Subject: [PATCH 4/4] libnm: use nm_getpwuid() in _permissions_user_allowed() No need to clone the string again. Use nm_getpwuid() directly and avoid an additional clone. --- src/libnm-core-impl/nm-setting-connection.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index 725626c835..dc067f09dc 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -358,7 +358,7 @@ invalid: static gboolean _permissions_user_allowed(NMSettingConnection *setting, const char *uname, gulong uid) { - gs_free char *uname_free = NULL; + gs_free struct passwd *pw = NULL; NMSettingConnectionPrivate *priv; guint i; @@ -379,11 +379,12 @@ _permissions_user_allowed(NMSettingConnection *setting, const char *uname, gulon continue; if (!uname) { - if (uid != G_MAXULONG) - uname_free = nm_utils_uid_to_name(uid); - if (!uname_free) + if (uid != G_MAXULONG) { + pw = nm_getpwuid(uid); + uname = nm_passwd_name(pw); + } + if (!uname) return FALSE; - uname = uname_free; } if (nm_streq(permission->item, uname))