2014-10-07 18:23:51

by Richard Guy Briggs

[permalink] [raw]
Subject: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

Log the event when a client attempts to connect to the netlink audit multicast
socket, requiring CAP_AUDIT_READ capability, binding to the AUDIT_NLGRP_READLOG
group. Log the disconnect too.

Sample output:
time->Tue Oct 7 14:15:19 2014
type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1 pid=3552 comm="audit-multicast" exe="/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0 op=connect res=1

Signed-off-by: Richard Guy Briggs <[email protected]>
---
For some reason unbind isn't being called on disconnect. I suspect missing
plumbing in netlink. Investigation needed...

include/uapi/linux/audit.h | 1 +
kernel/audit.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..7fa6e8f 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -110,6 +110,7 @@
#define AUDIT_SECCOMP 1326 /* Secure Computing event */
#define AUDIT_PROCTITLE 1327 /* Proctitle emit event */
#define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes */
+#define AUDIT_EVENT_LISTENER 1348 /* 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 53bb39b..74c81a7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff *skb)
mutex_unlock(&audit_cmd_mutex);
}

+static void audit_log_bind(int group, char *op, int err)
+{
+ struct audit_buffer *ab;
+ char comm[sizeof(current->comm)];
+ struct mm_struct *mm = current->mm;
+
+ ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
+ if (!ab)
+ return;
+
+ audit_log_format(ab, "auid=%d",
+ from_kuid(&init_user_ns, audit_get_loginuid(current)));
+ audit_log_format(ab, " uid=%d",
+ from_kuid(&init_user_ns, current_uid()));
+ audit_log_format(ab, " gid=%d",
+ from_kgid(&init_user_ns, current_gid()));
+ audit_log_format(ab, " ses=%d", audit_get_sessionid(current));
+ audit_log_format(ab, " pid=%d", task_pid_nr(current));
+ audit_log_format(ab, " comm=");
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ if (mm) {
+ down_read(&mm->mmap_sem);
+ if (mm->exe_file)
+ audit_log_d_path(ab, " exe=", &mm->exe_file->f_path);
+ up_read(&mm->mmap_sem);
+ } else
+ audit_log_format(ab, " exe=(null)");
+ audit_log_task_context(ab); /* subj= */
+ audit_log_format(ab, " group=%d", group);
+ audit_log_format(ab, " op=%s", op);
+ audit_log_format(ab, " res=%d", !err);
+ audit_log_end(ab);
+}
+
/* Run custom bind function on netlink socket group connect or bind requests. */
static int audit_bind(int group)
{
+ int err = 0;
+
if (!capable(CAP_AUDIT_READ))
- return -EPERM;
+ err = -EPERM;
+ audit_log_bind(group, "connect", err);
+ return err;
+}

- return 0;
+static void audit_unbind(int group)
+{
+ audit_log_bind(group, "disconnect", 0);
}

static int __net_init audit_net_init(struct net *net)
@@ -1124,6 +1165,7 @@ static int __net_init audit_net_init(struct net *net)
.bind = audit_bind,
.flags = NL_CFG_F_NONROOT_RECV,
.groups = AUDIT_NLGRP_MAX,
+ .unbind = audit_unbind,
};

struct audit_net *aunet = net_generic(net, audit_net_id);
--
1.7.1


2014-10-07 19:03:20

by Eric Paris

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
> Log the event when a client attempts to connect to the netlink audit multicast
> socket, requiring CAP_AUDIT_READ capability, binding to the AUDIT_NLGRP_READLOG
> group. Log the disconnect too.
>
> Sample output:
> time->Tue Oct 7 14:15:19 2014
> type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1 pid=3552 comm="audit-multicast" exe="/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0 op=connect res=1
>
> Signed-off-by: Richard Guy Briggs <[email protected]>
> ---
> For some reason unbind isn't being called on disconnect. I suspect missing
> plumbing in netlink. Investigation needed...
>
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 4d100c8..7fa6e8f 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
> #define AUDIT_SECCOMP 1326 /* Secure Computing event */
> #define AUDIT_PROCTITLE 1327 /* Proctitle emit event */
> #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes */
> +#define AUDIT_EVENT_LISTENER 1348 /* 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 53bb39b..74c81a7 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff *skb)
> mutex_unlock(&audit_cmd_mutex);
> }
>
> +static void audit_log_bind(int group, char *op, int err)
> +{
> + struct audit_buffer *ab;
> + char comm[sizeof(current->comm)];
> + struct mm_struct *mm = current->mm;
> +
> + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
> + if (!ab)
> + return;
> +
> + audit_log_format(ab, "auid=%d",
> + from_kuid(&init_user_ns, audit_get_loginuid(current)));
> + audit_log_format(ab, " uid=%d",
> + from_kuid(&init_user_ns, current_uid()));
> + audit_log_format(ab, " gid=%d",
> + from_kgid(&init_user_ns, current_gid()));
> + audit_log_format(ab, " ses=%d", audit_get_sessionid(current));
> + audit_log_format(ab, " pid=%d", task_pid_nr(current));
> + audit_log_format(ab, " comm=");
> + audit_log_untrustedstring(ab, get_task_comm(comm, current));
> + if (mm) {
> + down_read(&mm->mmap_sem);
> + if (mm->exe_file)
> + audit_log_d_path(ab, " exe=", &mm->exe_file->f_path);
> + up_read(&mm->mmap_sem);
> + } else
> + audit_log_format(ab, " exe=(null)");
> + audit_log_task_context(ab); /* subj= */

super crazy yuck. audit_log_task_info() ??

> + audit_log_format(ab, " group=%d", group);

group seems like too easily confused a name.

> + audit_log_format(ab, " op=%s", op);
> + audit_log_format(ab, " res=%d", !err);
> + audit_log_end(ab);
> +}
> +
> /* Run custom bind function on netlink socket group connect or bind requests. */
> static int audit_bind(int group)
> {
> + int err = 0;
> +
> if (!capable(CAP_AUDIT_READ))
> - return -EPERM;
> + err = -EPERM;
> + audit_log_bind(group, "connect", err);
> + return err;
> +}
>
> - return 0;
> +static void audit_unbind(int group)
> +{
> + audit_log_bind(group, "disconnect", 0);
> }
>
> static int __net_init audit_net_init(struct net *net)
> @@ -1124,6 +1165,7 @@ static int __net_init audit_net_init(struct net *net)
> .bind = audit_bind,
> .flags = NL_CFG_F_NONROOT_RECV,
> .groups = AUDIT_NLGRP_MAX,
> + .unbind = audit_unbind,
> };
>
> struct audit_net *aunet = net_generic(net, audit_net_id);

