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

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

[Richard Guy Briggs]

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 <rgb@redhat.com>
---
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

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

[Eric Paris]

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 <rgb@redhat.com>
> ---
> 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);

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

[Richard Guy Briggs]

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 <rgb@redhat.com>
> > ---
> > 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 <rbriggs@redhat.com>
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

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

[Paul Moore]

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

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

[Steve Grubb]

On Tue, 07 Oct 2014 18:06:51 -0400
Paul Moore <pmoore@redhat.com> 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

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

[Paul Moore]

On Saturday, October 11, 2014 11:42:06 AM Steve Grubb wrote:
> On Tue, 07 Oct 2014 18:06:51 -0400
> 
> Paul Moore <pmoore@redhat.com> 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

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

[Richard Guy Briggs]

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 <rbriggs@redhat.com>
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

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

[Steve Grubb]

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 <rgb@redhat.com>
> > ---
> > 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

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

[Richard Guy Briggs]

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 <rgb@redhat.com>
> > > ---
> > > 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 <rbriggs@redhat.com>
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

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

[Steve Grubb]

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 <rbriggs@redhat.com>
> 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

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

[Paul Moore]

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

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

[Eric Paris]

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 <rgb@redhat.com>
> > > > ---
> > > > 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....

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

[Paul Moore]

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

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

[Richard Guy Briggs]

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 <rgb@redhat.com>
> > > > > ---
> > > > > 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 <rbriggs@redhat.com>
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

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

[Richard Guy Briggs]

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 <rbriggs@redhat.com>
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

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

[Paul Moore]

[NOTE: Since we are wandering off the original topic I took the liberty of 
trimming the To/CC line to just the audit list folks, feel free to add back as 
necessary.]

On Tuesday, October 21, 2014 09:24:05 PM Richard Guy Briggs wrote:
> 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.

Exactly.  I'm starting to think that we need to resolve this issue before we 
introduce any new features into the kernel.

> > 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.

No, we're already seeing that a single fixed ordering is bad, adding an 
alternate fixed ordering just kicks the can down the road a bit further and 
sets a bad precedence for future development.  It is time to get rid of the 
fixed ordering in the audit records.

-- 
paul moore
security and virtualization @ redhat

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

[Steve Grubb]

On Tuesday, October 21, 2014 06:30:24 PM 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 ;)

Field ordering is important. For example, suppose we decide to make ordering 
changes to the AUDIT_AVC record to bring it in line with current standards. 
Would anyone care?

> Before we go to much farther, I'd really like us to agree that ordering is
> not important, can we do that? 

Its kind of doubtful we can do anything like this quickly. Maybe over time. 
But for entirely new events, we can create some canonical order and use it.

> As a follow up, what do we need to do to make that happen in the userspace
> tools?

I have serious doubts that this is worth doing right now. To me, these are the 
burning issues that I think should be on the table to be solved rather than 
field ordering:

1) For the *at syscalls, can we get the path from the FD being passed to be
able to reconstruct what is being accessed?
2) For the adjtimex syscall, how do we only get events where the time is set
rather than the clock being status'ed?
3) How do we select events to record based on values in structures being
passed? (This is related to #2)
4) How do we select events when a setuid/setgid/setcapabilities file is
executed when you do not have a file list? IOW, extend perms to allow 
selection.
5) Can tty be added to AUDIT_LOGIN event?
6) How do we handle user names that are not in /etc/passwd? IOW, someone 
logins in as root through some network protocol, like spice, and perform admin 
actions with no local account.
7) Does audit of /proc work? If not can it? (Security parameters can be set
through it.)
8) Can we audit NFS based files? Samba?
9) Can we get events for a watched file even when a user's permissions do not
allow full path resolution?
10) Can we optionally get events when a file is actually modified rather than
when opened with intent to modify?
11) Why is the kernel dumping events into syslog instead of waiting until the 
queue is full when auditd shuts down for a restart?
12) The struct audit_status was extended to include version and 
backlog_wait_time. I cannot determine at runtime if they exist, meaning that 
software compiled on a new kernel runs on an old kernel, it will be reading 
random stack or heap to make decisions. The correct solution would be to make 
a new struct with planned expansion capability with version as the first 
element so any changes can be signaled. This new struct would be sent using a 
new netlink command.
13) Is audit by process name ready to go?

I really think some of these issues are more important than re-ordering event 
fields. There is also the issue of having some test suite for robustness.

-Steve

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

[Eric Paris]

On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:

> 12) The struct audit_status was extended to include version and 
> backlog_wait_time. I cannot determine at runtime if they exist, meaning that 
> software compiled on a new kernel runs on an old kernel, it will be reading 
> random stack or heap to make decisions. The correct solution would be to make 
> a new struct with planned expansion capability with version as the first 
> element so any changes can be signaled. This new struct would be sent using a 
> new netlink command.

Incorrect.  The length of the message makes it perfectly clear how much
data the kernel sent, and thus if that data includes the version or
backlog_wait_time.  I thought this had been discussed before...

The answer is 'check how much data you got from the kernel'

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

[Steve Grubb]

On Tuesday, October 21, 2014 06:30:29 PM Eric Paris wrote:
> On Tue, 2014-10-21 at 17:08 -0400, Richard Guy Briggs wrote:
> > On 14/10/21, 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=
> > 
> > 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

It'll eat at least 60 bytes per record and its its an aggregated log, then 
throw in the length of the system names. Disk space is at a premium. We want 
as many records as possible in the logging partition. Also, this will 
translate into more network traffic, more buffers needed in the kernel queue, 
more buffers in audispd and remote logging.

 
> What does my UID have to do with a syscall?  Why is it in the record?

To save space. But also because it may be relevant to whatever is happening.

-Steve

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

[Steve Grubb]

On Tuesday, October 21, 2014 09:24:05 PM Richard Guy Briggs wrote:
> 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.

There really are worse problems to solve. Also, all this changing things will 
keep me from adding new analytical capabilities to the audit tools because I 
continue to have to re-do the bottom layers instead of progress towards making 
things better for admins.


> > 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.

I think we can create the audit_log_task_info_lite function and use it for 
your new events.

-Steve

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

[Steve Grubb]

On Wednesday, October 22, 2014 10:30:12 AM Eric Paris wrote:
> On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:
> > 12) The struct audit_status was extended to include version and
> > backlog_wait_time. I cannot determine at runtime if they exist, meaning
> > that software compiled on a new kernel runs on an old kernel, it will be
> > reading random stack or heap to make decisions. The correct solution
> > would be to make a new struct with planned expansion capability with
> > version as the first element so any changes can be signaled. This new
> > struct would be sent using a new netlink command.
> 
> Incorrect.  The length of the message makes it perfectly clear how much
> data the kernel sent, and thus if that data includes the version or
> backlog_wait_time.  I thought this had been discussed before...
> 
> The answer is 'check how much data you got from the kernel'

Is the padding the same for all arches? :-)

-Steve

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

[Eric Paris]

On Wed, 2014-10-22 at 10:36 -0400, Steve Grubb wrote:
> On Wednesday, October 22, 2014 10:30:12 AM Eric Paris wrote:
> > On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:
> > > 12) The struct audit_status was extended to include version and
> > > backlog_wait_time. I cannot determine at runtime if they exist, meaning
> > > that software compiled on a new kernel runs on an old kernel, it will be
> > > reading random stack or heap to make decisions. The correct solution
> > > would be to make a new struct with planned expansion capability with
> > > version as the first element so any changes can be signaled. This new
> > > struct would be sent using a new netlink command.
> > 
> > Incorrect.  The length of the message makes it perfectly clear how much
> > data the kernel sent, and thus if that data includes the version or
> > backlog_wait_time.  I thought this had been discussed before...
> > 
> > The answer is 'check how much data you got from the kernel'
> 
> Is the padding the same for all arches? :-)

completely irrelevant...

The code in audit_get_reply() seems like poor practice...  You aren't
doing any sanity checks on the data coming back.  I guess that's why
userspace has so much trouble handling things.  It's just completely
ignore the length the kernel gave you...

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

[Eric Paris]

On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:

> 1) For the *at syscalls, can we get the path from the FD being passed to be
> able to reconstruct what is being accessed?

You might sometimes be able to get A path.  But every time anyone ever
says THE path they've already lost.  There is no THE path.  There might
be NO path.  Every single request with THE path is always doomed to
fail.

> 2) For the adjtimex syscall, how do we only get events where the time is set
> rather than the clock being status'ed?

Now you want to match on stuff from 3)  it's certainly possible...  Not
necessarily going to be super easy, especially the rule definitions...

> 3) How do we select events to record based on values in structures being
> passed? (This is related to #2)

Same way you get the fd in a mmap record.  AUX records.

> 4) How do we select events when a setuid/setgid/setcapabilities file is
> executed when you do not have a file list? IOW, extend perms to allow 
> selection.
> 5) Can tty be added to AUDIT_LOGIN event?
> 6) How do we handle user names that are not in /etc/passwd? IOW, someone 
> logins in as root through some network protocol, like spice, and perform admin 
> actions with no local account.
> 7) Does audit of /proc work? If not can it? (Security parameters can be set
> through it.)

