Skip to contentSkip to conversation

Fix formatting of AUDIT_CONFIG_CHANGE events

PATCH In review Steve Grubb 3 participants 3 messages Nov 16, 2016 – Nov 20, 2016 Audit

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix formatting of AUDIT_CONFIG_CHANGE events

The AUDIT_CONFIG_CHANGE events sometimes use a op= field. The current code logs the value of the field with quotes. This field is documented to not be encoded, so it should not have quotes.

---

kernel/auditfilter.c +1 -2
diff -urp vanilla-4.9-rc5.orig/kernel/auditfilter.c vanilla-4.9-rc5/kernel/auditfilter.c
@@ -1074,8 +1074,7 @@ static void audit_log_rule_change(char *
 		return;
 	audit_log_format(ab, "auid=%u ses=%u" ,loginuid, sessionid);
 	audit_log_task_context(ab);
-	audit_log_format(ab, " op=");
-	audit_log_string(ab, action);
+	audit_log_format(ab, " op=%s", action);
 	audit_log_key(ab, rule->filterkey);
 	audit_log_format(ab, " list=%d res=%d", rule->listnr, res);
 	audit_log_end(ab);
kernel/audit_fsnotify.c +2 -3
diff -urp vanilla-4.9-rc5.orig/kernel/audit_fsnotify.c vanilla-4.9-rc5/kernel/audit_fsnotify.c
@@ -130,10 +130,9 @@ static void audit_mark_log_rule_change(s
 	ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
-	audit_log_format(ab, "auid=%u ses=%u op=",
+	audit_log_format(ab, "auid=%u ses=%u op=%s",
 			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
-			 audit_get_sessionid(current));
-	audit_log_string(ab, op);
+			 audit_get_sessionid(current), op);
 	audit_log_format(ab, " path=");
 	audit_log_untrustedstring(ab, audit_mark->path);
 	audit_log_key(ab, rule->filterkey);
kernel/audit_tree.c +1 -2
diff -urp vanilla-4.9-rc5.orig/kernel/audit_tree.c vanilla-4.9-rc5/kernel/audit_tree.c
@@ -458,8 +458,7 @@ static void audit_tree_log_remove_rule(s
 	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
-	audit_log_format(ab, "op=");
-	audit_log_string(ab, "remove_rule");
+	audit_log_format(ab, "op=remove_rule");
 	audit_log_format(ab, " dir=");
 	audit_log_untrustedstring(ab, rule->tree->pathname);
 	audit_log_key(ab, rule->filterkey);
kernel/audit_watch.c +2 -3
diff -urp vanilla-4.9-rc5.orig/kernel/audit_watch.c vanilla-4.9-rc5/kernel/audit_watch.c
@@ -242,10 +242,9 @@ static void audit_watch_log_rule_change(
 		ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
 		if (unlikely(!ab))
 			return;
-		audit_log_format(ab, "auid=%u ses=%u op=",
+		audit_log_format(ab, "auid=%u ses=%u op=%s",
 				 from_kuid(&init_user_ns, audit_get_loginuid(current)),
-				 audit_get_sessionid(current));
-		audit_log_string(ab, op);
+				 audit_get_sessionid(current), op);
 		audit_log_format(ab, " path=");
 		audit_log_untrustedstring(ab, w->path);
 		audit_log_key(ab, r->filterkey);
Reviewed-by on v1 1/1
#1 lore raw
Quoted 3 lines · Steve Grubb

The AUDIT_CONFIG_CHANGE events sometimes use a op= field. The current code logs the value of the field with quotes. This field is documented to not be encoded, so it should not have quotes.

There were a number of callers that had spaces in their "op" or "action" strings which I've addressed with several upstream patches, so this is the right direction to go. All the callers for the instances listed below are fine.

Quoted 1 line
Quoted 62 lines

---

diff -urp vanilla-4.9-rc5.orig/kernel/auditfilter.c vanilla-4.9-rc5/kernel/auditfilter.c

kernel/auditfilter.c +1 -2
@@ -1074,8 +1074,7 @@ static void audit_log_rule_change(char *
 		return;
 	audit_log_format(ab, "auid=%u ses=%u" ,loginuid, sessionid);
 	audit_log_task_context(ab);
-	audit_log_format(ab, " op=");
-	audit_log_string(ab, action);
+	audit_log_format(ab, " op=%s", action);
 	audit_log_key(ab, rule->filterkey);
 	audit_log_format(ab, " list=%d res=%d", rule->listnr, res);
 	audit_log_end(ab);

diff -urp vanilla-4.9-rc5.orig/kernel/audit_fsnotify.c vanilla-4.9-rc5/kernel/audit_fsnotify.c

kernel/audit_fsnotify.c +2 -3
@@ -130,10 +130,9 @@ static void audit_mark_log_rule_change(s
 	ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
-	audit_log_format(ab, "auid=%u ses=%u op=",
+	audit_log_format(ab, "auid=%u ses=%u op=%s",
 			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
-			 audit_get_sessionid(current));
-	audit_log_string(ab, op);
+			 audit_get_sessionid(current), op);
 	audit_log_format(ab, " path=");
 	audit_log_untrustedstring(ab, audit_mark->path);
 	audit_log_key(ab, rule->filterkey);

diff -urp vanilla-4.9-rc5.orig/kernel/audit_tree.c vanilla-4.9-rc5/kernel/audit_tree.c

kernel/audit_tree.c +1 -2
@@ -458,8 +458,7 @@ static void audit_tree_log_remove_rule(s
 	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
-	audit_log_format(ab, "op=");
-	audit_log_string(ab, "remove_rule");
+	audit_log_format(ab, "op=remove_rule");
 	audit_log_format(ab, " dir=");
 	audit_log_untrustedstring(ab, rule->tree->pathname);
 	audit_log_key(ab, rule->filterkey);

diff -urp vanilla-4.9-rc5.orig/kernel/audit_watch.c vanilla-4.9-rc5/kernel/audit_watch.c

kernel/audit_watch.c +2 -4
@@ -242,10 +242,9 @@ static void audit_watch_log_rule_change(
 		ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
 		if (unlikely(!ab))
 			return;
-		audit_log_format(ab, "auid=%u ses=%u op=",
+		audit_log_format(ab, "auid=%u ses=%u op=%s",
 				 from_kuid(&init_user_ns, audit_get_loginuid(current)),
-				 audit_get_sessionid(current));
-		audit_log_string(ab, op);
+				 audit_get_sessionid(current), op);
 		audit_log_format(ab, " path=");
 		audit_log_untrustedstring(ab, w->path);
 		audit_log_key(ab, r->filterkey);
 
--
Quoted 3 lines

Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <[email protected]>
Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

maintainer
#2 lore raw
In reply to Steve Grubb
Quoted 5 lines · Steve Grubb

The AUDIT_CONFIG_CHANGE events sometimes use a op= field. The current code logs the value of the field with quotes. This field is documented to not be encoded, so it should not have quotes.

Looks good to me, merged.

Signature · 2 lines
paul moore
www.paul-moore.com