2014-10-07 19:40:02

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On 14/10/07, Eric Paris wrote:
> On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
> > Log the event when a client attempts to connect to the netlink audit multicast
> > socket, requiring CAP_AUDIT_READ capability, binding to the AUDIT_NLGRP_READLOG
> > group. Log the disconnect too.
> >
> > Sample output:
> > time->Tue Oct 7 14:15:19 2014
> > type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1 pid=3552 comm="audit-multicast" exe="/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0 op=connect res=1
> >
> > Signed-off-by: Richard Guy Briggs <[email protected]>
> > ---
> > For some reason unbind isn't being called on disconnect. I suspect missing
> > plumbing in netlink. Investigation needed...
> >
> > include/uapi/linux/audit.h | 1 +
> > kernel/audit.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 45 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 4d100c8..7fa6e8f 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -110,6 +110,7 @@
> > #define AUDIT_SECCOMP 1326 /* Secure Computing event */
> > #define AUDIT_PROCTITLE 1327 /* Proctitle emit event */
> > #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes */
> > +#define AUDIT_EVENT_LISTENER 1348 /* 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 53bb39b..74c81a7 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff *skb)
> > mutex_unlock(&audit_cmd_mutex);
> > }
> >
> > +static void audit_log_bind(int group, char *op, int err)
> > +{
> > + struct audit_buffer *ab;
> > + char comm[sizeof(current->comm)];
> > + struct mm_struct *mm = current->mm;
> > +
> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > + if (!ab)
> > + return;
> > +
> > + audit_log_format(ab, "auid=%d",
> > + from_kuid(&init_user_ns, audit_get_loginuid(current)));
> > + audit_log_format(ab, " uid=%d",
> > + from_kuid(&init_user_ns, current_uid()));
> > + audit_log_format(ab, " gid=%d",
> > + from_kgid(&init_user_ns, current_gid()));
> > + audit_log_format(ab, " ses=%d", audit_get_sessionid(current));
> > + audit_log_format(ab, " pid=%d", task_pid_nr(current));
> > + audit_log_format(ab, " comm=");
> > + audit_log_untrustedstring(ab, get_task_comm(comm, current));
> > + if (mm) {
> > + down_read(&mm->mmap_sem);
> > + if (mm->exe_file)
> > + audit_log_d_path(ab, " exe=", &mm->exe_file->f_path);
> > + up_read(&mm->mmap_sem);
> > + } else
> > + audit_log_format(ab, " exe=(null)");
> > + audit_log_task_context(ab); /* subj= */
>
> super crazy yuck. audit_log_task_info() ??

I agree. I already suggested that a while ago. I'd love to. sgrubb
thinks it dumps way too much info. We still haven't got a definitive
answer about what is enough and what is too much info for any given type
of record.

I also thought of moving audit_log_task() from auditsc.c to audit.c
and using that. For that matter, both audit_log_task() and
audit_log_task_info() could use audit_log_session_info(), but they are
in slightly different order of keywords which will upset sgrubb's
parser.

What to do?

Another paragraph I'd like to see added to
http://people.redhat.com/sgrubb/audit/audit-parse.txt
would be a "canonical order" of keywords. However, that discussion went
nowhere. Would it be reasonable to suggest only two possible orders
instead of the almost infinite iterations possible and declare a
standard order of keywords and gradually move to it?

> > + audit_log_format(ab, " group=%d", group);
>
> group seems like too easily confused a name.

"multicast_group" or "mc_group"?

> > + audit_log_format(ab, " op=%s", op);
> > + audit_log_format(ab, " res=%d", !err);
> > + audit_log_end(ab);
> > +}
> > +
> > /* Run custom bind function on netlink socket group connect or bind requests. */
> > static int audit_bind(int group)
> > {
> > + int err = 0;
> > +
> > if (!capable(CAP_AUDIT_READ))
> > - return -EPERM;
> > + err = -EPERM;
> > + audit_log_bind(group, "connect", err);
> > + return err;
> > +}
> >
> > - return 0;
> > +static void audit_unbind(int group)
> > +{
> > + audit_log_bind(group, "disconnect", 0);
> > }
> >
> > static int __net_init audit_net_init(struct net *net)
> > @@ -1124,6 +1165,7 @@ static int __net_init audit_net_init(struct net *net)
> > .bind = audit_bind,
> > .flags = NL_CFG_F_NONROOT_RECV,
> > .groups = AUDIT_NLGRP_MAX,
> > + .unbind = audit_unbind,
> > };
> >
> > struct audit_net *aunet = net_generic(net, audit_net_id);
>
>

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-10-07 22:06:57