watches do not work.  watches can not work.  watches are inode based.
all of /proc has one inode.  I'm sure it's not completely unsolvable to
'watch' in proc, but it'll be a whole new thing JUST for proc...

> 8) Can we audit NFS based files? Samba?

Audit how?  Audit changes made by THIS system? yes.
Audit changes made by the server or another client?  no.
technologically impossible.

> 9) Can we get events for a watched file even when a user's permissions do not
> allow full path resolution?

No.

> 10) Can we optionally get events when a file is actually modified rather than
> when opened with intent to modify?

Possibly sometimes...   It can't be completely reliable.  Think about
mmap().   fsnotify_modify() tries to do something like this but it is
not reliable...

> 11) Why is the kernel dumping events into syslog instead of waiting until the 
> queue is full when auditd shuts down for a restart?

Interesting thought...

> 13) Is audit by process name ready to go?

That's up to you/paul/richard.

> I really think some of these issues are more important than re-ordering event 
> fields. There is also the issue of having some test suite for robustness.

I disagree completely   :)

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

[Paul Moore]

On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > 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 ;)
> 
> Field ordering is important. For example, suppose we decide to make ordering
> changes to the AUDIT_AVC record to bring it in line with current standards.
> Would anyone care?

That is an interesting example record considering everyone recognizes it to be 
an oddly formed, special case.  I'd like to discuss improving the AVC audit 
record, but that discussion is lower priority than the field ordering 
discussion.

Let's stick to correctly formed audit records that follow the name-value pair 
format perfectly; I argue that while we may get accustomed to a specific field 
ordering, the field ordering for well formed audit records should not be 
guaranteed.

> > Before we go to much farther, I'd really like us to agree that ordering is
> > not important, can we do that?
> 
> Its kind of doubtful we can do anything like this quickly. Maybe over time.

Why?  Why can we not do this now?  What, besides some assumptions by the 
userspace tools, is preventing us from fixing this?

> But for entirely new events, we can create some canonical order and use it.

No.  Field order is a problem, not a feature we need to promote.

> > As a follow up, what do we need to do to make that happen in the userspace
> > tools?
> 
> I have serious doubts that this is worth doing right now.

I disagree.  I think we need to resolve this before we go forward with adding, 
or modifying any audit records.

> To me, these are the burning issues that I think should be on the table to
> be solved rather than field ordering:
> 
> 1) ... {snip} ...

Ignoring the priority for a moment, thanks for posting these.  Is there an 
audit TODO list posted somewhere?

-- 
paul moore
security and virtualization @ redhat

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

[LC Bruzenak]

[-- Attachment #1.1: Type: text/plain, Size: 1051 bytes --]

On 10/22/2014 10:12 AM, Eric Paris wrote:
> On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:
>
>> 1) For the *at syscalls, can we get the path from the FD being passed to be
>> able to reconstruct what is being accessed?
> You might sometimes be able to get A path.  But every time anyone ever
> says THE path they've already lost.  There is no THE path.  There might
> be NO path.  Every single request with THE path is always doomed to
> fail.
IIUC we've got to have some assurance that the path is legit for forensics.
Technically I believe I understand and concur with what you are saying
Eric, but as a guy on the far end of the process I know I need to be
able to reference a complete path to a FD.
One which we believe did exist at the time the mod occurred. To me,
sometimes isn't really good enough. But A path probably is.
...
>> 9) Can we get events for a watched file even when a user's permissions do not
>> allow full path resolution?
> No.
No?

Thx,
LCB

-- 
LC (Lenny) Bruzenak
lenny@magitekltd.com



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Steve Grubb]

On Wednesday, October 22, 2014 10:51:49 AM LC Bruzenak wrote:
> On 10/22/2014 10:12 AM, Eric Paris wrote:
> > On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:
> >> 1) For the *at syscalls, can we get the path from the FD being passed to
> >> be
> >> able to reconstruct what is being accessed?
> > 
> > You might sometimes be able to get A path.  But every time anyone ever
> > says THE path they've already lost.  There is no THE path.  There might
> > be NO path.  Every single request with THE path is always doomed to
> > fail.
> 
> IIUC we've got to have some assurance that the path is legit for forensics.
> Technically I believe I understand and concur with what you are saying
> Eric, but as a guy on the far end of the process I know I need to be
> able to reference a complete path to a FD.
> One which we believe did exist at the time the mod occurred. To me,
> sometimes isn't really good enough. But A path probably is.
> ...

The thing is, that if an fd is open, there is an entry on 
/proc/<pid>/fd/<number> that you can use readlink on to get the path. So, if 
/proc has the info to show the outside world, why can't it be accessed from 
inside when needing it for an audit event?


> >> 9) Can we get events for a watched file even when a user's permissions do
> >> not allow full path resolution?
> > 
> > No.
> 
> No?

There are requirements that say audit should send notification on the attempted 
access in both success and failure scenarios. It doesn't say when convenient.

-Steve

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

[Steve Grubb]

On Wednesday, October 22, 2014 11:28:46 AM Paul Moore wrote:
> On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> > On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > > 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 ;)
> > 
> > Field ordering is important. For example, suppose we decide to make
> > ordering changes to the AUDIT_AVC record to bring it in line with current
> > standards. Would anyone care?
> 
> That is an interesting example record considering everyone recognizes it to
> be an oddly formed, special case.

But it illustrates the point. There are tools that depend on an ordering and 
format. There are more programs that just ausearch that needs to be considered 
if the fields change. For example, Someone could do things like this:

retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");

Where, if the field ordering can't be guaranteed, the code becomes:

retval = auparse_find_field(au, "auid");
retval = auparse_first_field(au);
retval = auparse_find_field(au, "pid");
retval = auparse_first_field(au);
retval = auparse_find_field(au, "uid");
retval = auparse_first_field(au);
retval = auparse_find_field(au, res");

Which of the two is likely to be faster? Especially when doing realtime 
analysis as a plugin and needing to finish because another event is coming in? 
Just like a binary struct has to maintain an order of data members if written 
to disk, the sequence of fields need to be maintained in a text record.


> I'd like to discuss improving the AVC
> audit record, but that discussion is lower priority than the field ordering
> discussion.
> 
> Let's stick to correctly formed audit records that follow the name-value
> pair format perfectly; I argue that while we may get accustomed to a
> specific field ordering, the field ordering for well formed audit records
> should not be guaranteed.

Its worked well for the 10 years I've been working on the audit code. There 
has to be a review cycle when any new events are created.  People generally 
make up field names without looking at the catalogue, they use an already 
assigned name for something different, they put them in the wrong order, they 
have dangling words instead of following name=value, they use the wrong event 
type, they might not put enough information in the event, or they put fields in 
the wrong order. All these issues are caught and fixed by review.


> > > Before we go to much farther, I'd really like us to agree that ordering
> > > is
> > > not important, can we do that?
> > 
> > Its kind of doubtful we can do anything like this quickly. Maybe over
> > time.
> 
> Why?  Why can we not do this now?

There are more pressing needs on the user space side of this. reordering fields 
means I have to drop all my current plans and redo something that's been 
working fine. This is why it takes so long to get audit utilities and reports 
that are fast, understandable, and the right information.

The problem at hand is Richard wants to make 2 new events. He wants to know 
what goes in them. We can make a function that pulls the right information 
together and add it to his new event. The immediate problem is solved.


> What, besides some assumptions by the
> userspace tools, is preventing us from fixing this?
> 
> > But for entirely new events, we can create some canonical order and use
> > it.
> 
> No.  Field order is a problem, not a feature we need to promote.
> 
> > > As a follow up, what do we need to do to make that happen in the
> > > userspace
> > > tools?
> > 
> > I have serious doubts that this is worth doing right now.
> 
> I disagree.  I think we need to resolve this before we go forward with
> adding, or modifying any audit records.

We shouldn't be doing much modifying of records. They really should be pretty 
static unless requirements change. For new records, there is no guarantee that 
the function created before is the right information for the new event. It 
just depends on what it is as to what needs to be collected. Then due to all 
the misused fields, we still need to have review to make sure there's no typo. 
Speaking of which, I just found a typo in 
AUDIT_FEATURE_CHANGE events.
 

> > To me, these are the burning issues that I think should be on the table to
> > be solved rather than field ordering:
> > 
> > 1) ... {snip} ...
> 
> Ignoring the priority for a moment, thanks for posting these.  Is there an
> audit TODO list posted somewhere?

That is just some kernel issues off the top of my head. Things come up all the 
time. Most of the time things are found because someone is asking questions or 
I am trying to make sense of the audit trail.

As for user space tools, yes there is a TODO file in the audit tarball. I don't 
know if the kernel maintainers have a TODO list published anywhere.

