Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1411641 > unrolled thread

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

Started byRichard Guy Briggs <rgb@redhat.com>
First post2016-06-02 01:00 +0200
Last post2016-06-03 22:40 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] audit: add fields to exclude filter by reusing user filter Richard Guy Briggs <rgb@redhat.com> - 2016-06-02 01:00 +0200
    Re: [PATCH] audit: add fields to exclude filter by reusing user filter Paul Moore <paul@paul-moore.com> - 2016-06-03 21:40 +0200
      Re: [PATCH] audit: add fields to exclude filter by reusing user  filter Richard Guy Briggs <rgb@redhat.com> - 2016-06-03 22:30 +0200
        Re: [PATCH] audit: add fields to exclude filter by reusing user filter Paul Moore <pmoore@redhat.com> - 2016-06-03 22:40 +0200

#1411641 — [PATCH] audit: add fields to exclude filter by reusing user filter

FromRichard Guy Briggs <rgb@redhat.com>
Date2016-06-02 01:00 +0200
Subject[PATCH] audit: add fields to exclude filter by reusing user filter
Message-ID<rFnhg-6gS-7@gated-at.bofh.it>
RFE: add additional fields for use in audit filter exclude rules
https://github.com/linux-audit/audit-kernel/issues/5

Re-factor audit_filter_type() to use audit_filter_user_rules() to enable
exclude filter to additionally filter on PID, UID, GID, AUID,
LOGINUID_SET, SUBJ_*.

Add check in audit_filter_user() to quit early if list is empty.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditfilter.c |   22 +++++++++-------------
 1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 96c9a1b..515c752 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1358,6 +1358,9 @@ int audit_filter_user(int type)
 	ret = 1; /* Audit by default */
 
 	rcu_read_lock();
+	if (list_empty(&audit_filter_list[AUDIT_FILTER_USER]))
+		goto unlock_and_return;
+
 	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
 		rc = audit_filter_user_rules(&e->rule, type, &state);
 		if (rc) {
@@ -1366,13 +1369,14 @@ int audit_filter_user(int type)
 			break;
 		}
 	}
+unlock_and_return:
 	rcu_read_unlock();
-
 	return ret;
 }
 
 int audit_filter_type(int type)
 {
+	enum audit_state state = AUDIT_DISABLED;
 	struct audit_entry *e;
 	int result = 0;
 
@@ -1380,19 +1384,11 @@ int audit_filter_type(int type)
 	if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
 		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[AUDIT_FILTER_TYPE], list) {
+		if (audit_filter_user_rules(&e->rule, type, &state) == 1) {
+			result = 1;
+			break;
 		}
-		if (result)
-			goto unlock_and_return;
 	}
 unlock_and_return:
 	rcu_read_unlock();
-- 
1.7.1

[toc] | [next] | [standalone]


#1413420

FromPaul Moore <paul@paul-moore.com>
Date2016-06-03 21:40 +0200
Message-ID<rG36O-7m7-11@gated-at.bofh.it>
In reply to#1411641
On Wed, Jun 1, 2016 at 6:50 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> RFE: add additional fields for use in audit filter exclude rules
> https://github.com/linux-audit/audit-kernel/issues/5
>
> Re-factor audit_filter_type() to use audit_filter_user_rules() to enable
> exclude filter to additionally filter on PID, UID, GID, AUID,
> LOGINUID_SET, SUBJ_*.
>
> Add check in audit_filter_user() to quit early if list is empty.
>
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/auditfilter.c |   22 +++++++++-------------
>  1 files changed, 9 insertions(+), 13 deletions(-)

I like the consolidation between audit_filter_type() and
audit_filter_user(), I like it so much I think we should take it
further.  Let's consolidate both functions into a single function (say
audit_filter()?) and update the callers to use the new function.  This
shouldn't be hard as the only callers are audit_receive_msg() and
audit_log_start(); you'll need to be careful as the return values of
the current functions are opposite of each other, but it should be
easy enough to update one of the callers.

Sound reasonable?

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 96c9a1b..515c752 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1358,6 +1358,9 @@ int audit_filter_user(int type)
>         ret = 1; /* Audit by default */
>
>         rcu_read_lock();
> +       if (list_empty(&audit_filter_list[AUDIT_FILTER_USER]))
> +               goto unlock_and_return;
> +
>         list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
>                 rc = audit_filter_user_rules(&e->rule, type, &state);
>                 if (rc) {
> @@ -1366,13 +1369,14 @@ int audit_filter_user(int type)
>                         break;
>                 }
>         }
> +unlock_and_return:
>         rcu_read_unlock();
> -
>         return ret;
>  }
>
>  int audit_filter_type(int type)
>  {
> +       enum audit_state state = AUDIT_DISABLED;
>         struct audit_entry *e;
>         int result = 0;
>
> @@ -1380,19 +1384,11 @@ int audit_filter_type(int type)
>         if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
>                 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[AUDIT_FILTER_TYPE], list) {
> +               if (audit_filter_user_rules(&e->rule, type, &state) == 1) {
> +                       result = 1;
> +                       break;
>                 }
> -               if (result)
> -                       goto unlock_and_return;
>         }
>  unlock_and_return:
>         rcu_read_unlock();
> --
> 1.7.1
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit



