linux-kernel - Re: [PATCH 04/13] Audit: maintain an lsmblob in 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] [day] [month] [year] [list]
Message-ID: <e0fc63dea4db21f53dd3358334029aae3981fc5f.camel@canonical.com>
Date: Tue, 27 Aug 2024 12:08:23 -0300
From: Georgia Garcia <georgia.garcia@...onical.com>
To: Casey Schaufler <casey@...aufler-ca.com>, paul@...l-moore.com, 
	linux-security-module@...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,
 mic@...ikod.net
Subject: Re: [PATCH 04/13] Audit: maintain an lsmblob in audit_context

On Tue, 2024-08-27 at 12:01 -0300, Georgia Garcia wrote:
> On Sun, 2024-08-25 at 12:00 -0700, Casey Schaufler wrote:
> > Replace the secid value stored in struct audit_context with a struct
> > lsmblob. Change the code that uses this value to accommodate the
> > change. security_audit_rule_match() expects a lsmblob, so existing
> > scaffolding can be removed. A call to security_secid_to_secctx()
> > is changed to security_lsmblob_to_secctx().  The call to
> > security_ipc_getsecid() is scaffolded.
> > 
> > A new function lsmblob_is_set() is introduced to identify whether
> > an lsmblob contains a non-zero value.
> > 
> > Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
> > ---
> >  include/linux/security.h | 13 +++++++++++++
> >  kernel/audit.h           |  3 ++-
> >  kernel/auditsc.c         | 19 ++++++++-----------
> >  3 files changed, 23 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index 457fafc32fb0..a0b23b6e8734 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -277,6 +277,19 @@ static inline const char *kernel_load_data_id_str(enum kernel_load_data_id id)
> >  	return kernel_load_data_str[id];
> >  }
> >  
> > +/**
> > + * lsmblob_is_set - report if there is a value in the lsmblob
> > + * @blob: Pointer to the exported LSM data
> > + *
> > + * Returns true if there is a value set, false otherwise
> > + */
> > +static inline bool lsmblob_is_set(struct lsmblob *blob)
> > +{
> > +	const struct lsmblob empty = {};
> > +
> > +	return !!memcmp(blob, &empty, sizeof(*blob));
> > +}
> > +
> >  #ifdef CONFIG_SECURITY
> >  
> >  int call_blocking_lsm_notifier(enum lsm_event event, void *data);
> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index a60d2840559e..b1f2de4d4f1e 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -11,6 +11,7 @@
> >  
> >  #include <linux/fs.h>
> >  #include <linux/audit.h>
> > +#include <linux/security.h>
> >  #include <linux/skbuff.h>
> >  #include <uapi/linux/mqueue.h>
> >  #include <linux/tty.h>
> > @@ -160,7 +161,7 @@ struct audit_context {
> >  			kuid_t			uid;
> >  			kgid_t			gid;
> >  			umode_t			mode;
> > -			u32			osid;
> > +			struct lsmblob		oblob;
> >  			int			has_perm;
> >  			uid_t			perm_uid;
> >  			gid_t			perm_gid;
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 23adb15cae43..84f6e9356b8f 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -724,9 +724,7 @@ static int audit_filter_rules(struct task_struct *tsk,
> >  				/* Find ipc objects that match */
> >  				if (!ctx || ctx->type != AUDIT_IPC)
> >  					break;
> > -				/* scaffolding */
> > -				blob.scaffold.secid = ctx->ipc.osid;
> > -				if (security_audit_rule_match(&blob,
> > +				if (security_audit_rule_match(&ctx->ipc.oblob,
> >  							      f->type, f->op,
> >  							      f->lsm_rule))
> >  					++result;
> > @@ -1394,19 +1392,17 @@ static void show_special(struct audit_context *context, int *call_panic)
> >  			audit_log_format(ab, " a%d=%lx", i,
> >  				context->socketcall.args[i]);
> >  		break; }
> > -	case AUDIT_IPC: {
> > -		u32 osid = context->ipc.osid;
> > -
> > +	case AUDIT_IPC:
> >  		audit_log_format(ab, "ouid=%u ogid=%u mode=%#ho",
> >  				 from_kuid(&init_user_ns, context->ipc.uid),
> >  				 from_kgid(&init_user_ns, context->ipc.gid),
> >  				 context->ipc.mode);
> > -		if (osid) {
> > +		if (lsmblob_is_set(&context->ipc.oblob)) {
> >  			char *ctx = NULL;
> >  			u32 len;
> >  
> > -			if (security_secid_to_secctx(osid, &ctx, &len)) {
> > -				audit_log_format(ab, " osid=%u", osid);
> > +			if (security_lsmblob_to_secctx(&context->ipc.oblob,
> > +						       &ctx, &len)) {
> 
> Is there any reason to stop auditing secid when we fail to get the
> security context?

Well, yes, if we're moving away from using secid for all LSMs, then it
doesn't make sense to audit it here anymore, so nevermind :)

> 
> >  				*call_panic = 1;
> >  			} else {
> >  				audit_log_format(ab, " obj=%s", ctx);
> > @@ -1426,7 +1422,7 @@ static void show_special(struct audit_context *context, int *call_panic)
> >  				context->ipc.perm_gid,
> >  				context->ipc.perm_mode);
> >  		}
> > -		break; }
> > +		break;
> >  	case AUDIT_MQ_OPEN:
> >  		audit_log_format(ab,
> >  			"oflag=0x%x mode=%#ho mq_flags=0x%lx mq_maxmsg=%ld "
> > @@ -2642,7 +2638,8 @@ void __audit_ipc_obj(struct kern_ipc_perm *ipcp)
> >  	context->ipc.gid = ipcp->gid;
> >  	context->ipc.mode = ipcp->mode;
> >  	context->ipc.has_perm = 0;
> > -	security_ipc_getsecid(ipcp, &context->ipc.osid);
> > +	/* scaffolding */
> > +	security_ipc_getsecid(ipcp, &context->ipc.oblob.scaffold.secid);
> >  	context->type = AUDIT_IPC;
> >  }
> >  
>