-Steve

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

[Eric Paris]

On Wed, 2014-10-22 at 10:51 -0500, LC Bruzenak wrote:
> On 10/22/2014 10:12 AM, Eric Paris wrote:
> > On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:
> >
> >> 1) For the *at syscalls, can we get the path from the FD being passed to be
> >> able to reconstruct what is being accessed?
> > You might sometimes be able to get A path.  But every time anyone ever
> > says THE path they've already lost.  There is no THE path.  There might
> > be NO path.  Every single request with THE path is always doomed to
> > fail.
> IIUC we've got to have some assurance that the path is legit for forensics.
> Technically I believe I understand and concur with what you are saying
> Eric, but as a guy on the far end of the process I know I need to be
> able to reference a complete path to a FD.
> One which we believe did exist at the time the mod occurred. To me,
> sometimes isn't really good enough. But A path probably is.
> ...

>From the PoV of the process in question there was, at some point, A
path.  That I agree with.  But imagine I clone a new mount  namespace
and don't share my changes with the parent namespace.  Now I mount
something new in that child namespace.  What is A path for a file in the
new mount?  From the parent namespace there is NO path, ABSOLUTELY NO
PATH.  (guess which mount namespace auditd lives in, by the way).  From
the PoV of the processes in the child mount namespace there is A path,
but it's possibly/probably completely meaningless to the
admin.  /etc/shadow != /etc/shadow the admin cares about...  readlink()
doesn't work in this case either.  Sometimes there just plain is no
path.  So yeah, I'm betting MOST of the time we can come up with A path,
but that's not exactly what you want either   :-(

> >> 9) Can we get events for a watched file even when a user's permissions do not
> >> allow full path resolution?
> > No.
> No?

Say I set a watch for failure to open /path/to/my/file.
If someone comes along and says open(/path/to/my/file) but they do not
have execute permissions on /path/to/ their request will be denied.  Not
because they didn't have permission to open /path/to/my/file, but
because they didn't have permission to open /path/to.  Watches do not,
and can not, emit a rule for that.  The rule you requested (failure to
open /path/to/my/file) was not violated.  The kernel did not try to
open /path/to/my/file.  It tried to open /path/to/ and died right there.
If you care about things being unable to open /path/to/, put a watch
on /path/to (although I'm not 100% such watches actually work, but at
least the theory is right and maybe that could be fixed)

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

[LC Bruzenak]

[-- Attachment #1.1: Type: text/plain, Size: 3598 bytes --]

On 10/22/2014 01:18 PM, Eric Paris wrote:
> On Wed, 2014-10-22 at 10:51 -0500, LC Bruzenak wrote:
>> On 10/22/2014 10:12 AM, Eric Paris wrote:
>>> On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:
>>>
>>>> 1) For the *at syscalls, can we get the path from the FD being passed to be
>>>> able to reconstruct what is being accessed?
>>> You might sometimes be able to get A path.  But every time anyone ever
>>> says THE path they've already lost.  There is no THE path.  There might
>>> be NO path.  Every single request with THE path is always doomed to
>>> fail.
>> IIUC we've got to have some assurance that the path is legit for forensics.
>> Technically I believe I understand and concur with what you are saying
>> Eric, but as a guy on the far end of the process I know I need to be
>> able to reference a complete path to a FD.
>> One which we believe did exist at the time the mod occurred. To me,
>> sometimes isn't really good enough. But A path probably is.
>> ...
> >From the PoV of the process in question there was, at some point, A
> path.  That I agree with.  But imagine I clone a new mount  namespace
> and don't share my changes with the parent namespace.  Now I mount
> something new in that child namespace.  What is A path for a file in the
> new mount?  From the parent namespace there is NO path, ABSOLUTELY NO
> PATH.  (guess which mount namespace auditd lives in, by the way).  From
> the PoV of the processes in the child mount namespace there is A path,
> but it's possibly/probably completely meaningless to the
> admin.  /etc/shadow != /etc/shadow the admin cares about...  readlink()
> doesn't work in this case either.  Sometimes there just plain is no
> path.  So yeah, I'm betting MOST of the time we can come up with A path,
> but that's not exactly what you want either   :-(
OK; interesting case. Now I hate namespaces.
:-)

Perhaps we'll have to get smarter in order to be able to work backwards
through namespaces.
Or if not solvable, maybe some of us will have to decide to not allow
ad-hoc mounted namespaces. Just saying. This stuff matters.
I'm assuming we get the namespace info in userland audit events? My
RHEL6 versions are behind where you guys are...
>
>>>> 9) Can we get events for a watched file even when a user's permissions do not
>>>> allow full path resolution?
>>> No.
>> No?
> Say I set a watch for failure to open /path/to/my/file.
> If someone comes along and says open(/path/to/my/file) but they do not
> have execute permissions on /path/to/ their request will be denied.  Not
> because they didn't have permission to open /path/to/my/file, but
> because they didn't have permission to open /path/to.  Watches do not,
> and can not, emit a rule for that.  The rule you requested (failure to
> open /path/to/my/file) was not violated.  The kernel did not try to
> open /path/to/my/file.  It tried to open /path/to/ and died right there.
> If you care about things being unable to open /path/to/, put a watch
> on /path/to (although I'm not 100% such watches actually work, but at
> least the theory is right and maybe that could be fixed)
I didn't match the "no" answer to what I read to be a more general question.
Per the STIG (& added) rules there are exit watches for EACCES and
EPERM, which IIUC would be caught in your example. Not all the way down
to the end of the path but to the point of failure. Good enough.
So I probably misinterpreted the question. Your answer cleared it up
nicely; thanks.

Thx,
LCB

-- 
LC (Lenny) Bruzenak
lenny@magitekltd.com



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Steve Grubb]

On Wednesday, October 22, 2014 02:18:00 PM Eric Paris wrote:
> On Wed, 2014-10-22 at 10:51 -0500, LC Bruzenak wrote:
> > On 10/22/2014 10:12 AM, Eric Paris wrote:
> > > On Wed, 2014-10-22 at 10:25 -0400, Steve Grubb wrote:
> > >> 1) For the *at syscalls, can we get the path from the FD being passed
> > >> to be
> > >> able to reconstruct what is being accessed?
> > > 
> > > You might sometimes be able to get A path.  But every time anyone ever
> > > says THE path they've already lost.  There is no THE path.  There might
> > > be NO path.  Every single request with THE path is always doomed to
> > > fail.
> > 
> > IIUC we've got to have some assurance that the path is legit for
> > forensics.
> > Technically I believe I understand and concur with what you are saying
> > Eric, but as a guy on the far end of the process I know I need to be
> > able to reference a complete path to a FD.
> > One which we believe did exist at the time the mod occurred. To me,
> > sometimes isn't really good enough. But A path probably is.
> > ...
> >
> From the PoV of the process in question there was, at some point, A
> path.  That I agree with.  But imagine I clone a new mount  namespace
> and don't share my changes with the parent namespace.  Now I mount
> something new in that child namespace. 

All of these are privileged ops. Meaning you would have to be tracking the 
actions of an admin who could do all kinds of bad things to a system.

> What is A path for a file in the new mount?

Does openat allow an fd in one name space and a file name in another?

> From the parent namespace there is NO path, ABSOLUTELY NO
> PATH.

Without getting into trick situations like this, how about we get it for the 
normal run-in-the-mill-nothing-tricky-going-on-here system? Even that would be 
immensely helpful. Especially since glibc has switched over to openat and 
newer platforms like aarch64 don't have open(2).


> (guess which mount namespace auditd lives in, by the way).  From
> the PoV of the processes in the child mount namespace there is A path,
> but it's possibly/probably completely meaningless to the
> admin.  /etc/shadow != /etc/shadow the admin cares about...  readlink()
> doesn't work in this case either.  Sometimes there just plain is no
> path.  So yeah, I'm betting MOST of the time we can come up with A path,
> but that's not exactly what you want either   :-(



> > >> 9) Can we get events for a watched file even when a user's permissions
> > >> do not allow full path resolution?
> > > 
> > > No.
> > 
> > No?
> 
> Say I set a watch for failure to open /path/to/my/file.
> If someone comes along and says open(/path/to/my/file) but they do not
> have execute permissions on /path/to/ their request will be denied.  Not
> because they didn't have permission to open /path/to/my/file, but
> because they didn't have permission to open /path/to.  Watches do not,
> and can not, emit a rule for that.  The rule you requested (failure to
> open /path/to/my/file) was not violated.  The kernel did not try to
> open /path/to/my/file.  It tried to open /path/to/ and died right there.
> If you care about things being unable to open /path/to/, put a watch
> on /path/to (although I'm not 100% such watches actually work, but at
> least the theory is right and maybe that could be fixed)

If I have this rule

-a always,exit -S openat -F exit=-EPERM

