Re: [PATCH v2 2/2] fuse: Send security context of inode on file creation

On Tue, Nov 02, 2021 at 04:38:06PM +0100, Miklos Szeredi wrote:

On Tue, 2 Nov 2021 at 16:30, Vivek Goyal [off-list ref] wrote:

quoted

On Tue, Nov 02, 2021 at 03:00:30PM +0100, Miklos Szeredi wrote:

quoted

On Tue, 12 Oct 2021 at 20:06, Vivek Goyal [off-list ref] wrote:

quoted

quoted

quoted

@@ -633,7 +713,29 @@ static int create_new_entry(struct fuse_mount *fm, struct fuse_args *args,
        args->out_numargs = 1;
        args->out_args[0].size = sizeof(outarg);
        args->out_args[0].value = &outarg;
+
+       if (init_security) {

Hi Miklos,

quoted

Instead of a new arg to create_new_entry(), this could check
args.opcode != FUSE_LINK.

Will do.

quoted

quoted

+               unsigned short idx = args->in_numargs;
+
+               if ((size_t)idx >= ARRAY_SIZE(args->in_args)) {
+                       err = -ENOMEM;
+                       goto out_put_forget_req;
+               }
+
+               err = get_security_context(entry, mode, &security_ctx,
+                                          &security_ctxlen);
+               if (err)
+                       goto out_put_forget_req;
+
+               if (security_ctxlen > 0) {

This doesn't seem right.  How would the server know if this is arg is missing?

I think if FUSE_SECURITY_CTX was negotiated, then the secctx header
will always need to be added to the MK* requests.

Even for the case of FUSE_LINK request? I think I put this check because
FUSE_LINK is not sending secctx header. Other requests are appending
this header even if a security module is not loaded/enabled.

No, FUSE_LINK wouldn't even get this far.

You are right. My bad. So looks like this check will always be true
given the current code. get_security_context() will either all
headers with security context or just return zeroed "struct fuse_secctxs",
indicating there are no security context. 

If that's the case, I should be able to get rid of this check. I will
do some more testing.

quoted

I guess it makes more sense to add secctx header even for FUSE_LINK
request. Just that header will mention 0 security contexts are
following. This will interface more uniform. I will make this change.

Why? FUSE_LINK is not an inode creation op, it just shares the
instantiation part with creation.

Ok, got it. Makes sense. So no sending of zeroed security context header
with FUSE_LINK.

Vivek