[PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Richard Guy Briggs]

Log information about programs connecting to and disconnecting from 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.  Here is the systemd startup event:

type=UNKNOWN[1335] msg=audit(2020-01-17 10:30:33.731:6) : pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd nl-mcgrp=1 op=connect res=yes

And the events from the test suite:

type=PROCTITLE msg=audit(2020-01-17 10:36:24.050:294) : proctitle=/usr/bin/perl -w amcast_joinpart/test
type=SOCKADDR msg=audit(2020-01-17 10:36:24.050:294) : saddr={ saddr_fam=netlink nlnk-fam=16 nlnk-pid=0 }
type=SYSCALL msg=audit(2020-01-17 10:36:24.050:294) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x7 a1=0x55d65cb79090 a2=0xc a3=0x0 items=0 ppid=671 pid=674 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=3 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.050:294) : pid=674 uid=root auid=root tty=ttyS0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=connect res=yes

type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.051:295) : pid=674 uid=root auid=root tty=ttyS0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=disconnect res=yes

Please see the upstream issue tracker:
  https://github.com/linux-audit/audit-kernel/issues/28
  https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Multicast-Socket-Join-Part
  https://github.com/rgbriggs/audit-testsuite/compare/ghak28-mcast-part-join

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>

---
Note: msg type 1334 was skipped due to BPF accepted in another tree.
Note: v5 due to previous 2014-10-07, 2015-07-23, 2016-11-30, 2017-10-13
Note: subj attrs included due to missing syscall record for systemd (audit=1)
Note: tried refactor of subj attrs, but this is yet another new order.
---
 include/uapi/linux/audit.h |  1 +
 kernel/audit.c             | 48 ++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 3ad935527177..67fb24472dc2 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -116,6 +116,7 @@
 #define AUDIT_FANOTIFY		1331	/* Fanotify access decision */
 #define AUDIT_TIME_INJOFFSET	1332	/* Timekeeping offset injected */
 #define AUDIT_TIME_ADJNTPVAL	1333	/* NTP value adjustment */
+#define AUDIT_EVENT_LISTENER	1335	/* Task joined multicast read socket */
 
 #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
 #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
diff --git a/kernel/audit.c b/kernel/audit.c
index 17b0d523afb3..478259f3fa53 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
 	audit_ctl_unlock();
 }
 
+/* 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;
+
+	if (!audit_enabled)
+		return;
+
+	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
+	if (!ab)
+		return;
+
+	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_format(ab, " nl-mcgrp=%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 err;
+}
 
-	return 0;
+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,
+		.unbind	= audit_multicast_unbind,
 		.flags	= NL_CFG_F_NONROOT_RECV,
 		.groups	= AUDIT_NLGRP_MAX,
 	};
-- 
1.8.3.1

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Paul Moore]

On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> Log information about programs connecting to and disconnecting from 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.  Here is the systemd startup event:
>
> type=UNKNOWN[1335] msg=audit(2020-01-17 10:30:33.731:6) : pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd nl-mcgrp=1 op=connect res=yes
>
> And the events from the test suite:
>
> type=PROCTITLE msg=audit(2020-01-17 10:36:24.050:294) : proctitle=/usr/bin/perl -w amcast_joinpart/test
> type=SOCKADDR msg=audit(2020-01-17 10:36:24.050:294) : saddr={ saddr_fam=netlink nlnk-fam=16 nlnk-pid=0 }
> type=SYSCALL msg=audit(2020-01-17 10:36:24.050:294) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x7 a1=0x55d65cb79090 a2=0xc a3=0x0 items=0 ppid=671 pid=674 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=3 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.050:294) : pid=674 uid=root auid=root tty=ttyS0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=connect res=yes
>
> type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.051:295) : pid=674 uid=root auid=root tty=ttyS0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=disconnect res=yes
>
> Please see the upstream issue tracker:
>   https://github.com/linux-audit/audit-kernel/issues/28
>   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Multicast-Socket-Join-Part
>   https://github.com/rgbriggs/audit-testsuite/compare/ghak28-mcast-part-join
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>
> ---
> Note: msg type 1334 was skipped due to BPF accepted in another tree.
> Note: v5 due to previous 2014-10-07, 2015-07-23, 2016-11-30, 2017-10-13
> Note: subj attrs included due to missing syscall record for systemd (audit=1)
> Note: tried refactor of subj attrs, but this is yet another new order.
> ---
>  include/uapi/linux/audit.h |  1 +
>  kernel/audit.c             | 48 ++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 3ad935527177..67fb24472dc2 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -116,6 +116,7 @@
>  #define AUDIT_FANOTIFY         1331    /* Fanotify access decision */
>  #define AUDIT_TIME_INJOFFSET   1332    /* Timekeeping offset injected */
>  #define AUDIT_TIME_ADJNTPVAL   1333    /* NTP value adjustment */
> +#define AUDIT_EVENT_LISTENER   1335    /* Task joined multicast read socket */
>
>  #define AUDIT_AVC              1400    /* SE Linux avc denial or grant */
>  #define AUDIT_SELINUX_ERR      1401    /* Internal SE Linux Errors */
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 17b0d523afb3..478259f3fa53 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
>         audit_ctl_unlock();
>  }
>
> +/* 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;
> +
> +       if (!audit_enabled)
> +               return;
> +
> +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> +       if (!ab)
> +               return;
> +
> +       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));

Don't we already get all of that information as part of the syscall record?

> +       audit_put_tty(tty);
> +       audit_log_task_context(ab); /* subj= */

Also part of the syscall record.

> +       audit_log_format(ab, " comm=");
> +       audit_log_untrustedstring(ab, get_task_comm(comm, current));

Again.

> +       audit_log_d_path_exe(ab, current->mm); /* exe= */

Again.

> +       audit_log_format(ab, " nl-mcgrp=%d op=%s res=%d", group, op, !err);

This part is new ;)

> +       audit_log_end(ab);
> +}

I'm pretty sure these are the same arguments I made when Steve posted
a prior version of this patch.

-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Richard Guy Briggs]

