On 2017-10-12 19:57, Steve Grubb wrote:
There are very important fields necessary to understand who is
adding
audit rules and a little more context about the environment in which
its happening. This adds pid, uid, tty, subj, comm, and exe
information to the event. These are required fields.
Could we add an issue number to the patch description?
Eg: See:
https://github.com/linux-audit/audit-kernel/issues/59
Signed-off-by: sgrubb <sgrubb(a)redhat.com>
---
kernel/audit_watch.c | 23 +++++++++++++++++++----
kernel/auditfilter.c | 18 +++++++++++++++---
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 9eb8b3511636..63abc2ba1372 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -239,14 +239,29 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch
*old)
static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w,
char *op)
{
if (audit_enabled) {
+ struct tty_struct *tty;
+ const struct cred *cred;
struct audit_buffer *ab;
+ char comm[sizeof(current->comm)];
+
ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
if (unlikely(!ab))
return;
- 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=");
+
+ tty = audit_get_tty(current);
+ cred = current_cred();
+ audit_log_format(ab, "pid=%d uid=%u auid=%u tty=%s ses=%u",
+ task_tgid_nr(current),
+ from_kuid(&init_user_ns, cred->uid),
+ from_kuid(&init_user_ns,
+ audit_get_loginuid(current)),
+ tty ? tty_name(tty) : "(none)",
+ audit_get_sessionid(current));
+ audit_log_task_context(ab);
+ audit_log_format(ab, " comm=");
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ audit_log_d_path_exe(ab, current->mm);
+ audit_log_format(ab, "op=%s path=", op);
audit_log_untrustedstring(ab, w->path);
audit_log_key(ab, r->filterkey);
audit_log_format(ab, " list=%d res=1", r->listnr);
As noted, audit_put_tty(tty) is missing...
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 0b0aa5854dac..5e2a953da29a 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1065,17 +1065,29 @@ static void audit_list_rules(int seq, struct sk_buff_head *q)
static void audit_log_rule_change(char *action, struct audit_krule *rule, int res)
{
struct audit_buffer *ab;
- uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(current));
- unsigned int sessionid = audit_get_sessionid(current);
+ struct tty_struct *tty;
+ const struct cred *cred;
+ char comm[sizeof(current->comm)];
if (!audit_enabled)
return;
+ tty = audit_get_tty(current);
+ cred = current_cred();
+
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
if (!ab)
return;
- audit_log_format(ab, "auid=%u ses=%u" ,loginuid, sessionid);
+ audit_log_format(ab, "pid=%d uid=%u auid=%u tty=%s ses=%u",
+ task_tgid_nr(current),
+ from_kuid(&init_user_ns, cred->uid),
+ from_kuid(&init_user_ns, audit_get_loginuid(current)),
+ tty ? tty_name(tty) : "(none)",
+ audit_get_sessionid(current));
audit_log_task_context(ab);
+ audit_log_format(ab, " comm=");
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ audit_log_d_path_exe(ab, current->mm);
audit_log_format(ab, " op=%s", action);
audit_log_key(ab, rule->filterkey);
audit_log_format(ab, " list=%d res=%d", rule->listnr, res);
Again, missing audit_put_tty(tty).
Since these are already standalone records (since the context passed to
audit_log_start() is NULL) this info is necessary.
I'm fine with the field ordering. If that is not acceptable, I'd
recommend a new record type (AUDIT_TASK) to act as an aux record to this
record that lists this information in a standard order that can be used
as an aux record for all the standalone records that are missing this
information.
Reviewed-by: Richard Guy Briggs <rgb(a)redhat.com>
- RGB
--
Richard Guy Briggs <rgb(a)redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635