Re: [PATCH v4] audit: add fields to exclude filter by reusing user filter

On Fri, Jun 24, 2016 at 4:35 PM, Richard Guy Briggs <rgb(a)redhat.com&gt; wrote: > RFE: add additional fields for use in audit filter exclude rules > https://github.com/linux-audit/audit-kernel/issues/5 > > Re-factor and combine audit_filter_type() with audit_filter_user() to > use audit_filter_user_rules() to enable the exclude filter to > additionally filter on PID, UID, GID, AUID, LOGINUID_SET, SUBJ_*. > > The process of combining the similar audit_filter_user() and > audit_filter_type() functions, required inverting the meaning and > including the ALWAYS action of the latter. > > Include audit_filter_user_rules() into audit_filter(), removing unneeded > logic in the process. > > Keep the check to quit early if the list is empty. > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > --- > v4: rebase on 4.6-based audit/next. > > v3: pull audit_filter_user_rules() into audit_filter() and simplify > logic. > > v2: combine audit_filter_user() and audit_filter_type() into > audit_filter(). > --- > include/linux/audit.h | 2 - > kernel/audit.c | 4 +- > kernel/audit.h | 2 + > kernel/auditfilter.c | 151 +++++++++++++++++-------------------------------- > 4 files changed, 57 insertions(+), 102 deletions(-) Merged, thanks. Please remember to run scripts/checkpatch.pl on your submissions, I had to fix up a couple of whitespace damaged lines.

