Re: [PATCH ghak25 v4 3/3] audit: add subj creds to NETFILTER_CFG record to cover async unregister

[Paul Moore]

On Wed, Apr 22, 2020 at 5:40 PM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> Some table unregister actions seem to be initiated by the kernel to
> garbage collect unused tables that are not initiated by any userspace
> actions.  It was found to be necessary to add the subject credentials to
> cover this case to reveal the source of these actions.  A sample record:
>
>   type=NETFILTER_CFG msg=audit(2020-03-11 21:25:21.491:269) : table=nat family=bridge entries=0 op=unregister pid=153 uid=root auid=unset tty=(none) ses=unset subj=system_u:system_r:kernel_t:s0 comm=kworker/u4:2 exe=(null)

[I'm going to comment up here instead of in the code because it is a
bit easier for everyone to see what the actual impact might be on the
records.]

Steve wants subject info in this case, okay, but let's try to trim out
some of the fields which simply don't make sense in this record; I'm
thinking of fields that are unset/empty in the kernel case and are
duplicates of other records in the userspace/syscall case.  I think
that means we can drop "tty", "ses", "comm", and "exe" ... yes?

While "auid" is a potential target for removal based on the
dup-or-unset criteria, I think it falls under Steve's request for
subject info here, even if it is garbage in this case.

> Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx>
> ---
>  kernel/auditsc.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index d281c18d1771..d7a45b181be0 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2557,12 +2557,30 @@ void __audit_log_nfcfg(const char *name, u8 af, unsigned int nentries,
>                        enum audit_nfcfgop op)
>  {
>         struct audit_buffer *ab;
> +       const struct cred *cred;
> +       struct tty_struct *tty;
> +       char comm[sizeof(current->comm)];
>
>         ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_NETFILTER_CFG);
>         if (!ab)
>                 return;
>         audit_log_format(ab, "table=%s family=%u entries=%u op=%s",
>                          name, af, nentries, audit_nfcfgs[op].s);
> +
> +       cred = current_cred();
> +       tty = audit_get_tty();
> +       audit_log_format(ab, " pid=%u uid=%u auid=%u tty=%s ses=%u",
> +                        task_pid_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_put_tty(tty);
> +       audit_log_task_context(ab); /* subj= */
> +       audit_log_format(ab, " comm=");
> +       audit_log_untrustedstring(ab, get_task_comm(comm, current));
> +       audit_log_d_path_exe(ab, current->mm); /* exe= */
> +
>         audit_log_end(ab);
>  }
>  EXPORT_SYMBOL_GPL(__audit_log_nfcfg);

-- 
paul moore
www.paul-moore.com