On Mon, Sep 27, 2021 at 08:22:48AM -0700, Casey Schaufler wrote: > On 9/27/2021 7:05 AM, Vivek Goyal wrote: > > On Sun, Sep 26, 2021 at 05:53:11PM -0700, Casey Schaufler wrote: > >> On 9/24/2021 4:32 PM, Vivek Goyal wrote: > >>> 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. > >> No. That's imprecise. It's what SELinux does. An LSM can add any > >> number of attributes on inode creation, or none. These attributes > >> may or may not be "security labels". Assuming that they are is the > >> kind of thinking that leads people like Linus to conclude that the > >> LSM community is clueless. > >> > >> > >>> 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. > >> There's a little bit of chicken-and-egg going on here. > >> There's no point in accommodating multiple labels in > >> this code because you can't have multiple labels. There's > >> no point in trying to support multiple labels because > >> you can't use them in virtiofs and a bunch of other > >> places. > > Once security subsystem provides a hook to support multiple lables, then > > atleast one filesystem will have to be converted to make use of this new > > hook at the same time and rest of the filesystems can catch up later. > > Clearly you haven't been following the work I've been doing on > module stacking. That's completely understandable. There aren't > new hooks being added, or at least haven't been yet. Some of the > existing hooks are getting changed to provide the data required > for multiple security modules (e.g. secids become a set of secids). > Filesystems that support xattrs properly are unaffected because, > for all it's shortcomings, the LSM layer hides the details of > the security modules sufficiently. > > Which filesystem are you saying will have to "be converted"? When I grep for "security_dentry_init_security()" in current code, I see two users, ceph and nfs. fs/ceph/xattr.c ceph_security_init_secctx() fs/nfs/nfs4proc.c nfs4_label_init_security() So looks like these two file systems will have to be converted (along with fuse). Vivek > NFS is going to require some work, but that's because it was > done as a special case for "MAC labels". The NFS support for > security.* xattrs came much later. This is one of the reasons > why I'm concerned about the virtiofs implementation you're > proposing. We were never able to get the NFS "MAC label" > implementation to work properly with Smack, even though there is > no obvious reason it wouldn't. > > > > > > Vivek > > >
