linux-kernel - Re: [PATCH v4 4/4] Audit: Add record for multiple object contexts

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: <88f740c7efa914435e2223e90666c8b2@paul-moore.com>
Date: Mon, 16 Jun 2025 16:54:52 -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 v4 4/4] Audit: Add record for multiple object contexts

On Jun  6, 2025 Casey Schaufler <casey@...aufler-ca.com> wrote:
> 
> Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
> An example of the MAC_OBJ_CONTEXTS record is:
> 
>     type=MAC_OBJ_CONTEXTS
>     msg=audit(1601152467.009:1050):
>     obj_selinux=unconfined_u:object_r:user_home_t:s0
> 
> When an audit event includes a AUDIT_MAC_OBJ_CONTEXTS record
> the "obj=" field in other records in the event will be "obj=?".
> An AUDIT_MAC_OBJ_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based
> on an object security context.
> 
> Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
> ---
>  include/linux/audit.h      |  7 +++++
>  include/uapi/linux/audit.h |  1 +
>  kernel/audit.c             | 58 +++++++++++++++++++++++++++++++++++++-
>  kernel/auditsc.c           | 45 ++++++++---------------------
>  security/selinux/hooks.c   |  3 +-
>  security/smack/smack_lsm.c |  3 +-
>  6 files changed, 80 insertions(+), 37 deletions(-)

...

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 0987b2f391cc..451c36965889 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2337,6 +2344,55 @@ int audit_log_task_context(struct audit_buffer *ab)
>  }
>  EXPORT_SYMBOL(audit_log_task_context);
>  
> +int audit_log_obj_ctx(struct audit_buffer *ab, struct lsm_prop *prop)
> +{
> +	int i;
> +	int rc;
> +	int error = 0;
> +	char *space = "";
> +	struct lsm_context ctx;
> +
> +	if (audit_obj_secctx_cnt < 2) {
> +		error = security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF);
> +		if (error < 0) {
> +			if (error != -EINVAL)
> +				goto error_path;
> +			return error;
> +		}
> +		audit_log_format(ab, " obj=%s", ctx.context);
> +		security_release_secctx(&ctx);
> +		return 0;
> +	}
> +	audit_log_format(ab, " obj=?");
> +	error = audit_buffer_aux_new(ab, AUDIT_MAC_OBJ_CONTEXTS);
> +	if (error)
> +		goto error_path;
> +
> +	for (i = 0; i < audit_obj_secctx_cnt; i++) {
> +		rc = security_lsmprop_to_secctx(prop, &ctx,
> +						audit_obj_lsms[i]->id);
> +		if (rc < 0) {
> +			audit_log_format(ab, "%sobj_%s=?", space,
> +					 audit_obj_lsms[i]->name);
> +			if (rc != -EINVAL)
> +				audit_panic("error in audit_log_obj_ctx");
> +			error = rc;

Do we need the same logic as in audit_log_subj_ctx()?

> +		} else {
> +			audit_log_format(ab, "%sobj_%s=%s", space,
> +					 audit_obj_lsms[i]->name, ctx.context);
> +			security_release_secctx(&ctx);
> +		}
> +		space = " ";
> +	}
> +
> +	audit_buffer_aux_end(ab);
> +	return error;
> +
> +error_path:
> +	audit_panic("error in audit_log_obj_ctx");
> +	return error;
> +}
> +
>  void audit_log_d_path_exe(struct audit_buffer *ab,
>  			  struct mm_struct *mm)
>  {
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 322d4e27f28e..0c28fa33d099 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1098,7 +1098,6 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>  				 char *comm)
>  {
>  	struct audit_buffer *ab;
> -	struct lsm_context ctx;
>  	int rc = 0;
>  
>  	ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
> @@ -1108,15 +1107,9 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>  	audit_log_format(ab, "opid=%d oauid=%d ouid=%d oses=%d", pid,
>  			 from_kuid(&init_user_ns, auid),
>  			 from_kuid(&init_user_ns, uid), sessionid);
> -	if (lsmprop_is_set(prop)) {
> -		if (security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF) < 0) {
> -			audit_log_format(ab, " obj=(none)");
> -			rc = 1;
> -		} else {
> -			audit_log_format(ab, " obj=%s", ctx.context);
> -			security_release_secctx(&ctx);
> -		}
> -	}
> +	if (lsmprop_is_set(prop) && audit_log_obj_ctx(ab, prop))
> +		rc = 1;

We should probably use the return value from audit_log_obj_ctx().

>  	audit_log_format(ab, " ocomm=");
>  	audit_log_untrustedstring(ab, comm);
>  	audit_log_end(ab);

...

> @@ -1780,15 +1756,16 @@ static void audit_log_exit(void)
>  						  axs->target_sessionid[i],
>  						  &axs->target_ref[i],
>  						  axs->target_comm[i]))
> -				call_panic = 1;
> +			call_panic = 1;
>  	}
>  
>  	if (context->target_pid &&
>  	    audit_log_pid_context(context, context->target_pid,
>  				  context->target_auid, context->target_uid,
>  				  context->target_sessionid,
> -				  &context->target_ref, context->target_comm))
> -			call_panic = 1;
> +				  &context->target_ref,
> +				  context->target_comm))
> +		call_panic = 1;

I appreciate the indent fixes, would you mind pulling this out and
submitting them separately?

--
paul-moore.com