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

2011-07-08 Thread Al Viro
On Wed, Dec 08, 2010 at 02:45:27PM -0500, Eric Paris wrote:
 SELinux would like to implement a new labeling behavior of newly created
 inodes.  We currently label new inodes based on the parent and the creating
 process.  This new behavior would also take into account the name of the
 new object when deciding the new label.  This is not the (supposed) full path,
 just the last component of the path.
 
 This is very useful because creating /etc/shadow is different than creating
 /etc/passwd but the kernel hooks are unable to differentiate these
 operations.  We currently require that userspace realize it is doing some
 difficult operation like that and than userspace jumps through SELinux hoops
 to get things set up correctly.  This patch does not implement new
 behavior, that is obviously contained in a seperate SELinux patch, but it
 does pass the needed name down to the correct LSM hook.  If no such name
 exists it is fine to pass NULL.

-ETOOFUCKINGUGLY...
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-09 Thread John Stoffel
 Eric == Eric Paris epa...@redhat.com writes:

Eric SELinux would like to implement a new labeling behavior of newly
Eric created inodes.  We currently label new inodes based on the
Eric parent and the creating process.  This new behavior would also
Eric take into account the name of the new object when deciding the
Eric new label.  This is not the (supposed) full path, just the last
Eric component of the path.

Eric This is very useful because creating /etc/shadow is different
Eric than creating /etc/passwd but the kernel hooks are unable to
Eric differentiate these operations.  We currently require that
Eric userspace realize it is doing some difficult operation like that
Eric and than userspace jumps through SELinux hoops to get things set
Eric up correctly.  This patch does not implement new behavior, that
Eric is obviously contained in a seperate SELinux patch, but it does
Eric pass the needed name down to the correct LSM hook.  If no such
Eric name exists it is fine to pass NULL.

I've looked this patch over, and maybe I'm missing something, but how
does knowing the name of the file really tell you anything, esp when
you only get the filename, not the path?  What threat are you
addressing with this change?  

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?  

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?

Thanks,
John
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-09 Thread Eric Paris
On Thu, 2010-12-09 at 10:05 -0500, John Stoffel wrote:
  Eric == Eric Paris epa...@redhat.com 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?  

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

http://marc.info/?t=12914130822r=1w=2

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

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

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

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

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

-Eric

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-09 Thread Serge Hallyn
Quoting John Stoffel (j...@stoffel.org):
  Eric == Eric Paris epa...@redhat.com writes:
 
 Eric SELinux would like to implement a new labeling behavior of newly
 Eric created inodes.  We currently label new inodes based on the
 Eric parent and the creating process.  This new behavior would also
 Eric take into account the name of the new object when deciding the
 Eric new label.  This is not the (supposed) full path, just the last
 Eric component of the path.
 
 Eric This is very useful because creating /etc/shadow is different
 Eric than creating /etc/passwd but the kernel hooks are unable to
 Eric differentiate these operations.  We currently require that
 Eric userspace realize it is doing some difficult operation like that
 Eric and than userspace jumps through SELinux hoops to get things set
 Eric up correctly.  This patch does not implement new behavior, that
 Eric is obviously contained in a seperate SELinux patch, but it does
 Eric pass the needed name down to the correct LSM hook.  If no such
 Eric name exists it is fine to pass NULL.
 
 I've looked this patch over, and maybe I'm missing something, but how
 does knowing the name of the file really tell you anything, esp when
 you only get the filename, not the path?  What threat are you
 addressing with this change?  

Like you, I keep thinking back to this patch and going back and forth.
But to answer your question:  in some cases, the name of the file
(plus the context of the directory in which it is created) can tell
you what assumptions userspace will make about it.  And userspace most
definately is a part of the TCB, i.e. /bin/passwd and /bin/login.

-serge
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-09 Thread John Stoffel
 Eric == Eric Paris epa...@redhat.com writes:

Eric On Thu, 2010-12-09 at 10:05 -0500, John Stoffel wrote:
  Eric == Eric Paris epa...@redhat.com 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=12914130822r=1w=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
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2010-12-09 Thread Eric Paris
On Thu, 2010-12-09 at 12:48 -0500, John Stoffel wrote:
  Eric == Eric Paris epa...@redhat.com writes:
 
 Eric On Thu, 2010-12-09 at 10:05 -0500, John Stoffel wrote:
   Eric == Eric Paris epa...@redhat.com writes:
 
 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?

Access decisions are still based solely on the label.  This patch can
influence how new objects get their label, which makes the access
decisions indirectly path based.  You'll find a reasonable summary and
commentary on lwn in this weeks security section.

--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html