* fuse doesn't use security_inode_init_security? @ 2020-05-01 6:55 Chirantan Ekbote 2020-05-01 7:53 ` Miklos Szeredi 2020-05-01 15:46 ` Vivek Goyal 0 siblings, 2 replies; 6+ messages in thread From: Chirantan Ekbote @ 2020-05-01 6:55 UTC (permalink / raw) To: Miklos Szeredi; +Cc: linux-fsdevel, fuse-devel Hello, I noticed that the fuse module doesn't currently call security_inode_init_security and I was wondering if there is a specific reason for that. I found a patch from 2013[1] that would change fuse so that it would call that function but it doesn't appear that the patch was merged. For background: I currently have a virtio-fs server with a guest VM that wants to use selinux. I was able to enable selinux support without much issue by adding fs_use_xattr virtiofs u:object_r:labeledfs:s0; to the selinux policy in the guest. This works for the most part except that `setfscreatecon` doesn't appear to work. From what I can tell, this ends up writing to `/proc/[pid]/attr/fscreate` and the attributes actually get set via the `inode_init_security` lsm hook in selinux. However, since fuse doesn't call `security_inode_init_security` the hook never runs so the file/directory doesn't have the right attributes. Is it safe to just call `security_inode_init_security` whenever fuse creates a new inode? How does this affect non-virtiofs fuse servers? Would we need a new flag so that servers could opt-in to this behavior like in the patch from [1]? Thank you, Chirantan [1] https://sourceforge.net/p/fuse/mailman/message/31624830/ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: fuse doesn't use security_inode_init_security? 2020-05-01 6:55 fuse doesn't use security_inode_init_security? Chirantan Ekbote @ 2020-05-01 7:53 ` Miklos Szeredi 2020-05-01 18:32 ` Stephen Smalley 2020-05-01 15:46 ` Vivek Goyal 1 sibling, 1 reply; 6+ messages in thread From: Miklos Szeredi @ 2020-05-01 7:53 UTC (permalink / raw) To: Chirantan Ekbote Cc: linux-fsdevel, fuse-devel, Vivek Goyal, LSM, virtio-fs-list On Fri, May 1, 2020 at 8:55 AM Chirantan Ekbote <chirantan@chromium.org> wrote: > > Hello, > > I noticed that the fuse module doesn't currently call > security_inode_init_security and I was wondering if there is a > specific reason for that. I found a patch from 2013[1] that would > change fuse so that it would call that function but it doesn't appear > that the patch was merged. > > For background: I currently have a virtio-fs server with a guest VM > that wants to use selinux. I was able to enable selinux support > without much issue by adding > > fs_use_xattr virtiofs u:object_r:labeledfs:s0; > > to the selinux policy in the guest. This works for the most part > except that `setfscreatecon` doesn't appear to work. From what I can > tell, this ends up writing to `/proc/[pid]/attr/fscreate` and the > attributes actually get set via the `inode_init_security` lsm hook in > selinux. However, since fuse doesn't call > `security_inode_init_security` the hook never runs so the > file/directory doesn't have the right attributes. > > Is it safe to just call `security_inode_init_security` whenever fuse > creates a new inode? How does this affect non-virtiofs fuse servers? Not sure, Adding more Cc's. I know there's a deadlock scenario with getxattr called on root inode before mount returns, which causes a deadlock unless mount is run in the background. Current libfuse doesn't handle this, but I think some fuse fs work around this by not using libfuse, or at least have some special setup code (glusterfs? ceph-fuse? not sure...). I also don't know whether the ->inode_init_security hook is related to this or not. Thanks, Miklos ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: fuse doesn't use security_inode_init_security? 2020-05-01 7:53 ` Miklos Szeredi @ 2020-05-01 18:32 ` Stephen Smalley 2020-05-07 7:53 ` Chirantan Ekbote 0 siblings, 1 reply; 6+ messages in thread From: Stephen Smalley @ 2020-05-01 18:32 UTC (permalink / raw) To: Miklos Szeredi Cc: Chirantan Ekbote, Linux FS Devel, fuse-devel, Vivek Goyal, LSM, virtio-fs-list, SElinux list On Fri, May 1, 2020 at 3:54 AM Miklos Szeredi <miklos@szeredi.hu> wrote: > > On Fri, May 1, 2020 at 8:55 AM Chirantan Ekbote <chirantan@chromium.org> wrote: > > > > Hello, > > > > I noticed that the fuse module doesn't currently call > > security_inode_init_security and I was wondering if there is a > > specific reason for that. I found a patch from 2013[1] that would > > change fuse so that it would call that function but it doesn't appear > > that the patch was merged. > > > > For background: I currently have a virtio-fs server with a guest VM > > that wants to use selinux. I was able to enable selinux support > > without much issue by adding > > > > fs_use_xattr virtiofs u:object_r:labeledfs:s0; > > > > to the selinux policy in the guest. This works for the most part > > except that `setfscreatecon` doesn't appear to work. From what I can > > tell, this ends up writing to `/proc/[pid]/attr/fscreate` and the > > attributes actually get set via the `inode_init_security` lsm hook in > > selinux. However, since fuse doesn't call > > `security_inode_init_security` the hook never runs so the > > file/directory doesn't have the right attributes. > > > > Is it safe to just call `security_inode_init_security` whenever fuse > > creates a new inode? How does this affect non-virtiofs fuse servers? > > Not sure, Adding more Cc's. > > I know there's a deadlock scenario with getxattr called on root inode > before mount returns, which causes a deadlock unless mount is run in > the background. Current libfuse doesn't handle this, but I think some > fuse fs work around this by not using libfuse, or at least have some > special setup code (glusterfs? ceph-fuse? not sure...). I also don't > know whether the ->inode_init_security hook is related to this or not. (cc selinux list) security_inode_init_security() calls the initxattrs callback to actually set each xattr in the backing store (if any), so unless you have a way to pass that to the daemon along with the create request the attribute won't be persisted with the file. Setting the xattrs is supposed to be atomic with the file creation, not a separate setxattr() operation after creating the file, similar to ACL inheritance on new files. Also possibly related https://lore.kernel.org/selinux/6df9b58c-fe9b-28f3-c151-f77aa6dd67e7@tycho.nsa.gov/. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: fuse doesn't use security_inode_init_security? 2020-05-01 18:32 ` Stephen Smalley @ 2020-05-07 7:53 ` Chirantan Ekbote 2020-05-07 13:06 ` Stephen Smalley 0 siblings, 1 reply; 6+ messages in thread From: Chirantan Ekbote @ 2020-05-07 7:53 UTC (permalink / raw) To: Stephen Smalley Cc: Miklos Szeredi, Linux FS Devel, fuse-devel, Vivek Goyal, LSM, virtio-fs-list, SElinux list On Sat, May 2, 2020 at 12:46 AM Vivek Goyal <vgoyal@redhat.com> wrote: > > I am wondering how would fuse initialize the security context of newly > created file. One way seems to be that it passes that information > as part of FUSE_CREATE/FUSE_MKNOD calls to server and server sets > its "fscreate" accordingly and then creates new file. This is similar > to virtiofsd changing its effective uid/gid to the fuse client so that > file is created with caller's uid/gid. Seems to be selinux context for > file creation probably should be handled similiarly. > How would the fuse driver get the correct context without going through security_inode_init_security? > Other method could be to first create new file and then new fuse > commands to do setxattrs. But that will be racy as file will have > some default label for sometime between creation and setxattr. > > Having said that, I have a question. How do you reconcile host selinux > policy and guest selinux labels. I am assuming host selinux policy > will have to know about guest labels so that it allows virtiofsd do > set those labels? Dan, you might have more thoughts on this. > My understanding is that we currently merge the guest and host policies so that all types in the guest are available on the host. The host itself uses selinux in permissive mode. The top-level directory is also owned exclusively by the guest and host processes don't have access to it. On Sat, May 2, 2020 at 3:32 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > > (cc selinux list) > > security_inode_init_security() calls the initxattrs callback to > actually set each xattr in the backing store (if any), so unless you > have a way to pass that to the daemon along with the create request > the attribute won't be persisted with the file. Setting the xattrs is > supposed to be atomic with the file creation, not a separate > setxattr() operation after creating the file, similar to ACL > inheritance on new files. > But it's not truly atomic, is it? The underlying file system creates the inode and then the inode_init_security selinux hook actually sets the attributes. What would happen if the computer lost power after the file system driver created the inode but before the selinux hook set the attributes? > Also possibly related > https://lore.kernel.org/selinux/6df9b58c-fe9b-28f3-c151-f77aa6dd67e7@tycho.nsa.gov/. Interesting. Let me pull out the relevant bits and respond inline. > - deadlock during mount with userspace waiting for mount(2) to complete > and the kernel blocked on requesting the security.selinux xattr of the > root directory as part of superblock setup during mount I haven't personally run into this. It Just Works, except for the fscreate issue. > - there was an attempt to introduce distinctions based on filesystem > "subtype" so that whitelisted fuse filesystems could have xattr support > enabled when it was known that their userspace would handle mount(2) > safely [3] but this was apparently always broken and later reverted [4]. I think we kind of side-stepped this issue. The fstype for virtiofs is "virtiofs" instead of something like "fuse.virtiofs" so there is no subtype handling required. > - there is the issue of trusting the fuse filesystem for its labeling > information and for domain/context transitions This is definitely an issue for regular fuse file systems. However, the virtiofs device has read/write access to all the VM's memory so there isn't much the VM can do if it doesn't trust the device. I guess what I'm trying to understand is: what are the issues with having the fuse driver call the inode_init_security hooks? Even if it's not something that can be turned on by default in mainline, I'd like to evaluate whether we can turn it on locally in our restricted environment. One issue is the lack of atomicity guarantees. This is likely a deal-breaker for general fuse usage. However, I don't think it's an issue for our restricted use of virtiofs because the attributes will be set "atomically" from the guest userspace's perspective. It won't be atomic on the host side, but host processes don't have access to those directories anyway. Are there any other issues? Thank you, Chirantan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: fuse doesn't use security_inode_init_security? 2020-05-07 7:53 ` Chirantan Ekbote @ 2020-05-07 13:06 ` Stephen Smalley 0 siblings, 0 replies; 6+ messages in thread From: Stephen Smalley @ 2020-05-07 13:06 UTC (permalink / raw) To: Chirantan Ekbote Cc: Miklos Szeredi, Linux FS Devel, fuse-devel, Vivek Goyal, LSM, virtio-fs-list, SElinux list On Thu, May 7, 2020 at 3:53 AM Chirantan Ekbote <chirantan@chromium.org> wrote: > On Sat, May 2, 2020 at 3:32 AM Stephen Smalley > <stephen.smalley.work@gmail.com> wrote: > > > > > > (cc selinux list) > > > > security_inode_init_security() calls the initxattrs callback to > > actually set each xattr in the backing store (if any), so unless you > > have a way to pass that to the daemon along with the create request > > the attribute won't be persisted with the file. Setting the xattrs is > > supposed to be atomic with the file creation, not a separate > > setxattr() operation after creating the file, similar to ACL > > inheritance on new files. > > > > But it's not truly atomic, is it? The underlying file system creates > the inode and then the inode_init_security selinux hook actually sets > the attributes. What would happen if the computer lost power after > the file system driver created the inode but before the selinux hook > set the attributes? IIUC, in the case of ext4 and xfs at least, the setting of the security.selinux xattr is performed in the same transaction as the file create, so a crash will either yield a file that has its xattr set or no file at all. This is also true of POSIX ACLs. Labeled NFS (NFSv4.2) likewise is supposed to send the MAC label with the file create request and either create it with that label or not create it at all. Note that nfs however uses security_dentry_init_security() to get the MAC label since it doesn't yet have an inode and MAC labels are a first class abstraction in NFS not merely xattrs. > > - deadlock during mount with userspace waiting for mount(2) to complete > > and the kernel blocked on requesting the security.selinux xattr of the > > root directory as part of superblock setup during mount > > I haven't personally run into this. It Just Works, except for the > fscreate issue. Yes, this can be worked around in your fuse daemon if it supports handling getxattr during mount (e.g. multi-threaded, other threads can service the getxattr request while the mount(2) is still in progress). But not supported by stock fuse userspace IIUC. > I guess what I'm trying to understand is: what are the issues with > having the fuse driver call the inode_init_security hooks? Even if > it's not something that can be turned on by default in mainline, I'd > like to evaluate whether we can turn it on locally in our restricted > environment. > > One issue is the lack of atomicity guarantees. This is likely a > deal-breaker for general fuse usage. However, I don't think it's an > issue for our restricted use of virtiofs because the attributes will > be set "atomically" from the guest userspace's perspective. It won't > be atomic on the host side, but host processes don't have access to > those directories anyway. > > Are there any other issues? I don't have a problem with fuse calling the hook (either security_inode_init_security or security_dentry_init_security). It is just a question of what it will do with the result (i.e. what its initxattr callback will do for the former or what it will do with the returned label in the latter). Optimally it will take the label information and bundle it up along with the create request to the daemon, which can then handle it as a single transaction. Failing that, it needs to support setting the label in some manner during file creation that at least provides atomicity with respect to the user of the filesystem (the guest in your case). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: fuse doesn't use security_inode_init_security? 2020-05-01 6:55 fuse doesn't use security_inode_init_security? Chirantan Ekbote 2020-05-01 7:53 ` Miklos Szeredi @ 2020-05-01 15:46 ` Vivek Goyal 1 sibling, 0 replies; 6+ messages in thread From: Vivek Goyal @ 2020-05-01 15:46 UTC (permalink / raw) To: Chirantan Ekbote Cc: Miklos Szeredi, linux-fsdevel, fuse-devel, Daniel J Walsh On Fri, May 01, 2020 at 03:55:20PM +0900, Chirantan Ekbote wrote: > Hello, > > I noticed that the fuse module doesn't currently call > security_inode_init_security and I was wondering if there is a > specific reason for that. I found a patch from 2013[1] that would > change fuse so that it would call that function but it doesn't appear > that the patch was merged. > > For background: I currently have a virtio-fs server with a guest VM > that wants to use selinux. I was able to enable selinux support > without much issue by adding > > fs_use_xattr virtiofs u:object_r:labeledfs:s0; > > to the selinux policy in the guest. This works for the most part > except that `setfscreatecon` doesn't appear to work. From what I can > tell, this ends up writing to `/proc/[pid]/attr/fscreate` and the > attributes actually get set via the `inode_init_security` lsm hook in > selinux. However, since fuse doesn't call > `security_inode_init_security` the hook never runs so the > file/directory doesn't have the right attributes. > > Is it safe to just call `security_inode_init_security` whenever fuse > creates a new inode? How does this affect non-virtiofs fuse servers? > Would we need a new flag so that servers could opt-in to this behavior > like in the patch from [1]? I am wondering how would fuse initialize the security context of newly created file. One way seems to be that it passes that information as part of FUSE_CREATE/FUSE_MKNOD calls to server and server sets its "fscreate" accordingly and then creates new file. This is similar to virtiofsd changing its effective uid/gid to the fuse client so that file is created with caller's uid/gid. Seems to be selinux context for file creation probably should be handled similiarly. Other method could be to first create new file and then new fuse commands to do setxattrs. But that will be racy as file will have some default label for sometime between creation and setxattr. Having said that, I have a question. How do you reconcile host selinux policy and guest selinux labels. I am assuming host selinux policy will have to know about guest labels so that it allows virtiofsd do set those labels? Dan, you might have more thoughts on this. Thanks Vivek ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-07 13:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-01 6:55 fuse doesn't use security_inode_init_security? Chirantan Ekbote 2020-05-01 7:53 ` Miklos Szeredi 2020-05-01 18:32 ` Stephen Smalley 2020-05-07 7:53 ` Chirantan Ekbote 2020-05-07 13:06 ` Stephen Smalley 2020-05-01 15:46 ` Vivek Goyal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).