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