On 2020-01-22 17:40, Paul Moore wrote:
> On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > Log information about programs connecting to and disconnecting from 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.  Here is the systemd startup event:
> >
> > type=UNKNOWN[1335] msg=audit(2020-01-17 10:30:33.731:6) : pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd nl-mcgrp=1 op=connect res=yes
> >
> > And the events from the test suite:
> >
> > type=PROCTITLE msg=audit(2020-01-17 10:36:24.050:294) : proctitle=/usr/bin/perl -w amcast_joinpart/test
> > type=SOCKADDR msg=audit(2020-01-17 10:36:24.050:294) : saddr={ saddr_fam=netlink nlnk-fam=16 nlnk-pid=0 }
> > type=SYSCALL msg=audit(2020-01-17 10:36:24.050:294) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x7 a1=0x55d65cb79090 a2=0xc a3=0x0 items=0 ppid=671 pid=674 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=3 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> > type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.050:294) : pid=674 uid=root auid=root tty=ttyS0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=connect res=yes
> >
> > type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.051:295) : pid=674 uid=root auid=root tty=ttyS0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl exe=/usr/bin/perl nl-mcgrp=1 op=disconnect res=yes
> >
> > Please see the upstream issue tracker:
> >   https://github.com/linux-audit/audit-kernel/issues/28
> >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Multicast-Socket-Join-Part
> >   https://github.com/rgbriggs/audit-testsuite/compare/ghak28-mcast-part-join
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> >
> > ---
> > Note: msg type 1334 was skipped due to BPF accepted in another tree.
> > Note: v5 due to previous 2014-10-07, 2015-07-23, 2016-11-30, 2017-10-13
> > Note: subj attrs included due to missing syscall record for systemd (audit=1)
> > Note: tried refactor of subj attrs, but this is yet another new order.
> > ---
> >  include/uapi/linux/audit.h |  1 +
> >  kernel/audit.c             | 48 ++++++++++++++++++++++++++++++++++++++++++----
> >  2 files changed, 45 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 3ad935527177..67fb24472dc2 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -116,6 +116,7 @@
> >  #define AUDIT_FANOTIFY         1331    /* Fanotify access decision */
> >  #define AUDIT_TIME_INJOFFSET   1332    /* Timekeeping offset injected */
> >  #define AUDIT_TIME_ADJNTPVAL   1333    /* NTP value adjustment */
> > +#define AUDIT_EVENT_LISTENER   1335    /* Task joined multicast read socket */
> >
> >  #define AUDIT_AVC              1400    /* SE Linux avc denial or grant */
> >  #define AUDIT_SELINUX_ERR      1401    /* Internal SE Linux Errors */
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 17b0d523afb3..478259f3fa53 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> >         audit_ctl_unlock();
> >  }
> >
> > +/* 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;
> > +
> > +       if (!audit_enabled)
> > +               return;
> > +
> > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > +       if (!ab)
> > +               return;
> > +
> > +       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));
> 
> Don't we already get all of that information as part of the syscall record?

Yes.  However, the syscall record isn't always present.  One example is
systemd, shown above.  The other is the disconnect record, shown above,
which may be asynchronous, or an unmonitored syscall (It could only be
setsockopt, close, shutdown.).

> > +       audit_put_tty(tty);
> > +       audit_log_task_context(ab); /* subj= */
> 
> Also part of the syscall record.
> 
> > +       audit_log_format(ab, " comm=");
> > +       audit_log_untrustedstring(ab, get_task_comm(comm, current));
> 
> Again.
> 
> > +       audit_log_d_path_exe(ab, current->mm); /* exe= */
> 
> Again.
> 
> > +       audit_log_format(ab, " nl-mcgrp=%d op=%s res=%d", group, op, !err);
> 
> This part is new ;)
> 
> > +       audit_log_end(ab);
> > +}
> 
> I'm pretty sure these are the same arguments I made when Steve posted
> a prior version of this patch.

You did.  I would really like to have dropped them, but they aren't
reliably available.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@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

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Paul Moore]

On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2020-01-22 17:40, Paul Moore wrote:
> > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:

...

> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index 17b0d523afb3..478259f3fa53 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > >         audit_ctl_unlock();
> > >  }
> > >
> > > +/* 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;
> > > +
> > > +       if (!audit_enabled)
> > > +               return;
> > > +
> > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > +       if (!ab)
> > > +               return;
> > > +
> > > +       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));
> >
> > Don't we already get all of that information as part of the syscall record?
>
> Yes.  However, the syscall record isn't always present.  One example is
> systemd, shown above.

Assuming that the system supports syscall auditing, the absence of a
syscall record is a configuration choice made by the admin.  If the
system doesn't support syscall auditing the obvious "fix" is to do the
work to enable syscall auditing on that platform ... but now we're
starting to get off topic.

> The other is the disconnect record, shown above,
> which may be asynchronous, or an unmonitored syscall (It could only be
> setsockopt, close, shutdown.).

An unmonitored syscall still falls under the category of a
configuration choice so I'm not too concerned about that, but the
async disconnect record is legitimate.  Can you provide more
information about when this occurs?  I'm guessing this is pretty much
just an abrupt/abnormal program exit?

> > I'm pretty sure these are the same arguments I made when Steve posted
> > a prior version of this patch.
>
> You did.  I would really like to have dropped them, but they aren't
> reliably available.

Personally I'm not too worried if we have duplicate information spread
across records in a single event, as long as they are consistent.
However, I remember Steve complaining rather loudly about duplicated
fields across records in a single event some time back; perhaps that
is not a concern of his anymore (perhaps it was a narrow case at the
time), I don't know.

Here is the deal, either duplicated information is something we are
okay with, or it is something to avoid; we need to pick one.  As
mentioned above, I don't really care that much either way (I have a
slight preference, but I don't feel strongly enough to fight for it),
so let's hear the arguments both for and against and decide - whatever
we pick I'll enforce so long as we are stuck with this string format.

-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Richard Guy Briggs]

On 2020-01-23 09:32, Paul Moore wrote:
> On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2020-01-22 17:40, Paul Moore wrote:
> > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> ...
> 
> > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > index 17b0d523afb3..478259f3fa53 100644
> > > > --- a/kernel/audit.c
> > > > +++ b/kernel/audit.c
> > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > >         audit_ctl_unlock();
> > > >  }
> > > >
> > > > +/* 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;
> > > > +
> > > > +       if (!audit_enabled)
> > > > +               return;
> > > > +
> > > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > +       if (!ab)
> > > > +               return;
> > > > +
> > > > +       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));
> > >
> > > Don't we already get all of that information as part of the syscall record?
> >
> > Yes.  However, the syscall record isn't always present.  One example is
> > systemd, shown above.
> 
> Assuming that the system supports syscall auditing, the absence of a
> syscall record is a configuration choice made by the admin.  If the
> system doesn't support syscall auditing the obvious "fix" is to do the
> work to enable syscall auditing on that platform ... but now we're
> starting to get off topic.

Well, the system did spit out a syscall record with the example above,
so it has support for syscall auditing.

I'm testing on f30 with an upstream kernel, the standard 30-stig ruleset and
with kernel command line audit=1.  What else is needed to support a syscall
record on systemd before any audit rules have been put in place?  We may still
have a bug here that affects early process auditing.  What am I missing?

If we can get that sorted out, we don't need subject attributes in this record.

> > The other is the disconnect record, shown above,
> > which may be asynchronous, or an unmonitored syscall (It could only be
> > setsockopt, close, shutdown.).
> 
> An unmonitored syscall still falls under the category of a
> configuration choice so I'm not too concerned about that, but the
> async disconnect record is legitimate.  Can you provide more
> information about when this occurs?  I'm guessing this is pretty much
> just an abrupt/abnormal program exit?

Again, what configuration choice are you talking about?  
"-a task,never"?  That isn't active on this system.

The output was produced by the test case quoted in the patch description.

I should not have had to put a rule in place to do syscall auditing on connect,
bind, setsockopt, close, shutdown.

The disconnect would have been due to a perl close() call.  I would not have
expected that to be async, but I don't know the details of what the perl
implementation does.

> > > I'm pretty sure these are the same arguments I made when Steve posted
> > > a prior version of this patch.
> >
> > You did.  I would really like to have dropped them, but they aren't
> > reliably available.
> 
> Personally I'm not too worried if we have duplicate information spread
> across records in a single event, as long as they are consistent.
> However, I remember Steve complaining rather loudly about duplicated
> fields across records in a single event some time back; perhaps that
> is not a concern of his anymore (perhaps it was a narrow case at the
> time), I don't know.
> 
> Here is the deal, either duplicated information is something we are
> okay with, or it is something to avoid; we need to pick one.  As
> mentioned above, I don't really care that much either way (I have a
> slight preference, but I don't feel strongly enough to fight for it),
> so let's hear the arguments both for and against and decide - whatever
> we pick I'll enforce so long as we are stuck with this string format.

Steve, can you say why this order should be the standard?  From:
	http://people.redhat.com/sgrubb/audit/record-fields.html

I get:
        SYSCALL/ANOM_LINK/FEATURE_CHANGE
                ppid    pid     auid    uid     gid     euid    suid    fsuid   egid    sgid    fsgid   tty     ses     comm    exe     subj
        ANOM_ABEND/SECCOMP
                                auid    uid     gid     ses     subj    pid     comm    exe
        LOGIN
                pid     uid     subj    old-auid        auid    tty     old-ses ses
        SYSTEM_BOOT/SYSTEM_SHUTDOWN
                pid     uid     auid    ses     subj    comm    exe
        USER_LOGIN
                pid     uid     auid    ses     subj    uid     exe
        DAEMON_START
                                auid    pid     uid     ses     subj
        DAEMON_CONFIG/DAEMON_END
                                auid    pid     subj
        ANOM_PROMISCUOUS
                                auid    uid     gid     ses
        52msgs
                pid     uid     auid    ses     subj	*
        CONFIG_CHANGE
                                auid    ses     subj

This new record is:
        EVENT_LISTENER
                pid     uid     auid    tty     ses     subj    comm    exe

And using the search criteria following, I get no other matches:
        /pid.*uid.*auid.*tty.*ses.*subj.*comm.*exe
so this appears to be a new field order.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@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

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Paul Moore]

On Thu, Jan 23, 2020 at 11:14 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2020-01-23 09:32, Paul Moore wrote:
> > On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > On 2020-01-22 17:40, Paul Moore wrote:
> > > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > ...
> >
> > > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > > index 17b0d523afb3..478259f3fa53 100644
> > > > > --- a/kernel/audit.c
> > > > > +++ b/kernel/audit.c
> > > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > > >         audit_ctl_unlock();
> > > > >  }
> > > > >
> > > > > +/* 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;
> > > > > +
> > > > > +       if (!audit_enabled)
> > > > > +               return;
> > > > > +
> > > > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > > +       if (!ab)
> > > > > +               return;
> > > > > +
> > > > > +       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));
> > > >
> > > > Don't we already get all of that information as part of the syscall record?
> > >
> > > Yes.  However, the syscall record isn't always present.  One example is
> > > systemd, shown above.
> >
> > Assuming that the system supports syscall auditing, the absence of a
> > syscall record is a configuration choice made by the admin.  If the
> > system doesn't support syscall auditing the obvious "fix" is to do the
> > work to enable syscall auditing on that platform ... but now we're
> > starting to get off topic.
>
> Well, the system did spit out a syscall record with the example above,
> so it has support for syscall auditing.
>
> I'm testing on f30 with an upstream kernel, the standard 30-stig ruleset and
> with kernel command line audit=1.  What else is needed to support a syscall
> record on systemd before any audit rules have been put in place?  We may still
> have a bug here that affects early process auditing.  What am I missing?
>
> If we can get that sorted out, we don't need subject attributes in this record.

It looks like some debugging is in order.  There must be some sort of
action initiated by userspace which is causing the multicast
"op=connect", right?  Find out what that is and why it isn't
generating a syscall record (maybe it's not a syscall? I don't know
what systemd is doing here).

> > > The other is the disconnect record, shown above,
> > > which may be asynchronous, or an unmonitored syscall (It could only be
> > > setsockopt, close, shutdown.).
> >
> > An unmonitored syscall still falls under the category of a
> > configuration choice so I'm not too concerned about that, but the
> > async disconnect record is legitimate.  Can you provide more
> > information about when this occurs?  I'm guessing this is pretty much
> > just an abrupt/abnormal program exit?
>
> Again, what configuration choice are you talking about?
> "-a task,never"?  That isn't active on this system.
>
> The output was produced by the test case quoted in the patch description.
>
> I should not have had to put a rule in place to do syscall auditing on connect,
> bind, setsockopt, close, shutdown.
>
> The disconnect would have been due to a perl close() call.  I would not have
> expected that to be async, but I don't know the details of what the perl
> implementation does.

You mentioned two cases: unmonitored syscalls and async records (I
assumed these were just "disconnect").  Monitoring a syscall is a
configuration choice, regardless of what the defaults may be, and
since the folks likely to care about these multicast events are the
same sort of folks that care deeply about audit, asking them to do
some additional configuration tweaks seems like a reasonable thing to
get this new record with the proper information.  The async records
are potentially more interesting, but less clear, which is why I asked
for more info.

Regardless, all of this is pretty moot if we decide we don't care
about duplicate information.  Let's make a decision on duplicate
fields across multiple records before we worry too much about the rest
of what we are discussing.

> > > > I'm pretty sure these are the same arguments I made when Steve posted
> > > > a prior version of this patch.
> > >
> > > You did.  I would really like to have dropped them, but they aren't
> > > reliably available.
> >
> > Personally I'm not too worried if we have duplicate information spread
> > across records in a single event, as long as they are consistent.
> > However, I remember Steve complaining rather loudly about duplicated
> > fields across records in a single event some time back; perhaps that
> > is not a concern of his anymore (perhaps it was a narrow case at the
> > time), I don't know.
> >
> > Here is the deal, either duplicated information is something we are
> > okay with, or it is something to avoid; we need to pick one.  As
> > mentioned above, I don't really care that much either way (I have a
> > slight preference, but I don't feel strongly enough to fight for it),
> > so let's hear the arguments both for and against and decide - whatever
> > we pick I'll enforce so long as we are stuck with this string format.
>
> Steve, can you say why this order should be the standard?  From:
>         http://people.redhat.com/sgrubb/audit/record-fields.html
>
> I get:
>         SYSCALL/ANOM_LINK/FEATURE_CHANGE
>                 ppid    pid     auid    uid     gid     euid    suid    fsuid   egid    sgid    fsgid   tty     ses     comm    exe     subj

Oh man, let's please not have *another* debate about field ordering
before we answer the duplicate field question.  If history has shown
us anything, it is that debates around audit record field ordering
tend to kill progress.  Let's try to stay focused.

-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Richard Guy Briggs]

On 2020-01-23 11:57, Paul Moore wrote:
> On Thu, Jan 23, 2020 at 11:14 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2020-01-23 09:32, Paul Moore wrote:
> > > On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > On 2020-01-22 17:40, Paul Moore wrote:
> > > > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > >
> > > ...
> > >
> > > > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > > > index 17b0d523afb3..478259f3fa53 100644
> > > > > > --- a/kernel/audit.c
> > > > > > +++ b/kernel/audit.c
> > > > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > > > >         audit_ctl_unlock();
> > > > > >  }
> > > > > >
> > > > > > +/* 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;
> > > > > > +
> > > > > > +       if (!audit_enabled)
> > > > > > +               return;
> > > > > > +
> > > > > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > > > +       if (!ab)
> > > > > > +               return;
> > > > > > +
> > > > > > +       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));
> > > > >
> > > > > Don't we already get all of that information as part of the syscall record?
> > > >
> > > > Yes.  However, the syscall record isn't always present.  One example is
> > > > systemd, shown above.
> > >
> > > Assuming that the system supports syscall auditing, the absence of a
> > > syscall record is a configuration choice made by the admin.  If the
> > > system doesn't support syscall auditing the obvious "fix" is to do the
> > > work to enable syscall auditing on that platform ... but now we're
> > > starting to get off topic.
> >
> > Well, the system did spit out a syscall record with the example above,
> > so it has support for syscall auditing.
> >
> > I'm testing on f30 with an upstream kernel, the standard 30-stig ruleset and
> > with kernel command line audit=1.  What else is needed to support a syscall
> > record on systemd before any audit rules have been put in place?  We may still
> > have a bug here that affects early process auditing.  What am I missing?
> >
> > If we can get that sorted out, we don't need subject attributes in this record.
> 
> It looks like some debugging is in order.  There must be some sort of
> action initiated by userspace which is causing the multicast
> "op=connect", right?  Find out what that is and why it isn't
> generating a syscall record (maybe it's not a syscall? I don't know
> what systemd is doing here).

One clue is that subj=kernel and auid, ttye and ses are unset, despite
the rest checking out:
	pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd

> > > > The other is the disconnect record, shown above,
> > > > which may be asynchronous, or an unmonitored syscall (It could only be
> > > > setsockopt, close, shutdown.).
> > >
> > > An unmonitored syscall still falls under the category of a
> > > configuration choice so I'm not too concerned about that, but the
> > > async disconnect record is legitimate.  Can you provide more
> > > information about when this occurs?  I'm guessing this is pretty much
> > > just an abrupt/abnormal program exit?
> >
> > Again, what configuration choice are you talking about?
> > "-a task,never"?  That isn't active on this system.
> >
> > The output was produced by the test case quoted in the patch description.
> >
> > I should not have had to put a rule in place to do syscall auditing on connect,
> > bind, setsockopt, close, shutdown.
> >
> > The disconnect would have been due to a perl close() call.  I would not have
> > expected that to be async, but I don't know the details of what the perl
> > implementation does.
> 
> You mentioned two cases: unmonitored syscalls and async records (I
> assumed these were just "disconnect").  Monitoring a syscall is a
> configuration choice, regardless of what the defaults may be, and
> since the folks likely to care about these multicast events are the
> same sort of folks that care deeply about audit, asking them to do
> some additional configuration tweaks seems like a reasonable thing to
> get this new record with the proper information.  The async records
> are potentially more interesting, but less clear, which is why I asked
> for more info.

I don't know what other config choices are going to make a difference
for pid=1 which is the primary user of this multicast socket other than
audit=1 unless we add another kernel boot parameter.

I'm puzzled why the production of this record doesn't automatically
trigger a syscall record on exit since that act of producing this record
will populate the audit context.

> Regardless, all of this is pretty moot if we decide we don't care
> about duplicate information.  Let's make a decision on duplicate
> fields across multiple records before we worry too much about the rest
> of what we are discussing.

I don't have a problem with duplicate information, but I'm not the
consumer.  I can fix situations where that duplicate information turns
out to be inconsistent though.

> > > > > I'm pretty sure these are the same arguments I made when Steve posted
> > > > > a prior version of this patch.
> > > >
> > > > You did.  I would really like to have dropped them, but they aren't
> > > > reliably available.
> > >
> > > Personally I'm not too worried if we have duplicate information spread
> > > across records in a single event, as long as they are consistent.
> > > However, I remember Steve complaining rather loudly about duplicated
> > > fields across records in a single event some time back; perhaps that
> > > is not a concern of his anymore (perhaps it was a narrow case at the
> > > time), I don't know.
> > >
> > > Here is the deal, either duplicated information is something we are
> > > okay with, or it is something to avoid; we need to pick one.  As
> > > mentioned above, I don't really care that much either way (I have a
> > > slight preference, but I don't feel strongly enough to fight for it),
> > > so let's hear the arguments both for and against and decide - whatever
> > > we pick I'll enforce so long as we are stuck with this string format.
> >
> > Steve, can you say why this order should be the standard?  From:
> >         http://people.redhat.com/sgrubb/audit/record-fields.html
> >
> > I get:
> >         SYSCALL/ANOM_LINK/FEATURE_CHANGE
> >                 ppid    pid     auid    uid     gid     euid    suid    fsuid   egid    sgid    fsgid   tty     ses     comm    exe     subj
> 
> Oh man, let's please not have *another* debate about field ordering
> before we answer the duplicate field question.  If history has shown
> us anything, it is that debates around audit record field ordering
> tend to kill progress.  Let's try to stay focused.

I agree that is a different thread.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@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

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Paul Moore]

On Thu, Jan 23, 2020 at 1:52 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2020-01-23 11:57, Paul Moore wrote:
> > On Thu, Jan 23, 2020 at 11:14 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > On 2020-01-23 09:32, Paul Moore wrote:
> > > > On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > On 2020-01-22 17:40, Paul Moore wrote:
> > > > > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > >
> > > > ...
> > > >
> > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > > > > index 17b0d523afb3..478259f3fa53 100644
> > > > > > > --- a/kernel/audit.c
> > > > > > > +++ b/kernel/audit.c
> > > > > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > > > > >         audit_ctl_unlock();
> > > > > > >  }
> > > > > > >
> > > > > > > +/* 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;
> > > > > > > +
> > > > > > > +       if (!audit_enabled)
> > > > > > > +               return;
> > > > > > > +
> > > > > > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > > > > +       if (!ab)
> > > > > > > +               return;
> > > > > > > +
> > > > > > > +       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));
> > > > > >
> > > > > > Don't we already get all of that information as part of the syscall record?
> > > > >
> > > > > Yes.  However, the syscall record isn't always present.  One example is
> > > > > systemd, shown above.
> > > >
> > > > Assuming that the system supports syscall auditing, the absence of a
> > > > syscall record is a configuration choice made by the admin.  If the
> > > > system doesn't support syscall auditing the obvious "fix" is to do the
> > > > work to enable syscall auditing on that platform ... but now we're
> > > > starting to get off topic.
> > >
> > > Well, the system did spit out a syscall record with the example above,
> > > so it has support for syscall auditing.
> > >
> > > I'm testing on f30 with an upstream kernel, the standard 30-stig ruleset and
> > > with kernel command line audit=1.  What else is needed to support a syscall
> > > record on systemd before any audit rules have been put in place?  We may still
> > > have a bug here that affects early process auditing.  What am I missing?
> > >
> > > If we can get that sorted out, we don't need subject attributes in this record.
> >
> > It looks like some debugging is in order.  There must be some sort of
> > action initiated by userspace which is causing the multicast
> > "op=connect", right?  Find out what that is and why it isn't
> > generating a syscall record (maybe it's not a syscall? I don't know
> > what systemd is doing here).
>
> One clue is that subj=kernel and auid, ttye and ses are unset, despite
> the rest checking out:
>         pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd

Does Fedora use systemd in its initramfs (I'm guessing the answer is
"yes")?  If so, I wonder if that is the source of this record.

-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Richard Guy Briggs]

On 2020-01-23 14:07, Paul Moore wrote:
> On Thu, Jan 23, 2020 at 1:52 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2020-01-23 11:57, Paul Moore wrote:
> > > On Thu, Jan 23, 2020 at 11:14 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > On 2020-01-23 09:32, Paul Moore wrote:
> > > > > On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > > On 2020-01-22 17:40, Paul Moore wrote:
> > > > > > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > >
> > > > > ...
> > > > >
> > > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > > > > > index 17b0d523afb3..478259f3fa53 100644
> > > > > > > > --- a/kernel/audit.c
> > > > > > > > +++ b/kernel/audit.c
> > > > > > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > > > > > >         audit_ctl_unlock();
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +/* 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;
> > > > > > > > +
> > > > > > > > +       if (!audit_enabled)
> > > > > > > > +               return;
> > > > > > > > +
> > > > > > > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > > > > > +       if (!ab)
> > > > > > > > +               return;
> > > > > > > > +
> > > > > > > > +       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));
> > > > > > >
> > > > > > > Don't we already get all of that information as part of the syscall record?
> > > > > >
> > > > > > Yes.  However, the syscall record isn't always present.  One example is
> > > > > > systemd, shown above.
> > > > >
> > > > > Assuming that the system supports syscall auditing, the absence of a
> > > > > syscall record is a configuration choice made by the admin.  If the
> > > > > system doesn't support syscall auditing the obvious "fix" is to do the
> > > > > work to enable syscall auditing on that platform ... but now we're
> > > > > starting to get off topic.
> > > >
> > > > Well, the system did spit out a syscall record with the example above,
> > > > so it has support for syscall auditing.
> > > >
> > > > I'm testing on f30 with an upstream kernel, the standard 30-stig ruleset and
> > > > with kernel command line audit=1.  What else is needed to support a syscall
> > > > record on systemd before any audit rules have been put in place?  We may still
> > > > have a bug here that affects early process auditing.  What am I missing?
> > > >
> > > > If we can get that sorted out, we don't need subject attributes in this record.
> > >
> > > It looks like some debugging is in order.  There must be some sort of
> > > action initiated by userspace which is causing the multicast
> > > "op=connect", right?  Find out what that is and why it isn't
> > > generating a syscall record (maybe it's not a syscall? I don't know
> > > what systemd is doing here).
> >
> > One clue is that subj=kernel and auid, ttye and ses are unset, despite
> > the rest checking out:
> >         pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd
> 
> Does Fedora use systemd in its initramfs (I'm guessing the answer is
> "yes")?  If so, I wonder if that is the source of this record.

Asking around, I got: "yes, dracut uses systemd these days"

So, yes, that is the source of this record.

So if there is no syscall associated with that record, it appears we
need those subject attributes.

Next question, why do the other records generated from the test not
automatically trigger a syscall record when audit=1 on the kernel
command line?

> paul moore

- RGB

--
Richard Guy Briggs <rgb@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

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Paul Moore]

On Thu, Jan 23, 2020 at 3:15 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2020-01-23 14:07, Paul Moore wrote:
> > On Thu, Jan 23, 2020 at 1:52 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > On 2020-01-23 11:57, Paul Moore wrote:
> > > > On Thu, Jan 23, 2020 at 11:14 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > On 2020-01-23 09:32, Paul Moore wrote:
> > > > > > On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > > > On 2020-01-22 17:40, Paul Moore wrote:
> > > > > > > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > >
> > > > > > ...
> > > > > >
> > > > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > > > > > > index 17b0d523afb3..478259f3fa53 100644
> > > > > > > > > --- a/kernel/audit.c
> > > > > > > > > +++ b/kernel/audit.c
> > > > > > > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > > > > > > >         audit_ctl_unlock();
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > +/* 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;
> > > > > > > > > +
> > > > > > > > > +       if (!audit_enabled)
> > > > > > > > > +               return;
> > > > > > > > > +
> > > > > > > > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > > > > > > +       if (!ab)
> > > > > > > > > +               return;
> > > > > > > > > +
> > > > > > > > > +       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));
> > > > > > > >
> > > > > > > > Don't we already get all of that information as part of the syscall record?
> > > > > > >
> > > > > > > Yes.  However, the syscall record isn't always present.  One example is
> > > > > > > systemd, shown above.
> > > > > >
> > > > > > Assuming that the system supports syscall auditing, the absence of a
> > > > > > syscall record is a configuration choice made by the admin.  If the
> > > > > > system doesn't support syscall auditing the obvious "fix" is to do the
> > > > > > work to enable syscall auditing on that platform ... but now we're
> > > > > > starting to get off topic.
> > > > >
> > > > > Well, the system did spit out a syscall record with the example above,
> > > > > so it has support for syscall auditing.
> > > > >
> > > > > I'm testing on f30 with an upstream kernel, the standard 30-stig ruleset and
> > > > > with kernel command line audit=1.  What else is needed to support a syscall
> > > > > record on systemd before any audit rules have been put in place?  We may still
> > > > > have a bug here that affects early process auditing.  What am I missing?
> > > > >
> > > > > If we can get that sorted out, we don't need subject attributes in this record.
> > > >
> > > > It looks like some debugging is in order.  There must be some sort of
> > > > action initiated by userspace which is causing the multicast
> > > > "op=connect", right?  Find out what that is and why it isn't
> > > > generating a syscall record (maybe it's not a syscall? I don't know
> > > > what systemd is doing here).
> > >
> > > One clue is that subj=kernel and auid, ttye and ses are unset, despite
> > > the rest checking out:
> > >         pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd
> >
> > Does Fedora use systemd in its initramfs (I'm guessing the answer is
> > "yes")?  If so, I wonder if that is the source of this record.
>
> Asking around, I got: "yes, dracut uses systemd these days"
>
> So, yes, that is the source of this record.
>
> So if there is no syscall associated with that record, it appears we
> need those subject attributes.

Well, I'm fairly certain that the record in question was the result of
a syscall made by systemd, the question is why was it not recorded?
That is something that needs to be answered.

-- 
paul moore
www.paul-moore.com

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Richard Guy Briggs]

On 2020-01-23 16:45, Paul Moore wrote:
> On Thu, Jan 23, 2020 at 3:15 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2020-01-23 14:07, Paul Moore wrote:
> > > On Thu, Jan 23, 2020 at 1:52 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > On 2020-01-23 11:57, Paul Moore wrote:
> > > > > On Thu, Jan 23, 2020 at 11:14 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > > On 2020-01-23 09:32, Paul Moore wrote:
> > > > > > > On Wed, Jan 22, 2020 at 6:07 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > > > > On 2020-01-22 17:40, Paul Moore wrote:
> > > > > > > > > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > > >
> > > > > > > ...
> > > > > > >
> > > > > > > > > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > > > > > > > > index 17b0d523afb3..478259f3fa53 100644
> > > > > > > > > > --- a/kernel/audit.c
> > > > > > > > > > +++ b/kernel/audit.c
> > > > > > > > > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > > > > > > > >         audit_ctl_unlock();
> > > > > > > > > >  }
> > > > > > > > > >
> > > > > > > > > > +/* 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;
> > > > > > > > > > +
> > > > > > > > > > +       if (!audit_enabled)
> > > > > > > > > > +               return;
> > > > > > > > > > +
> > > > > > > > > > +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > > > > > > > +       if (!ab)
> > > > > > > > > > +               return;
> > > > > > > > > > +
> > > > > > > > > > +       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));
> > > > > > > > >
> > > > > > > > > Don't we already get all of that information as part of the syscall record?
> > > > > > > >
> > > > > > > > Yes.  However, the syscall record isn't always present.  One example is
> > > > > > > > systemd, shown above.
> > > > > > >
> > > > > > > Assuming that the system supports syscall auditing, the absence of a
> > > > > > > syscall record is a configuration choice made by the admin.  If the
> > > > > > > system doesn't support syscall auditing the obvious "fix" is to do the
> > > > > > > work to enable syscall auditing on that platform ... but now we're
> > > > > > > starting to get off topic.
> > > > > >
> > > > > > Well, the system did spit out a syscall record with the example above,
> > > > > > so it has support for syscall auditing.
> > > > > >
> > > > > > I'm testing on f30 with an upstream kernel, the standard 30-stig ruleset and
> > > > > > with kernel command line audit=1.  What else is needed to support a syscall
> > > > > > record on systemd before any audit rules have been put in place?  We may still
> > > > > > have a bug here that affects early process auditing.  What am I missing?
> > > > > >
> > > > > > If we can get that sorted out, we don't need subject attributes in this record.
> > > > >
> > > > > It looks like some debugging is in order.  There must be some sort of
> > > > > action initiated by userspace which is causing the multicast
> > > > > "op=connect", right?  Find out what that is and why it isn't
> > > > > generating a syscall record (maybe it's not a syscall? I don't know
> > > > > what systemd is doing here).
> > > >
> > > > One clue is that subj=kernel and auid, ttye and ses are unset, despite
> > > > the rest checking out:
> > > >         pid=1 uid=root auid=unset tty=(none) ses=unset subj=kernel comm=systemd exe=/usr/lib/systemd/systemd
> > >
> > > Does Fedora use systemd in its initramfs (I'm guessing the answer is
> > > "yes")?  If so, I wonder if that is the source of this record.
> >
> > Asking around, I got: "yes, dracut uses systemd these days"
> >
> > So, yes, that is the source of this record.
> >
> > So if there is no syscall associated with that record, it appears we
> > need those subject attributes.
> 
> Well, I'm fairly certain that the record in question was the result of
> a syscall made by systemd, the question is why was it not recorded?
> That is something that needs to be answered.

The answer is in the ghak120 patch just posted.  See:
	https://github.com/linux-audit/audit-kernel/issues/120

As for the drop, well it appears that more than one termination records
are asynchronous (due to rcu locking) and will not have a directly
attributable syscall.  This applies to this issue and to ghak25.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@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

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Steve Grubb]

On Thursday, January 23, 2020 11:13:49 AM EST Richard Guy Briggs wrote:
> Steve, can you say why this order should be the standard?  From:
>         http://people.redhat.com/sgrubb/audit/record-fields.html

The majority of events go down the path of:
pid,uid,auid,ses,subj,op,comm,exe,res

Which lands on the parse_user() function.

If for some reason we really wanted to stay on a "kernel" parser, then I'd 
recommend:
auid,uid,ses,subj,pid,comm,exe,op,res

which lands on the parse_kernel_anom() function.

Either of those have complete information and requires no syscall record.

-Steve


> I get:
>         SYSCALL/ANOM_LINK/FEATURE_CHANGE
>                 ppid    pid     auid    uid     gid     euid    suid   
> fsuid   egid    sgid    fsgid   tty     ses     comm    exe     subj
> ANOM_ABEND/SECCOMP
>                                 auid    uid     gid     ses     subj    pid
>     comm    exe LOGIN
>                 pid     uid     subj    old-auid        auid    tty    
> old-ses ses SYSTEM_BOOT/SYSTEM_SHUTDOWN
>                 pid     uid     auid    ses     subj    comm    exe
>         USER_LOGIN
>                 pid     uid     auid    ses     subj    uid     exe
>         DAEMON_START
>                                 auid    pid     uid     ses     subj
>         DAEMON_CONFIG/DAEMON_END
>                                 auid    pid     subj
>         ANOM_PROMISCUOUS
>                                 auid    uid     gid     ses
>         52msgs
>                 pid     uid     auid    ses     subj    *
>         CONFIG_CHANGE
>                                 auid    ses     subj
> 
> This new record is:
>         EVENT_LISTENER
>                 pid     uid     auid    tty     ses     subj    comm    exe
> 
> And using the search criteria following, I get no other matches:
>         /pid.*uid.*auid.*tty.*ses.*subj.*comm.*exe
> so this appears to be a new field order.

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Steve Grubb]

On Wednesday, January 22, 2020 5:40:10 PM EST Paul Moore wrote:
> On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > Log information about programs connecting to and disconnecting from 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.  Here is the systemd startup event:
> > 
> > type=UNKNOWN[1335] msg=audit(2020-01-17 10:30:33.731:6) : pid=1 uid=root
> > auid=unset tty=(none) ses=unset subj=kernel comm=systemd
> > exe=/usr/lib/systemd/systemd nl-mcgrp=1 op=connect res=yes
> > 
> > And the events from the test suite:
> > 
> > type=PROCTITLE msg=audit(2020-01-17 10:36:24.050:294) :
> > proctitle=/usr/bin/perl -w amcast_joinpart/test type=SOCKADDR
> > msg=audit(2020-01-17 10:36:24.050:294) : saddr={ saddr_fam=netlink
> > nlnk-fam=16 nlnk-pid=0 } type=SYSCALL msg=audit(2020-01-17
> > 10:36:24.050:294) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x7
> > a1=0x55d65cb79090 a2=0xc a3=0x0 items=0 ppid=671 pid=674 auid=root
> > uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
> > fsgid=root tty=ttyS0 ses=3 comm=perl exe=/usr/bin/perl
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> > type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.050:294) : pid=674
> > uid=root auid=root tty=ttyS0 ses=3
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl
> > exe=/usr/bin/perl nl-mcgrp=1 op=connect res=yes
> > 
> > type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.051:295) : pid=674
> > uid=root auid=root tty=ttyS0 ses=3
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl
> > exe=/usr/bin/perl nl-mcgrp=1 op=disconnect res=yes> 
> > Please see the upstream issue tracker:
> >   https://github.com/linux-audit/audit-kernel/issues/28
> >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Multicast-So
> >   cket-Join-Part
> >   https://github.com/rgbriggs/audit-testsuite/compare/ghak28-mcast-part-> >   join> 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > 
> > ---
> > Note: msg type 1334 was skipped due to BPF accepted in another tree.
> > Note: v5 due to previous 2014-10-07, 2015-07-23, 2016-11-30, 2017-10-13
> > Note: subj attrs included due to missing syscall record for systemd
> > (audit=1) Note: tried refactor of subj attrs, but this is yet another
> > new order. ---
> > 
> >  include/uapi/linux/audit.h |  1 +
> >  kernel/audit.c             | 48
> >  ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45
> >  insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 3ad935527177..67fb24472dc2 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -116,6 +116,7 @@
> > 
> >  #define AUDIT_FANOTIFY         1331    /* Fanotify access decision */
> >  #define AUDIT_TIME_INJOFFSET   1332    /* Timekeeping offset injected */
> >  #define AUDIT_TIME_ADJNTPVAL   1333    /* NTP value adjustment */
> > 
> > +#define AUDIT_EVENT_LISTENER   1335    /* Task joined multicast read
> > socket */> 
> >  #define AUDIT_AVC              1400    /* SE Linux avc denial or grant
> >  */
> >  #define AUDIT_SELINUX_ERR      1401    /* Internal SE Linux Errors */
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 17b0d523afb3..478259f3fa53 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > 
> >         audit_ctl_unlock();
> >  
> >  }
> > 
> > +/* 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;
> > +
> > +       if (!audit_enabled)
> > +               return;
> > +
> > +       ab = audit_log_start(audit_context(), GFP_KERNEL,
> > AUDIT_EVENT_LISTENER); +       if (!ab)
> > +               return;
> > +
> > +       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));
> 
> Don't we already get all of that information as part of the syscall record?

We don't want or need a syscall record. It doesn't add anything to the 
necessary information. Also, when we have syscall records, people expect that 
they obey the syscall auditing. Especially wrt "never" audit rules.


> > +       audit_put_tty(tty);
> > +       audit_log_task_context(ab); /* subj= */
> 
> Also part of the syscall record.
> 
> > +       audit_log_format(ab, " comm=");
> > +       audit_log_untrustedstring(ab, get_task_comm(comm, current));
> 
> Again.
> 
> > +       audit_log_d_path_exe(ab, current->mm); /* exe= */
> 
> Again.
> 
> > +       audit_log_format(ab, " nl-mcgrp=%d op=%s res=%d", group, op,
> > !err);
> This part is new ;)
> 
> > +       audit_log_end(ab);
> > +}
> 
> I'm pretty sure these are the same arguments I made when Steve posted
> a prior version of this patch.

