Re: [PATCH v2 00/30] acl: add vfs posix acl api

From: Casey Schaufler <casey@schaufler-ca.com>
To: Seth Forshee <sforshee@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>,
	Christian Brauner <brauner@kernel.org>,
	linux-fsdevel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	v9fs-developer@lists.sourceforge.net, linux-cifs@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org, casey@schaufler-ca.com
Subject: Re: [PATCH v2 00/30] acl: add vfs posix acl api
Date: Tue, 27 Sep 2022 08:55:07 -0700	[thread overview]
Message-ID: <4954ef66-52ff-52de-9d7d-41da1b070b13@schaufler-ca.com> (raw)
In-Reply-To: <YzMT2axDeni7L1O8@do-x1extreme>

On 9/27/2022 8:16 AM, Seth Forshee wrote:
> On Tue, Sep 27, 2022 at 07:11:17AM -0700, Casey Schaufler wrote:
>> On 9/27/2022 12:41 AM, Christoph Hellwig wrote:
>>> On Mon, Sep 26, 2022 at 05:22:45PM -0700, Casey Schaufler wrote:
>>>> I suggest that you might focus on the acl/evm interface rather than the entire
>>>> LSM interface. Unless there's a serious plan to make ima/evm into a proper LSM
>>>> I don't see how the breadth of this patch set is appropriate.
>>> Umm. The problem is the historically the Linux xattr interface was
>>> intended for unstructured data, while some of it is very much structured
>>> and requires interpretation by the VFS and associated entities.  So
>>> splitting these out and add proper interface is absolutely the right
>>> thing to do and long overdue (also for other thing like capabilities).
>>> It might make things a little more verbose for LSM, but it fixes a very
>>> real problem.
>> Here's the problem I see. All of the LSMs see xattrs, except for their own,
>> as opaque objects. Introducing LSM hooks to address the data interpretation
>> issues between VFS and EVM, which is not an LSM, adds to an already overlarge
>> and interface. And the "real" users of the interface don't need the new hook.
>> I'm not saying that the ACL doesn't have problems. I'm not saying that the
>> solution you've proposed isn't better than what's there now. I am saying that
>> using LSM as a conduit between VFS and EVM at the expense of the rest of the
>> modules is dubious. A lot of change to LSM for no value to LSM.
>>
>> I am not adamant about this. A whole lot worse has been done for worse reasons.
>> But as Paul says, we're overdue to make an effort to keep the LSM interface sane.
> So I assume the alternative you have in mind would be to use the
> existing setxattr hook?

That is how it works today.

>  I worry about type confusion if an LSM does
> someday want to look inside the ACL data.

I suggest that changes to system behavior based on the content of
an ACL really belongs in the ACL code, not in an LSM. Can I imagine
someone wanting to add SELinux policy that controls what entries
are allowed to be set by a particular domain? Sure, but I can't see
how that would be popular with existing ACL fans.

>  Unless LSMs aren't supposed to
> look inside of xattr data, but in that case why pass the data pointer on
> to the LSMs?

So that the LSM can look at it's own xattr data.

> Note that the caller of this new hook does not have access to the uapi
> xattr data, and I think this is the right place for the new hook to be
> called as it's the interface that stacked filesystems like overlayfs
> will use to write ACLs to the lower filesystems.

I'm not saying anything about the organization of the calling code.
Why is it calling

	security_acl_hooha(...)

instead of

	evm_acl_hooha(...)

>
> Seth