linux-kernel - Re: [PATCH v4 8/30] landlock: Add AUDIT_LANDLOCK

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]

Message-ID: <081bd4a2a44a80e046662667e0aeb309@paul-moore.com>
Date: Wed, 15 Jan 2025 18:53:06 -0500
From: Paul Moore <paul@...l-moore.com>
To: Mickaël Salaün <mic@...ikod.net>, Eric Paris <eparis@...hat.com>, Günther Noack <gnoack@...gle.com>, "Serge E . Hallyn" <serge@...lyn.com>
Cc: Mickaël Salaün <mic@...ikod.net>, Ben Scarlato <akhna@...gle.com>, Casey Schaufler <casey@...aufler-ca.com>, Charles Zaffery <czaffery@...lox.com>, Daniel Burgener <dburgener@...ux.microsoft.com>, Francis Laniel <flaniel@...ux.microsoft.com>, James Morris <jmorris@...ei.org>, Jann Horn <jannh@...gle.com>, Jeff Xu <jeffxu@...gle.com>, Jorge Lucangeli Obes <jorgelo@...gle.com>, Kees Cook <kees@...nel.org>, Konstantin Meskhidze <konstantin.meskhidze@...wei.com>, Matt Bobrowski <mattbobrowski@...gle.com>, Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>, Phil Sutter <phil@....cc>, Praveen K Paladugu <prapal@...ux.microsoft.com>, Robert Salvet <robert.salvet@...lox.com>, Shervin Oloumi <enlightened@...gle.com>, Song Liu <song@...nel.org>, Tahera Fahimi <fahimitahera@...il.com>, Tyler Hicks <code@...icks.com>, audit@...r.kernel.org, linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v4 8/30] landlock: Add AUDIT_LANDLOCK_DENY and log ptrace  denials

On Jan  8, 2025 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@...ikod.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.

> 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.

> 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.

--
paul-moore.com