[PATCH 1/1] audit: add missing fields to AUDIT_CONFIG_CHANGE event

Richard Guy Briggs rgb at redhat.com
Fri Oct 13 19:54:39 UTC 2017


On 2017-10-12 19:57, Steve Grubb wrote:
> There are very important fields necessary to understand who is adding
> audit rules and a little more context about the environment in which
> its happening. This adds pid, uid, tty, subj, comm, and exe
> information to the event. These are required fields.

Could we add an issue number to the patch description?

Eg: See: https://github.com/linux-audit/audit-kernel/issues/59

> Signed-off-by: sgrubb <sgrubb at redhat.com>
> ---
>  kernel/audit_watch.c | 23 +++++++++++++++++++----
>  kernel/auditfilter.c | 18 +++++++++++++++---
>  2 files changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
> index 9eb8b3511636..63abc2ba1372 100644
> --- a/kernel/audit_watch.c
> +++ b/kernel/audit_watch.c
> @@ -239,14 +239,29 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)
>  static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
>  {
>  	if (audit_enabled) {
> +		struct tty_struct *tty;
> +		const struct cred *cred;
>  		struct audit_buffer *ab;
> +		char comm[sizeof(current->comm)];
> +
>  		ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
>  		if (unlikely(!ab))
>  			return;
> -		audit_log_format(ab, "auid=%u ses=%u op=%s",
> -				 from_kuid(&init_user_ns, audit_get_loginuid(current)),
> -				 audit_get_sessionid(current), op);
> -		audit_log_format(ab, " path=");
> +
> +		tty = audit_get_tty(current);
> +		cred = current_cred();
> +		audit_log_format(ab, "pid=%d uid=%u auid=%u tty=%s ses=%u",
> +				task_tgid_nr(current),
> +				from_kuid(&init_user_ns, cred->uid),
> +				from_kuid(&init_user_ns,
> +				audit_get_loginuid(current)),
> +				tty ? tty_name(tty) : "(none)",
> +				audit_get_sessionid(current));
> +		audit_log_task_context(ab);
> +		audit_log_format(ab, " comm=");
> +		audit_log_untrustedstring(ab, get_task_comm(comm, current));
> +		audit_log_d_path_exe(ab, current->mm);
> +		audit_log_format(ab, "op=%s path=", op);
>  		audit_log_untrustedstring(ab, w->path);
>  		audit_log_key(ab, r->filterkey);
>  		audit_log_format(ab, " list=%d res=1", r->listnr);

As noted, audit_put_tty(tty) is missing...

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 0b0aa5854dac..5e2a953da29a 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1065,17 +1065,29 @@ static void audit_list_rules(int seq, struct sk_buff_head *q)
>  static void audit_log_rule_change(char *action, struct audit_krule *rule, int res)
>  {
>  	struct audit_buffer *ab;
> -	uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(current));
> -	unsigned int sessionid = audit_get_sessionid(current);
> +	struct tty_struct *tty;
> +	const struct cred *cred;
> +	char comm[sizeof(current->comm)];
>  
>  	if (!audit_enabled)
>  		return;
>  
> +	tty = audit_get_tty(current);
> +	cred = current_cred();
> +
>  	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>  	if (!ab)
>  		return;
> -	audit_log_format(ab, "auid=%u ses=%u" ,loginuid, sessionid);
> +	audit_log_format(ab, "pid=%d uid=%u auid=%u tty=%s ses=%u",
> +			task_tgid_nr(current),
> +			from_kuid(&init_user_ns, cred->uid),
> +			from_kuid(&init_user_ns, audit_get_loginuid(current)),
> +			tty ? tty_name(tty) : "(none)",
> +			audit_get_sessionid(current));
>  	audit_log_task_context(ab);
> +	audit_log_format(ab, " comm=");
> +	audit_log_untrustedstring(ab, get_task_comm(comm, current));
> +	audit_log_d_path_exe(ab, current->mm);
>  	audit_log_format(ab, " op=%s", action);
>  	audit_log_key(ab, rule->filterkey);
>  	audit_log_format(ab, " list=%d res=%d", rule->listnr, res);

Again, missing audit_put_tty(tty).

Since these are already standalone records (since the context passed to
audit_log_start() is NULL) this info is necessary.

I'm fine with the field ordering.  If that is not acceptable, I'd
recommend a new record type (AUDIT_TASK) to act as an aux record to this
record that lists this information in a standard order that can be used
as an aux record for all the standalone records that are missing this
information.

Reviewed-by: Richard Guy Briggs <rgb at redhat.com>


- RGB

--
Richard Guy Briggs <rgb at redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635




More information about the Linux-audit mailing list