by Paul Moore

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tuesday, October 07, 2014 03:39:51 PM Richard Guy Briggs wrote:
> I also thought of moving audit_log_task() from auditsc.c to audit.c
> and using that. For that matter, both audit_log_task() and
> audit_log_task_info() could use audit_log_session_info(), but they are
> in slightly different order of keywords which will upset sgrubb's
> parser.

A bit of an aside from the patch, but in my opinion the parser should be made
a bit more robust so that it can handle fields in any particular order. I
agree that having fields in a "canonical ordering" is helpful, both for tools
and people, but the tools shouldn't require it in my opinion.

Steve, why exactly can't the userspace parser handle fields in any order? How
difficult would it be to fix?

--
paul moore
security and virtualization @ redhat

2014-10-11 15:42:15

by Steve Grubb

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tue, 07 Oct 2014 18:06:51 -0400
Paul Moore <[email protected]> wrote:

> On Tuesday, October 07, 2014 03:39:51 PM Richard Guy Briggs wrote:
> > I also thought of moving audit_log_task() from auditsc.c to audit.c
> > and using that. For that matter, both audit_log_task() and
> > audit_log_task_info() could use audit_log_session_info(), but they
> > are in slightly different order of keywords which will upset
> > sgrubb's parser.
>
> A bit of an aside from the patch, but in my opinion the parser should
> be made a bit more robust so that it can handle fields in any
> particular order. I agree that having fields in a "canonical
> ordering" is helpful, both for tools and people, but the tools
> shouldn't require it in my opinion.
>
> Steve, why exactly can't the userspace parser handle fields in any
> order? How difficult would it be to fix?

The issue is that people that really use audit, really get vast
quanities of logs. The tools expect things in a specific order so that
it can pick things out of events as quickly as possible. IOW, it
knows when it can discard the line because its grabbed everything it
needs. A casual audit user would never see this. I'm really optimizing
for the people whose use ausearch and it takes 10 minutes to run.

-Steve

2014-10-11 20:00:38

by Paul Moore

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Saturday, October 11, 2014 11:42:06 AM Steve Grubb wrote:
> On Tue, 07 Oct 2014 18:06:51 -0400
>
> Paul Moore <[email protected]> wrote:
> > On Tuesday, October 07, 2014 03:39:51 PM Richard Guy Briggs wrote:
> > > I also thought of moving audit_log_task() from auditsc.c to audit.c
> > > and using that. For that matter, both audit_log_task() and
> > > audit_log_task_info() could use audit_log_session_info(), but they
> > > are in slightly different order of keywords which will upset
> > > sgrubb's parser.
> >
> > A bit of an aside from the patch, but in my opinion the parser should
> > be made a bit more robust so that it can handle fields in any
> > particular order. I agree that having fields in a "canonical
> > ordering" is helpful, both for tools and people, but the tools
> > shouldn't require it in my opinion.
> >
> > Steve, why exactly can't the userspace parser handle fields in any
> > order? How difficult would it be to fix?
>
> The issue is that people that really use audit, really get vast
> quanities of logs. The tools expect things in a specific order so that
> it can pick things out of events as quickly as possible. IOW, it
> knows when it can discard the line because its grabbed everything it
> needs. A casual audit user would never see this. I'm really optimizing
> for the people whose use ausearch and it takes 10 minutes to run.

I understand you are catering to the "power user" here, but I don't see that
as an excuse for not being able to parse well formed name/value audit record
string if the order isn't exactly what you expect. I believe this will only
become more and more of a problem as things move forward. I think this is
something we need to fix soon.

Steve, would you be willing to fix the audit userspace parser so it can handle
fields in an arbitrary order? If not, would you be willing to accept patches
for the userspace that would accomplish this?

--
paul moore
security and virtualization @ redhat

2014-10-21 16:41:16

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On 14/10/07, Richard Guy Briggs wrote:
> On 14/10/07, Eric Paris wrote:
> > On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
> > > Log the event when a client attempts to connect to the netlink audit multicast
> > > socket, requiring CAP_AUDIT_READ capability, binding to the AUDIT_NLGRP_READLOG
> > > group. Log the disconnect too.

> > super crazy yuck. audit_log_task_info() ??
>
> I agree. I already suggested that a while ago. I'd love to. sgrubb
> thinks it dumps way too much info. We still haven't got a definitive
> answer about what is enough and what is too much info for any given type
> of record.
>
> I also thought of moving audit_log_task() from auditsc.c to audit.c
> and using that. For that matter, both audit_log_task() and
> audit_log_task_info() could use audit_log_session_info(), but they are
> in slightly different order of keywords which will upset sgrubb's
> parser.
>
> What to do?
>
> Another paragraph I'd like to see added to
> http://people.redhat.com/sgrubb/audit/audit-parse.txt
> would be a "canonical order" of keywords. However, that discussion went
> nowhere. Would it be reasonable to suggest only two possible orders
> instead of the almost infinite iterations possible and declare a
> standard order of keywords and gradually move to it?

Steve,

Can we agree to *two* orders (instead of the full set of iterations) for
these keywords so that we can start to sort things in a canonical order?
This random order per type of audit log message is chaos.

