Re: [PATCH V9 2/3] audit: implement audit by executable

On Wednesday, August 05, 2015 04:29:37 PM Richard Guy Briggs wrote: > This adds the ability audit the actions of a not-yet-running process. > > This patch implements the ability to filter on the executable path. Instead > of just hard coding the ino and dev of the executable we care about at the > moment the rule is inserted into the kernel, use the new audit_fsnotify > infrastructure to manage this dynamically. This means that if the filename > does not yet exist but the containing directory does, or if the inode in > question is unlinked and creat'd (aka updated) the rule will just continue > to work. If the containing directory is moved or deleted or the filesystem > is unmounted, the rule is deleted automatically. A future enhancement > would be to have the rule survive across directory disruptions. > > This is a heavily modified version of a patch originally submitted by Eric > Paris with some ideas from Peter Moody. > > Cc: Peter Moody <peter(a)hda3.com&gt; > Cc: Eric Paris <eparis(a)redhat.com&gt; > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > --- > include/linux/audit.h | 1 + > include/uapi/linux/audit.h | 5 +++- > kernel/audit.h | 4 +++ > kernel/audit_tree.c | 2 + > kernel/audit_watch.c | 31 +++++++++++++++++++++++++ > kernel/auditfilter.c | 53 ++++++++++++++++++++++++++++++++++++++++- > kernel/auditsc.c | 3 ++ > 7 files changed, 97 insertions(+), 2 deletions(-) Merged, although some more minor whitespace tweaks were necessary for checkpatch. On a related note, if you're not running ./scripts/checlpatch.pl on your patches before sending them out, I would recommend it. It isn't perfect, but it can catch some silly things that we all do from time to time.

Also, one last thing. It is pretty late in the -rcX cycle to merge these two patches, but considering that we've been talking about these for a while, I'm reasonably okay merging them. In the future, if it isn't in audit#next by the time -rc5 is released, it isn't going to make the merge window.

