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: <4895afcecf518a0744b964cfa4c16e3a@paul-moore.com>
Date: Thu, 24 Apr 2025 18:18:32 -0400
From: Paul Moore <paul@...l-moore.com>
To: Casey Schaufler <casey@...aufler-ca.com>, casey@...aufler-ca.com, eparis@...hat.com, linux-security-module@...r.kernel.org, audit@...r.kernel.org
Cc: 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, selinux@...r.kernel.org
Subject: Re: [PATCH v3 3/5] Audit: Add record for multiple task security  contexts

On Mar 19, 2025 Casey Schaufler <casey@...aufler-ca.com> wrote:
> 
> Replace the single skb pointer in an audit_buffer with a list of
> skb pointers. Add the audit_stamp information to the audit_buffer as
> there's no guarantee that there will be an audit_context containing
> the stamp associated with the event. At audit_log_end() time create
> auxiliary records (none are currently defined) as have been added to the
> list. Functions are created to manage the skb list in the audit_buffer.
> 
> Create a new audit record AUDIT_MAC_TASK_CONTEXTS.
> An example of the MAC_TASK_CONTEXTS (1423) record is:
> 
>     type=MAC_TASK_CONTEXTS[1423]
>     msg=audit(1600880931.832:113)
>     subj_apparmor=unconfined
>     subj_smack=_
> 
> When an audit event includes a AUDIT_MAC_TASK_CONTEXTS record the
> "subj=" field in other records in the event will be "subj=?".
> An AUDIT_MAC_TASK_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based on a
> subject security context.
> 
> Suggested-by: Paul Moore <paul@...l-moore.com>
> Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
> ---
>  include/linux/audit.h      |   6 ++
>  include/uapi/linux/audit.h |   1 +
>  kernel/audit.c             | 171 ++++++++++++++++++++++++++++++-------
>  security/apparmor/lsm.c    |   3 +
>  security/selinux/hooks.c   |   3 +
>  security/smack/smack_lsm.c |   3 +
>  6 files changed, 158 insertions(+), 29 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 0050ef288ab3..b493ca5976cf 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -37,6 +37,7 @@ struct audit_watch;
>  struct audit_tree;
>  struct sk_buff;
>  struct kern_ipc_perm;
> +struct lsm_id;
>  
>  struct audit_krule {
>  	u32			pflags;
> @@ -210,6 +211,8 @@ extern u32 audit_enabled;
>  
>  extern int audit_signal_info(int sig, struct task_struct *t);
>  
> +extern void audit_lsm_secctx(const struct lsm_id *lsmid);
> +
>  #else /* CONFIG_AUDIT */
>  static inline __printf(4, 5)
>  void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
> @@ -269,6 +272,9 @@ static inline int audit_signal_info(int sig, struct task_struct *t)
>  	return 0;
>  }
>  
> +static inline void audit_lsm_secctx(const struct lsm_id *lsmid)
> +{ }
> +
>  #endif /* CONFIG_AUDIT */
>  
>  #ifdef CONFIG_AUDIT_COMPAT_GENERIC
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index d9a069b4a775..5ebb5d80363d 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -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_MAC_TASK_CONTEXTS	1423	/* Multiple LSM task contexts */
>  
>  #define AUDIT_FIRST_KERN_ANOM_MSG   1700
>  #define AUDIT_LAST_KERN_ANOM_MSG    1799
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 6bbadb605ca3..7ec3919ae925 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -54,6 +54,7 @@
>  #include <net/netlink.h>
>  #include <linux/skbuff.h>
>  #include <linux/security.h>
> +#include <linux/lsm_hooks.h>
>  #include <linux/freezer.h>
>  #include <linux/pid_namespace.h>
>  #include <net/netns/generic.h>
> @@ -81,6 +82,11 @@ static u32	audit_failure = AUDIT_FAIL_PRINTK;
>  /* private audit network namespace index */
>  static unsigned int audit_net_id;
>  
> +/* Number of modules that provide a security context.
> +   List of lsms that provide a security context */
> +static u32 audit_secctx_cnt = 0;
> +static const struct lsm_id *audit_lsms[MAX_LSM_COUNT];

We've already talked about this in other threads, offline, etc., but
for the sake of others, this should be adjusted to use the counts
provided in the LSM initialization code rework.

https://lore.kernel.org/linux-security-module/20250409185019.238841-31-paul@paul-moore.com/

> @@ -2412,26 +2517,14 @@ int audit_signal_info(int sig, struct task_struct *t)
>  }
>  
>  /**
> - * audit_log_end - end one audit record
> - * @ab: the audit_buffer
> - *
> - * We can not do a netlink send inside an irq context because it blocks (last
> - * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a
> - * queue and a kthread is scheduled to remove them from the queue outside the
> - * irq context.  May be called in any context.
> + * __audit_log_end - enqueue one audit record
> + * @skb: the buffer to send
>   */
> -void audit_log_end(struct audit_buffer *ab)
> +static void __audit_log_end(struct sk_buff *skb)
>  {
> -	struct sk_buff *skb;
>  	struct nlmsghdr *nlh;
>  
> -	if (!ab)
> -		return;
> -
>  	if (audit_rate_check()) {
> -		skb = ab->skb;
> -		ab->skb = NULL;
> -
>  		/* setup the netlink header, see the comments in
>  		 * kauditd_send_multicast_skb() for length quirks */
>  		nlh = nlmsg_hdr(skb);
> @@ -2442,6 +2535,26 @@ void audit_log_end(struct audit_buffer *ab)
>  		wake_up_interruptible(&kauditd_wait);
>  	} else
>  		audit_log_lost("rate limit exceeded");
> +}

Okay, this is twice now in one patchset ... as I mentioned in the v2
review:

 "We should probably move the kauditd thread wake into
  audit_log_end() so we don't end up poking the scheduler
  multiple times."

https://lore.kernel.org/audit/69ee16ce82a564e09b2060d46fa2be0d@paul-moore.com/

> +/**
> + * audit_log_end - end one audit record
> + * @ab: the audit_buffer
> + *
> + * We can not do a netlink send inside an irq context because it blocks (last
> + * arg, flags, is not set to MSG_DONTWAIT), so the audit buffer is placed on a
> + * queue and a kthread is scheduled to remove them from the queue outside the
> + * irq context.  May be called in any context.
> + */
> +void audit_log_end(struct audit_buffer *ab)
> +{
> +	struct sk_buff *skb;
> +
> +	if (!ab)
> +		return;
> +
> +	while ((skb = skb_dequeue(&ab->skb_list)))
> +		__audit_log_end(skb);

The wakeup should go here.

>  	audit_buffer_free(ab);
>  }

--
paul-moore.com

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