SELinux Archive on lore.kernel.org
 help / color / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: selinux@vger.kernel.org
Cc: linux-security-module@vger.kernel.org, Tejun Heo <tj@kernel.org>,
	linux-fsdevel@vger.kernel.org, cgroups@vger.kernel.org,
	Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH v2 0/3] Allow initializing the kernfs node's secctx based on its parent
Date: Thu, 10 Jan 2019 09:54:59 -0800
Message-ID: <1062e181-16fa-66b8-a48c-786edcb9deb7@schaufler-ca.com> (raw)
In-Reply-To: <93d310a7-0bce-dccd-8136-c659a460e084@tycho.nsa.gov>


Resending after email configuration repair.

On 1/10/2019 6:15 AM, Stephen Smalley wrote:
> On 1/9/19 5:03 PM, Casey Schaufler wrote:
>> On 1/9/2019 12:37 PM, Stephen Smalley wrote:
>>> On 1/9/19 12:19 PM, Casey Schaufler wrote:
>>>> On 1/9/2019 8:28 AM, Ondrej Mosnacek wrote:
>>>>> Changes in v2:
>>>>> - add docstring for the new hook in union security_list_options
>>>>> - initialize *ctx to NULL and *ctxlen to 0 in case the hook is not
>>>>>     implemented
>>>>> v1: https://lore.kernel.org/selinux/20190109091028.24485-1-omosnace@redhat.com/T/
>>>>>
>>>>> This series adds a new security hook that allows to initialize the security
>>>>> context of kernfs properly, taking into account the parent context. Kernfs
>>>>> nodes require special handling here, since they are not bound to specific
>>>>> inodes/superblocks, but instead represent the backing tree structure that
>>>>> is used to build the VFS tree when the kernfs tree is mounted.
>>>>>
>>>>> The kernfs nodes initially do not store any security context and rely on
>>>>> the LSM to assign some default context to inodes created over them.
>>>>
>>>> This seems like a bug in kernfs. Why doesn't kernfs adhere to the usual
>>>> and expected filesystem behavior?
>>>
>>> sysfs / kernfs didn't support xattrs at all when we first added support for setting security contexts to it, so originally all sysfs / kernfs inodes had a single security context, and we only required separate storage for the inodes that were explicitly labeled by userspace.
>>>
>>> Later kernfs grew support for trusted.* xattrs using simple_xattrs but the existing security.* support was left mostly unchanged.
>>
>> OK, so as I said, this seems like a bug in kernfs.
>>
>>>
>>>>
>>>>> Kernfs
>>>>> inodes, however, allow setting an explicit context via the *setxattr(2)
>>>>> syscalls, in which case the context is stored inside the kernfs node's
>>>>> metadata.
>>>>>
>>>>> SELinux (and possibly other LSMs) initialize the context of newly created
>>>>> FS objects based on the parent object's context (usually the child inherits
>>>>> the parent's context, unless the policy dictates otherwise).
>>>>
>>>> An LSM might use information about the parent other than the "context".
>>>> Smack, for example, uses an attribute SMACK64TRANSMUTE from the parent
>>>> to determine whether the Smack label of the new object should be taken
>>>> from the parent or the process. Passing the "context" of the parent is
>>>> insufficient for Smack.
>>>
>>> IIUC, this would involve switching the handling of security.* xattrs in kernfs over to use simple_xattrs too (so that we can store multiple such attributes), and then pass the entire simple_xattrs list or at least anything with a security.* prefix when initializing a new node or refreshing an existing inode.  Then the security module could extract any security.* attributes of interest for use in determining the label of new inodes and in refreshing the label of an inode.
>>
>> Right. But I'll point out that there is nothing to prevent an
>> LSM from using inode information outside of the xattrs (e.g. uids)
>> to determine the security state it wants to give a new object.
>
> If that's a real concern, the hook could pass the ia_iattr structure in addition to the simple_xattrs list and the security module could use any inode attributes it likes in making the decision.  Effectively it would be passing the entire kernfs_iattrs structure, but probably not directly since that definition is presently private to kernfs.

Yes, it's a real concern. And no, just passing all of the kernfs internal data
out in j-random formats does not pass muster. Al Viro was commenting the other
day on how bad the LSM infrastructure interfaces are. The original proposal here
is already big, cluttered and inadequate. Adding more to it to make up for its
shortcomings should be sending up red flags.

I've been wallowing in the LSM infrastructure for the past seven years.
Interfaces like this one, that propagate the idiosyncrasies of both
the caller (kernfs) and one potential callee (SELinux) are much too
common. I understand that there is a problem that needs a solution.
This isn't it.


>> I suggest that the better solution would be for kernfs to
>> use inodes like a real filesystem. Every special case like this
>> results in special cases like this special hook. It's hard
>> enough to keep track of the general case in the Linux kernel.
>
> Feel free to propose an implementation if you like, but doing a complete rewrite of kernfs internals seems a bit out of scope.

If this issue points out a serious problem with the kernfs implementation
then I would expect that addressing the problem at its source would be in
everyone's best interest. Did anyone even look at the possibility? If I
said that I would do that, how long would you be willing to wait for it?


  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 16:28 Ondrej Mosnacek
2019-01-09 16:28 ` [PATCH v2 1/3] LSM: Add new hook for generic node initialization Ondrej Mosnacek
2019-01-09 17:08   ` Casey Schaufler
2019-01-11  1:57     ` Paul Moore
2019-01-11 18:30       ` Casey Schaufler
2019-01-14  9:01       ` Ondrej Mosnacek
2019-01-09 16:28 ` [PATCH v2 2/3] selinux: Implement the object_init_security hook Ondrej Mosnacek
2019-01-09 16:28 ` [PATCH v2 3/3] kernfs: Initialize security of newly created nodes Ondrej Mosnacek
2019-01-11 20:52   ` Tejun Heo
2019-01-09 17:19 ` [PATCH v2 0/3] Allow initializing the kernfs node's secctx based on its parent Casey Schaufler
2019-01-09 20:37   ` Stephen Smalley
2019-01-09 22:03     ` Casey Schaufler
2019-01-10 14:15       ` Stephen Smalley
2019-01-10 17:54         ` Casey Schaufler [this message]
2019-01-10 19:37           ` Stephen Smalley
2019-01-11  2:20             ` Paul Moore
2019-01-14  9:01               ` Ondrej Mosnacek
2019-01-11 18:22             ` Casey Schaufler
2019-01-14  9:01           ` Ondrej Mosnacek
2019-01-22  8:49             ` Ondrej Mosnacek
2019-01-22 14:17               ` Stephen Smalley
2019-01-22 15:26                 ` Stephen Smalley

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1062e181-16fa-66b8-a48c-786edcb9deb7@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=selinux@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

SELinux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \
		selinux@vger.kernel.org
	public-inbox-index selinux

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git