Re: [PATCH review 48/85] sunrpc: Update gss uid to security contextmapping.

From: J. Bruce Fields
Date: Wed Feb 13 2013 - 15:35:45 EST


On Wed, Feb 13, 2013 at 09:51:37AM -0800, Eric W. Biederman wrote:
> From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>
> - Use from_kuid when generating the on the wire uid values.
> - Use make_kuid when reading on the wire values.
>
> In gss_encode_v0_msg, since the uid in gss_upcall_msg is now a kuid_t
> generate the necessary uid_t value on the stack copy it into
> gss_msg->databuf where it can safely live until the message is no
> longer needed.

Apologies, I haven't been following the user namespace work.

If I understand correctly, you're expecting the id's seen in nfs
protocol messages to be the same as the id's seen in the initial user
namespace.

Why is that right, and not, say, the user namespace in which the mount
was originally performed? (Just asking, I honestly haven't thought
about it before.)

Also:

> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> index afbbcfb..a360067 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -395,8 +395,11 @@ gss_upcall_callback(struct rpc_task *task)
>
> static void gss_encode_v0_msg(struct gss_upcall_msg *gss_msg)
> {
> - gss_msg->msg.data = &gss_msg->uid;
> - gss_msg->msg.len = sizeof(gss_msg->uid);
> + uid_t uid = from_kuid(&init_user_ns, gss_msg->uid);
> + memcpy(gss_msg->databuf, &uid, sizeof(uid));
> + gss_msg->msg.data = gss_msg->databuf;
> + gss_msg->msg.len = sizeof(uid);
> + BUG_ON(sizeof(uid) > UPCALL_BUF_LEN);

This message is going to gssd, not to the server. Should it be encoded
for whatever namespace gssd lives in?

--b.

> }
>
> static void gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
> @@ -409,7 +412,7 @@ static void gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
>
> gss_msg->msg.len = sprintf(gss_msg->databuf, "mech=%s uid=%d ",
> mech->gm_name,
> - gss_msg->uid);
> + from_kuid(&init_user_ns, gss_msg->uid));
> p += gss_msg->msg.len;
> if (clnt->cl_principal) {
> len = sprintf(p, "target=%s ", clnt->cl_principal);
> @@ -620,7 +623,8 @@ gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
> struct gss_upcall_msg *gss_msg;
> struct rpc_pipe *pipe = RPC_I(filp->f_dentry->d_inode)->pipe;
> struct gss_cl_ctx *ctx;
> - uid_t uid;
> + uid_t id;
> + kuid_t uid;
> ssize_t err = -EFBIG;
>
> if (mlen > MSG_BUF_MAXSIZE)
> @@ -635,12 +639,18 @@ gss_pipe_downcall(struct file *filp, const char __user *src, size_t mlen)
> goto err;
>
> end = (const void *)((char *)buf + mlen);
> - p = simple_get_bytes(buf, end, &uid, sizeof(uid));
> + p = simple_get_bytes(buf, end, &id, sizeof(id));
> if (IS_ERR(p)) {
> err = PTR_ERR(p);
> goto err;
> }
>
> + uid = make_kuid(&init_user_ns, id);
> + if (!uid_valid(uid)) {
> + err = -EINVAL;
> + goto err;
> + }
> +
> err = -ENOMEM;
> ctx = gss_alloc_context();
> if (ctx == NULL)
> --
> 1.7.5.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/