linux-kernel - Re: [PATCH v39 01/42] integrity: disassociate ima_filter_rule from security_audit

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]

Date: Mon, 24 Jun 2024 19:05:22 -0400
From: Paul Moore <paul@...l-moore.com>
To: Casey Schaufler <casey@...aufler-ca.com>
Cc: Mimi Zohar <zohar@...ux.ibm.com>, Roberto Sassu <roberto.sassu@...weicloud.com>, 
	Roberto Sassu <roberto.sassu@...wei.com>, linux-security-module@...r.kernel.org, 
	jmorris@...ei.org, serge@...lyn.com, keescook@...omium.org, 
	john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp, 
	stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org, mic@...ikod.net, 
	linux-integrity@...r.kernel.org
Subject: Re: [PATCH v39 01/42] integrity: disassociate ima_filter_rule from security_audit_rule

On Mon, Jun 24, 2024 at 6:19 PM Casey Schaufler <casey@...aufler-ca.com> wrote:
> On 6/24/2024 3:03 PM, Paul Moore wrote:
> > On Mon, Jun 24, 2024 at 9:57 AM Mimi Zohar <zohar@...ux.ibm.com> wrote:
> >> On Mon, 2024-06-24 at 10:45 +0200, Roberto Sassu wrote:
> >>> My only comment would be that I would not call the new functions with
> >>> the ima_ prefix, being those in security.c, which is LSM agnostic, but
> >>> I would rather use a name that more resembles the differences, if any.
> >> Commit 4af4662fa4a9 ("integrity: IMA policy") originally referred to these hooks
> >> as security_filter_rule_XXXX, but commit b8867eedcf76 ("ima: Rename internal
> >> filter rule functions") renamed the function to ima_filter_rule_XXX) to avoid
> >> security namespace polution.
> >>
> >> If these were regular security hooks, the hooks would be named:
> >> filter_rule_init, filter_rule_free, filter_rule_match with the matching
> >> "security" prefix functions. Audit and IMA would then register the hooks.
> >>
> >> I agree these functions should probably be renamed again, probably to
> >> security_ima_filter_rule_XXXX.
> > It's funny, my mind saw that the patch was removing those preprocessor
> > macros and was so happy it must have shut off, because we already have
> > security_XXX functions for these :)
> >
> > See security_audit_rule_init(), security_audit_rule_free(), and
> > security_audit_rule_match().
> >
> > Casey, do you want to respin this patch to use the existing LSM
> > functions?
>
> If you want to use shared functions they shouldn't be security_audit_blah().
> I like Mimi's suggestion. Rename security_audit_filter_rule_init() to
> security_filter_rule_init() and use that in both places.

They are currently shared, the preprocessor macros just hide that fact
(which is not a good thing, IMO).  Renaming the existing LSM functions
to drop the "audit" in the name doesn't really solve the problem as
you still end up with "Audit_equal", etc. constants (which are awful
for multiple reasons, some having nothing to do with the LSM) in the
callers.

... and let me just get ahead of this, please do not do a macro-based
rename of "Audit_equal" to something else to "fix" that problem;
that's just as bad as what we have now.

Properly fixing this may be worthwhile, but I think it's an
unnecessary distraction at this point from improving that state of
multiple LSMs.  If you aren't comfortable submitting a patch that just
does a "/ima_filter_rule_match/security_audit_rule_match/" style
rename, or if Mimi and Roberto aren't supportive of that, you might as
well just drop this from the patchset.

--
paul-moore.com