Re: [PATCH v4 8/30] landlock: Add AUDIT_LANDLOCK_DENY and log ptrace denials

["Mickaël Salaün" <mic@digikod.net>]

On Wed, Jan 15, 2025 at 06:53:06PM -0500, Paul Moore wrote:
> On Jan  8, 2025 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> > 
> > Add a new AUDIT_LANDLOCK_DENY record type dedicated to any Landlock
> > denials.
> 
> ...
> 
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 75e21a135483..60c909c396c0 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -33,7 +33,7 @@
> >   * 1100 - 1199 user space trusted application messages
> >   * 1200 - 1299 messages internal to the audit daemon
> >   * 1300 - 1399 audit event messages
> > - * 1400 - 1499 SE Linux use
> > + * 1400 - 1499 access control messages
> >   * 1500 - 1599 kernel LSPP events
> >   * 1600 - 1699 kernel crypto events
> >   * 1700 - 1799 kernel anomaly records
> > @@ -146,6 +146,7 @@
> >  #define AUDIT_IPE_ACCESS	1420	/* IPE denial or grant */
> >  #define AUDIT_IPE_CONFIG_CHANGE	1421	/* IPE config change */
> >  #define AUDIT_IPE_POLICY_LOAD	1422	/* IPE policy load */
> > +#define AUDIT_LANDLOCK_DENY	1423	/* Landlock denial */
> 
> I didn't have an opportunity to respond to your reply to my v3 comments
> before you posted v4, but I see you've decided to stick with _DENY as
> opposed to _ACCESS (or something similar).  Let me copy your reply
> below so I can respond appropriately ...
> 
> > A stronger type with the "denied" semantic makes more sense to me,
> > especially for Landlock which is unprivileged, and it makes it clear
> > that it should only impact performance and log size (i.e. audit log
> > creation) for denied actions.
> 
> This is not consistent with how audit is typically used.  Please
> convert to AUDIT_LANDLOCK_ACCESS, or something similar.

OK

> 
> > The next patch
> > series will also contain a new kind of audit rule to specifically
> > identify the origin of the policy that created this denied event, which
> > should make more sense.
> 
> Generally speaking audit only wants to support a small number of message
> types dedicated to a specific LSM.  If you're aware of additional message
> types that you plan to propose in a future patchset, it's probably a
> time to discuss those now.

The only other audit record type I'm thinking about would be one
dedicated to "potentially denied access", something similar to SELinux's
permissive mode.

> 
> > Because of its unprivileged nature, Landlock will never log granted
> > accesses by default.  In the future, we might want a permissive-like
> > mode for Landlock, but this will be optional, and I would also strongly
> > prefer to add new audit record types for new semantics.
> 
> Once again, this isn't consistent with how audit is typically used and
> I'm not seeing a compelling reason to rework how things are done.  Please
> stick with encoding the success/failure, accept/reject, etc. states in
> audit record fields, not the message types themselves.

OK