-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1413449 — Re: [PATCH] audit: add fields to exclude filter by reusing user filter

FromRichard Guy Briggs <rgb@redhat.com>
Date2016-06-03 22:30 +0200
SubjectRe: [PATCH] audit: add fields to exclude filter by reusing user filter
Message-ID<rG3Tc-7Tx-27@gated-at.bofh.it>
In reply to#1413420
On 16/06/03, Paul Moore wrote:
> On Wed, Jun 1, 2016 at 6:50 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > RFE: add additional fields for use in audit filter exclude rules
> > https://github.com/linux-audit/audit-kernel/issues/5
> >
> > Re-factor audit_filter_type() to use audit_filter_user_rules() to enable
> > exclude filter to additionally filter on PID, UID, GID, AUID,
> > LOGINUID_SET, SUBJ_*.
> >
> > Add check in audit_filter_user() to quit early if list is empty.
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  kernel/auditfilter.c |   22 +++++++++-------------
> >  1 files changed, 9 insertions(+), 13 deletions(-)
> 
> I like the consolidation between audit_filter_type() and
> audit_filter_user(), I like it so much I think we should take it
> further.  Let's consolidate both functions into a single function (say
> audit_filter()?) and update the callers to use the new function.  This
> shouldn't be hard as the only callers are audit_receive_msg() and
> audit_log_start(); you'll need to be careful as the return values of
> the current functions are opposite of each other, but it should be
> easy enough to update one of the callers.
> 
> Sound reasonable?

Potentially...  I was even eyeing kernel/auditsc.c::audit_filter_rules()
for re-factoring...

> > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > index 96c9a1b..515c752 100644
> > --- a/kernel/auditfilter.c
> > +++ b/kernel/auditfilter.c
> > @@ -1358,6 +1358,9 @@ int audit_filter_user(int type)
> >         ret = 1; /* Audit by default */
> >
> >         rcu_read_lock();
> > +       if (list_empty(&audit_filter_list[AUDIT_FILTER_USER]))
> > +               goto unlock_and_return;
> > +
> >         list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_USER], list) {
> >                 rc = audit_filter_user_rules(&e->rule, type, &state);
> >                 if (rc) {
> > @@ -1366,13 +1369,14 @@ int audit_filter_user(int type)
> >                         break;
> >                 }
> >         }
> > +unlock_and_return:
> >         rcu_read_unlock();
> > -
> >         return ret;
> >  }
> >
> >  int audit_filter_type(int type)
> >  {
> > +       enum audit_state state = AUDIT_DISABLED;
> >         struct audit_entry *e;
> >         int result = 0;
> >
> > @@ -1380,19 +1384,11 @@ int audit_filter_type(int type)
> >         if (list_empty(&audit_filter_list[AUDIT_FILTER_TYPE]))
> >                 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[AUDIT_FILTER_TYPE], list) {
> > +               if (audit_filter_user_rules(&e->rule, type, &state) == 1) {
> > +                       result = 1;
> > +                       break;
> >                 }
> > -               if (result)
> > -                       goto unlock_and_return;
> >         }
> >  unlock_and_return:
> >         rcu_read_unlock();
> > --
> > 1.7.1
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit
> 
> 
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

[toc] | [prev] | [next] | [standalone]


#1413457

FromPaul Moore <pmoore@redhat.com>
Date2016-06-03 22:40 +0200
Message-ID<rG42R-7WA-15@gated-at.bofh.it>
In reply to#1413449
On Fri, Jun 3, 2016 at 4:24 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 16/06/03, Paul Moore wrote:
>> On Wed, Jun 1, 2016 at 6:50 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > RFE: add additional fields for use in audit filter exclude rules
>> > https://github.com/linux-audit/audit-kernel/issues/5
>> >
>> > Re-factor audit_filter_type() to use audit_filter_user_rules() to enable
>> > exclude filter to additionally filter on PID, UID, GID, AUID,
>> > LOGINUID_SET, SUBJ_*.
>> >
>> > Add check in audit_filter_user() to quit early if list is empty.
>> >
>> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> > ---
>> >  kernel/auditfilter.c |   22 +++++++++-------------
>> >  1 files changed, 9 insertions(+), 13 deletions(-)
>>
>> I like the consolidation between audit_filter_type() and
>> audit_filter_user(), I like it so much I think we should take it
>> further.  Let's consolidate both functions into a single function (say
>> audit_filter()?) and update the callers to use the new function.  This
>> shouldn't be hard as the only callers are audit_receive_msg() and
>> audit_log_start(); you'll need to be careful as the return values of
>> the current functions are opposite of each other, but it should be
>> easy enough to update one of the callers.
>>
>> Sound reasonable?
>
> Potentially...  I was even eyeing kernel/auditsc.c::audit_filter_rules()
> for re-factoring...

It's possible that we may be able to do some work on eliminating
duplication between the audit_filter_user()/audit_filter_type() and
audit_filter_rules(), but we'll always need audit_filter_rules()
simply because it can filter on more things, e.g. it has access to
object information.

I would suggest working on just audit_filter_user() and
audit_filter_type() right now so we can get something ready for
upstream before -rc5 or so, and then look into possible refactoring
with audit_filter_rules().

-- 
paul moore
security @ redhat

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web