> diff --git a/include/linux/audit.h b/include/linux/audit.h > index e38e3fc..9d4443f 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -163,8 +163,6 @@ extern void audit_log_task_info(struct audit_buffer *ab, > extern int audit_update_lsm_rules(void); > > /* Private API (for audit.c only) */ > -extern int audit_filter_user(int type); > -extern int audit_filter_type(int type); > extern int audit_rule_change(int type, __u32 portid, int seq, > void *data, size_t datasz); > extern int audit_list_rules_send(struct sk_buff *request_skb, int seq); > diff --git a/kernel/audit.c b/kernel/audit.c > index 678c3f0..994588e 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -934,7 +934,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) > if (!audit_enabled && msg_type != AUDIT_USER_AVC) > return 0; > > - err = audit_filter_user(msg_type); > + err = audit_filter(msg_type, AUDIT_FILTER_USER); > if (err == 1) { /* match or error */ > err = 0; > if (msg_type == AUDIT_USER_TTY) { > @@ -1382,7 +1382,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, > if (audit_initialized != AUDIT_INITIALIZED) > return NULL; > > - if (unlikely(audit_filter_type(type))) > + if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE))) > return NULL; > > if (gfp_mask & __GFP_DIRECT_RECLAIM) { > diff --git a/kernel/audit.h b/kernel/audit.h > index cbbe6bb..1879f02 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -327,6 +327,8 @@ extern pid_t audit_sig_pid; > extern kuid_t audit_sig_uid; > extern u32 audit_sig_sid; > > +extern int audit_filter(int msgtype, unsigned int listtype); > + > #ifdef CONFIG_AUDITSYSCALL > extern int __audit_signal_info(int sig, struct task_struct *t); > static inline int audit_signal_info(int sig, struct task_struct *t) > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index ff59a5e..3a67acf 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -1290,117 +1290,72 @@ int audit_compare_dname_path(const char *dname, const char *path, int parentlen) > return strncmp(p, dname, dlen); > } > > -static int audit_filter_user_rules(struct audit_krule *rule, int type, > - enum audit_state *state) > -{ > - int i; > - > - for (i = 0; i < rule->field_count; i++) { > - struct audit_field *f = &rule->fields[i]; > - pid_t pid; > - int result = 0; > - u32 sid; > - > - switch (f->type) { > - case AUDIT_PID: > - pid = task_pid_nr(current); > - result = audit_comparator(pid, f->op, f->val); > - break; > - case AUDIT_UID: > - result = audit_uid_comparator(current_uid(), f->op, f->uid); > - break; > - case AUDIT_GID: > - result = audit_gid_comparator(current_gid(), f->op, f->gid); > - break; > - case AUDIT_LOGINUID: > - result = audit_uid_comparator(audit_get_loginuid(current), > - f->op, f->uid); > - break; > - case AUDIT_LOGINUID_SET: > - result = audit_comparator(audit_loginuid_set(current), > - f->op, f->val); > - break; > - case AUDIT_MSGTYPE: > - result = audit_comparator(type, f->op, f->val); > - break; > - case AUDIT_SUBJ_USER: > - case AUDIT_SUBJ_ROLE: > - case AUDIT_SUBJ_TYPE: > - case AUDIT_SUBJ_SEN: > - case AUDIT_SUBJ_CLR: > - if (f->lsm_rule) { > - security_task_getsecid(current, &sid); > - result = security_audit_rule_match(sid, > - f->type, > - f->op, > - f->lsm_rule, > - NULL); > - } > - break; > - } > - > - if (result <= 0) > - return result; > - } > - switch (rule->action) { > - case AUDIT_NEVER: > - *state = AUDIT_DISABLED; > - break; > - case AUDIT_ALWAYS: > - *state = AUDIT_RECORD_CONTEXT; > - break; > - } > - return 1; > -} > - > -int audit_filter_user(int type) > -{ > - enum audit_state state = AUDIT_DISABLED; > - struct audit_entry *e; > - int rc, ret; > - > - ret = 1; /* Audit by default */ > - > - rcu_read_lock(); > - list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) { > - rc = audit_filter_user_rules(&e->rule, type, &state); > - if (rc) { > - if (rc > 0 && state == AUDIT_DISABLED) > - ret = 0; > - break; > - } > - } > - rcu_read_unlock(); > - > - return ret; > -} > - > -int audit_filter_type(int type) > +int audit_filter(int msgtype, unsigned int listtype) > { > struct audit_entry *e; > - int result = 0; > + int ret = 1; /* Audit by default */ > > rcu_read_lock(); > - if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE])) > + if (list_empty(&audit_filter_list[listtype])) > goto unlock_and_return; > + list_for_each_entry_rcu(e, &audit_filter_list[listtype], list) { > + int i, result = 0; > > - list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TYPE], > - list) { > - int i; > for (i = 0; i < e->rule.field_count; i++) { > struct audit_field *f = &e->rule.fields[i]; > - if (f->type == AUDIT_MSGTYPE) { > - result = audit_comparator(type, f->op, f->val); > - if (!result) > - break; > + pid_t pid; > + u32 sid; > + > + switch (f->type) { > + case AUDIT_PID: > + pid = task_pid_nr(current); > + result = audit_comparator(pid, f->op, f->val); > + break; > + case AUDIT_UID: > + result = audit_uid_comparator(current_uid(), f->op, f->uid); > + break; > + case AUDIT_GID: > + result = audit_gid_comparator(current_gid(), f->op, f->gid); > + break; > + case AUDIT_LOGINUID: > + result = audit_uid_comparator(audit_get_loginuid(current), > + f->op, f->uid); > + break; > + case AUDIT_LOGINUID_SET: > + result = audit_comparator(audit_loginuid_set(current), > + f->op, f->val); > + break; > + case AUDIT_MSGTYPE: > + result = audit_comparator(msgtype, f->op, f->val); > + break; > + case AUDIT_SUBJ_USER: > + case AUDIT_SUBJ_ROLE: > + case AUDIT_SUBJ_TYPE: > + case AUDIT_SUBJ_SEN: > + case AUDIT_SUBJ_CLR: > + if (f->lsm_rule) { > + security_task_getsecid(current, &sid); > + result = security_audit_rule_match(sid, > + f->type, f->op, f->lsm_rule, NULL); > + } > + break; > + default: > + goto unlock_and_return; > } > + if (result < 0) /* error */ > + goto unlock_and_return; > + if (!result) > + break; > + } > + if (result > 0) { > + if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_TYPE) > + ret = 0; > + break; > } > - if (result) > - goto unlock_and_return; > } > unlock_and_return: > rcu_read_unlock(); > - return result; > + return ret; > } > > static int update_lsm_rule(struct audit_krule *r) > -- > 1.7.1 paul moore