Re: [PATCH 1/1] audit: add missing fields to AUDIT_CONFIG_CHANGE event

On Thursday, October 12, 2017 6:51:19 PM EDT Paul Moore wrote: I'm speaking on behalf of everyone that has to deal with the events. If we need to have 9 parsers for the same event, its really doing a disservice to everyone that works on audit events. Besides, since you don't write any user space code or do things that have to make sense of it, why would you block something that is fixing problems? Did you run any tests with the events I supplied to verify things for yourself? Look at this mess: 1) audit_log_format(ab, "%s=%u old=%u", function_name, new, old); audit_log_session_info(ab); rc = audit_log_task_context(ab); 2) audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); audit_log_format(ab, " audit_enabled=%d res=0", audit_enabled); 3) audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); audit_log_format(ab, " op=trim res=1"); 4) audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); audit_log_format(ab, " op=make_equiv old="); audit_log_untrustedstring(ab, old); audit_log_format(ab, " new="); audit_log_untrustedstring(ab, new); audit_log_format(ab, " res=%d", !err); 5) audit_log_common_recv_msg(&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, s.log_passwd, !err); 6) audit_log_format(ab, "auid=%u ses=%u" ,loginuid, sessionid); audit_log_task_context(ab); audit_log_format(ab, " op=%s", action); audit_log_key(ab, rule->filterkey); audit_log_format(ab, " list=%d res=%d", rule->listnr, res); 7) audit_log_format(ab, "auid=%u ses=%u op=%s", from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current), op); audit_log_format(ab, " path="); audit_log_untrustedstring(ab, audit_mark->path); audit_log_key(ab, rule->filterkey); audit_log_format(ab, " list=%d res=1", rule->listnr); 8) 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); audit_log_format(ab, " list=%d res=1", rule->listnr); 9) audit_log_format(ab, "auid=%u ses=%u op=%s", from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current), op); audit_log_format(ab, " path="); audit_log_untrustedstring(ab, w->path); audit_log_key(ab, r->filterkey); audit_log_format(ab, " list=%d res=1", r->listnr); Of these 7 & 9 are the same. So that means following your suggestion, everyone has to write 8 parsers for the same event. Does that sound like good engineering practice? Also, it will cause subtle changes that break bigger rules than this - like all events everywhere end with the results. They are always the last item. Everywhere. This is a self evident rule that you are suggesting we break. We can't do that. Meanwhile...there is a problem that needs to be fixed.... -Steve