No. You didn't mind it then. What you objected to was that I wrote a helper 
function that could be used by future audit events to start a format 
standardization process.

The event looks good to me. Ack for the format being acceptable to existing 
tools.

-Steve

Re: [PATCH ghak28 V4] audit: log audit netlink multicast bind and unbind events

[Richard Guy Briggs]

On 2020-01-22 18:12, Steve Grubb wrote:
> On Wednesday, January 22, 2020 5:40:10 PM EST Paul Moore wrote:
> > On Fri, Jan 17, 2020 at 3:21 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > Log information about programs connecting to and disconnecting from 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.  Here is the systemd startup event:
> > > 
> > > type=UNKNOWN[1335] msg=audit(2020-01-17 10:30:33.731:6) : pid=1 uid=root
> > > auid=unset tty=(none) ses=unset subj=kernel comm=systemd
> > > exe=/usr/lib/systemd/systemd nl-mcgrp=1 op=connect res=yes
> > > 
> > > And the events from the test suite:
> > > 
> > > type=PROCTITLE msg=audit(2020-01-17 10:36:24.050:294) :
> > > proctitle=/usr/bin/perl -w amcast_joinpart/test type=SOCKADDR
> > > msg=audit(2020-01-17 10:36:24.050:294) : saddr={ saddr_fam=netlink
> > > nlnk-fam=16 nlnk-pid=0 } type=SYSCALL msg=audit(2020-01-17
> > > 10:36:24.050:294) : arch=x86_64 syscall=bind success=yes exit=0 a0=0x7
> > > a1=0x55d65cb79090 a2=0xc a3=0x0 items=0 ppid=671 pid=674 auid=root
> > > uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root
> > > fsgid=root tty=ttyS0 ses=3 comm=perl exe=/usr/bin/perl
> > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null)
> > > type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.050:294) : pid=674
> > > uid=root auid=root tty=ttyS0 ses=3
> > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl
> > > exe=/usr/bin/perl nl-mcgrp=1 op=connect res=yes
> > > 
> > > type=UNKNOWN[1335] msg=audit(2020-01-17 10:36:24.051:295) : pid=674
> > > uid=root auid=root tty=ttyS0 ses=3
> > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=perl
> > > exe=/usr/bin/perl nl-mcgrp=1 op=disconnect res=yes> 
> > > Please see the upstream issue tracker:
> > >   https://github.com/linux-audit/audit-kernel/issues/28
> > >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Multicast-So
> > >   cket-Join-Part
> > >   https://github.com/rgbriggs/audit-testsuite/compare/ghak28-mcast-part-> >   join> 
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > 
> > > ---
> > > Note: msg type 1334 was skipped due to BPF accepted in another tree.
> > > Note: v5 due to previous 2014-10-07, 2015-07-23, 2016-11-30, 2017-10-13
> > > Note: subj attrs included due to missing syscall record for systemd
> > > (audit=1) Note: tried refactor of subj attrs, but this is yet another
> > > new order. ---
> > > 
> > >  include/uapi/linux/audit.h |  1 +
> > >  kernel/audit.c             | 48
> > >  ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 45
> > >  insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > index 3ad935527177..67fb24472dc2 100644
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -116,6 +116,7 @@
> > > 
> > >  #define AUDIT_FANOTIFY         1331    /* Fanotify access decision */
> > >  #define AUDIT_TIME_INJOFFSET   1332    /* Timekeeping offset injected */
> > >  #define AUDIT_TIME_ADJNTPVAL   1333    /* NTP value adjustment */
> > > 
> > > +#define AUDIT_EVENT_LISTENER   1335    /* Task joined multicast read
> > > socket */> 
> > >  #define AUDIT_AVC              1400    /* SE Linux avc denial or grant
> > >  */
> > >  #define AUDIT_SELINUX_ERR      1401    /* Internal SE Linux Errors */
> > > 
> > > diff --git a/kernel/audit.c b/kernel/audit.c
> > > index 17b0d523afb3..478259f3fa53 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -1520,20 +1520,60 @@ static void audit_receive(struct sk_buff  *skb)
> > > 
> > >         audit_ctl_unlock();
> > >  
> > >  }
> > > 
> > > +/* 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;
> > > +
> > > +       if (!audit_enabled)
> > > +               return;
> > > +
> > > +       ab = audit_log_start(audit_context(), GFP_KERNEL,
> > > AUDIT_EVENT_LISTENER); +       if (!ab)
> > > +               return;
> > > +
> > > +       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));
> > 
> > Don't we already get all of that information as part of the syscall record?
> 
> We don't want or need a syscall record. It doesn't add anything to the 
> necessary information. Also, when we have syscall records, people expect that 
> they obey the syscall auditing. Especially wrt "never" audit rules.

Did both of you see the 4 "Note:" lines between the description and the
patch?  I'm caught in the middle here.

> > > +       audit_put_tty(tty);
> > > +       audit_log_task_context(ab); /* subj= */
> > 
> > Also part of the syscall record.
> > 
> > > +       audit_log_format(ab, " comm=");
> > > +       audit_log_untrustedstring(ab, get_task_comm(comm, current));
> > 
> > Again.
> > 
> > > +       audit_log_d_path_exe(ab, current->mm); /* exe= */
> > 
> > Again.
> > 
> > > +       audit_log_format(ab, " nl-mcgrp=%d op=%s res=%d", group, op,
> > > !err);
> > This part is new ;)
> > 
> > > +       audit_log_end(ab);
> > > +}
> > 
> > I'm pretty sure these are the same arguments I made when Steve posted
> > a prior version of this patch.
> 
> No. You didn't mind it then. What you objected to was that I wrote a helper 
> function that could be used by future audit events to start a format 
> standardization process.

Again, see the 4 notes.  I was not able to refactor any of the subject
attributes since this is yet another audit subject attributes order
(YAASAO) that hasn't been seen yet.  Why are we creatting YAASAO?

> The event looks good to me. Ack for the format being acceptable to existing 
> tools.
> 
> -Steve

- RGB

--
Richard Guy Briggs <rgb@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