[PATCH 1/1] audit: log binding and unbinding to netlink multicast

[Paul Moore]

On Fri, Oct 13, 2017 at 3:58 PM, Steve Grubb <sgrubb at redhat.com> wrote:
> Log information about programs connecting and disconnecting to the audit
> netlink multicast socket. This is needed so that during investigations a
> security officer can tell who or what had access to the audit trail. This
> helps to meet the FAU_SAR.2 requirement for Common Criteria. Sample
> event:
>
> type=UNKNOWN[1332] msg=audit(1507924331.540:3): pid=1 uid=0
> auid=4294967295 tty=(none) ses=4294967295 subj=kernel comm="systemd"
> exe="/usr/lib/systemd/systemd" nlnk-grp=1 op=connect res=1
>
> Signed-off-by: sgrubb <sgrubb at redhat.com>
> ---
>  include/uapi/linux/audit.h |  1 +
>  kernel/audit.c             | 48 ++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 45 insertions(+), 4 deletions(-)

Since I think this is going to involve a respin, I just want to
mention again "sgrubb" vs "Steve Grubb".  More comments inline ...

> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 0714a66f0e0c..892e63d9f2c1 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -112,6 +112,7 @@
>  #define AUDIT_FEATURE_CHANGE   1328    /* audit log listing feature changes */
>  #define AUDIT_REPLACE          1329    /* Replace auditd if this packet unanswerd */
>  #define AUDIT_KERN_MODULE      1330    /* Kernel Module events */
> +#define AUDIT_EVENT_LISTENER   1332    /* Task joined multicast read socket */

What Richard said.  Basically AUDIT_EVENT_LISTENER should be 1331 or
have a *really* good explanation as to why it needs to be 1332.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 1baabc9539b4..3d2461be83a8 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1476,22 +1476,61 @@ static void audit_receive(struct sk_buff  *skb)
>         mutex_unlock(&audit_cmd_mutex);
>  }
>
> +/* Log information about who is connecting to the audit multicast socket */
> +static void audit_log_multicast_bind(int group, const char *op, int err)
> +{
> +       const struct cred *cred;
> +       struct tty_struct *tty;
> +       char comm[sizeof(current->comm)];
> +       struct audit_buffer *ab;
> +
> +       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);

It really seems like this should be associated with the current task,
e.g. "audit_log_start(current->audit_context, ...)".  After all, the
whole point of this record is to capture information about the subject
who is binding to the multicast socket.

> +       if (!ab)
> +               return;
> +
> +       cred = current_cred();
> +       tty = audit_get_tty(current);
> +
> +       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= */

We could drop a lot of the above if this was an associated record.

> +       audit_log_format(ab, " nlnk-grp=%d op=%s res=%d", group, op, !err);
> +       audit_log_end(ab);
> +}
> +
>  /* Run custom bind function on netlink socket group connect or bind requests. */
> -static int audit_bind(struct net *net, int group)
> +static int audit_multicast_bind(struct net *net, int group)
>  {
> +       int err = 0;
> +
>         if (!capable(CAP_AUDIT_READ))
> -               return -EPERM;
> +               err = -EPERM;
> +       audit_log_multicast_bind(group, "connect", err);
>
> -       return 0;
> +       return err;
> +}
> +
> +static void audit_multicast_unbind(struct net *net, int group)
> +{
> +       audit_log_multicast_bind(group, "disconnect", 0);
>  }
>
>  static int __net_init audit_net_init(struct net *net)
>  {
>         struct netlink_kernel_cfg cfg = {
>                 .input  = audit_receive,
> -               .bind   = audit_bind,
> +               .bind   = audit_multicast_bind,
>                 .flags  = NL_CFG_F_NONROOT_RECV,
>                 .groups = AUDIT_NLGRP_MAX,
> +               .unbind = audit_multicast_unbind,

Please group the unbind with the bind, it is much easier to read that way.

>         };
>
>         struct audit_net *aunet = net_generic(net, audit_net_id);

-- 
paul moore
www.paul-moore.com