Messages in this thread | | | Subject | Re: [PATCH] security: selinux: allow per-file labeling for bpffs | From | Stephen Smalley <> | Date | Wed, 12 Feb 2020 13:09:04 -0500 |
| |
On 2/12/20 12:46 PM, Steven Moreland wrote: > And I strongly encourage our downstream in the same way :) I try, I > try. However, I am a n00b here (thanks for merging "my" first linux > patch). > > Looking at this code, I was wondering, why isn't SELinux labelling > completely orthogonal to the fs type? Is this a measurable > memory/performance thing?
If you just mean why don't we turn on SE_SBGENFS for all filesystem types, that's discussed in https://github.com/SELinuxProject/selinux-kernel/issues/2
It isn't always safe so we have been whitelisting the filesystem types that are supported.
More generally, labeling in SELinux goes beyond just GENFS; there are the SECURITY_FS_USE_* filesystem labeling behaviors defined by policy and those are also based on fstype, just not hardcoded in the kernel.
> > > On Tue, Feb 11, 2020 at 7:17 PM Paul Moore <paul@paul-moore.com> wrote: >> >> On Thu, Feb 6, 2020 at 1:12 PM Stephen Smalley <sds@tycho.nsa.gov> wrote: >>> On 2/6/20 12:41 PM, Steven Moreland wrote: >>>> On Thu, Feb 6, 2020 at 9:35 AM Stephen Smalley <sds@tycho.nsa.gov> wrote: >>>>> >>>>> On 2/6/20 12:21 PM, Stephen Smalley wrote: >>>>>> On 2/6/20 11:55 AM, Steven Moreland wrote: >>>>>>> From: Connor O'Brien <connoro@google.com> >>>>>>> >>>>>>> Add support for genfscon per-file labeling of bpffs files. This allows >>>>>>> for separate permissions for different pinned bpf objects, which may >>>>>>> be completely unrelated to each other. >>>>>> >>>>>> Do you want bpf fs to also support userspace labeling of files via >>>>>> setxattr()? If so, you'll want to also add it to >>>>>> selinux_is_genfs_special_handling() as well. >>>>>> >>>> >>>> Android doesn't currently have this use case. >>>> >>>>>> The only caveat I would note here is that it appears that bpf fs >>>>>> supports rename, link, unlink, rmdir etc by userspace, which means that >>>>>> name-based labeling via genfscon isn't necessarily safe/stable. See >>>>>> https://github.com/SELinuxProject/selinux-kernel/issues/2 >>>>>> >>>> >>>> Android restricts ownership of these files to a single process (bpfloader) and >>>> so this isn't a concern in our architecture. Is it a concern in general? >>> >>> I guess if the inodes are pinned in memory, then only the original name >>> under which the file is created will be relevant to determining the >>> label and subsequent rename/link operations won't have any effect. So as >>> long as the bpfloader creates the files with the same names being >>> specified in policy, that should line up and be stable for the lifecycle >>> of the inode. >>> >>> The alternative model is to have bpfloader look up a context from the >>> userspace file_contexts configuration via selabel_lookup(3) and friends, >>> and set it on the file explicitly. That's what e.g. ueventd does for >>> device nodes. However, one difference here is that you could currently >>> only do this via setxattr()/setfilecon() after creating the file so that >>> the file would temporarily exist in the default label for bpf fs, if >>> that matters. ueventd can instead use setfscreatecon(3) before creating >>> the file so that it is originally created in the right label but that >>> requires the filesystem to call security_inode_init_security() from its >>> function that originally creates the inode, which tmpfs/devtmpfs does >>> but bpf does not. So you'd have to add that to the bpf filesystem code >>> if you wanted to support setfscreatecon(3) on it. >> >> Considering the relative maturity of bpf, and bpffs, I think it's okay >> to take this small step right now, with the understanding that more >> work may need to be done, depending on how this is generally adopted >> by distros and users (for those of you not following the other thread, >> I've merged the v3 draft of this patch). >> >> However, I've been noticing a trend from the Android folks of tossing >> patches over the wall without much thought beyond the Android use >> case. I understand the Android devs have a job to do, and products to >> focus on, but I would strongly encourage them to think a bit longer >> about more general use cases before submitting patches upstream. >> >> -- >> paul moore >> www.paul-moore.com
| |