> - RGB

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-10-21 19:56:17

by Steve Grubb

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tuesday, October 07, 2014 03:03:14 PM Eric Paris wrote:
> On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
> > Log the event when a client attempts to connect to the netlink audit
> > multicast socket, requiring CAP_AUDIT_READ capability, binding to the
> > AUDIT_NLGRP_READLOG group. Log the disconnect too.
> >
> >
> >
> > Sample output:
> > time->Tue Oct 7 14:15:19 2014
> > type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1
> > pid=3552 comm="audit-multicast"
> > exe="/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen"
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0
> > op=connect res=1>
> >
> >
> > Signed-off-by: Richard Guy Briggs <[email protected]>
> > ---
> > For some reason unbind isn't being called on disconnect. I suspect
> > missing
> > plumbing in netlink. Investigation needed...
> >
> >
> > include/uapi/linux/audit.h | 1 +
> > kernel/audit.c | 46
> >++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 45
> >insertions(+), 2 deletions(-)
> >
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 4d100c8..7fa6e8f 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -110,6 +110,7 @@
> >
> > #define AUDIT_SECCOMP 1326 /* Secure Computing event */
> > #define AUDIT_PROCTITLE 1327 /* Proctitle emit event */
> > #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes
> >*/>
> > +#define AUDIT_EVENT_LISTENER 1348 /* 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 53bb39b..74c81a7 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff *skb)
> >
> > mutex_unlock(&audit_cmd_mutex);
> > }
> >
> >
> > +static void audit_log_bind(int group, char *op, int err)
> > +{
> > + struct audit_buffer *ab;
> > + char comm[sizeof(current->comm)];
> > + struct mm_struct *mm = current->mm;
> > +
> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > + if (!ab)
> > + return;
> > +
> > + audit_log_format(ab, "auid=%d",
> > + from_kuid(&init_user_ns,
> > audit_get_loginuid(current))); + audit_log_format(ab, " uid=%d",
> > + from_kuid(&init_user_ns, current_uid()));
> > + audit_log_format(ab, " gid=%d",
> > + from_kgid(&init_user_ns, current_gid()));
> > + audit_log_format(ab, " ses=%d", audit_get_sessionid(current));
> > + audit_log_format(ab, " pid=%d", task_pid_nr(current));
> > + audit_log_format(ab, " comm=");
> > + audit_log_untrustedstring(ab, get_task_comm(comm, current));
> > + if (mm) {
> > + down_read(&mm->mmap_sem);
> > + if (mm->exe_file)
> > + audit_log_d_path(ab, " exe=",
> > &mm->exe_file->f_path);
> > + up_read(&mm->mmap_sem);
> > + } else
> > + audit_log_format(ab, " exe=(null)");
> > + audit_log_task_context(ab); /* subj= */
>
> super crazy yuck. audit_log_task_info() ??

audit_log_task_info logs too much information for typical use. There are times
when you might want to know everything about what's connecting. But in this
case, we don't need anything about groups, saved uids, fsuid, or ppid.

Its a shame we don't have a audit_log_task_info_light function which only
records:

pid= auid= uid= subj= comm= exe= ses= tty=



> > + audit_log_format(ab, " group=%d", group);
>
> group seems like too easily confused a name.

nlnk-grp is better if its what I think it is.

-Steve

2014-10-21 21:08:30

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On 14/10/21, Steve Grubb wrote:
> On Tuesday, October 07, 2014 03:03:14 PM Eric Paris wrote:
> > On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
> > > Log the event when a client attempts to connect to the netlink audit
> > > multicast socket, requiring CAP_AUDIT_READ capability, binding to the
> > > AUDIT_NLGRP_READLOG group. Log the disconnect too.
> > >
> > >
> > >
> > > Sample output:
> > > time->Tue Oct 7 14:15:19 2014
> > > type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1
> > > pid=3552 comm="audit-multicast"
> > > exe="/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen"
> > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0
> > > op=connect res=1>
> > >
> > >
> > > Signed-off-by: Richard Guy Briggs <[email protected]>
> > > ---
> > > For some reason unbind isn't being called on disconnect. I suspect
> > > missing
> > > plumbing in netlink. Investigation needed...
> > >
> > >
> > > include/uapi/linux/audit.h | 1 +
> > > kernel/audit.c | 46
> > >++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 45
> > >insertions(+), 2 deletions(-)
> > >
> > >
> > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > index 4d100c8..7fa6e8f 100644
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -110,6 +110,7 @@
> > >
> > > #define AUDIT_SECCOMP 1326 /* Secure Computing event */
> > > #define AUDIT_PROCTITLE 1327 /* Proctitle emit event */
> > > #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes
> > >*/>
> > > +#define AUDIT_EVENT_LISTENER 1348 /* 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 53bb39b..74c81a7 100644
> > > --- a/kernel/audit.c
> > > +++ b/kernel/audit.c
> > > @@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff *skb)
> > >
> > > mutex_unlock(&audit_cmd_mutex);
> > > }
> > >
> > >
> > > +static void audit_log_bind(int group, char *op, int err)
> > > +{
> > > + struct audit_buffer *ab;
> > > + char comm[sizeof(current->comm)];
> > > + struct mm_struct *mm = current->mm;
> > > +
> > > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > + if (!ab)
> > > + return;
> > > +
> > > + audit_log_format(ab, "auid=%d",
> > > + from_kuid(&init_user_ns,
> > > audit_get_loginuid(current))); + audit_log_format(ab, " uid=%d",
> > > + from_kuid(&init_user_ns, current_uid()));
> > > + audit_log_format(ab, " gid=%d",
> > > + from_kgid(&init_user_ns, current_gid()));
> > > + audit_log_format(ab, " ses=%d", audit_get_sessionid(current));
> > > + audit_log_format(ab, " pid=%d", task_pid_nr(current));
> > > + audit_log_format(ab, " comm=");
> > > + audit_log_untrustedstring(ab, get_task_comm(comm, current));
> > > + if (mm) {
> > > + down_read(&mm->mmap_sem);
> > > + if (mm->exe_file)
> > > + audit_log_d_path(ab, " exe=",
> > > &mm->exe_file->f_path);
> > > + up_read(&mm->mmap_sem);
> > > + } else
> > > + audit_log_format(ab, " exe=(null)");
> > > + audit_log_task_context(ab); /* subj= */
> >
> > super crazy yuck. audit_log_task_info() ??
>
> audit_log_task_info logs too much information for typical use. There are times
> when you might want to know everything about what's connecting. But in this
> case, we don't need anything about groups, saved uids, fsuid, or ppid.
>
> Its a shame we don't have a audit_log_task_info_light function which only
> records:
>
> pid= auid= uid= subj= comm= exe= ses= tty=

We already have audit_log_task() which gives:
auid=
uid=
gid=
ses=
subj=
pid=
comm=
exe=
This is missing tty=, but has gid=. Can we please use that function
instead and add tty=? And while we are at it, refactor
audit_log_task_info() to call audit_log_task()?

Is this standard set above what should be used for certain classes of
log messages?

Yes, it will be in a different order because we don't have a canonical
order yet. Can we accept two orders of keywords so we can start
canonicalizing, please?

> > > + audit_log_format(ab, " group=%d", group);
> >
> > group seems like too easily confused a name.
>
> nlnk-grp is better if its what I think it is.

Where did you find that name? That could work and it is shorter, but it
seems awkwardly optimized. "nlnk" doesn't appear once in the kernel.
"nl" is already recognized for netlink, "mcgrp" is already used for
"multicast group(s)", so I would suggest "nl-mcgrp".

> -Steve

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-10-21 21:40:45

by Steve Grubb

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tuesday, October 21, 2014 05:08:22 PM Richard Guy Briggs wrote:
> On 14/10/21, Steve Grubb wrote:
> > > super crazy yuck. audit_log_task_info() ??
> >
> > audit_log_task_info logs too much information for typical use. There are
> > times when you might want to know everything about what's connecting. But
> > in this case, we don't need anything about groups, saved uids, fsuid, or
> > ppid.
> >
> > Its a shame we don't have a audit_log_task_info_light function which only
> > records:
> >
> > pid= auid= uid= subj= comm= exe= ses= tty=
>
> We already have audit_log_task() which gives:
> auid=
> uid=
> gid=
> ses=
> subj=
> pid=
> comm=
> exe=
> This is missing tty=, but has gid=. Can we please use that function
> instead and add tty=?

gid is important for things that might allow access by file permissions. But
the syscall logging is going to have that and much more. In this case, access
is granted by having a posix capability. So, all we want is what's the
process, who's the user, which session/tty is this coming from to find all
events that might be related.


> And while we are at it, refactor audit_log_task_info() to call
> audit_log_task()?

That will cause problems at this point.

> Is this standard set above what should be used for certain classes of
> log messages?

Its hard to say if that is sufficient for all cases. When access is granted by
posix capability, sure. This is probably good for most kernel originating
events. But there are times extra info is needed.

> Yes, it will be in a different order because we don't have a canonical
> order yet. Can we accept two orders of keywords so we can start
> canonicalizing, please?

I don't understand what you are getting at.

-Steve

> > > > + audit_log_format(ab, " group=%d", group);
> > >
> > > group seems like too easily confused a name.
> >
> > nlnk-grp is better if its what I think it is.
>
> Where did you find that name? That could work and it is shorter, but it
> seems awkwardly optimized. "nlnk" doesn't appear once in the kernel.
> "nl" is already recognized for netlink, "mcgrp" is already used for
> "multicast group(s)", so I would suggest "nl-mcgrp".
>
> > -Steve
>
> - RGB
>
> --
> Richard Guy Briggs <[email protected]>
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems,
> Red Hat Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-10-21 22:30:31

by Paul Moore

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tuesday, October 21, 2014 03:56:10 PM Steve Grubb wrote:
> audit_log_task_info logs too much information for typical use. There are
> times when you might want to know everything about what's connecting. But
> in this case, we don't need anything about groups, saved uids, fsuid, or
> ppid.
>
> Its a shame we don't have a audit_log_task_info_light function which only
> records:
>
> pid= auid= uid= subj= comm= exe= ses= tty=

This is getting back to my earlier concerns/questions about field ordering, or
at the very least I'm going to hijack this conversation and steer it towards
field ordering ;)

