ksmbd-tools: make @group works in ksmbd.conf

[iamwudy]

This commit add the feature that you can use "@groupname" in valid users/invalid users/read list/write list/admin users in ksmbd.conf when you want to add all users in this group, as samba do in smb.conf.

To reach this, some codes moved form parse_list() to cp_insert_user(), and a new function cp_insert_user_group() was added. cp_insert_user_group() scans the file "/etc/group" to find the user list of the group, and than calls cp_insert_user() for each of those users.

After those changes, parse_list() now can correctly deal with "@groupname", not trying to add a user named "@groupname".

Signed-off-by: Wang Junjie iamwudy@qq.com

[namjaejeon]

Thanks for your patch, Please add prefix(ksmbd: ) in patch subject, need to add patch description. Please refer other commits in ksmbd-tools.(https://github.com/cifsd-team/ksmbd-tools/commits/master)

[iamwudy]

I have changed the title and the description. Is anything else should be adjusted?

[namjaejeon]

Ah... You need to change it in your patch, not PR.

[atheik]

@iamwudy, @namjaejeon,

Would you consider the following approach instead of commit fefa793?

[PATCH] ksmbd-tools: add @group support to ksmbd.conf for lists of users (click the arrow to expand)

```diff From d5217735bcb645b40369f077fd4bca2ed1c4c508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Atte=20Heikkil=C3=A4?= Date: Wed, 14 Sep 2022 19:55:19 +0300 Subject: [PATCH] ksmbd-tools: add @group support to ksmbd.conf for lists of users MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For parameters that accept a list of users, make it possible to specify all users belonging to a system group by prefixing the group name with `@'. Add the `grc' parameter to parse_list(), which is the character prefix that identifies a group name. Group members are fetched with getgrnam(), which returns them in a format identical to the `list' parameter of parse_list(), i.e. a NULL terminated list of strings. Since the strings returned by getgrnam() should not be freed, being available only until a subsequent getgrnam() call is made, make it so that the caller is responsible for freeing `list'. It is then possible to simply call parse_list() again in parse_list() to parse the group members. When `grc' is the null terminator, group member lookup is disabled. Note, that it is not possible to confuse an empty string for the disabled prefix due to the preceding cp_ltrim() condition. Co-authored-by: Wang Junjie Signed-off-by: Atte Heikkilä --- ksmbd.conf.5.in | 1 + lib/management/share.c | 54 +++++++++++++++++++++++++++++++++++------- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/ksmbd.conf.5.in b/ksmbd.conf.5.in index cf894ea..da3932d 100644 --- a/ksmbd.conf.5.in +++ b/ksmbd.conf.5.in @@ -15,6 +15,7 @@ A parameter entry consists of a parameter and a value, in that order, separated The parameter may contain leading and trailing tabs and spaces. The value, which begins immediately after the equal sign, may contain leading tabs and spaces or be empty. Some parameter entries can be given a list of multiple values, in which case they are separated by commas, tabs, or spaces. +For a list of users, all users in a system group can be specified by giving the group name prefixed with \fB@\fR. A semicolon (\fB;\fR) or a hash (\fB#\fR) marks the beginning of a comment which continues until the end of the line. .SH SHARES Each section name, except that of the \fBglobal\fR section, defines a shared resource, commonly referred to as a share. diff --git a/lib/management/share.c b/lib/management/share.c index 22e6a83..83b6b1d 100644 --- a/lib/management/share.c +++ b/lib/management/share.c @@ -256,7 +256,7 @@ struct ksmbd_share *shm_lookup_share(char *name) return share; } -static GHashTable *parse_list(GHashTable *map, char **list) +static GHashTable *parse_list(GHashTable *map, char **list, char grc) { int i; @@ -276,6 +276,15 @@ static GHashTable *parse_list(GHashTable *map, char **list) if (!p) continue; + if (*p == grc) { + struct group *gr; + + gr = getgrnam(p + 1); + if (gr) + parse_list(map, gr->gr_mem, 0x00); + continue; + } + user = usm_lookup_user(p); if (!user) { pr_info("Drop non-existing user `%s'\n", p); @@ -291,7 +300,6 @@ static GHashTable *parse_list(GHashTable *map, char **list) g_hash_table_insert(map, user->name, user); } - cp_group_kv_list_free(list); return map; } @@ -472,63 +480,91 @@ static void process_group_kv(gpointer _k, gpointer _v, gpointer user_data) } if (shm_share_config(k, KSMBD_SHARE_CONF_VALID_USERS)) { + char **users_list; + + users_list = cp_get_group_kv_list(v); share->maps[KSMBD_SHARE_VALID_USERS_MAP] = parse_list(share->maps[KSMBD_SHARE_VALID_USERS_MAP], - cp_get_group_kv_list(v)); + users_list, '@'); if (share->maps[KSMBD_SHARE_VALID_USERS_MAP] == NULL) set_share_flag(share, KSMBD_SHARE_FLAG_INVALID); + cp_group_kv_list_free(users_list); return; } if (shm_share_config(k, KSMBD_SHARE_CONF_INVALID_USERS)) { + char **users_list; + + users_list = cp_get_group_kv_list(v); share->maps[KSMBD_SHARE_INVALID_USERS_MAP] = parse_list(share->maps[KSMBD_SHARE_INVALID_USERS_MAP], - cp_get_group_kv_list(v)); + users_list, '@'); if (share->maps[KSMBD_SHARE_INVALID_USERS_MAP] == NULL) set_share_flag(share, KSMBD_SHARE_FLAG_INVALID); + cp_group_kv_list_free(users_list); return; } if (shm_share_config(k, KSMBD_SHARE_CONF_READ_LIST)) { + char **users_list; + + users_list = cp_get_group_kv_list(v); share->maps[KSMBD_SHARE_READ_LIST_MAP] = parse_list(share->maps[KSMBD_SHARE_READ_LIST_MAP], - cp_get_group_kv_list(v)); + users_list, '@'); if (share->maps[KSMBD_SHARE_READ_LIST_MAP] == NULL) set_share_flag(share, KSMBD_SHARE_FLAG_INVALID); + cp_group_kv_list_free(users_list); return; } if (shm_share_config(k, KSMBD_SHARE_CONF_WRITE_LIST)) { + char **users_list; + + users_list = cp_get_group_kv_list(v); share->maps[KSMBD_SHARE_WRITE_LIST_MAP] = parse_list(share->maps[KSMBD_SHARE_WRITE_LIST_MAP], - cp_get_group_kv_list(v)); + users_list, '@'); if (share->maps[KSMBD_SHARE_WRITE_LIST_MAP] == NULL) set_share_flag(share, KSMBD_SHARE_FLAG_INVALID); + cp_group_kv_list_free(users_list); return; } if (shm_share_config(k, KSMBD_SHARE_CONF_ADMIN_USERS)) { + char **users_list; + + users_list = cp_get_group_kv_list(v); share->maps[KSMBD_SHARE_ADMIN_USERS_MAP] = parse_list(share->maps[KSMBD_SHARE_ADMIN_USERS_MAP], - cp_get_group_kv_list(v)); + users_list, '@'); if (share->maps[KSMBD_SHARE_ADMIN_USERS_MAP] == NULL) set_share_flag(share, KSMBD_SHARE_FLAG_INVALID); + cp_group_kv_list_free(users_list); return; } if (shm_share_config(k, KSMBD_SHARE_CONF_HOSTS_ALLOW)) { + char **hosts_list; + + hosts_list = cp_get_group_kv_list(v); share->hosts_allow_map = parse_list(share->hosts_allow_map, - cp_get_group_kv_list(v)); + hosts_list, 0x00); if (share->hosts_allow_map == NULL) set_share_flag(share, KSMBD_SHARE_FLAG_INVALID); + cp_group_kv_list_free(hosts_list); return; } if (shm_share_config(k, KSMBD_SHARE_CONF_HOSTS_DENY)) { + char **hosts_list; + + hosts_list = cp_get_group_kv_list(v); share->hosts_deny_map = parse_list(share->hosts_deny_map, - cp_get_group_kv_list(v)); + hosts_list, 0x00); if (share->hosts_deny_map == NULL) set_share_flag(share, KSMBD_SHARE_FLAG_INVALID); + cp_group_kv_list_free(hosts_list); return; } -- 2.37.3 ```

[namjaejeon]

@iamwudy Can you give ack for @atheik 's change ?

[iamwudy]

I agree with @atheik 's change. It solve a problem that my parse_list() can only deal with valid users/invalid users/read list/write list/admin users. But parse_list() need deal with hosts_allow and hosts_deny too, both them don't have @-syntax. Well done!

[atheik]

@iamwudy,

Thanks for the approval. My approach does disable @ for hosts lists, that's true, but hosts allow and hosts deny do not work at all. parse_list() only supports lists of users and my patch doesn't change that.

[namjaejeon]

@atheik @iamwudy I have applied Atte's patch to mainline. Thanks for your work!

[atheik]

@namjaejeon,

@atheik @iamwudy I have applied Atte's patch to mainline.

You lost the author information when you resolved conflicts in commit 736454f.

[namjaejeon]

You lost the author information when you resolved conflicts in commit https://github.com/cifsd-team/ksmbd-tools/commit/736454faea32b53f97dc395b4be6382f826b2e28.

Sorry, I have recreate the patch because there is some hunk failed on the latest mainline. I have fixed it. Thanks for your report!

[namjaejeon]

This patch was applied, Close this PR. Thanks for your work!