The path is sitting in arg2. It not lost or inaccessible. We can't see it in 
current audit records because we get a pointer to the string as arg2. I would 
think if path resolution couldn't happen, we could get a substitute record 
that identifies the string passed as arg2.

-Steve

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

[Paul Moore]

On Wednesday, October 22, 2014 01:56:13 PM Steve Grubb wrote:
> On Wednesday, October 22, 2014 11:28:46 AM Paul Moore wrote:
> > On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> > > On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > > > 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 ;)
> > > 
> > > Field ordering is important. For example, suppose we decide to make
> > > ordering changes to the AUDIT_AVC record to bring it in line with
> > > current standards. Would anyone care?
> > 
> > That is an interesting example record considering everyone recognizes it
> > to be an oddly formed, special case.
> 
> But it illustrates the point. There are tools that depend on an ordering and
> format. There are more programs that just ausearch that needs to be
> considered if the fields change. For example, Someone could do things like
> this:
> 
> retval = auparse_find_field(au, "auid");
> retval = auparse_next_field(au);
> retval = auparse_next_field(au);
> retval = auparse_find_field(au, res");
> 
> Where, if the field ordering can't be guaranteed, the code becomes:
> 
> retval = auparse_find_field(au, "auid");
> retval = auparse_first_field(au);
> retval = auparse_find_field(au, "pid");
> retval = auparse_first_field(au);
> retval = auparse_find_field(au, "uid");
> retval = auparse_first_field(au);
> retval = auparse_find_field(au, res");

In my mind the latter code is more robust and preferable.

> Which of the two is likely to be faster? Especially when doing realtime
> analysis as a plugin and needing to finish because another event is coming
> in? Just like a binary struct has to maintain an order of data members if
> written to disk, the sequence of fields need to be maintained in a text
> record.

What about the speed and performance of the code in the kernel?  What about 
the maintenance burden of having to duplicate code to ensure a fixed format?

I'm sorry, I don't find your argument above to be compelling.

> > I'd like to discuss improving the AVC audit record, but that discussion is
> > lower priority than the field ordering discussion.
> > 
> > Let's stick to correctly formed audit records that follow the name-value
> > pair format perfectly; I argue that while we may get accustomed to a
> > specific field ordering, the field ordering for well formed audit records
> > should not be guaranteed.
> 
> Its worked well for the 10 years I've been working on the audit code.

It has worked.  It is causing problems now, and these issues will likely only 
increase as things progress.

> There has to be a review cycle when any new events are created.  People
> generally make up field names without looking at the catalogue, they use an
> already assigned name for something different, they put them in the wrong
> order, they have dangling words instead of following name=value, they use
> the wrong event type, they might not put enough information in the event, or
> they put fields in the wrong order. All these issues are caught and fixed
> by review.

In summary, code needs to be reviewed; I think we all agree on that.

> > > > Before we go to much farther, I'd really like us to agree that
> > > > ordering is not important, can we do that?
> > > 
> > > Its kind of doubtful we can do anything like this quickly. Maybe over
> > > time.
> > 
> > Why?  Why can we not do this now?
> 
> There are more pressing needs on the user space side of this. reordering
> fields means I have to drop all my current plans and redo something that's
> been working fine. This is why it takes so long to get audit utilities and
> reports that are fast, understandable, and the right information.

I disagree about the priority.  Eric disagrees about the priority.  Richard 
hasn't explicitly stated he disagrees with the priority but he has made 
several comments on this list about ordering being an issue (Richard, my 
apologies if I am putting words in your mouth).

Does the audit userspace still live in SVN on fedorahosted.org?  What would we 
need to change in the userspace to eliminate the reliance on field ordering?  
I understand if you don't have a well developed list of items, but surely you 
must have some idea?

I'm willing to help here, and I suspect there might be some others as well, 
just let us know what the pressing issues are in the audit userspace.

> The problem at hand is Richard wants to make 2 new events. He wants to know
> what goes in them. We can make a function that pulls the right information
> together and add it to his new event. The immediate problem is solved.

... and the field ordering issue is swept under the run again.  I feel this is 
an important issue and we should deal with it now; it will only be harder to 
resolve later.

> > What, besides some assumptions by the userspace tools, is preventing us
> > from fixing this?
> > 
> > > But for entirely new events, we can create some canonical order and use
> > > it.
> > 
> > No.  Field order is a problem, not a feature we need to promote.
> > 
> > > > As a follow up, what do we need to do to make that happen in the
> > > > userspace tools?
> > > 
> > > I have serious doubts that this is worth doing right now.
> > 
> > I disagree.  I think we need to resolve this before we go forward with
> > adding, or modifying any audit records.
> 
> We shouldn't be doing much modifying of records. They really should be
> pretty static unless requirements change. For new records, there is no
> guarantee that the function created before is the right information for the
> new event. It just depends on what it is as to what needs to be collected.
> Then due to all the misused fields, we still need to have review to make
> sure there's no typo. Speaking of which, I just found a typo in
> AUDIT_FEATURE_CHANGE events.

We're seeing at least one case where our inability to change the ordering of 
the audit fields is causing problems.

> > > To me, these are the burning issues that I think should be on the table
> > > to be solved rather than field ordering:
> > > 
> > > 1) ... {snip} ...
> > 
> > Ignoring the priority for a moment, thanks for posting these.  Is there an
> > audit TODO list posted somewhere?
> 
> That is just some kernel issues off the top of my head. Things come up all
> the time. Most of the time things are found because someone is asking
> questions or I am trying to make sense of the audit trail.
> 
> As for user space tools, yes there is a TODO file in the audit tarball. I
> don't know if the kernel maintainers have a TODO list published anywhere.

Eric, do you have one published somewhere?

Assuming that Eric doesn't have a TODO list posted somewhere, do you have any 
objections to my posting and maintaining an audit kernel TODO list on the 
audit fedorahosted.org wiki?

-- 
paul moore
security and virtualization @ redhat

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

[LC Bruzenak]

[-- Attachment #1.1: Type: text/plain, Size: 1309 bytes --]

On 10/22/2014 03:06 PM, Paul Moore wrote:
>> > But it illustrates the point. There are tools that depend on an ordering and
>> > format. There are more programs that just ausearch that needs to be
>> > considered if the fields change. For example, Someone could do things like
>> > this:
>> > 
>> > retval = auparse_find_field(au, "auid");
>> > retval = auparse_next_field(au);
>> > retval = auparse_next_field(au);
>> > retval = auparse_find_field(au, res");
>> > 
>> > Where, if the field ordering can't be guaranteed, the code becomes:
>> > 
>> > retval = auparse_find_field(au, "auid");
>> > retval = auparse_first_field(au);
>> > retval = auparse_find_field(au, "pid");
>> > retval = auparse_first_field(au);
>> > retval = auparse_find_field(au, "uid");
>> > retval = auparse_first_field(au);
>> > retval = auparse_find_field(au, res");
> In my mind the latter code is more robust and preferable.
>
OK; I swear if you change this I'm going to parse EVERY field straight
into a SQLite file first, since I'd have to go change code anyway.
:-)

I have code which is based on the examples, from years back, which
believe there is order. It can be changed if needed; rather not but could.
I suspect there are others...

LCB

-- 
LC (Lenny) Bruzenak
lenny@magitekltd.com



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Steve Grubb]

On Wednesday, October 22, 2014 04:06:47 PM Paul Moore wrote:
> On Wednesday, October 22, 2014 01:56:13 PM Steve Grubb wrote:
> > On Wednesday, October 22, 2014 11:28:46 AM Paul Moore wrote:
> > > On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> > > > On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > > > > 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 ;)
> > > > 
> > > > Field ordering is important. For example, suppose we decide to make
> > > > ordering changes to the AUDIT_AVC record to bring it in line with
> > > > current standards. Would anyone care?
> > > 
> > > That is an interesting example record considering everyone recognizes it
> > > to be an oddly formed, special case.
> > 
> > But it illustrates the point. There are tools that depend on an ordering
> > and format. There are more programs that just ausearch that needs to be
> > considered if the fields change. For example, Someone could do things
> > like this:
> > 
> > retval = auparse_find_field(au, "auid");
> > retval = auparse_next_field(au);
> > retval = auparse_next_field(au);
> > retval = auparse_find_field(au, res");
> > 
> > Where, if the field ordering can't be guaranteed, the code becomes:
> > 
> > retval = auparse_find_field(au, "auid");
> > retval = auparse_first_field(au);
> > retval = auparse_find_field(au, "pid");
> > retval = auparse_first_field(au);
> > retval = auparse_find_field(au, "uid");
> > retval = auparse_first_field(au);
> > retval = auparse_find_field(au, res");
> 
> In my mind the latter code is more robust and preferable.

Except you can have problems when the event is like this
auid= pid= old uid= new uid= res=

and yes there are places like that. The performance really is the main issue.


> > Which of the two is likely to be faster? Especially when doing realtime
> > analysis as a plugin and needing to finish because another event is coming
> > in? Just like a binary struct has to maintain an order of data members if
> > written to disk, the sequence of fields need to be maintained in a text
> > record.
> 
> What about the speed and performance of the code in the kernel?

kprintf is the same speed no matter what the order.

> What about the maintenance burden of having to duplicate code to ensure a
> fixed format?

There just isn't a need to be altering events. We've had to add a few things 
here and there, but its only been a couple changes.
 
> > > > > Before we go to much farther, I'd really like us to agree that
> > > > > ordering is not important, can we do that?
> > > > 
> > > > Its kind of doubtful we can do anything like this quickly. Maybe over
> > > > time.
> > > 
> > > Why?  Why can we not do this now?
> > 
> > There are more pressing needs on the user space side of this. reordering
> > fields means I have to drop all my current plans and redo something that's
> > been working fine. This is why it takes so long to get audit utilities and
> > reports that are fast, understandable, and the right information.
> 
> I disagree about the priority.  Eric disagrees about the priority.  Richard
> hasn't explicitly stated he disagrees with the priority but he has made
> several comments on this list about ordering being an issue (Richard, my
> apologies if I am putting words in your mouth).

What events do people need to change and why? There's not been any discussion 
that I know of saying we need to add fields or change them around.


> Does the audit userspace still live in SVN on fedorahosted.org?

Yes.

> What would we need to change in the userspace to eliminate the reliance on
> field ordering?

Many of the utilities. ausyscall & autrace might be the only ones not affected.


> I understand if you don't have a well developed list of items,
> but surely you must have some idea?
> 
> I'm willing to help here, and I suspect there might be some others as well,
> just let us know what the pressing issues are in the audit userspace.

People have been asking for 
1) compressed logs
2) auparse needs to untangle interlaced events
3) people want to suppress certain records or events to save disk space
4) we need to handle federated IDs
5) we need to enrich events on the fly so that uids are preserved
6) we need a validation suite to ensure that user space apps are generating 
correct events (lightdm for example is not audit aware meaning many desktops 
can't be analyzed)
7) we need to get better at handling vast quantities of logs
8) we need some plugin for logstash
9) we need to allow people to format events their way
10) we need easier to understand reports
11) we need to be able to compare in kernel and on disk audit rules
12) auvirt is very broken and in need of rewrite
13) bash tab completions might be helpful
14) eventually combine audispd and auditd into 1 process.

