Re: [PATCH] context based audit filtering (take 3)

Darrel Goeddel wrote: ...snip... The updated version of Dustin's patch I referred to is below. The changes are are follows: - Add the se_str char * to the audit_field. This stores a copy of the string target that the rule is based on. This is needed for the audit_compare_rule addition (below). It will also be used in the callback for policy reloads (also below). This is managed along with the se_rule field. This also gets rid of a memory leak where the unpacked string was not being freed. - printk a warning and ignore invalid selinux rules (but still hang on to them so they may be activated with a later policy reload). - Change audit_compare_rule to compare the string values of the selinux rules to see if the rule exists. Previously, it thought that any selinux filters based on equal length string were equal. - Change audit_krule_to_data to put in the string target of the rule for the selinux filters. This will hopefully allow the display of the type, level, etc. when dumping rules via AUDIT_LIST_RULES (I'll test tommorrow). - Add a selinux callback for re-initializing the se_rule field when there is a policy reload. THIS NEEDS WORK - It doesn't obey proper locking yet, but it is functional. I need to get my head around the locking of the audit structures a little better - I'll also take suggestions ;) - Obtain the sid of the task in audit_filter_rules instead of the callers obtaining it and passing it in. diff --git a/kernel/audit.h b/kernel/audit.h index eb33354..3ffc70a 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -59,9 +59,11 @@ struct audit_watch { }; struct audit_field { - u32 type; - u32 val; - u32 op; + u32 type; + u32 val; + u32 op; + char *se_str; + struct selinux_audit_rule *se_rule; }; struct audit_krule { diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index 3712295..306e941 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -25,6 +25,7 @@ #include <linux/fs.h> #include <linux/namei.h> #include <linux/netlink.h> +#include <linux/selinux.h> #include "audit.h" /* There are three lists of rules -- one to search at task creation @@ -50,6 +51,12 @@ static inline void audit_free_watch(stru static inline void audit_free_rule(struct audit_entry *e) { + int i; + for (i = 0; i < e->rule.field_count; i++) { + struct audit_field *f = &e->rule.fields[i]; + kfree(f->se_str); + selinux_audit_rule_free(f->se_rule); + } kfree(e->rule.fields); kfree(e); } @@ -222,7 +229,7 @@ static struct audit_entry *audit_data_to void *bufp; size_t remain = datasz - sizeof(struct audit_rule_data); int i; - char *path; + char *str; entry = audit_to_entry_common((struct audit_rule *)data); if (IS_ERR(entry)) @@ -241,16 +248,40 @@ static struct audit_entry *audit_data_to f->op = data->fieldflags[i] & AUDIT_OPERATORS; f->type = data->fields[i]; f->val = data->values[i]; + f->se_str = NULL; + f->se_rule = NULL; switch(f->type) { + case AUDIT_SE_USER: + case AUDIT_SE_ROLE: + case AUDIT_SE_TYPE: + case AUDIT_SE_SEN: + case AUDIT_SE_CLR: + str = audit_unpack_string(&bufp, &remain, f->val); + if (IS_ERR(str)) + goto exit_free; + entry->rule.buflen += f->val; + + err = selinux_audit_rule_init(f->type, f->op, str, + &f->se_rule); + if (err == -EINVAL) { + printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", str); + err = 0; + } + if (err) { + kfree(str); + goto exit_free; + } else + f->se_str = str; + break; case AUDIT_WATCH: - path = audit_unpack_string(&bufp, &remain, f->val); - if (IS_ERR(path)) + str = audit_unpack_string(&bufp, &remain, f->val); + if (IS_ERR(str)) goto exit_free; entry->rule.buflen += f->val; - err = audit_to_watch(path, &entry->rule, i); + err = audit_to_watch(str, &entry->rule, i); if (err) { - kfree(path); + kfree(str); goto exit_free; } break; @@ -333,6 +364,15 @@ static struct audit_rule_data *audit_kru data->buflen += data->values[i] = audit_pack_string(&bufp, krule->watch->path); break; + case AUDIT_SE_USER: + case AUDIT_SE_ROLE: + case AUDIT_SE_TYPE: + case AUDIT_SE_SEN: + case AUDIT_SE_CLR: + data->buflen += data->values[i] = + audit_pack_string(&bufp, + krule->fields[i].se_str); + break; default: data->values[i] = f->val; } @@ -370,6 +410,14 @@ static int audit_compare_rule(struct aud if (audit_compare_watch(a->watch, b->watch)) return 1; break; + case AUDIT_SE_USER: + case AUDIT_SE_ROLE: + case AUDIT_SE_TYPE: + case AUDIT_SE_SEN: + case AUDIT_SE_CLR: + if (strcmp(a->fields[i].se_str, b->fields[i].se_str)) + return 1; + break; default: if (a->fields[i].val != b->fields[i].val) return 1; @@ -640,6 +688,9 @@ int audit_comparator(const u32 left, con default: return -EINVAL; } + /* should NEVER get here */ + BUG(); + return 0; } @@ -726,3 +777,45 @@ unlock_and_return: rcu_read_unlock(); return result; } + +static int selinux_callback(void) +{ + struct audit_entry *entry; + int i, j, err = 0; + + /* TODO: add proper locking. */ + for (i = 0; i < AUDIT_NR_FILTERS; i++) { + list_for_each_entry(entry, &audit_filter_list[i], list) { + for (j = 0; j < entry->rule.field_count; j++) { + struct audit_field *f = &entry->rule.fields[j]; + switch (f->type) { + case AUDIT_SE_USER: + case AUDIT_SE_ROLE: + case AUDIT_SE_TYPE: + case AUDIT_SE_SEN: + case AUDIT_SE_CLR: + selinux_audit_rule_free(f->se_rule); + err = selinux_audit_rule_init(f->type, + f->op, f->se_str, &f->se_rule); + if (err == -EINVAL) { + printk(KERN_WARNING "selinux audit rule for item %s is invalid\n", f->se_str); + err = 0; + } + if (err) + goto out; + } + } + } + } + +out: + return err; +} + +static int __init register_selinux_callback(void) +{ + selinux_audit_set_callback(&selinux_callback); + return 0; +} +__initcall(register_selinux_callback); + diff --git a/kernel/auditsc.c b/kernel/auditsc.c index cd83289..c52c80a 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -58,6 +58,7 @@ #include <linux/security.h> #include <linux/list.h> #include <linux/tty.h> +#include <linux/selinux.h> #include "audit.h" @@ -168,6 +169,9 @@ static int audit_filter_rules(struct tas enum audit_state *state) { int i, j; + u32 sid; + + selinux_task_ctxid(tsk, &sid); for (i = 0; i < rule->field_count; i++) { struct audit_field *f = &rule->fields[i]; @@ -258,6 +262,18 @@ static int audit_filter_rules(struct tas if (ctx) result = audit_comparator(ctx->loginuid, f->op, f->val); break; + case AUDIT_SE_USER: + case AUDIT_SE_ROLE: + case AUDIT_SE_TYPE: + case AUDIT_SE_SEN: + case AUDIT_SE_CLR: + /* NOTE: this may return negative values indicating + a temporary error. We simply treat this as a + match for now to avoid losing information that + may be wanted. */ + result = selinux_audit_rule_match(sid, f->type, f->op, + f->se_rule); + break; case AUDIT_ARG0: case AUDIT_ARG1: case AUDIT_ARG2: -- Darrel