Discussion:
[RFC][PATCH] audit: log join and part events to the read-only multicast log socket
(too old to reply)
Richard Guy Briggs
2014-10-07 18:23:19 UTC
Permalink
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 <***@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
Eric Paris
2014-10-07 19:03:14 UTC
Permalink
Post by 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.
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
---
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() ??
Post by Richard Guy Briggs
+ audit_log_format(ab, " group=%d", group);
group seems like too easily confused a name.
Post by Richard Guy Briggs
+ 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);
Richard Guy Briggs
2014-10-07 19:39:51 UTC
Permalink
Post by Eric Paris
Post by 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.
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
---
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?
Post by Eric Paris
Post by Richard Guy Briggs
+ audit_log_format(ab, " group=%d", group);
group seems like too easily confused a name.
"multicast_group" or "mc_group"?
Post by Eric Paris
Post by Richard Guy Briggs
+ 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 <***@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
Paul Moore
2014-10-07 22:06:51 UTC
Permalink
Post by Richard Guy Briggs
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
Steve Grubb
2014-10-11 15:42:06 UTC
Permalink
On Tue, 07 Oct 2014 18:06:51 -0400
Post by Paul Moore
Post by Richard Guy Briggs
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
Paul Moore
2014-10-11 20:00:30 UTC
Permalink
Post by Steve Grubb
On Tue, 07 Oct 2014 18:06:51 -0400
Post by Paul Moore
Post by Richard Guy Briggs
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
Richard Guy Briggs
2014-10-21 16:41:09 UTC
Permalink
Post by Richard Guy Briggs
Post by Eric Paris
Post by 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.
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.
Post by Richard Guy Briggs
- RGB
- RGB

--
Richard Guy Briggs <***@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
Steve Grubb
2014-10-21 19:56:10 UTC
Permalink
Post by Eric Paris
Post by 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.
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>
---
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=
Post by Eric Paris
Post by Richard Guy Briggs
+ 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
Richard Guy Briggs
2014-10-21 21:08:22 UTC
Permalink
Post by Steve Grubb
Post by Eric Paris
Post by 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.
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>
---
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
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?
Post by Steve Grubb
Post by Eric Paris
Post by Richard Guy Briggs
+ 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".
Post by Steve Grubb
-Steve
- RGB

--
Richard Guy Briggs <***@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
Steve Grubb
2014-10-21 21:40:43 UTC
Permalink
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Eric Paris
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
pid= auid= uid= subj= comm= exe= ses= tty=
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.
Post by Richard Guy Briggs
And while we are at it, refactor audit_log_task_info() to call
audit_log_task()?
That will cause problems at this point.
Post by Richard Guy Briggs
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.
Post by Richard Guy Briggs
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
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Eric Paris
Post by Richard Guy Briggs
+ 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".
Post by Steve Grubb
-Steve
- RGB
--
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
Richard Guy Briggs
2014-10-29 20:23:15 UTC
Permalink
Post by Steve Grubb
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Eric Paris
super crazy yuck. audit_log_task_info() ??
Its a shame we don't have a audit_log_task_info_light function which only
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.
Post by Steve Grubb
Post by Richard Guy Briggs
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.
Post by Steve Grubb
-Steve
Post by Richard Guy Briggs
Post by Steve Grubb
-Steve
- RGB
- RGB

--
Richard Guy Briggs <***@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
Eric Paris
2014-10-21 22:30:29 UTC
Permalink
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Eric Paris
Post by 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.
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>
---
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
pid= auid= uid= subj= comm= exe= ses= tty=
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....
Paul Moore
2014-10-21 23:14:16 UTC
Permalink
Post by Eric Paris
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...
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)
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
Richard Guy Briggs
2014-10-22 01:18:36 UTC
Permalink
Post by Eric Paris
Post by Richard Guy Briggs
Post by Steve Grubb
Post by Eric Paris
Post by 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.
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>
---
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
pid= auid= uid= subj= comm= exe= ses= tty=
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...
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)
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 <***@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
Steve Grubb
2014-10-22 14:30:43 UTC
Permalink
Post by Eric Paris
Post by Richard Guy Briggs
Post by Steve Grubb
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
pid= auid= uid= subj= comm= exe= ses= tty=
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.
Post by Eric Paris
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
Paul Moore
2014-10-21 22:30:24 UTC
Permalink
Post by Steve Grubb
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
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
Richard Guy Briggs
2014-10-22 01:24:05 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
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
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.
Post by Paul Moore
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.
Post by Paul Moore
paul moore
- RGB

--
Richard Guy Briggs <***@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
Paul Moore
2014-10-22 13:34:41 UTC
Permalink
[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.]
Post by Richard Guy Briggs
Post by Paul Moore
Post by Steve Grubb
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
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.
Post by Richard Guy Briggs
Post by Paul Moore
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
Richard Guy Briggs
2014-10-29 21:09:59 UTC
Permalink
Post by Paul Moore
Post by Richard Guy Briggs
Post by Paul Moore
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".
Post by Paul Moore
paul moore
- RGB