There's more than that. But it would be nice of people using audit and its 
tools also say what they are needing.


> > We shouldn't be doing much modifying of records. They really should be
> > pretty static unless requirements change. For new records, there is no
> > guarantee that the function created before is the right information for
> > the
> > new event. It just depends on what it is as to what needs to be collected.
> > Then due to all the misused fields, we still need to have review to make
> > sure there's no typo. Speaking of which, I just found a typo in
> > AUDIT_FEATURE_CHANGE events.
> 
> We're seeing at least one case where our inability to change the ordering of
> the audit fields is causing problems.

What field ordering problem is preventing kernel work?


> > > > To me, these are the burning issues that I think should be on the
> > > > table
> > > > to be solved rather than field ordering:
> > > > 
> > > > 1) ... {snip} ...
> > > 
> > > Ignoring the priority for a moment, thanks for posting these.  Is there
> > > an
> > > audit TODO list posted somewhere?
> > 
> > That is just some kernel issues off the top of my head. Things come up all
> > the time. Most of the time things are found because someone is asking
> > questions or I am trying to make sense of the audit trail.
> > 
> > As for user space tools, yes there is a TODO file in the audit tarball. I
> > don't know if the kernel maintainers have a TODO list published anywhere.
> 
> Eric, do you have one published somewhere?
> 
> Assuming that Eric doesn't have a TODO list posted somewhere, do you have
> any objections to my posting and maintaining an audit kernel TODO list on
> the audit fedorahosted.org wiki?

That would be nice.

-Steve

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

[Paul Moore]

On Wednesday, October 22, 2014 03:34:24 PM LC Bruzenak wrote:
> On 10/22/2014 03:06 PM, Paul Moore wrote:
> >> > But it illustrates the point. There are tools that depend on an
> >> > ordering and format. There are more programs that just ausearch that
> >> > needs to be considered if the fields change. For example, Someone
> >> > could do things like this:
> >> > 
> >> > retval = auparse_find_field(au, "auid");
> >> > retval = auparse_next_field(au);
> >> > retval = auparse_next_field(au);
> >> > retval = auparse_find_field(au, res");
> >> > 
> >> > Where, if the field ordering can't be guaranteed, the code becomes:
> >> > 
> >> > retval = auparse_find_field(au, "auid");
> >> > retval = auparse_first_field(au);
> >> > retval = auparse_find_field(au, "pid");
> >> > retval = auparse_first_field(au);
> >> > retval = auparse_find_field(au, "uid");
> >> > retval = auparse_first_field(au);
> >> > retval = auparse_find_field(au, res");
> > 
> > In my mind the latter code is more robust and preferable.
> 
> OK; I swear if you change this I'm going to parse EVERY field straight
> into a SQLite file first, since I'd have to go change code anyway.
> 
> :-)

 :)
 
> I have code which is based on the examples, from years back, which
> believe there is order. It can be changed if needed; rather not but could.
> I suspect there are others...

We haven't changed anything yet, but I strongly believe we need to do away 
with field ordering.  The good news is that if you explicitly search for the 
field instead of relying on a fixed order the code should be more robust and 
work either way. ;)

-- 
paul moore
security and virtualization @ redhat

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

[Paul Moore]

On Wednesday, October 22, 2014 04:39:49 PM Steve Grubb wrote:
> On Wednesday, October 22, 2014 04:06:47 PM Paul Moore wrote:
> > On Wednesday, October 22, 2014 01:56:13 PM Steve Grubb wrote:
> > > On Wednesday, October 22, 2014 11:28:46 AM Paul Moore wrote:
> > > > On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> > > > > On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > > > > > 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 ;)
> > > > > 
> > > > > Field ordering is important. For example, suppose we decide to make
> > > > > ordering changes to the AUDIT_AVC record to bring it in line with
> > > > > current standards. Would anyone care?
> > > > 
> > > > That is an interesting example record considering everyone recognizes
> > > > it to be an oddly formed, special case.
> > > 
> > > But it illustrates the point. There are tools that depend on an ordering
> > > and format. There are more programs that just ausearch that needs to be
> > > considered if the fields change. For example, Someone could do things
> > > like this:
> > > 
> > > retval = auparse_find_field(au, "auid");
> > > retval = auparse_next_field(au);
> > > retval = auparse_next_field(au);
> > > retval = auparse_find_field(au, res");
> > > 
> > > Where, if the field ordering can't be guaranteed, the code becomes:
> > > 
> > > retval = auparse_find_field(au, "auid");
> > > retval = auparse_first_field(au);
> > > retval = auparse_find_field(au, "pid");
> > > retval = auparse_first_field(au);
> > > retval = auparse_find_field(au, "uid");
> > > retval = auparse_first_field(au);
> > > retval = auparse_find_field(au, res");
> > 
> > In my mind the latter code is more robust and preferable.
> 
> Except you can have problems when the event is like this
> auid= pid= old uid= new uid= res=

I honestly don't see the problem here.
 
> and yes there are places like that. The performance really is the main
> issue.

So you've said.  As I've said, I'm not convinced.

> > > Which of the two is likely to be faster? Especially when doing realtime
> > > analysis as a plugin and needing to finish because another event is
> > > coming in? Just like a binary struct has to maintain an order of data
> > > members if written to disk, the sequence of fields need to be maintained
> > > in a text record.
> > 
> > What about the speed and performance of the code in the kernel?
> 
> kprintf is the same speed no matter what the order.

Having to duplicate code in the kernel has a negative effect.

> > What about the maintenance burden of having to duplicate code to ensure a
> > fixed format?
> 
> There just isn't a need to be altering events. We've had to add a few things
> here and there, but its only been a couple changes.

See the earlier comments on Richard's patch.

> > > > > > Before we go to much farther, I'd really like us to agree that
> > > > > > ordering is not important, can we do that?
> > > > > 
> > > > > Its kind of doubtful we can do anything like this quickly. Maybe
> > > > > over time.
> > > > 
> > > > Why?  Why can we not do this now?
> > > 
> > > There are more pressing needs on the user space side of this. reordering
> > > fields means I have to drop all my current plans and redo something
> > > that's been working fine. This is why it takes so long to get audit
> > > utilities and reports that are fast, understandable, and the right
> > > information.
> > 
> > I disagree about the priority.  Eric disagrees about the priority. 
> > Richard hasn't explicitly stated he disagrees with the priority but he has
> > made several comments on this list about ordering being an issue (Richard,
> > my apologies if I am putting words in your mouth).
> 
> What events do people need to change and why? There's not been any
> discussion that I know of saying we need to add fields or change them
> around.

See the earlier comments on Richard's patch.

> > What would we need to change in the userspace to eliminate the reliance on
> > field ordering?
> 
> Many of the utilities. ausyscall & autrace might be the only ones not
> affected.

So we would need to change ausyscall and autrace, possibly others.

Do you expect to need any changes to the deamon or audit libraries?

> > I understand if you don't have a well developed list of items,
> > but surely you must have some idea?
> > 
> > I'm willing to help here, and I suspect there might be some others as
> > well, just let us know what the pressing issues are in the audit
> > userspace.
> 
> People have been asking for ... {snip} ...

Right now I was asking for input on what would need to change in userspace to 
eliminate the reliance on field ordering, not a full TODO list.
 
> > > We shouldn't be doing much modifying of records. They really should be
> > > pretty static unless requirements change. For new records, there is no
> > > guarantee that the function created before is the right information for
> > > the new event. It just depends on what it is as to what needs to be
> > > collected. Then due to all the misused fields, we still need to have
> > > review to make sure there's no typo. Speaking of which, I just found a
> > > typo in AUDIT_FEATURE_CHANGE events.
> > 
> > We're seeing at least one case where our inability to change the ordering
> > of the audit fields is causing problems.
> 
> What field ordering problem is preventing kernel work?

It is making Richard's patch undesirable.

> > > > > To me, these are the burning issues that I think should be on the
> > > > > table to be solved rather than field ordering:
> > > > > 
> > > > > 1) ... {snip} ...
> > > > 
> > > > Ignoring the priority for a moment, thanks for posting these.  Is
> > > > there an audit TODO list posted somewhere?
> > > 
> > > That is just some kernel issues off the top of my head. Things come up
> > > all the time. Most of the time things are found because someone is
> > > asking questions or I am trying to make sense of the audit trail.
> > > 
> > > As for user space tools, yes there is a TODO file in the audit tarball.
> > > I don't know if the kernel maintainers have a TODO list published
> > > anywhere.
> > 
> > Eric, do you have one published somewhere?
> > 
> > Assuming that Eric doesn't have a TODO list posted somewhere, do you have
> > any objections to my posting and maintaining an audit kernel TODO list on
> > the audit fedorahosted.org wiki?
> 
> That would be nice.

Great, I'll look into that once I hear back from Eric on any old kernel audit 
TODO lists he might have.

-- 
paul moore
security and virtualization @ redhat

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

[LC Bruzenak]

[-- Attachment #1.1: Type: text/plain, Size: 612 bytes --]

On 10/22/2014 03:44 PM, Paul Moore wrote:
> We haven't changed anything yet, but I strongly believe we need to do away 
> with field ordering.  The good news is that if you explicitly search for the 
> field instead of relying on a fixed order the code should be more robust and 
> work either way. ;)
I have no doubt my old code looks like Steve's first example, not the
second.
But as I said, code can be changed if the assumptions about ordering are
thrown out.

You're making a pretty big splash over here Paul! Very impressive...
:-)

