5:06 p.m.
On Monday, October 16, 2017 5:35:55 PM EDT Paul Moore wrote:
On Fri, Oct 13, 2017 at 3:58 PM, Steve Grubb
<sgrubb(a)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(a)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.
Because 1331 is already assigned and in https://git.kernel.org/pub/scm/linux/
kernel/git/jack/linux-fs.git/log/?h=for_next as commit
de8cd83e91bc3ee212b3e6ec6e4283af9e4ab269.
If you want me to assign 1331 which is already assigned to AUDIT_FANOTIFY in
the user space piece, then it will make your testing...not look right. So, how
do you want it?
> 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.
OK, easy enough.
> + 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.
Really do not want that to be the way we do things. We never have relied on
syscall records for simple events.
> + 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.
No problem.
-Steve
> };
>
> struct audit_net *aunet = net_generic(net, audit_net_id);