Skip to content

Commit

Permalink
ksmbd: add support for supplementary groups
Browse files Browse the repository at this point in the history 
Even though system user has a supplementary group, It gets
NT_STATUS_ACCESS_DENIED when attempting to create file or directory.
This patch add KSMBD_EVENT_LOGIN_REQUEST_EXT/RESPONSE_EXT netlink events
to get supplementary groups list. The new netlink event doesn't break
backward compatibility when using old ksmbd-tools.

Co-authored-by: Atte Heikkilä <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
  • Loading branch information
@namjaejeon @atheik
namjaejeon and atheik committed Oct 7, 2024
1 parent 51480c0 commit be97dff
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 17 deletions.
6 changes: 5 additions & 1 deletion auth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ int ksmbd_krb5_authenticate(struct ksmbd_session *sess, char *in_blob,
int in_len, char *out_blob, int *out_len)
{
struct ksmbd_spnego_authen_response *resp;
struct ksmbd_login_response_ext *resp_ext;

This comment has been minimized.

Sign in to view

Copy link
@atheik

atheik Oct 7, 2024 •
edited
Loading

Author Contributor

struct ksmbd_login_response_ext *resp_ext should be initialized to NULL.
struct ksmbd_user *user = NULL;
int retval;

Expand Down Expand Up @@ -826,7 +827,10 @@ int ksmbd_krb5_authenticate(struct ksmbd_session *sess, char *in_blob,
goto out;
}

user = ksmbd_alloc_user(&resp->login_response);
if (resp->login_response.status & KSMBD_USER_FLAG_EXTENSION)
resp_ext = ksmbd_ipc_login_request_ext(resp->login_response.account);

user = ksmbd_alloc_user(&resp->login_response, resp_ext);
if (!user) {
ksmbd_debug(AUTH, "login failure\n");
retval = -ENOMEM;
Expand Down
17 changes: 17 additions & 0 deletions ksmbd_netlink.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@
* - KSMBD_EVENT_SPNEGO_AUTHEN_REQUEST/RESPONSE(ksmbd_spnego_authen_request/response)
* This event is to make kerberos authentication to be processed in
* userspace.
*
* - KSMBD_EVENT_LOGIN_REQUEST_EXT/RESPONSE_EXT(ksmbd_login_request_ext/response_ext)
* This event is to get user account extension info to user IPC daemon.
*/

#define KSMBD_GENL_NAME "SMBD_GENL"
Expand Down Expand Up @@ -145,6 +148,16 @@ struct ksmbd_login_response {
__u32 reserved[16]; /* Reserved room */
};

/*
* IPC user login response extension.
*/
struct ksmbd_login_response_ext {
__u32 handle;
__u32 ngroups; /* supplementary group count */
__s8 reserved[128]; /* Reserved room */
__s8 ____payload[];
};

/*
* IPC request to fetch net share config.
*/
Expand Down Expand Up @@ -306,6 +319,9 @@ enum ksmbd_event {
KSMBD_EVENT_SPNEGO_AUTHEN_REQUEST,
KSMBD_EVENT_SPNEGO_AUTHEN_RESPONSE = 15,

KSMBD_EVENT_LOGIN_REQUEST_EXT,
KSMBD_EVENT_LOGIN_RESPONSE_EXT,

__KSMBD_EVENT_MAX,
KSMBD_EVENT_MAX = __KSMBD_EVENT_MAX - 1
};
Expand Down Expand Up @@ -336,6 +352,7 @@ enum KSMBD_TREE_CONN_STATUS {
#define KSMBD_USER_FLAG_BAD_USER BIT(3)
#define KSMBD_USER_FLAG_GUEST_ACCOUNT BIT(4)
#define KSMBD_USER_FLAG_DELAY_SESSION BIT(5)
#define KSMBD_USER_FLAG_EXTENSION BIT(6)

/*
* Share config flags.
Expand Down
43 changes: 35 additions & 8 deletions mgmt/user_config.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
struct ksmbd_user *ksmbd_login_user(const char *account)
{
struct ksmbd_login_response *resp;
struct ksmbd_login_response_ext *resp_ext = NULL;
struct ksmbd_user *user = NULL;

resp = ksmbd_ipc_login_request(account);
Expand All @@ -21,15 +22,19 @@ struct ksmbd_user *ksmbd_login_user(const char *account)
if (!(resp->status & KSMBD_USER_FLAG_OK))
goto out;

user = ksmbd_alloc_user(resp);
if (resp->status & KSMBD_USER_FLAG_EXTENSION)
resp_ext = ksmbd_ipc_login_request_ext(account);

user = ksmbd_alloc_user(resp, resp_ext);
out:
kvfree(resp);
return user;
}

struct ksmbd_user *ksmbd_alloc_user(struct ksmbd_login_response *resp)
struct ksmbd_user *ksmbd_alloc_user(struct ksmbd_login_response *resp,
struct ksmbd_login_response_ext *resp_ext)
{
struct ksmbd_user *user = NULL;
struct ksmbd_user *user;

user = kmalloc(sizeof(struct ksmbd_user), GFP_KERNEL);
if (!user)
Expand All @@ -44,18 +49,40 @@ struct ksmbd_user *ksmbd_alloc_user(struct ksmbd_login_response *resp)
if (user->passkey)
memcpy(user->passkey, resp->hash, resp->hash_sz);

if (!user->name || !user->passkey) {
kfree(user->name);
kfree(user->passkey);
kfree(user);
user = NULL;
user->ngroups = 0;
user->sgid = NULL;

if (!user->name || !user->passkey)
goto err_free;

if (resp_ext) {
if (resp_ext->ngroups > NGROUPS_MAX) {
pr_err("ngroups(%u) from login response exceeds max groups(%d)\n",
resp_ext->ngroups, NGROUPS_MAX);
goto err_free;
}

user->sgid = kmemdup(resp_ext->____payload,
resp_ext->ngroups * sizeof(gid_t),

This comment has been minimized.

Sign in to view

Copy link
@atheik

atheik Oct 7, 2024

Author Contributor

Consider resp_ext->ngroups * sizeof(unsigned int) for consistency.
GFP_KERNEL);
if (user->sgid)

This comment has been minimized.

Sign in to view

Copy link
@atheik

atheik Oct 7, 2024

Author Contributor

kmemdup() fails only on OoM, right?
Consider being consistent with the rest of ksmbd_alloc_user():

if (!user->sgid)
	goto err_free;
user->ngroups = resp_ext->ngroups;
ksmbd_debug(SMB, "supplementary groups : %d\n", user->ngroups);
user->ngroups = resp_ext->ngroups;
ksmbd_debug(SMB, "supplementary groups : %d\n", user->ngroups);
}

return user;

err_free:
kfree(user->name);
kfree(user->passkey);
kfree(user);
return NULL;
}

void ksmbd_free_user(struct ksmbd_user *user)
{
ksmbd_ipc_logout_request(user->name, user->flags);
kfree(user->sgid);
kfree(user->name);
kfree(user->passkey);
kfree(user);
Expand Down
5 changes: 4 additions & 1 deletion mgmt/user_config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ struct ksmbd_user {

size_t passkey_sz;
char *passkey;
unsigned int ngroups;
unsigned int *sgid;
};

static inline bool user_guest(struct ksmbd_user *user)
Expand Down Expand Up @@ -60,7 +62,8 @@ static inline unsigned int user_gid(struct ksmbd_user *user)
}

struct ksmbd_user *ksmbd_login_user(const char *account);
struct ksmbd_user *ksmbd_alloc_user(struct ksmbd_login_response *resp);
struct ksmbd_user *ksmbd_alloc_user(struct ksmbd_login_response *resp,
struct ksmbd_login_response_ext *resp_ext);
void ksmbd_free_user(struct ksmbd_user *user);
int ksmbd_anonymous_user(struct ksmbd_user *user);
bool ksmbd_compare_user(struct ksmbd_user *u1, struct ksmbd_user *u2);
Expand Down
21 changes: 18 additions & 3 deletions smb_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -781,13 +781,15 @@ int __ksmbd_override_fsids(struct ksmbd_work *work,
struct ksmbd_share_config *share)
{
struct ksmbd_session *sess = work->sess;
struct ksmbd_user *user = sess->user;
struct cred *cred;
struct group_info *gi;
unsigned int uid;
unsigned int gid;
int i;

uid = user_uid(sess->user);
gid = user_gid(sess->user);
uid = user_uid(user);
gid = user_gid(user);
if (share->force_uid != KSMBD_SHARE_INVALID_UID)
uid = share->force_uid;
if (share->force_gid != KSMBD_SHARE_INVALID_GID)
Expand All @@ -800,11 +802,24 @@ int __ksmbd_override_fsids(struct ksmbd_work *work,
cred->fsuid = make_kuid(&init_user_ns, uid);
cred->fsgid = make_kgid(&init_user_ns, gid);

gi = groups_alloc(0);
gi = groups_alloc(user->ngroups);
if (!gi) {
abort_creds(cred);
return -ENOMEM;
}

for (i = 0; i < user->ngroups; i++) {
if (gid_eq(GLOBAL_ROOT_GID,
make_kgid(&init_user_ns, user->sgid[i])))
gi->gid[i] = cred->fsgid;
else
gi->gid[i] = make_kgid(&init_user_ns,
user->sgid[i]);
}

if (user->ngroups)
groups_sort(gi);

set_groups(cred, gi);
put_group_info(gi);

Expand Down
64 changes: 60 additions & 4 deletions transport_ipc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ static const struct nla_policy ksmbd_nl_policy[KSMBD_EVENT_MAX + 1] = {
},
[KSMBD_EVENT_SPNEGO_AUTHEN_RESPONSE] = {
},
[KSMBD_EVENT_LOGIN_REQUEST_EXT] = {
.len = sizeof(struct ksmbd_login_request),
},
[KSMBD_EVENT_LOGIN_RESPONSE_EXT] = {
.len = sizeof(struct ksmbd_login_response_ext),
},
};

static struct genl_ops ksmbd_genl_ops[] = {
Expand Down Expand Up @@ -187,6 +193,14 @@ static struct genl_ops ksmbd_genl_ops[] = {
.cmd = KSMBD_EVENT_SPNEGO_AUTHEN_RESPONSE,
.doit = handle_generic_event,
},
{
.cmd = KSMBD_EVENT_LOGIN_REQUEST_EXT,
.doit = handle_unsupported_event,
},
{
.cmd = KSMBD_EVENT_LOGIN_RESPONSE_EXT,
.doit = handle_generic_event,
},
};

static struct genl_family ksmbd_genl_family = {
Expand All @@ -199,7 +213,7 @@ static struct genl_family ksmbd_genl_family = {
.ops = ksmbd_genl_ops,
.n_ops = ARRAY_SIZE(ksmbd_genl_ops),
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 1, 0)
.resv_start_op = KSMBD_EVENT_SPNEGO_AUTHEN_RESPONSE + 1,
.resv_start_op = KSMBD_EVENT_LOGIN_RESPONSE_EXT + 1,
#endif
};

Expand Down Expand Up @@ -461,16 +475,24 @@ static int ipc_validate_msg(struct ipc_msg_table_entry *entry)
{
unsigned int msg_sz = entry->msg_sz;

if (entry->type == KSMBD_EVENT_RPC_REQUEST) {
switch (entry->type) {
case KSMBD_EVENT_RPC_REQUEST:
{
struct ksmbd_rpc_command *resp = entry->response;

msg_sz = sizeof(struct ksmbd_rpc_command) + resp->payload_sz;
} else if (entry->type == KSMBD_EVENT_SPNEGO_AUTHEN_REQUEST) {
break;
}
case KSMBD_EVENT_SPNEGO_AUTHEN_REQUEST:
{
struct ksmbd_spnego_authen_response *resp = entry->response;

msg_sz = sizeof(struct ksmbd_spnego_authen_response) +
resp->session_key_len + resp->spnego_blob_len;
} else if (entry->type == KSMBD_EVENT_SHARE_CONFIG_REQUEST) {
break;
}
case KSMBD_EVENT_SHARE_CONFIG_REQUEST:
{
struct ksmbd_share_config_response *resp = entry->response;

if (resp->payload_sz) {
Expand All @@ -480,6 +502,17 @@ static int ipc_validate_msg(struct ipc_msg_table_entry *entry)
msg_sz = sizeof(struct ksmbd_share_config_response) +
resp->payload_sz;
}
break;
}
case KSMBD_EVENT_LOGIN_REQUEST_EXT:
{
struct ksmbd_login_response_ext *resp = entry->response;

if (resp->ngroups) {
msg_sz = sizeof(struct ksmbd_login_response_ext) +
resp->ngroups * sizeof(gid_t);

This comment has been minimized.

Sign in to view

Copy link
@atheik

atheik Oct 7, 2024

Author Contributor

Is it sizeof(gid_t) or sizeof(unsigned int)?

They are the same size and signedness but what is the motivation for ksmbd sometimes using the former and sometimes using the latter, e.g. struct ksmbd_user uses unsigned int instead of gid_t?
}
}
}

return entry->msg_sz != msg_sz ? -EINVAL : 0;
Expand Down Expand Up @@ -561,6 +594,29 @@ struct ksmbd_login_response *ksmbd_ipc_login_request(const char *account)
return resp;
}

struct ksmbd_login_response_ext *ksmbd_ipc_login_request_ext(const char *account)
{
struct ksmbd_ipc_msg *msg;
struct ksmbd_login_request *req;
struct ksmbd_login_response_ext *resp;

if (strlen(account) >= KSMBD_REQ_MAX_ACCOUNT_NAME_SZ)
return NULL;

msg = ipc_msg_alloc(sizeof(struct ksmbd_login_request));
if (!msg)
return NULL;

msg->type = KSMBD_EVENT_LOGIN_REQUEST_EXT;
req = (struct ksmbd_login_request *)msg->payload;
req->handle = ksmbd_acquire_id(&ipc_ida);
strscpy(req->account, account, KSMBD_REQ_MAX_ACCOUNT_NAME_SZ);
resp = ipc_msg_send_request(msg, req->handle);
ipc_msg_handle_free(req->handle);
ipc_msg_free(msg);
return resp;
}

struct ksmbd_spnego_authen_response *
ksmbd_ipc_spnego_authen_request(const char *spnego_blob, int blob_len)
{
Expand Down
2 changes: 2 additions & 0 deletions transport_ipc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

struct ksmbd_login_response *
ksmbd_ipc_login_request(const char *account);
struct ksmbd_login_response_ext *
ksmbd_ipc_login_request_ext(const char *account);

struct ksmbd_session;
struct ksmbd_share_config;
Expand Down

0 comments on commit be97dff

Please sign in to comment.