Before we go to much farther, I'd really like us to agree that ordering is not
important, can we do that? As a follow up, what do we need to do to make that
happen in the userspace tools?

--
paul moore
security and virtualization @ redhat

2014-10-21 22:30:34

by Eric Paris

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tue, 2014-10-21 at 17:08 -0400, Richard Guy Briggs wrote:
> On 14/10/21, Steve Grubb wrote:
> > On Tuesday, October 07, 2014 03:03:14 PM Eric Paris wrote:
> > > On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
> > > > Log the event when a client attempts to connect to the netlink audit
> > > > multicast socket, requiring CAP_AUDIT_READ capability, binding to the
> > > > AUDIT_NLGRP_READLOG group. Log the disconnect too.
> > > >
> > > >
> > > >
> > > > Sample output:
> > > > time->Tue Oct 7 14:15:19 2014
> > > > type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1
> > > > pid=3552 comm="audit-multicast"
> > > > exe="/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen"
> > > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0
> > > > op=connect res=1>
> > > >
> > > >
> > > > Signed-off-by: Richard Guy Briggs <[email protected]>
> > > > ---
> > > > For some reason unbind isn't being called on disconnect. I suspect
> > > > missing
> > > > plumbing in netlink. Investigation needed...
> > > >
> > > >
> > > > include/uapi/linux/audit.h | 1 +
> > > > kernel/audit.c | 46
> > > >++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 45
> > > >insertions(+), 2 deletions(-)
> > > >
> > > >
> > > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > > index 4d100c8..7fa6e8f 100644
> > > > --- a/include/uapi/linux/audit.h
> > > > +++ b/include/uapi/linux/audit.h
> > > > @@ -110,6 +110,7 @@
> > > >
> > > > #define AUDIT_SECCOMP 1326 /* Secure Computing event */
> > > > #define AUDIT_PROCTITLE 1327 /* Proctitle emit event */
> > > > #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes
> > > >*/>
> > > > +#define AUDIT_EVENT_LISTENER 1348 /* 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 53bb39b..74c81a7 100644
> > > > --- a/kernel/audit.c
> > > > +++ b/kernel/audit.c
> > > > @@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff *skb)
> > > >
> > > > mutex_unlock(&audit_cmd_mutex);
> > > > }
> > > >
> > > >
> > > > +static void audit_log_bind(int group, char *op, int err)
> > > > +{
> > > > + struct audit_buffer *ab;
> > > > + char comm[sizeof(current->comm)];
> > > > + struct mm_struct *mm = current->mm;
> > > > +
> > > > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > + if (!ab)
> > > > + return;
> > > > +
> > > > + audit_log_format(ab, "auid=%d",
> > > > + from_kuid(&init_user_ns,
> > > > audit_get_loginuid(current))); + audit_log_format(ab, " uid=%d",
> > > > + from_kuid(&init_user_ns, current_uid()));
> > > > + audit_log_format(ab, " gid=%d",
> > > > + from_kgid(&init_user_ns, current_gid()));
> > > > + audit_log_format(ab, " ses=%d", audit_get_sessionid(current));
> > > > + audit_log_format(ab, " pid=%d", task_pid_nr(current));
> > > > + audit_log_format(ab, " comm=");
> > > > + audit_log_untrustedstring(ab, get_task_comm(comm, current));
> > > > + if (mm) {
> > > > + down_read(&mm->mmap_sem);
> > > > + if (mm->exe_file)
> > > > + audit_log_d_path(ab, " exe=",
> > > > &mm->exe_file->f_path);
> > > > + up_read(&mm->mmap_sem);
> > > > + } else
> > > > + audit_log_format(ab, " exe=(null)");
> > > > + audit_log_task_context(ab); /* subj= */
> > >
> > > super crazy yuck. audit_log_task_info() ??
> >
> > audit_log_task_info logs too much information for typical use. There are times
> > when you might want to know everything about what's connecting. But in this
> > case, we don't need anything about groups, saved uids, fsuid, or ppid.
> >
> > Its a shame we don't have a audit_log_task_info_light function which only
> > records:
> >
> > pid= auid= uid= subj= comm= exe= ses= tty=
>
> We already have audit_log_task() which gives:
> auid=
> uid=
> gid=
> ses=
> subj=
> pid=
> comm=
> exe=
> This is missing tty=, but has gid=. Can we please use that function
> instead and add tty=? And while we are at it, refactor
> audit_log_task_info() to call audit_log_task()?
>
> Is this standard set above what should be used for certain classes of
> log messages?
>
> Yes, it will be in a different order because we don't have a canonical
> order yet. Can we accept two orders of keywords so we can start
> canonicalizing, please?

I've always hated the fact that we include this in ANY current audit
message. I truly believe we need two new record types.

AUDIT_PROCESS_INFO
AUDIT_EXTENDED_PROCESS_INFO

What does my UID have to do with a syscall? Why is it in the record?
It's a pretty big change, like, RHEL8, but splitting the reporting of
process info from other records will make all matter of things, in the
kernel and in userspace so much cleaner...

Nuts:
type=SYSCALL msg=audit(10/13/2014 03:07:39.919:117953) : arch=x86_64 syscall=stat success=yes exit=0 a0=0x1332f60 a1=0x7fff8749e6d0 a2=0x7fff8749e6d0 a3=0x0 items=1 ppid=28212 pid=30066 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=(none) ses=367 comm=lsof exe=/usr/sbin/lsof subj=system_u:system_r:system_cronjob_t:s0-s0:c0.c1023 key=(null)

Slightly (and yes, just slightly) Less Nuts:
type=SYSCALL msg=audit(10/13/2014 03:07:39.919:117953) : arch=x86_64 syscall=stat success=yes exit=0 a0=0x1332f60 a1=0x7fff8749e6d0 a2=0x7fff8749e6d0 a3=0x0
type=AUDIT_PROCESS_INFO msg=audit(10/13/2014 03:07:39.919:117953) : pid=30066 auid=root uid=root gid=root tty=(none) ses=367 comm=lsof exe=/usr/sbin/lsof subj=system_u:system_r:system_cronjob_t:s0-s0:c0.c1023 key=(null)
type=AUDIT_EXTENDED_PROCESS_INFO msg=audit(10/13/2014 03:07:39.919:117953) : ppid=28212 euid=root suid=root fsuid=root egid=root sgid=root fsgid=root key=(null)

It'd be weird is a syscall record actually only had syscall information....

2014-10-21 23:14:19

by Paul Moore

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On Tuesday, October 21, 2014 06:30:29 PM Eric Paris wrote:
> I've always hated the fact that we include this in ANY current audit
> message. I truly believe we need two new record types.
>
> AUDIT_PROCESS_INFO
> AUDIT_EXTENDED_PROCESS_INFO
>
> What does my UID have to do with a syscall? Why is it in the record?
> It's a pretty big change, like, RHEL8, but splitting the reporting of
> process info from other records will make all matter of things, in the
> kernel and in userspace so much cleaner...
>
> Nuts:
> type=SYSCALL msg=audit(10/13/2014 03:07:39.919:117953) : arch=x86_64
> syscall=stat success=yes exit=0 a0=0x1332f60 a1=0x7fff8749e6d0
> a2=0x7fff8749e6d0 a3=0x0 items=1 ppid=28212 pid=30066 auid=root uid=root
> gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root
> tty=(none) ses=367 comm=lsof exe=/usr/sbin/lsof
> subj=system_u:system_r:system_cronjob_t:s0-s0:c0.c1023 key=(null)
>
> Slightly (and yes, just slightly) Less Nuts:
> type=SYSCALL msg=audit(10/13/2014 03:07:39.919:117953) : arch=x86_64
> syscall=stat success=yes exit=0 a0=0x1332f60 a1=0x7fff8749e6d0
> a2=0x7fff8749e6d0 a3=0x0 type=AUDIT_PROCESS_INFO msg=audit(10/13/2014
> 03:07:39.919:117953) : pid=30066 auid=root uid=root gid=root tty=(none)
> ses=367 comm=lsof exe=/usr/sbin/lsof
> subj=system_u:system_r:system_cronjob_t:s0-s0:c0.c1023 key=(null)
> type=AUDIT_EXTENDED_PROCESS_INFO msg=audit(10/13/2014 03:07:39.919:117953)
> : ppid=28212 euid=root suid=root fsuid=root egid=root sgid=root fsgid=root
> key=(null)
>
> It'd be weird is a syscall record actually only had syscall information....

I'm definitely in favor of this change. We already have the concept of
multiple records per event, we should use this to our advantage to make things
a bit more sane.

--
paul moore
security and virtualization @ redhat

2014-10-22 01:18:41

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On 14/10/21, Eric Paris wrote:
> On Tue, 2014-10-21 at 17:08 -0400, Richard Guy Briggs wrote:
> > On 14/10/21, Steve Grubb wrote:
> > > On Tuesday, October 07, 2014 03:03:14 PM Eric Paris wrote:
> > > > On Tue, 2014-10-07 at 14:23 -0400, Richard Guy Briggs wrote:
> > > > > Log the event when a client attempts to connect to the netlink audit
> > > > > multicast socket, requiring CAP_AUDIT_READ capability, binding to the
> > > > > AUDIT_NLGRP_READLOG group. Log the disconnect too.
> > > > >
> > > > >
> > > > >
> > > > > Sample output:
> > > > > time->Tue Oct 7 14:15:19 2014
> > > > > type=UNKNOWN[1348] msg=audit(1412705719.316:117): auid=0 uid=0 gid=0 ses=1
> > > > > pid=3552 comm="audit-multicast"
> > > > > exe="/home/rgb/rgb/git/audit-multicast-listen/audit-multicast-listen"
> > > > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 group=0
> > > > > op=connect res=1>
> > > > >
> > > > >
> > > > > Signed-off-by: Richard Guy Briggs <[email protected]>
> > > > > ---
> > > > > For some reason unbind isn't being called on disconnect. I suspect
> > > > > missing
> > > > > plumbing in netlink. Investigation needed...
> > > > >
> > > > >
> > > > > include/uapi/linux/audit.h | 1 +
> > > > > kernel/audit.c | 46
> > > > >++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 45
> > > > >insertions(+), 2 deletions(-)
> > > > >
> > > > >
> > > > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > > > index 4d100c8..7fa6e8f 100644
> > > > > --- a/include/uapi/linux/audit.h
> > > > > +++ b/include/uapi/linux/audit.h
> > > > > @@ -110,6 +110,7 @@
> > > > >
> > > > > #define AUDIT_SECCOMP 1326 /* Secure Computing event */
> > > > > #define AUDIT_PROCTITLE 1327 /* Proctitle emit event */
> > > > > #define AUDIT_FEATURE_CHANGE 1328 /* audit log listing feature changes
> > > > >*/>
> > > > > +#define AUDIT_EVENT_LISTENER 1348 /* 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 53bb39b..74c81a7 100644
> > > > > --- a/kernel/audit.c
> > > > > +++ b/kernel/audit.c
> > > > > @@ -1108,13 +1108,54 @@ static void audit_receive(struct sk_buff *skb)
> > > > >
> > > > > mutex_unlock(&audit_cmd_mutex);
> > > > > }
> > > > >
> > > > >
> > > > > +static void audit_log_bind(int group, char *op, int err)
> > > > > +{
> > > > > + struct audit_buffer *ab;
> > > > > + char comm[sizeof(current->comm)];
> > > > > + struct mm_struct *mm = current->mm;
> > > > > +
> > > > > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
> > > > > + if (!ab)
> > > > > + return;
> > > > > +
> > > > > + audit_log_format(ab, "auid=%d",
> > > > > + from_kuid(&init_user_ns,
> > > > > audit_get_loginuid(current))); + audit_log_format(ab, " uid=%d",
> > > > > + from_kuid(&init_user_ns, current_uid()));
> > > > > + audit_log_format(ab, " gid=%d",
> > > > > + from_kgid(&init_user_ns, current_gid()));
> > > > > + audit_log_format(ab, " ses=%d", audit_get_sessionid(current));
> > > > > + audit_log_format(ab, " pid=%d", task_pid_nr(current));
> > > > > + audit_log_format(ab, " comm=");
> > > > > + audit_log_untrustedstring(ab, get_task_comm(comm, current));
> > > > > + if (mm) {
> > > > > + down_read(&mm->mmap_sem);
> > > > > + if (mm->exe_file)
> > > > > + audit_log_d_path(ab, " exe=",
> > > > > &mm->exe_file->f_path);
> > > > > + up_read(&mm->mmap_sem);
> > > > > + } else
> > > > > + audit_log_format(ab, " exe=(null)");
> > > > > + audit_log_task_context(ab); /* subj= */
> > > >
> > > > super crazy yuck. audit_log_task_info() ??
> > >
> > > audit_log_task_info logs too much information for typical use. There are times
> > > when you might want to know everything about what's connecting. But in this
> > > case, we don't need anything about groups, saved uids, fsuid, or ppid.
> > >
> > > Its a shame we don't have a audit_log_task_info_light function which only
> > > records:
> > >
> > > pid= auid= uid= subj= comm= exe= ses= tty=
> >
> > We already have audit_log_task() which gives:
> > auid=
> > uid=
> > gid=
> > ses=
> > subj=
> > pid=
> > comm=
> > exe=
> > This is missing tty=, but has gid=. Can we please use that function
> > instead and add tty=? And while we are at it, refactor
> > audit_log_task_info() to call audit_log_task()?
> >
> > Is this standard set above what should be used for certain classes of
> > log messages?
> >
> > Yes, it will be in a different order because we don't have a canonical
> > order yet. Can we accept two orders of keywords so we can start
> > canonicalizing, please?
>
> I've always hated the fact that we include this in ANY current audit
> message. I truly believe we need two new record types.
>
> AUDIT_PROCESS_INFO
> AUDIT_EXTENDED_PROCESS_INFO
>
> What does my UID have to do with a syscall? Why is it in the record?
> It's a pretty big change, like, RHEL8, but splitting the reporting of
> process info from other records will make all matter of things, in the
> kernel and in userspace so much cleaner...
>
> Nuts:
> type=SYSCALL msg=audit(10/13/2014 03:07:39.919:117953) : arch=x86_64 syscall=stat success=yes exit=0 a0=0x1332f60 a1=0x7fff8749e6d0 a2=0x7fff8749e6d0 a3=0x0 items=1 ppid=28212 pid=30066 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=(none) ses=367 comm=lsof exe=/usr/sbin/lsof subj=system_u:system_r:system_cronjob_t:s0-s0:c0.c1023 key=(null)
>
> Slightly (and yes, just slightly) Less Nuts:
> type=SYSCALL msg=audit(10/13/2014 03:07:39.919:117953) : arch=x86_64 syscall=stat success=yes exit=0 a0=0x1332f60 a1=0x7fff8749e6d0 a2=0x7fff8749e6d0 a3=0x0
> type=AUDIT_PROCESS_INFO msg=audit(10/13/2014 03:07:39.919:117953) : pid=30066 auid=root uid=root gid=root tty=(none) ses=367 comm=lsof exe=/usr/sbin/lsof subj=system_u:system_r:system_cronjob_t:s0-s0:c0.c1023 key=(null)
> type=AUDIT_EXTENDED_PROCESS_INFO msg=audit(10/13/2014 03:07:39.919:117953) : ppid=28212 euid=root suid=root fsuid=root egid=root sgid=root fsgid=root key=(null)
>
> It'd be weird is a syscall record actually only had syscall information....

I am definitely in favour of this approach.

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-10-22 01:24:13

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

On 14/10/21, Paul Moore wrote:
> On Tuesday, October 21, 2014 03:56:10 PM Steve Grubb wrote:
> > audit_log_task_info logs too much information for typical use. There are
> > times when you might want to know everything about what's connecting. But
> > in this case, we don't need anything about groups, saved uids, fsuid, or
> > ppid.
> >
> > Its a shame we don't have a audit_log_task_info_light function which only
> > records:
> >
> > pid= auid= uid= subj= comm= exe= ses= tty=
>
> This is getting back to my earlier concerns/questions about field ordering, or
> at the very least I'm going to hijack this conversation and steer it towards
> field ordering ;)

Well, I've already been pushing it that way because it interferes with
any sort of refactoring that needs to be done to simplify and clean up
the kernel log code.

> Before we go to much farther, I'd really like us to agree that ordering is not
> important, can we do that? As a follow up, what do we need to do to make that
> happen in the userspace tools?

At the very least, as I've suggested, agree on at least one more order,
a canonical one, that can provide a much more firm guide how to present
the keywords so that we're not stuck with an arbitrary order that turns
out not to make sense for some reason or another.

> paul moore

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545