| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHC9VhSv2uo8PdT1XJRELHWsS03TdiwB09DG7zXDmP-dTHn9=Q@mail.gmail.com>
Date: Thu, 16 Jun 2016 16:54:42 -0400
From: Paul Moore <paul@...l-moore.com>
To: Richard Guy Briggs <rgb@...hat.com>
Cc: linux-audit@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] audit: add fields to exclude filter by reusing user filter
On Tue, Jun 14, 2016 at 5:04 PM, Richard Guy Briggs <rgb@...hat.com> 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.
>
> Keep the check to quit early if the list is empty.
>
> Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
> ---
> 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 | 46 ++++++++++------------------------------------
> 4 files changed, 14 insertions(+), 40 deletions(-)
Patches that remove more code than the add always make me happy :)
Comment below ...
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 32cdafb..539c1d9 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -160,8 +160,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 384374a..2dfaa19 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -914,7 +914,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) {
> @@ -1362,7 +1362,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 96c9a1b..f90c042 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1349,50 +1349,24 @@ static int audit_filter_user_rules(struct audit_krule *rule, int type,
> return 1;
> }
>
> -int audit_filter_user(int type)
> +int audit_filter(int msgtype, unsigned int listtype)
> {
> enum audit_state state = AUDIT_DISABLED;
> struct audit_entry *e;
> - int rc, ret;
> -
> - ret = 1; /* Audit by default */
> + int rc, result = 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)
> -{
> - struct audit_entry *e;
> - int result = 0;
> -
> - 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[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;
> - }
> + list_for_each_entry_rcu(e, &audit_filter_list[listtype], list) {
> + rc = audit_filter_user_rules(&e->rule, msgtype, &state);
Any reason not to do away with audit_filter_user_rules() and just
insert the code here? As far as I can tell there are no other callers
...
> + if (rc) {
> + if (rc > 0 && ((state == AUDIT_DISABLED) ||
> + (listtype == AUDIT_FILTER_TYPE)))
> + result = 0;
> + break;
> }
> - if (result)
> - goto unlock_and_return;
> }
> unlock_and_return:
> rcu_read_unlock();
--
paul moore
www.paul-moore.com
Powered by blists - more mailing lists