LCB

-- 
LC (Lenny) Bruzenak
lenny@magitekltd.com



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Steve Grubb]

On Wednesday, October 22, 2014 05:00:03 PM Paul Moore wrote:
> On Wednesday, October 22, 2014 04:39:49 PM Steve Grubb wrote:
> > On Wednesday, October 22, 2014 04:06:47 PM Paul Moore wrote:
> > > On Wednesday, October 22, 2014 01:56:13 PM Steve Grubb wrote:
> > > > On Wednesday, October 22, 2014 11:28:46 AM Paul Moore wrote:
> > > > > On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> > > > > > On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > > > > > > 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 ;)
> > > > > > 
> > > > > > Field ordering is important. For example, suppose we decide to
> > > > > > make
> > > > > > ordering changes to the AUDIT_AVC record to bring it in line with
> > > > > > current standards. Would anyone care?
> > > > > 
> > > > > That is an interesting example record considering everyone
> > > > > recognizes
> > > > > it to be an oddly formed, special case.
> > > > 
> > > > But it illustrates the point. There are tools that depend on an
> > > > ordering
> > > > and format. There are more programs that just ausearch that needs to
> > > > be
> > > > considered if the fields change. For example, Someone could do things
> > > > like this:
> > > > 
> > > > retval = auparse_find_field(au, "auid");
> > > > retval = auparse_next_field(au);
> > > > retval = auparse_next_field(au);
> > > > retval = auparse_find_field(au, res");
> > > > 
> > > > Where, if the field ordering can't be guaranteed, the code becomes:
> > > > 
> > > > retval = auparse_find_field(au, "auid");
> > > > retval = auparse_first_field(au);
> > > > retval = auparse_find_field(au, "pid");
> > > > retval = auparse_first_field(au);
> > > > retval = auparse_find_field(au, "uid");
> > > > retval = auparse_first_field(au);
> > > > retval = auparse_find_field(au, res");
> > > 
> > > In my mind the latter code is more robust and preferable.
> > 
> > Except you can have problems when the event is like this
> > auid= pid= old uid= new uid= res=
> 
> I honestly don't see the problem here.

You'll never get new uid which is really the one you want.


> > > I disagree about the priority.  Eric disagrees about the priority.
> > > Richard hasn't explicitly stated he disagrees with the priority but he
> > > has  made several comments on this list about ordering being an issue
> > > (Richard, my apologies if I am putting words in your mouth).

I thought it was a question of what to put in an event. As for ordering all 
you have to do is check the event with ausearch-test to see if its well 
formed.


> > What events do people need to change and why? There's not been any
> > discussion that I know of saying we need to add fields or change them
> > around.
> 
> See the earlier comments on Richard's patch.

He's making a new event. Its not changing things around.

 
> > > What would we need to change in the userspace to eliminate the reliance
> > > on field ordering?
> > 
> > Many of the utilities. ausyscall & autrace might be the only ones not
> > affected.
> 
> So we would need to change ausyscall and autrace, possibly others.

Exactly the opposite, those are about the only ones clean because they are the 
only ones not parsing logs.


> Do you expect to need any changes to the deamon or audit libraries?

Not the daemon or library directly for this. But if you want to look into 
this, you'll need some really big logs for testing. You'll need at least 100MB 
to see performance variations. If we can keep performance reasonably close, 
I'd take patches. I know it will be slower.
 
> > > I understand if you don't have a well developed list of items,
> > > but surely you must have some idea?
> > > 
> > > I'm willing to help here, and I suspect there might be some others as
> > > well, just let us know what the pressing issues are in the audit
> > > userspace.
> > 
> > People have been asking for ... {snip} ...
> 
> Right now I was asking for input on what would need to change in userspace
> to eliminate the reliance on field ordering, not a full TODO list.

Well, its this TODO list that makes it a bad time for me to even consider re-
doing something that is working fine. We can't have nice things because we keep 
mucking in the bottom layers.


> > > > We shouldn't be doing much modifying of records. They really should be
> > > > pretty static unless requirements change. For new records, there is no
> > > > guarantee that the function created before is the right information
> > > > for
> > > > the new event. It just depends on what it is as to what needs to be
> > > > collected. Then due to all the misused fields, we still need to have
> > > > review to make sure there's no typo. Speaking of which, I just found a
> > > > typo in AUDIT_FEATURE_CHANGE events.
> > > 
> > > We're seeing at least one case where our inability to change the
> > > ordering
> > > of the audit fields is causing problems.
> > 
> > What field ordering problem is preventing kernel work?
> 
> It is making Richard's patch undesirable.

I thought there was some agreement to write a function and use it. User space 
doesn't need re-writing for that.

-Steve

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

[Paul Moore]

On Wednesday, October 22, 2014 04:11:08 PM LC Bruzenak wrote:
> On 10/22/2014 03:44 PM, Paul Moore wrote:
> > We haven't changed anything yet, but I strongly believe we need to do away
> > with field ordering.  The good news is that if you explicitly search for
> > the field instead of relying on a fixed order the code should be more
> > robust and work either way. ;)
> 
> I have no doubt my old code looks like Steve's first example, not the
> second.
> But as I said, code can be changed if the assumptions about ordering are
> thrown out.

Well, like I said, It's probably safer that way as the code will work 
regardless.  Time to break bad habits :)

> You're making a pretty big splash over here Paul! Very impressive...
> 
> :-)

Yeah "splash" ... it's been an interesting week.

-- 
paul moore
security and virtualization @ redhat

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

[LC Bruzenak]

[-- Attachment #1.1: Type: text/plain, Size: 8139 bytes --]

On 10/22/2014 04:29 PM, Paul Moore wrote:
> Well, like I said, It's probably safer that way as the code will work 
> regardless.  Time to break bad habits :)
>

I hear you. But there's working and there's working well.
As long as we don't suffer a search response degradation by changing the
assumptive order, as I said, I'm OK with going back and reworking code.
If it makes searching real data unusable, it's now broken some
operational stuff.