> diff --git a/include/linux/audit.h b/include/linux/audit.h > index c2e7e3a..aee456f 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -59,6 +59,7 @@ struct audit_krule { > struct audit_field *inode_f; /* quick access to an inode field */ > struct audit_watch *watch; /* associated watch */ > struct audit_tree *tree; /* associated watched tree */ > + struct audit_fsnotify_mark *exe; > struct list_head rlist; /* entry in audit_{watch,tree}.rules list */ > struct list_head list; /* for AUDIT_LIST* purposes only */ > u64 prio; > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h > index 971df22..e2ca600 100644 > --- a/include/uapi/linux/audit.h > +++ b/include/uapi/linux/audit.h > @@ -266,6 +266,7 @@ > #define AUDIT_OBJ_UID 109 > #define AUDIT_OBJ_GID 110 > #define AUDIT_FIELD_COMPARE 111 > +#define AUDIT_EXE 112 > > #define AUDIT_ARG0 200 > #define AUDIT_ARG1 (AUDIT_ARG0+1) > @@ -324,8 +325,10 @@ enum { > > #define AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT 0x00000001 > #define AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME 0x00000002 > +#define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004 > #define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \ > - AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME) > + AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \ > + AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH ) > > /* deprecated: AUDIT_VERSION_* */ > #define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL > diff --git a/kernel/audit.h b/kernel/audit.h > index 46d10dd..dadf86a 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -277,6 +277,8 @@ extern char *audit_mark_path(struct audit_fsnotify_mark > *mark); extern void audit_remove_mark(struct audit_fsnotify_mark > *audit_mark); extern void audit_remove_mark_rule(struct audit_krule > *krule); > extern int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned > long ino, dev_t dev); +extern int audit_dupe_exe(struct audit_krule *new, > struct audit_krule *old); +extern int audit_exe_compare(struct task_struct > *tsk, struct audit_fsnotify_mark *mark); > > #else > #define audit_put_watch(w) {} > @@ -292,6 +294,8 @@ extern int audit_mark_compare(struct audit_fsnotify_mark > *mark, unsigned long in #define audit_remove_mark(m) > #define audit_remove_mark_rule(k) > #define audit_mark_compare(m, i, d) 0 > +#define audit_exe_compare(t, m) (-EINVAL) > +#define audit_dupe_exe(n, o) (-EINVAL) > #endif /* CONFIG_AUDIT_WATCH */ > > #ifdef CONFIG_AUDIT_TREE > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > index b0f9877..94ecdab 100644 > --- a/kernel/audit_tree.c > +++ b/kernel/audit_tree.c > @@ -479,6 +479,8 @@ static void kill_rules(struct audit_tree *tree) > if (rule->tree) { > /* not a half-baked one */ > audit_tree_log_remove_rule(rule); > + if (entry->rule.exe) > + audit_remove_mark(entry->rule.exe); > rule->tree = NULL; > list_del_rcu(&entry->list); > list_del(&entry->rule.list); > diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c > index c668bfc..1255dbf 100644 > --- a/kernel/audit_watch.c > +++ b/kernel/audit_watch.c > @@ -312,6 +312,8 @@ static void audit_update_watch(struct audit_parent > *parent, list_replace(&oentry->rule.list, > &nentry->rule.list); > } > + if (oentry->rule.exe) > + audit_remove_mark(oentry->rule.exe); > > audit_watch_log_rule_change(r, owatch, "updated_rules"); > > @@ -342,6 +344,8 @@ static void audit_remove_parent_watches(struct > audit_parent *parent) list_for_each_entry_safe(r, nextr, &w->rules, rlist) > { > e = container_of(r, struct audit_entry, rule); > audit_watch_log_rule_change(r, w, "remove_rule"); > + if (e->rule.exe) > + audit_remove_mark(e->rule.exe); > list_del(&r->rlist); > list_del(&r->list); > list_del_rcu(&e->list); > @@ -514,3 +518,30 @@ static int __init audit_watch_init(void) > return 0; > } > device_initcall(audit_watch_init); > + > +int audit_dupe_exe(struct audit_krule *new, struct audit_krule *old) > +{ > + struct audit_fsnotify_mark *audit_mark; > + char *pathname; > + > + pathname = kstrdup(audit_mark_path(old->exe), GFP_KERNEL); > + if (!pathname) > + return -ENOMEM; > + > + audit_mark = audit_alloc_mark(new, pathname, strlen(pathname)); > + if (IS_ERR(audit_mark)) { > + kfree(pathname); > + return PTR_ERR(audit_mark); > + } > + new->exe = audit_mark; > + > + return 0; > +} > + > +int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark > *mark) +{ > + unsigned long ino = tsk->mm->exe_file->f_inode->i_ino; > + dev_t dev = tsk->mm->exe_file->f_inode->i_sb->s_dev; > + > + return audit_mark_compare(mark, ino, dev); > +} > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 3d99196..c662638 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -405,6 +405,12 @@ static int audit_field_valid(struct audit_entry *entry, > struct audit_field *f) if (f->val > AUDIT_MAX_FIELD_COMPARE) > return -EINVAL; > break; > + case AUDIT_EXE: > + if (f->op != Audit_equal) > + return -EINVAL; > + if (entry->rule.listnr != AUDIT_FILTER_EXIT) > + return -EINVAL; > + break; > }; > return 0; > } > @@ -419,6 +425,7 @@ static struct audit_entry *audit_data_to_entry(struct > audit_rule_data *data, size_t remain = datasz - sizeof(struct > audit_rule_data); > int i; > char *str; > + struct audit_fsnotify_mark *audit_mark; > > entry = audit_to_entry_common(data); > if (IS_ERR(entry)) > @@ -539,6 +546,24 @@ static struct audit_entry *audit_data_to_entry(struct > audit_rule_data *data, entry->rule.buflen += f->val; > entry->rule.filterkey = str; > break; > + case AUDIT_EXE: > + if (entry->rule.exe || f->val > PATH_MAX) > + goto exit_free; > + str = audit_unpack_string(&bufp, &remain, f->val); > + if (IS_ERR(str)) { > + err = PTR_ERR(str); > + goto exit_free; > + } > + entry->rule.buflen += f->val; > + > + audit_mark = audit_alloc_mark(&entry->rule, str, f->val); > + if (IS_ERR(audit_mark)) { > + kfree(str); > + err = PTR_ERR(audit_mark); > + goto exit_free; > + } > + entry->rule.exe = audit_mark; > + break; > } > } > > @@ -551,6 +576,8 @@ exit_nofree: > exit_free: > if (entry->rule.tree) > audit_put_tree(entry->rule.tree); /* that's the temporary one */ > + if (entry->rule.exe) > + audit_remove_mark(entry->rule.exe); /* that's the template one */ > audit_free_rule(entry); > return ERR_PTR(err); > } > @@ -615,6 +642,10 @@ static struct audit_rule_data > *audit_krule_to_data(struct audit_krule *krule) data->buflen += > data->values[i] = > audit_pack_string(&bufp, krule->filterkey); > break; > + case AUDIT_EXE: > + data->buflen += data->values[i] = > + audit_pack_string(&bufp, audit_mark_path(krule->exe)); > + break; > case AUDIT_LOGINUID_SET: > if (krule->pflags & AUDIT_LOGINUID_LEGACY && !f->val) { > data->fields[i] = AUDIT_LOGINUID; > @@ -678,6 +709,12 @@ static int audit_compare_rule(struct audit_krule *a, > struct audit_krule *b) if (strcmp(a->filterkey, b->filterkey)) > return 1; > break; > + case AUDIT_EXE: > + /* both paths exist based on above type compare */ > + if (strcmp(audit_mark_path(a->exe), > + audit_mark_path(b->exe))) > + return 1; > + break; > case AUDIT_UID: > case AUDIT_EUID: > case AUDIT_SUID: > @@ -799,8 +836,14 @@ struct audit_entry *audit_dupe_rule(struct audit_krule > *old) err = -ENOMEM; > else > new->filterkey = fk; > + break; > + case AUDIT_EXE: > + err = audit_dupe_exe(new, old); > + break; > } > if (err) { > + if (new->exe) > + audit_remove_mark(new->exe); > audit_free_rule(entry); > return ERR_PTR(err); > } > @@ -963,6 +1006,9 @@ int audit_del_rule(struct audit_entry *entry) > if (e->rule.tree) > audit_remove_tree_rule(&e->rule); > > + if (e->rule.exe) > + audit_remove_mark_rule(&e->rule); > + > #ifdef CONFIG_AUDITSYSCALL > if (!dont_count) > audit_n_rules--; > @@ -1067,8 +1113,11 @@ int audit_rule_change(int type, __u32 portid, int > seq, void *data, WARN_ON(1); > } > > - if (err || type == AUDIT_DEL_RULE) > + if (err || type == AUDIT_DEL_RULE) { > + if (entry->rule.exe) > + audit_remove_mark(entry->rule.exe); > audit_free_rule(entry); > + } > > return err; > } > @@ -1360,6 +1409,8 @@ static int update_lsm_rule(struct audit_krule *r) > return 0; > > nentry = audit_dupe_rule(r); > + if (entry->rule.exe) > + audit_remove_mark(entry->rule.exe); > if (IS_ERR(nentry)) { > /* save the first error encountered for the > * return value */ > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 701ea5c..e9bac2b 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -466,6 +466,9 @@ static int audit_filter_rules(struct task_struct *tsk, > result = audit_comparator(ctx->ppid, f->op, f->val); > } > break; > + case AUDIT_EXE: > + result = audit_exe_compare(tsk, rule->exe); > + break; > case AUDIT_UID: > result = audit_uid_comparator(cred->uid, f->op, f->uid); > break; -- paul moore security @ redhat