xfs
[Top] [All Lists]

Re: [PATCH] fs/vfs/security: pass last path component to LSM on inode cr

To: Eric Paris <eparis@xxxxxxxxxx>
Subject: Re: [PATCH] fs/vfs/security: pass last path component to LSM on inode creation
From: "John Stoffel" <john@xxxxxxxxxxx>
Date: Thu, 9 Dec 2010 12:48:26 -0500
Cc: John Stoffel <john@xxxxxxxxxxx>, xfs-masters@xxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx, cluster-devel@xxxxxxxxxx, linux-mtd@xxxxxxxxxxxxxxxxxxx, jfs-discussion@xxxxxxxxxxxxxxxxxxxxx, ocfs2-devel@xxxxxxxxxxxxxx, reiserfs-devel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, linux-mm@xxxxxxxxx, linux-security-module@xxxxxxxxxxxxxxx, chris.mason@xxxxxxxxxx, jack@xxxxxxx, akpm@xxxxxxxxxxxxxxxxxxxx, adilger.kernel@xxxxxxxxx, tytso@xxxxxxx, swhiteho@xxxxxxxxxx, dwmw2@xxxxxxxxxxxxx, shaggy@xxxxxxxxxxxxxxxxxx, mfasheh@xxxxxxxx, joel.becker@xxxxxxxxxx, aelder@xxxxxxx, hughd@xxxxxxxxxx, jmorris@xxxxxxxxx, sds@xxxxxxxxxxxxx, eparis@xxxxxxxxxxxxxx, hch@xxxxxx, dchinner@xxxxxxxxxx, viro@xxxxxxxxxxxxxxxxxx, tao.ma@xxxxxxxxxx, shemminger@xxxxxxxxxx, jeffm@xxxxxxxx, paul.moore@xxxxxx, penguin-kernel@xxxxxxxxxxxxxxxxxxx, casey@xxxxxxxxxxxxxxxx, kees.cook@xxxxxxxxxxxxx, dhowells@xxxxxxxxxx
In-reply-to: <1291909941.3072.70.camel@xxxxxxxxxxxxxxxxxxxxx>
References: <20101208194527.13537.77202.stgit@xxxxxxxxxxxxxxxxxxxx> <19712.61515.201226.938553@xxxxxxxxxxxxxxxxx> <1291909941.3072.70.camel@xxxxxxxxxxxxxxxxxxxxx>
>>>>> "Eric" == Eric Paris <eparis@xxxxxxxxxx> writes:

Eric> On Thu, 2010-12-09 at 10:05 -0500, John Stoffel wrote:
>> >>>>> "Eric" == Eric Paris <eparis@xxxxxxxxxx> writes:

>> So what happens when I create a file /home/john/shadow, does selinux
>> (or LSM in general) then run extra checks because the filename is
>> 'shadow' in your model?  

Eric> It's entirely a question of labeling and one that was discussed on the
Eric> LSM list in some detail:

Eric> http://marc.info/?t=129141308200002&r=1&w=2

Thank you for pointing me at this discussion.  I'm working my way
through it, but so far I'm not seeing any consensus that this is
really the proper thing to do.  I personally feel this should be in
userspace if at all possible.

Eric> The basic synopsis is that when a new inode is created SELinux
Eric> must apply some label.  It makes the decision for what label to
Eric> apply based on 3 pieces of information.

Eric> The label of the parent inode.
Eric> The label of the process creating the new inode.
Eric> The 'class' of the inode, S_ISREG, S_ISDIR, S_ISLNK, etc

These seem to be ok, if you're using label based security.  But since
I freely admit I'm not an expert or even a user, I'm just trying to
understand and push back to make sure we do what's good.  And which
doesn't impact non-SElinux users.  

Eric> This patch adds a 4th piece of information, the name of the
Eric> object being created.  An obvious situation where this will be
Eric> useful is devtmpfs (although you'll find other examples in the
Eric> above thread).  devtmpfs when it creates char/block devices is
Eric> unable to distinguish between kmem and console and so they are
Eric> created with a generic label.  hotplug/udev is then called which
Eric> does some pathname like matching and relabels them to something
Eric> more specific.  We've found that many people are able to race
Eric> against this particular updating and get spurious denials in
Eric> /dev.  With this patch devtmpfs will be able to get the labels
Eric> correct to begin with.

So your Label based access controls are *also* based on pathnames?
Right?  

Eric> I'm certainly willing to discuss the security implications of this
Eric> patch, but that would probably be best done with a significantly
Eric> shortened cc-list.  You'll see in the above mentioned thread that a
Eric> number of 'security' people (even those who are staunchly anti-SELinux)
Eric> recognize there is value in this and that it is certainly much better
Eric> than we have today.

>> I *think* the overhead shouldn't be there if SELINUX is disabled, but
>> have you confirmed this?  How you run performance tests before/after
>> this change when doing lots of creations of inodes to see what sort of
>> performance changes might be there?

Eric> I've actually recently done some perf testing on creating large
Eric> numbers of inodes using bonnie++, since SELinux was a noticeable
Eric> overhead in that operation.  Doing that same test with SELinux
Eric> disabled (or enabled) I do not see a noticeable difference when
Eric> this patch is applied or not.  It's just an extra argument to a
Eric> function that goes unused.

That answers alot of my concerns then.  Not having it impact users in
a non-SELinux context is vitally important to me.

Thanks,
John

<Prev in Thread] Current Thread [Next in Thread>