As it is, I already have to move the last 24 hours off to a different
area and make searching go day-by-day only. Otherwise the logs are too big.
I chose a 100MB file size a while back as a sweet spot based on the
observation that each file could be parsed in some time acceptable to
security managers.



Just some info on the amount of data I've seen in real world examples:
On a normally-busy site, each day, we will generate ~1-4GB of events. It
used to be much more, but our team has managed to keep it down. I'd say
that often more time is spent reducing the events than analyzing them.
If not, they grow to a size you simply cannot parse in people-time. I
did have way more, but we've become better adept at spotting and
preventing event storms.

Here is a current test machine's audit log directory. I have to go look
to see what went wrong, obviously something did, but this kind of thing
can and does happen in the real world.
Note the time stamps. 100MB every minute or two is moving right along.
We push all audit data off to a collector machine so it doesn't dominate
the disks or otherwise hog the business server resources. BTW - funny
that I just happened to login to this machine to check its status.
Whatever was running amok stopped soon after 00:57. This sometimes just
happens. I've seen this go on for hours more in fielded systems. Almost
always something is really wrong, but the system is supposed to maintain
security and be usable through these times as well.

[root@audit audit]# ls -alt
total 4179016
-rw-------.  1 root root  84835735 Oct 23 02:49 audit.log
drwxr-x---.  2 root root      4096 Oct 23 00:57 .
-r--------.  1 root root 104857789 Oct 23 00:57 audit.log.1
-r--------.  1 root root 104857663 Oct 23 00:56 audit.log.2
-r--------.  1 root root 104857639 Oct 23 00:55 audit.log.3
-r--------.  1 root root 104857608 Oct 23 00:54 audit.log.4
-r--------.  1 root root 104857874 Oct 23 00:52 audit.log.5
-r--------.  1 root root 104857864 Oct 23 00:51 audit.log.6
-r--------.  1 root root 104857670 Oct 23 00:50 audit.log.7
-r--------.  1 root root 104857740 Oct 23 00:49 audit.log.8
-r--------.  1 root root 104857644 Oct 23 00:48 audit.log.9
-r--------.  1 root root 104857871 Oct 23 00:47 audit.log.10
-r--------.  1 root root 104857602 Oct 23 00:46 audit.log.11
-r--------.  1 root root 104857755 Oct 23 00:45 audit.log.12
-r--------.  1 root root 104857907 Oct 23 00:44 audit.log.13
-r--------.  1 root root 104857973 Oct 23 00:43 audit.log.14
-r--------.  1 root root 104857632 Oct 23 00:42 audit.log.15
-r--------.  1 root root 104857843 Oct 23 00:42 audit.log.16
-r--------.  1 root root 104857769 Oct 23 00:41 audit.log.17
-r--------.  1 root root 104857864 Oct 23 00:40 audit.log.18
-r--------.  1 root root 104857862 Oct 23 00:39 audit.log.19
-r--------.  1 root root 104857757 Oct 23 00:38 audit.log.20
-r--------.  1 root root 104857601 Oct 23 00:36 audit.log.21
-r--------.  1 root root 104857663 Oct 23 00:35 audit.log.22
-r--------.  1 root root 104857711 Oct 23 00:34 audit.log.23
-r--------.  1 root root 104857806 Oct 23 00:33 audit.log.24
-r--------.  1 root root 104857722 Oct 23 00:32 audit.log.25
-r--------.  1 root root 104857684 Oct 23 00:31 audit.log.26
-r--------.  1 root root 104857761 Oct 23 00:30 audit.log.27
-r--------.  1 root root 104857975 Oct 23 00:29 audit.log.28
-r--------.  1 root root 104857702 Oct 23 00:28 audit.log.29
-r--------.  1 root root 104857771 Oct 23 00:27 audit.log.30
-r--------.  1 root root 104857990 Oct 23 00:25 audit.log.31
-r--------.  1 root root 104857629 Oct 23 00:24 audit.log.32
-r--------.  1 root root 104857663 Oct 23 00:23 audit.log.33
-r--------.  1 root root 104857852 Oct 23 00:22 audit.log.34
-r--------.  1 root root 104857841 Oct 23 00:21 audit.log.35
-r--------.  1 root root 104857634 Oct 23 00:20 audit.log.36
-r--------.  1 root root 104857690 Oct 23 00:19 audit.log.37
-r--------.  1 root root 104857623 Oct 23 00:18 audit.log.38
-r--------.  1 root root 104857711 Oct 23 00:17 audit.log.39
-r--------.  1 root root 104857867 Oct 23 00:02 audit.log.40


I started an "aureport -i --summary". I realize my aureport and ausearch
don't use the auparse library, but they will soon, if not already, in
the newer code.

I just wanted to give you a feel for data amounts in your
considerations. Also I have designs for some new tools I want to deploy
which definitely will use libauparse, if it's usable.

At 4GB+ of audit data, this report isn't returning yet. The nice thing
is it isn't leaking memory, or if it is, it isn't gushing (from "top"):
31485 root      20   0  217m 104m  89m R 99.7  0.4  22:29.91 aureport

So I guess from a real user perspective I'm just worried that fixing an
order dependency will have adverse effects on retrieval.
So my vote would be to promote anything which makes searching/parsing
faster.
Or we need a different paradigm entirely.

I've been doing some other stuff while writing this. The aureport isn't
done after 37+ minutes:
31485 root      20   0  220m  96m  78m R 100.0  0.4  37:22.38 aureport

I realize if I were sitting inside a google infrastructure this would be
different, but I'm not.
So when guys try to understand what happened during, in this case, the
first hour of the day, the time it takes to return the answer can make
them think it's broken.

This is why to me, speed of parse/retrieval is important.
The aureport is still cranking:
31485 root      20   0  221m  76m  57m R 100.0  0.3  44:21.15 aureport

Now if a few queries happen at the same time, and they can, we start
seeing some real work on this machine.
It's a reasonable machine - dual quad core Xeon with 24GB of RAM -
dedicated to storage and retrieval of system audit data (from "cat
/proc/cpuinfo"):
...
processor    : 7
vendor_id    : GenuineIntel
cpu family    : 6
model        : 44
model name    : Intel(R) Xeon(R) CPU           X5677  @ 3.47GHz
stepping    : 2
cpu MHz        : 1600.000
cache size    : 12288 KB



So hopefully this gives you a sense of the way the audit data can stack
up; to me every millisecond/event saved is potentially hours earned.
Report is still running btw:
31485 root      20   0  222m 103m  83m R 99.9  0.4  56:18.64 aureport


From what I've read on the list, there are other users who have similar
loads.
I'm not sure if this makes any real difference in your thought process
or not, and you probably knew most of this anyway.
Just wanted to throw out some data points for consideration in case they
matter.

I waited until this morning to send this so I could see how long the
aureport took.

[root@audit audit]# time aureport -i --summary

Summary Report
======================
Range of time in logs: 10/23/2014 00:01:02.305 - 10/23/2014 09:58:40.745
Selected time for report: 10/23/2014 00:01:02 - 10/23/2014 09:58:40.745
Number of changes in configuration: 2892
Number of changes to accounts, groups, or roles: 0
Number of logins: 47
Number of failed logins: 2
Number of authentications: 2712
Number of failed authentications: 355
Number of users: 8
Number of terminals: 41
Number of host names: 7
Number of executables: 112
Number of files: 536772
Number of AVC's: 5791
Number of MAC events: 3045
Number of failed syscalls: 2502700
Number of anomaly events: 525
Number of responses to anomaly events: 0
Number of crypto events: 14
Number of keys: 36
Number of process IDs: 30494
Number of events: 8526658


real    428m11.405s
user    427m21.833s
sys    0m4.014s


Thank you!
LCB

-- 
LC (Lenny) Bruzenak
lenny@magitekltd.com



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

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

[Paul Moore]

On Thursday, October 23, 2014 09:19:49 AM LC Bruzenak wrote:
> On 10/22/2014 04:29 PM, Paul Moore wrote:
> > Well, like I said, It's probably safer that way as the code will work
> > regardless.  Time to break bad habits :)
> 
> I hear you. But there's working and there's working well.
> As long as we don't suffer a search response degradation by changing the
> assumptive order, as I said, I'm OK with going back and reworking code.
> If it makes searching real data unusable, it's now broken some
> operational stuff.

Performance is a big deal, I think we've all been hearing that for some time 
now.  I get it, and it is something that is and will remain *a* priority.  
However, this fixed ordering is something that is Just Plain Wrong and is 
likely to make life much more difficult for us as we try to improve audit.

-- 
paul moore
security and virtualization @ redhat

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

[Paul Moore]

On Wednesday, October 22, 2014 05:18:37 PM Steve Grubb wrote:
> On Wednesday, October 22, 2014 05:00:03 PM Paul Moore wrote:
> > On Wednesday, October 22, 2014 04:39:49 PM Steve Grubb wrote:
> > > Except you can have problems when the event is like this
> > > auid= pid= old uid= new uid= res=
> > 
> > I honestly don't see the problem here.
> 
> You'll never get new uid which is really the one you want.

