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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 18 Aug 2009 10:23:06 -0400
From:	Stephen Smalley <sds@...ho.nsa.gov>
To:	Casey Schaufler <casey@...aufler-ca.com>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"David P. Quigley" <dpquigl@...ho.nsa.gov>, jmorris@...ei.org,
	gregkh@...e.de, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, selinux@...ho.nsa.gov
Subject: Re: [PATCH] Security/sysfs: v2 - Enable security xattrs to be set
 on sysfs files, directories, and symlinks.

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@...aufler-ca.com>
> >>
> >> 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.

> I'll have another go. Thank you for the clarification.

-- 
Stephen Smalley
National Security Agency

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