--
Richard Guy Briggs <***@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
Steve Grubb
2014-10-22 14:34:40 UTC
Permalink
Post by Richard Guy Briggs
Post by Paul Moore
Post by Steve Grubb
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
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.
Post by Richard Guy Briggs
Post by Paul Moore
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
Steve Grubb
2014-10-22 14:25:35 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
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
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?
Post by Paul Moore
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.
Post by Paul Moore
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
Eric Paris
2014-10-22 14:30:12 UTC
Permalink
Post by Steve Grubb
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'
Steve Grubb
2014-10-22 14:36:49 UTC
Permalink
Post by Eric Paris
Post by Steve Grubb
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
Eric Paris
2014-10-22 15:08:02 UTC
Permalink
Post by Steve Grubb
Post by Eric Paris
Post by Steve Grubb
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...
Eric Paris
2014-10-22 15:12:05 UTC
Permalink
Post by Steve Grubb
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.
Post by Steve Grubb
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...
Post by Steve Grubb
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.
Post by Steve Grubb
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...
Post by Steve Grubb
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.
Post by Steve Grubb
9) Can we get events for a watched file even when a user's permissions do not
allow full path resolution?
No.
Post by Steve Grubb
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...
Post by Steve Grubb
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...
Post by Steve Grubb
13) Is audit by process name ready to go?
That's up to you/paul/richard.
Post by Steve Grubb
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 :)
LC Bruzenak
2014-10-22 15:51:49 UTC
Permalink
Post by Eric Paris
Post by Steve Grubb
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.
...
Post by Eric Paris
Post by Steve Grubb
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
***@magitekltd.com
Steve Grubb
2014-10-22 16:24:54 UTC
Permalink
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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?
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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
Eric Paris
2014-10-22 18:18:00 UTC
Permalink
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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 :-(
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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)
LC Bruzenak
2014-10-22 19:36:43 UTC
Permalink
Post by Eric Paris
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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...
Post by Eric Paris
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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
***@magitekltd.com
Steve Grubb
2014-10-22 20:00:25 UTC
Permalink
Post by LC Bruzenak
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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.
Post by LC Bruzenak
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?
Post by LC Bruzenak
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).
Post by LC Bruzenak
(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 :-(
Post by LC Bruzenak
Post by Eric Paris
Post by Steve Grubb
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
Paul Moore
2014-10-22 15:28:46 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
Post by Paul Moore
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?
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
To me, these are the burning issues that I think should be on the table to
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
Steve Grubb
2014-10-22 17:56:13 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Paul Moore
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.
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Paul Moore
What, besides some assumptions by the
userspace tools, is preventing us from fixing this?
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
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.
Post by Paul Moore
Post by Steve Grubb
To me, these are the burning issues that I think should be on the table to
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
Paul Moore
2014-10-22 20:06:47 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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
retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");
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.
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
What, besides some assumptions by the userspace tools, is preventing us
from fixing this?
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
To me, these are the burning issues that I think should be on the table
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
LC Bruzenak
2014-10-22 20:34:24 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
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
retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");
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
***@magitekltd.com
Paul Moore
2014-10-22 20:44:05 UTC
Permalink
Post by LC Bruzenak
Post by Paul Moore
Post by Steve Grubb
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
retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");
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.
:-)
:)
Post by LC Bruzenak
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
LC Bruzenak
2014-10-22 21:11:08 UTC
Permalink
Post by Paul Moore
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
***@magitekltd.com
Paul Moore
2014-10-22 21:29:55 UTC
Permalink
Post by LC Bruzenak
Post by Paul Moore
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 :)
Post by LC Bruzenak
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
LC Bruzenak
2014-10-23 14:19:49 UTC
Permalink
Post by Paul Moore
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.

[***@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.

[***@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
***@magitekltd.com
Paul Moore
2014-10-23 19:08:04 UTC
Permalink
Post by LC Bruzenak
Post by Paul Moore
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
Steve Grubb
2014-10-22 20:39:49 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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
retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");
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.
Post by Paul Moore
Post by Steve Grubb
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.
Post by Paul Moore
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.
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Paul Moore
Does the audit userspace still live in SVN on fedorahosted.org?
Yes.
Post by Paul Moore
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.
Post by Paul Moore
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.
Post by Paul Moore
Post by Steve Grubb
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?
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
To me, these are the burning issues that I think should be on the table
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
Paul Moore
2014-10-22 21:00:03 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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
retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");
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.
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
Post by Paul Moore
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?
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
To me, these are the burning issues that I think should be on the
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
Steve Grubb
2014-10-22 21:18:37 UTC
Permalink
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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
retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");
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.
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Paul Moore
Post by Steve Grubb
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.
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Paul Moore
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.
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
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
Paul Moore
2014-10-23 19:15:32 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
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.
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
Post by Paul Moore
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
Richard Guy Briggs
2014-10-30 14:55:40 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
Post by Steve Grubb
Post by Paul Moore
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
retval = auparse_find_field(au, "auid");
retval = auparse_next_field(au);
retval = auparse_next_field(au);
retval = auparse_find_field(au, res");
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.
Post by Steve Grubb
-Steve
- RGB

--
Richard Guy Briggs <***@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
Richard Guy Briggs
2014-10-30 14:48:28 UTC
Permalink
Post by Steve Grubb
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",
Post by Steve Grubb
-Steve
- RGB

--
Richard Guy Briggs <***@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
Steve Grubb
2014-10-30 15:10:48 UTC
Permalink
Post by Richard Guy Briggs
Post by Steve Grubb
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
Richard Guy Briggs
2014-10-30 15:23:10 UTC
Permalink
Post by Steve Grubb
Post by Richard Guy Briggs
Post by Steve Grubb
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
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.
Post by Steve Grubb
-Steve
- RGB

--
Richard Guy Briggs <***@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
Richard Guy Briggs
2014-10-29 21:38:18 UTC
Permalink
Post by Steve Grubb
Post by Paul Moore
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.
Post by Steve Grubb
-Steve
- RGB

--
Richard Guy Briggs <***@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
Continue reading on narkive:
Loading...