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]

Message-ID: <4A8B81A0.6070206@schaufler-ca.com>
Date:	Tue, 18 Aug 2009 21:37:52 -0700
From:	Casey Schaufler <casey@...aufler-ca.com>
To:	Stephen Smalley <sds@...ho.nsa.gov>
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,
	Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH] Security/sysfs: v2 - Enable security xattrs to be set
 on sysfs files, directories, and symlinks.

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@...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.
>   

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.

--
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/