All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Colin Walters <walters@verbum.org>
Cc: linux-fsdevel@vger.kernel.org, virtio-fs@redhat.com,
	selinux@vger.kernel.org,
	LSM List <linux-security-module@vger.kernel.org>,
	chirantan@chromium.org, Miklos Szeredi <miklos@szeredi.hu>,
	stephen.smalley.work@gmail.com,
	Daniel J Walsh <dwalsh@redhat.com>
Subject: Re: [PATCH 2/2] fuse: Send security context of inode on file creation
Date: Fri, 24 Sep 2021 19:32:39 -0400	[thread overview]
Message-ID: <YU5gF9xDhj4g+0Oe@redhat.com> (raw)
In-Reply-To: <a02d3e08-3abc-448a-be32-2640d8a991e0@www.fastmail.com>

On Fri, Sep 24, 2021 at 06:00:10PM -0400, Colin Walters wrote:
> 
> 
> On Fri, Sep 24, 2021, at 3:24 PM, Vivek Goyal wrote:
> > When a new inode is created, send its security context to server along
> > with creation request (FUSE_CREAT, FUSE_MKNOD, FUSE_MKDIR and FUSE_SYMLINK).
> > This gives server an opportunity to create new file and set security
> > context (possibly atomically). In all the configurations it might not
> > be possible to set context atomically.
> >
> > Like nfs and ceph, use security_dentry_init_security() to dermine security
> > context of inode and send it with create, mkdir, mknod, and symlink requests.
> >
> > Following is the information sent to server.
> >
> > - struct fuse_secctx.
> >   This contains total size of security context which follows this structure.
> >
> > - xattr name string.
> >   This string represents name of xattr which should be used while setting
> >   security context. As of now it is hardcoded to "security.selinux".
> 
> Any reason not to just send all `security.*` xattrs found on the inode? 
> 
> (I'm not super familiar with this code, it looks like we're going from the LSM-cached version attached to the inode, but presumably since we're sending bytes we can just ask the filesytem for the raw data instead)

So this inode is about to be created. There are no xattrs yet. And
filesystem is asking LSMs, what security labels should be set on this
inode before it is published. 

For local filesystems it is somewhat easy. They are the one creating
inode and can set all xattrs/labels before inode is added to inode
cache.

But for remote like filesystems, it is more tricky. Actual inode
creation first will happen on server and then client will instantiate
an inode based on information returned by server (Atleast that's
what fuse does).

So security_dentry_init_security() was created (I think by NFS folks)
so that they can query the label and send it along with create
request and server can take care of setting label (along with file
creation).

One limitation of security_dentry_init_security() is that it practically
supports only one label. And only SELinux has implemented. So for
all practical purposes this is a hook to obtain selinux label. NFS
and ceph already use it in that way.

Now there is a desire to be able to return more than one security
labels and support smack and possibly other LSMs. Sure, that great.
But I think for that we will have to implement a new hook which
can return multiple labels and filesystems like nfs, ceph and fuse
will have to be modified to cope with this new hook to support
multiple lables. 

And I am arguing that we can modify fuse when that hook has been
implemented. There is no point in adding that complexity in fuse
code as well all fuse-server implementations when there is nobody
generating multiple labels. We can't even test it.

Thanks
Vivek


WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Colin Walters <walters@verbum.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	selinux@vger.kernel.org, stephen.smalley.work@gmail.com,
	virtio-fs@redhat.com,
	LSM List <linux-security-module@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [Virtio-fs] [PATCH 2/2] fuse: Send security context of inode on file creation
Date: Fri, 24 Sep 2021 19:32:39 -0400	[thread overview]
Message-ID: <YU5gF9xDhj4g+0Oe@redhat.com> (raw)
In-Reply-To: <a02d3e08-3abc-448a-be32-2640d8a991e0@www.fastmail.com>

On Fri, Sep 24, 2021 at 06:00:10PM -0400, Colin Walters wrote:
> 
> 
> On Fri, Sep 24, 2021, at 3:24 PM, Vivek Goyal wrote:
> > When a new inode is created, send its security context to server along
> > with creation request (FUSE_CREAT, FUSE_MKNOD, FUSE_MKDIR and FUSE_SYMLINK).
> > This gives server an opportunity to create new file and set security
> > context (possibly atomically). In all the configurations it might not
> > be possible to set context atomically.
> >
> > Like nfs and ceph, use security_dentry_init_security() to dermine security
> > context of inode and send it with create, mkdir, mknod, and symlink requests.
> >
> > Following is the information sent to server.
> >
> > - struct fuse_secctx.
> >   This contains total size of security context which follows this structure.
> >
> > - xattr name string.
> >   This string represents name of xattr which should be used while setting
> >   security context. As of now it is hardcoded to "security.selinux".
> 
> Any reason not to just send all `security.*` xattrs found on the inode? 
> 
> (I'm not super familiar with this code, it looks like we're going from the LSM-cached version attached to the inode, but presumably since we're sending bytes we can just ask the filesytem for the raw data instead)

So this inode is about to be created. There are no xattrs yet. And
filesystem is asking LSMs, what security labels should be set on this
inode before it is published. 

For local filesystems it is somewhat easy. They are the one creating
inode and can set all xattrs/labels before inode is added to inode
cache.

But for remote like filesystems, it is more tricky. Actual inode
creation first will happen on server and then client will instantiate
an inode based on information returned by server (Atleast that's
what fuse does).

So security_dentry_init_security() was created (I think by NFS folks)
so that they can query the label and send it along with create
request and server can take care of setting label (along with file
creation).

One limitation of security_dentry_init_security() is that it practically
supports only one label. And only SELinux has implemented. So for
all practical purposes this is a hook to obtain selinux label. NFS
and ceph already use it in that way.

Now there is a desire to be able to return more than one security
labels and support smack and possibly other LSMs. Sure, that great.
But I think for that we will have to implement a new hook which
can return multiple labels and filesystems like nfs, ceph and fuse
will have to be modified to cope with this new hook to support
multiple lables. 

And I am arguing that we can modify fuse when that hook has been
implemented. There is no point in adding that complexity in fuse
code as well all fuse-server implementations when there is nobody
generating multiple labels. We can't even test it.

Thanks
Vivek


  reply	other threads:[~2021-09-24 23:32 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24 19:24 [PATCH 0/2] fuse: Send file/inode security context during creation Vivek Goyal
2021-09-24 19:24 ` [Virtio-fs] " Vivek Goyal
2021-09-24 19:24 ` [PATCH 1/2] fuse: Add a flag FUSE_SECURITY_CTX Vivek Goyal
2021-09-24 19:24   ` [Virtio-fs] " Vivek Goyal
2021-09-24 19:24 ` [PATCH 2/2] fuse: Send security context of inode on file creation Vivek Goyal
2021-09-24 19:24   ` [Virtio-fs] " Vivek Goyal
2021-09-24 19:58   ` Casey Schaufler
2021-09-24 19:58     ` [Virtio-fs] " Casey Schaufler
2021-09-24 20:18     ` Vivek Goyal
2021-09-24 20:18       ` [Virtio-fs] " Vivek Goyal
2021-09-24 20:54       ` Casey Schaufler
2021-09-24 20:54         ` [Virtio-fs] " Casey Schaufler
2021-09-24 21:16         ` Vivek Goyal
2021-09-24 21:16           ` [Virtio-fs] " Vivek Goyal
2021-09-24 21:55           ` Casey Schaufler
2021-09-24 21:55             ` [Virtio-fs] " Casey Schaufler
2021-09-24 22:00   ` Colin Walters
2021-09-24 22:00     ` [Virtio-fs] " Colin Walters
2021-09-24 23:32     ` Vivek Goyal [this message]
2021-09-24 23:32       ` Vivek Goyal
2021-09-27  0:53       ` Casey Schaufler
2021-09-27  0:53         ` [Virtio-fs] " Casey Schaufler
2021-09-27 14:05         ` Vivek Goyal
2021-09-27 14:05           ` [Virtio-fs] " Vivek Goyal
2021-09-27 15:22           ` Casey Schaufler
2021-09-27 15:22             ` [Virtio-fs] " Casey Schaufler
2021-09-27 15:56             ` Vivek Goyal
2021-09-27 15:56               ` [Virtio-fs] " Vivek Goyal
2021-09-27 17:56               ` Casey Schaufler
2021-09-27 17:56                 ` [Virtio-fs] " Casey Schaufler
2021-09-27 19:20                 ` Vivek Goyal
2021-09-27 19:20                   ` [Virtio-fs] " Vivek Goyal
2021-09-27 20:19                   ` Casey Schaufler
2021-09-27 20:19                     ` [Virtio-fs] " Casey Schaufler
2021-09-27 20:45                     ` Vivek Goyal
2021-09-27 20:45                       ` [Virtio-fs] " Vivek Goyal
2021-09-27 21:45                       ` Casey Schaufler
2021-09-27 21:45                         ` [Virtio-fs] " Casey Schaufler
2021-09-28 12:49                         ` Vivek Goyal
2021-09-28 12:49                           ` [Virtio-fs] " Vivek Goyal
2021-09-28 14:25                           ` Casey Schaufler
2021-09-28 14:25                             ` [Virtio-fs] " Casey Schaufler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YU5gF9xDhj4g+0Oe@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=chirantan@chromium.org \
    --cc=dwalsh@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=virtio-fs@redhat.com \
    --cc=walters@verbum.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.