Once again, I honestly don't see the problem here as I think we should be able 
to write a parser to handle this.
 
> > > > I disagree about the priority.  Eric disagrees about the priority.
> > > > Richard hasn't explicitly stated he disagrees with the priority but he
> > > > has  made several comments on this list about ordering being an issue
> > > > (Richard, my apologies if I am putting words in your mouth).
> 
> I thought it was a question of what to put in an event.

It is an issue of code reuse/duplication and how the fixed field ordering is 
turning the kernel code into a mess.

> > > What events do people need to change and why? There's not been any
> > > discussion that I know of saying we need to add fields or change them
> > > around.
> > 
> > See the earlier comments on Richard's patch.
> 
> He's making a new event. Its not changing things around.

See above.

> > > > What would we need to change in the userspace to eliminate the
> > > > reliance on field ordering?
> > > 
> > > Many of the utilities. ausyscall & autrace might be the only ones not
> > > affected.
> > 
> > So we would need to change ausyscall and autrace, possibly others.
> 
> Exactly the opposite, those are about the only ones clean because they are
> the only ones not parsing logs.

Gotcha, I misread that sentence.

> > Do you expect to need any changes to the deamon or audit libraries?
> 
> Not the daemon or library directly for this. But if you want to look into
> this, you'll need some really big logs for testing. You'll need at least
> 100MB to see performance variations. If we can keep performance reasonably
> close, I'd take patches. I know it will be slower.

Define "reasonably close".  Also, do you have any "really big" logs you use 
for testing?

-- 
paul moore
security and virtualization @ redhat

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

[Richard Guy Briggs]

On 14/10/21, Steve Grubb wrote:
> 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() ??
> > > 
> > > Its a shame we don't have a audit_log_task_info_light function which only
> > > records:
> > 
> > We already have audit_log_task() which gives:
> 
> > And while we are at it, refactor audit_log_task_info() to call
> > audit_log_task()?
> 
> That will cause problems at this point.

Yup.  We already have problems caused by not having this.

> > 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.

To clarify, if two orders are permitted per message type, one can be the
old one per message type, the second can be a standard order for all
messages, so that any given fields/keywords can be expected eventually
to found in this order, regardless of whether or not they are included
in that particular message type.

If we have a standard order in which keywords/fields are to be presented
then there will be no need to have as much duplicitous code in the
kernel and it will be much easier to get the order "right" in new
messages, but also much easier to scan any message to see if there is
information missing, similar, duplicated or superfluous.

> -Steve
> 
> > > -Steve
> > 
> > - RGB

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
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

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

[Richard Guy Briggs]

On 14/10/22, Paul Moore wrote:
> On Tuesday, October 21, 2014 09:24:05 PM Richard Guy Briggs wrote:
> > On 14/10/21, Paul Moore wrote:
> > > 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.
> 
> No, we're already seeing that a single fixed ordering is bad, adding an 
> alternate fixed ordering just kicks the can down the road a bit further and 
> sets a bad precedence for future development.  It is time to get rid of the 
> fixed ordering in the audit records.

The problem is that we don't just have a single fixed ordering.  We have
a different fixed ordering for each type of audit log message.  This
makes refactoring pretty much impossible or very inefficient at best.

I agree that eliminating that dependency on ordering would be a great
thing.  This sounds like a great time to reference Postel's Law or
Robustness Principle first introduced in IETF RFC760 and reworded in
RFC1122: "Be conservative in what you send, be liberal in what you
accept".

> paul moore

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
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

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

[Richard Guy Briggs]

On 14/10/22, Steve Grubb wrote:
> On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > On Tuesday, October 21, 2014 03:56:10 PM Steve Grubb wrote:
> > Before we go to much farther, I'd really like us to agree that ordering is
> > not important, can we do that? 
> 
> Its kind of doubtful we can do anything like this quickly. Maybe over time. 
> But for entirely new events, we can create some canonical order and use it.

Good.  Where do we start?  Alphabetical order seems like an obvious but
not very useful order.

How about an order based on classes of fields and importance or length
of data within them?  Start with who (subject), did what (verb) to what
(object) with what result.  Within each of those, have a standard order.
Can that go in the "Audit Event Parsing Library Specifications"?

There is also the standardization of field keywords that has already had
some action/correction.

> -Steve

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
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

Typo in AUDIT_FEATURE_CHANGE events [was: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket]

[Richard Guy Briggs]

On 14/10/22, Steve Grubb wrote:
> Speaking of which, I just found a typo in 
> AUDIT_FEATURE_CHANGE events.

Just so I don't lose this, what's the problem there?  I don't see a
typo, but question the field names.

	audit_log_format(ab, "feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d",                 

> -Steve

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
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

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

[Richard Guy Briggs]

On 14/10/22, Steve Grubb wrote:
> On Wednesday, October 22, 2014 04:06:47 PM Paul Moore wrote:
> > On Wednesday, October 22, 2014 01:56:13 PM Steve Grubb wrote:
> > > On Wednesday, October 22, 2014 11:28:46 AM Paul Moore wrote:
> > > > On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> > > > > On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > > > > > 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 ;)
> > > > > 
> > > > > Field ordering is important. For example, suppose we decide to make
> > > > > ordering changes to the AUDIT_AVC record to bring it in line with
> > > > > current standards. Would anyone care?
> > > > 
> > > > That is an interesting example record considering everyone recognizes it
> > > > to be an oddly formed, special case.
> > > 
> > > But it illustrates the point. There are tools that depend on an ordering
> > > and format. There are more programs that just ausearch that needs to be
> > > considered if the fields change. For example, Someone could do things
> > > like this:
> > > 
> > > retval = auparse_find_field(au, "auid");
> > > retval = auparse_next_field(au);
> > > retval = auparse_next_field(au);
> > > retval = auparse_find_field(au, res");
> > > 
> > > Where, if the field ordering can't be guaranteed, the code becomes:
> > > 
> > > retval = auparse_find_field(au, "auid");
> > > retval = auparse_first_field(au);
> > > retval = auparse_find_field(au, "pid");
> > > retval = auparse_first_field(au);
> > > retval = auparse_find_field(au, "uid");
> > > retval = auparse_first_field(au);
> > > retval = auparse_find_field(au, res");
> > 
> > In my mind the latter code is more robust and preferable.
> 
> Except you can have problems when the event is like this
> auid= pid= old uid= new uid= res=
> 
> and yes there are places like that. The performance really is the main issue.

And this is the type of thing that needs to be cleaned up,
disambiguating which field you actually want.  Both "old" and "new" are
orphaned keywords that you have indicated are ignored by the parser, so
should be cleaned up to old_uid= and uid=, according to the rules you
have set out.

> -Steve

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
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

Re: Typo in AUDIT_FEATURE_CHANGE events [was: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket]

[Steve Grubb]

On Thursday, October 30, 2014 10:48:28 AM Richard Guy Briggs wrote:
> On 14/10/22, Steve Grubb wrote:
> > Speaking of which, I just found a typo in
> > AUDIT_FEATURE_CHANGE events.
> 
> Just so I don't lose this, what's the problem there?  I don't see a
> typo, but question the field names.
> 
> 	audit_log_format(ab, "feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d",

You need to start feature= with a space. For example, see how it gets
appended to subj=:

time->Mon Oct 27 16:11:21 2014
type=FEATURE_CHANGE msg=audit(1414440681.713:17710):  ppid=13599 pid=13618 auid=4294967295
 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="auditctl"
 exe="/usr/sbin/auditctl" subj=system_u:system_r:auditctl_t:s0feature=loginuid_immutable old=0 new=1 
old_lock=0 new_lock=1 res=1


-Steve

Re: Typo in AUDIT_FEATURE_CHANGE events [was: Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket]

[Richard Guy Briggs]

On 14/10/30, Steve Grubb wrote:
> On Thursday, October 30, 2014 10:48:28 AM Richard Guy Briggs wrote:
> > On 14/10/22, Steve Grubb wrote:
> > > Speaking of which, I just found a typo in
> > > AUDIT_FEATURE_CHANGE events.
> > 
> > Just so I don't lose this, what's the problem there?  I don't see a
> > typo, but question the field names.
> > 
> > 	audit_log_format(ab, "feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d",
> 
> You need to start feature= with a space. For example, see how it gets
> appended to subj=:
> 
> time->Mon Oct 27 16:11:21 2014
> type=FEATURE_CHANGE msg=audit(1414440681.713:17710):  ppid=13599 pid=13618 auid=4294967295
>  uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="auditctl"
>  exe="/usr/sbin/auditctl" subj=system_u:system_r:auditctl_t:s0feature=loginuid_immutable old=0 new=1 
> old_lock=0 new_lock=1 res=1

Got it, thanks.

> -Steve

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
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