Re: [PATCH] Security/sysfs: v2 - Enable security xattrs to be seton sysfs files, directories, and symlinks.

[Stephen Smalley]

On Tue, 2009-08-18 at 21:37 -0700, Casey Schaufler wrote:
> Stephen Smalley wrote:
> > On Tue, 2009-08-18 at 07:12 -0700, Casey Schaufler wrote:
> >   
> >> Stephen Smalley wrote:
> >>     
> >>> On Mon, 2009-08-17 at 20:55 -0700, Casey Schaufler wrote:
> >>>   
> >>>       
> >>>> From: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
> >>>>
> >>>> Another approach to limited xattr support in sysfs.
> >>>>
> >>>> I tried to listen to the objections to a linked list representation
> >>>> and I think that I understand that there isn't really any interest
> >>>> in supporting xattrs for real, only for those maintained by LSMs.
> >>>> I also looked carefully into the claims that memory usage is
> >>>> critical and that the code I had before was duplicating effort.
> >>>>
> >>>> This version lets the surrounding code do as much of the work as
> >>>> possible. Unlike the initial proposal for sysfs xattrs, it does not
> >>>> introduce any new LSM hooks, it uses hooks that already exist. It
> >>>> does not support any attributes on its own, it only provides for
> >>>> the attribute advertised by security_inode_listsecurity(). It could
> >>>> easily be used by other filesystems to provide the same LSM xattr
> >>>> support. It could also be extended to do the list based support for
> >>>> arbitrary xattrs without too much effort.
> >>>>
> >>>> Probably the oddest bit is that the inode_getsecurity hooks need to
> >>>> check to see if they are getting called before the inode is instantiated
> >>>> and return -ENODATA in that event. It would be possible to do a
> >>>> filesystem specific check instead, but this way provides for generally
> >>>> correct behavior at small cost.
> >>>>
> >>>> This has been tested with Smack, but not SELinux. I think that
> >>>> SELinux will work correctly, but it could be that a labeling
> >>>> behavior that is different than the "usual" instantiation labeling
> >>>> is actually desired. That would be an easy change.
> >>>>
> >>>> As always, let me know if I missed something obvious or if there's a
> >>>> fatal flaw in the scheme.
> >>>>     
> >>>>         
> >>> The point of the David's patch was to provide a way to save the security
> >>> xattr in the backing data structure for sysfs entries when an attribute
> >>> value is set from userspace so that the value can be preserved if the
> >>> inode is evicted from memory and later re-instantiated.  AFAICS, your
> >>> patch completely misses the problem.  How about we just go back to
> >>> David's patch?
> >>>   
> >>>       
> >> Oh no, that would use too much memory!
> >>
> >> Either you care about the value the user set, in which case you
> >> want to save the value the user set, or you don't. If you do, you
> >> have to save that value, not an LSM's interpretation of that value.
> >> No secids. No new hooks.
> >>     
> >
> > As the security module is the only component of the kernel that
> > uses/interprets that value, it isn't unreasonable for the security
> > module's interpretation of that value to be considered canonical.  In
> > fact, that is already the case - the hooks within vfs_getxattr() enable
> > the security module to override/replace the actual security xattr value
> > returned to userspace.
> >
> > In the case of Smack, Smack could just provide a pointer to its own
> > internal copy of the string, and that could be stored in the wrapped
> > iattr.  In the case of SELinux, we could provide a secid that could be
> > stored in the wrapped iattr.  The hook interface could just handle it as
> > a blob if you prefer.  But either way we don't need extra storage aside
> > from a pointer-size field in the wrapped iattr.
> >   
> 
> So how often is the SELinux label going to get explicitly set in /sys ?
> I'm grappling with the value of going hog-wild in optimizing this. If
> it is something that's quite rare I can see the concern with expanding
> the d_entry. If it is common, the storage associated with storing the
> xattr could be an issue. If it is uncommon but not rare there's another
> story again.
> 
> I'm looking at addressing the issues. Thank you.

I'd expect most sysfs nodes to be left in the default label, although we
don't really know as this would be the first time that people have the
option of finer-grained control to sysfs.

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/