[PATCH] lsm: make SECURITY_PATH always enabled

[Paul Moore]

On Wed, Apr 23, 2025 at 12:57 AM Song Liu <songliubraving at meta.com> wrote:
> > On Apr 22, 2025, at 2:13 PM, Paul Moore <paul at paul-moore.com> wrote:
>
> [...]
>
> >>
> >> However, I don't think existing CONFIG_SECURITY_* are doing the right
> >> things. Among all the configs, CONFIG_SECURITY_PATH is the most awkward
> >> to me. Say , if we have CONFIG_SECURITY_PATH, shouldn't we also have
> >> CONFIG_SECURITY_INODE? IOW, something like:
> >>
> >> #ifdef CONFIG_SECURITY_INODE
> >> int security_inode_rmdir(struct inode *dir, struct dentry *dentry);
> >> #endif
> >>
> >> #ifdef CONFIG_SECURITY_PATH
> >> int security_path_rmdir(struct inode *dir, struct dentry *dentry);
> >> #endif
> >
> > Without putting much thought into what would fall under
> > CONFIG_SECURITY_INODE, I think it would be interesting to see what
> > hooks one might be able to make conditional on such a Kconfig knob.
> > Using security_inode_permission() as a simple test, it looks like only
> > SELinux and Smack provide implementations, spot checks on a few other
> > security_*inode*() hooks shows similar, or even more limited, results.
> >
> > You would need to spend some time to determine what LSM hooks are used
> > by which LSMs and adjust their Kconfigs appropriately for the new
> > CONFIG_SECURITY_INODE knob, but if you do that then I think that would
> > be okay.
>
> Well, I was hoping to simplify the CONFIGs by removing one. So I am
> not sure whether adding a new CONFIG is the right thing to do.

Ah, in that case I suspect you're going to be disappointed as I'm
fairly confident we don't want to consolidate the inode and path based
hooks under one Kconfig knob at this point in time.  If anything, I
think there may be some value in adding an inode Kconfig as described
above, which I realize isn't your original goal, but still ... :)

> >> OR, maybe we should just remove security_inode_rmdir(), and users of
> >> security_inode_rmdir() can just use security_path_rmdir() instead?
> >
> > Those two LSM hooks are called from slightly different places in the
> > codepath which has an impact on their environment.  For example, the
> > inode variant doesn't have to deal with directory inodes that don't
> > have a defined rmdir op, whereas the path variant does; the inode
> > variant doesn't have to worry about S_KERNEL_FILE files, the inode
> > variant has a refcount'd and locked dentry, etc.  Moving an existing
> > LSM, especially complex ones, from one LSM hook to another, is a
> > delicate operation and might not be worth it for such a small return.
>
> Given there is pushback when a new LSM hook is added, I assume
> removing a hook (or merge two hooks into one) may be a good move.
> Well, it is totally possible that I underestimated the complexity of
> the work.

The funny thing is that the difficulty in adding LSM hooks is one of
the main reasons why I am so hesitant to remove an existing hook; you
can consider it as perhaps an unintended consequence of a general
hostility towards the LSM.

Regardless of the above, yes, there can be a lot of complexity
involved in adding, modifying, or consolidating LSM hooks as there can
be decades worth of assumptions both in the LSMs and in the caller
that need to be considered for each change.  Of course that doesn't
mean such change can't happen - we have plenty of examples where it
has - but such changes are often more complicated than they appear.

-- 
paul-moore.com