[PATCH ghak59 V3 2/4] audit: add syscall information to CONFIG_CHANGE records
Paul Moore
paul at paul-moore.com
Thu Jan 17 17:58:14 UTC 2019
On Thu, Jan 17, 2019 at 10:34 AM Richard Guy Briggs <rgb at redhat.com> wrote:
>
> On 2019-01-17 08:21, Paul Moore wrote:
> > On Thu, Jan 17, 2019 at 4:33 AM Steve Grubb <sgrubb at redhat.com> wrote:
> > > On Mon, 14 Jan 2019 17:58:58 -0500 Paul Moore <paul at paul-moore.com> wrote:
> > > > On Mon, Dec 10, 2018 at 5:18 PM Richard Guy Briggs <rgb at redhat.com> wrote:
> > > > > Tie syscall information to all CONFIG_CHANGE calls since they are
> > > > > all a result of user actions.
> > >
> > > Please don't tie syscall information to this. The syscall will be
> > > sendto. We don't need that information, its implicit. Also, doing this
> > > will possibly wreck things in libauparse. Please test the events with
> > > ausearch --format csv and --format text. IFF the event looks better or
> > > the same should we do this. If stuff disappears, the patch is
> > > breaking things
> >
> > We've discussed this quite a bit already; connecting associated
> > records into a single event is something that should happen, needs to
> > happen, and will happen. Conceptually it makes no sense to record the
> > syscall (and any other associated records) which triggers the audit
> > configuration change, and the configuration change record itself as
> > two distinct events - they are the same event. We've also heard from
> > a prominent user that associating records in this way is desirable.
> >
> > If the ausearch csv and text audit log transformations can't handle
> > this particular change, I would consider that a shortcoming of that
> > code. We have multi-record events now, and this is only going to
> > increase in the future.
> >
> > Richard, if you can't make the requested changes to this patch and
> > resubmit by ... let's say the middle of next week? that should be
> > enough time, yes? ... please let me know and I'll make the changes and
> > get this merged.
>
> I would do the change, which should be very trivial, but I'm dense
> enough that I still don't know what you want. In the last 6 months I've
> asked a number of direct questions that have not been directly
> addressed. Perhaps I should be able to figure it out from the more
> general or fundamental principles replies I've gotten (which have been
> helpful, but perhaps incomplete), but I'm still having some trouble.
> Perhaps I'm exposing my limitations.
Since code is unambiguous, let me just cut and paste what I was
thinking (be warned, this is a cut-n-paste, so the whitespace is
probably mangled):
diff --git a/kernel/audit.c b/kernel/audit.c
index d412fb4ae6d5..d2caef6ef09e 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -396,7 +396,7 @@ static int audit_log_config_change(char
*function_name, u32 new, u32 old,
struct audit_buffer *ab;
int rc = 0;
- ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+ ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
if (unlikely(!ab))
return rc;
audit_log_format(ab, "op=set %s=%u old=%u ", function_name, new, old);
@@ -1053,7 +1053,8 @@ static int audit_netlink_ok(struct sk_buff *skb,
u16 msg_type)
return err;
}
-static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
+static void audit_log_common_recv_msg(struct audit_context *context,
+ struct audit_buffer **ab, u16 msg_type)
{
uid_t uid = from_kuid(&init_user_ns, current_uid());
pid_t pid = task_tgid_nr(current);
@@ -1063,7 +1064,7 @@ static void audit_log_common_recv_msg(struct
audit_buffer **ab, u16 msg_type)
return;
}
- *ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
+ *ab = audit_log_start(context, GFP_KERNEL, msg_type);
if (unlikely(!*ab))
return;
audit_log_format(*ab, "pid=%d uid=%u ", pid, uid);
@@ -1071,6 +1072,11 @@ static void audit_log_common_recv_msg(struct
audit_buffer **ab, u16 msg_type)
audit_log_task_context(*ab);
}
+static inline void audit_log_user_recv_msg(struct audit_buffer **ab,
u16 msg_type)
+{
+ audit_log_common_recv_msg(NULL, ab, msg_type);
+}
+
int is_audit_feature_set(int i)
{
return af.features & AUDIT_FEATURE_TO_MASK(i);
@@ -1338,7 +1344,7 @@ static int audit_receive_msg(struct sk_buff
*skb, struct nlmsghdr *nlh)
if (err)
break;
}
- audit_log_common_recv_msg(&ab, msg_type);
+ audit_log_user_recv_msg(&ab, msg_type);
if (msg_type != AUDIT_USER_TTY)
audit_log_format(ab, " msg='%.*s'",
AUDIT_MESSAGE_TEXT_MAX,
@@ -1361,7 +1367,8 @@ static int audit_receive_msg(struct sk_buff
*skb, struct nlmsghdr *nlh)
if (nlmsg_len(nlh) < sizeof(struct audit_rule_data))
return -EINVAL;
if (audit_enabled == AUDIT_LOCKED) {
- audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+ audit_log_common_recv_msg(audit_context(), &ab,
+ AUDIT_CONFIG_CHANGE);
audit_log_format(ab, " op=%s audit_enabled=%d res=0",
msg_type == AUDIT_ADD_RULE ?
"add_rule" : "remove_rule",
@@ -1376,7 +1383,8 @@ static int audit_receive_msg(struct sk_buff
*skb, struct nlmsghdr *nlh)
break;
case AUDIT_TRIM:
audit_trim_trees();
- audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+ audit_log_common_recv_msg(audit_context(), &ab,
+ AUDIT_CONFIG_CHANGE);
audit_log_format(ab, " op=trim res=1");
audit_log_end(ab);
break;
@@ -1406,7 +1414,8 @@ static int audit_receive_msg(struct sk_buff
*skb, struct nlmsghdr *nlh)
/* OK, here comes... */
err = audit_tag_tree(old, new);
- audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+ audit_log_common_recv_msg(audit_context(), &ab,
+ AUDIT_CONFIG_CHANGE);
audit_log_format(ab, " op=make_equiv old=");
audit_log_untrustedstring(ab, old);
@@ -1474,7 +1483,8 @@ static int audit_receive_msg(struct sk_buff
*skb, struct nlmsghdr *nlh)
old.enabled = t & AUDIT_TTY_ENABLE;
old.log_passwd = !!(t & AUDIT_TTY_LOG_PASSWD);
- audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+ audit_log_common_recv_msg(audit_context(), &ab,
+ AUDIT_CONFIG_CHANGE);
audit_log_format(ab, " op=tty_set old-enabled=%d new-enabled=%d"
" old-log_passwd=%d new-log_passwd=%d res=%d",
old.enabled, s.enabled, old.log_passwd,
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
index cf4512a33675..37ae95cfb7f4 100644
--- a/kernel/audit_fsnotify.c
+++ b/kernel/audit_fsnotify.c
@@ -127,7 +127,7 @@ static void audit_mark_log_rule_change(struct
audit_fsnotify_mark *audit_mark, c
if (!audit_enabled)
return;
- ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+ ab = audit_log_start(audit_context(), GFP_NOFS, AUDIT_CONFIG_CHANGE);
if (unlikely(!ab))
return;
audit_log_session_info(ab);
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 20ef9ba134b0..e8d1adeb2223 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -242,7 +242,7 @@ static void audit_watch_log_rule_change(struct
audit_krule *r, struct audit_watc
if (!audit_enabled)
return;
- ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+ ab = audit_log_start(audit_context(), GFP_NOFS, AUDIT_CONFIG_CHANGE);
if (!ab)
return;
audit_log_session_info(ab);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index bf309f2592c4..26a80a9d43a9 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1091,7 +1091,7 @@ static void audit_log_rule_change(char *action,
struct audit_krule *rule, int re
if (!audit_enabled)
return;
- ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+ ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
if (!ab)
return;
audit_log_session_info(ab);
--
paul moore
www.paul-moore.com
More information about the Linux-audit
mailing list