public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steve Grubb <sgrubb@redhat.com>
To: Paul Moore <paul@paul-moore.com>
Cc: Richard Guy Briggs <rgb@redhat.com>,
	linux-kernel@vger.kernel.org, linux-audit@redhat.com,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH ALT4 V3 2/2] audit: filter PATH records keyed on filesystem magic
Date: Mon, 09 Oct 2017 20:13:30 -0400	[thread overview]
Message-ID: <5615140.bAjOPuJM3L@x2> (raw)
In-Reply-To: <CAHC9VhRTm_9VG2won+w=fJOrkXpzWvpJ1s+xVTddLfwsvFq0=Q@mail.gmail.com>

On Thursday, September 7, 2017 6:36:32 PM EDT Paul Moore wrote:
> On Wed, Aug 23, 2017 at 7:03 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > Tracefs or debugfs were causing hundreds to thousands of PATH records to
> > be associated with the init_module and finit_module SYSCALL records on a
> > 
> > few modules when the following rule was in place for startup:
> >         -a always,exit -F arch=x86_64 -S init_module -F key=mod-load
> > 
> > Provide a method to ignore these large number of PATH records from
> > overwhelming the logs if they are not of interest.  Introduce a new
> > filter list "AUDIT_FILTER_FS", with a new field type AUDIT_FSTYPE,
> > which keys off the filesystem 4-octet hexadecimal magic identifier to
> > filter specific filesystem PATH records.
> > 
> > An example rule would look like:
> >         -a never,filesystem -F fstype=0x74726163 -F key=ignore_tracefs
> >         -a never,filesystem -F fstype=0x64626720 -F key=ignore_debugfs
> > 
> > Arguably the better way to address this issue is to disable tracefs and
> > debugfs on boot from production systems.
> > 
> > See: https://github.com/linux-audit/audit-kernel/issues/16
> > See: https://github.com/linux-audit/audit-userspace/issues/8
> > Test case: https://github.com/linux-audit/audit-testsuite/issues/42
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > 
> > ---
> > v3: rebase
> > v2: convert AUDIT_FILTER_PATH to AUDIT_FILTER_FS
> > ---
> > 
> >  include/uapi/linux/audit.h |    8 ++++++--
> >  kernel/auditfilter.c       |   39 ++++++++++++++++++++++++++++++++-------
> >  kernel/auditsc.c           |   23 +++++++++++++++++++++++
> >  3 files changed, 61 insertions(+), 9 deletions(-)
> 
> Both this patch and 1/2 look okay, but I'm not going to merge either
> until after the merge window closes.
> 
> Steve, I assume you are still happy with the kernel/userspace
> interface for this?

The user space portion was applied a week or two ago. So, I guess proceed with 
merging this.

-Steve

> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 0714a66..be71134 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -155,8 +155,9 @@
> > 
> >  #define AUDIT_FILTER_WATCH     0x03    /* Apply rule to file system
> >  watches */ #define AUDIT_FILTER_EXIT      0x04    /* Apply rule at
> >  syscall exit */ #define AUDIT_FILTER_TYPE      0x05    /* Apply rule at
> >  audit_log_start */> 
> > +#define AUDIT_FILTER_FS                0x06    /* Apply rule at
> > __audit_inode_child */
> > 
> > -#define AUDIT_NR_FILTERS       6
> > +#define AUDIT_NR_FILTERS       7
> > 
> >  #define AUDIT_FILTER_PREPEND   0x10    /* Prepend to front of list */
> > 
> > @@ -256,6 +257,7 @@
> > 
> >  #define AUDIT_OBJ_LEV_HIGH     23
> >  #define AUDIT_LOGINUID_SET     24
> >  #define AUDIT_SESSIONID        25      /* Session ID */
> > 
> > +#define AUDIT_FSTYPE   26      /* FileSystem Type */
> > 
> >                                 /* These are ONLY useful when checking
> >                                 
> >                                  * at syscall exit time (AUDIT_AT_EXIT).
> >                                  */
> > 
> > @@ -335,13 +337,15 @@ enum {
> > 
> >  #define AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND    0x00000008
> >  #define AUDIT_FEATURE_BITMAP_SESSIONID_FILTER  0x00000010
> >  #define AUDIT_FEATURE_BITMAP_LOST_RESET                0x00000020
> > 
> > +#define AUDIT_FEATURE_BITMAP_FILTER_FS         0x00000040
> > 
> >  #define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
> >  
> >                                   AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME |
> >                                   \
> >                                   AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH | \
> >                                   AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
> >                                   AUDIT_FEATURE_BITMAP_SESSIONID_FILTER |
> >                                   \
> > 
> > -                                 AUDIT_FEATURE_BITMAP_LOST_RESET)
> > +                                 AUDIT_FEATURE_BITMAP_LOST_RESET | \
> > +                                 AUDIT_FEATURE_BITMAP_FILTER_FS)
> > 
> >  /* deprecated: AUDIT_VERSION_* */
> >  #define AUDIT_VERSION_LATEST           AUDIT_FEATURE_BITMAP_ALL
> > 
> > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > index 0b0aa58..4a1758a 100644
> > --- a/kernel/auditfilter.c
> > +++ b/kernel/auditfilter.c
> > @@ -56,7 +56,8 @@ struct list_head audit_filter_list[AUDIT_NR_FILTERS] = {
> > 
> >         LIST_HEAD_INIT(audit_filter_list[3]),
> >         LIST_HEAD_INIT(audit_filter_list[4]),
> >         LIST_HEAD_INIT(audit_filter_list[5]),
> > 
> > -#if AUDIT_NR_FILTERS != 6
> > +       LIST_HEAD_INIT(audit_filter_list[6]),
> > +#if AUDIT_NR_FILTERS != 7
> > 
> >  #error Fix audit_filter_list initialiser
> >  #endif
> >  };
> > 
> > @@ -67,6 +68,7 @@ struct list_head audit_filter_list[AUDIT_NR_FILTERS] = {
> > 
> >         LIST_HEAD_INIT(audit_rules_list[3]),
> >         LIST_HEAD_INIT(audit_rules_list[4]),
> >         LIST_HEAD_INIT(audit_rules_list[5]),
> > 
> > +       LIST_HEAD_INIT(audit_rules_list[6]),
> > 
> >  };
> >  
> >  DEFINE_MUTEX(audit_filter_mutex);
> > 
> > @@ -263,6 +265,7 @@ static int audit_match_signal(struct audit_entry
> > *entry)> 
> >  #endif
> >  
> >         case AUDIT_FILTER_USER:
> > 
> >         case AUDIT_FILTER_TYPE:
> > +       case AUDIT_FILTER_FS:
> >                 ;
> >         
> >         }
> >         if (unlikely(rule->action == AUDIT_POSSIBLE)) {
> > 
> > @@ -338,6 +341,21 @@ static int audit_field_valid(struct audit_entry
> > *entry, struct audit_field *f)> 
> >                     entry->rule.listnr != AUDIT_FILTER_USER)
> >                     
> >                         return -EINVAL;
> >                 
> >                 break;
> > 
> > +       case AUDIT_FSTYPE:
> > +               if (entry->rule.listnr != AUDIT_FILTER_FS)
> > +                       return -EINVAL;
> > +               break;
> > +       }
> > +
> > +       switch(entry->rule.listnr) {
> > +       case AUDIT_FILTER_FS:
> > +               switch(f->type) {
> > +               case AUDIT_FSTYPE:
> > +               case AUDIT_FILTERKEY:
> > +                       break;
> > +               default:
> > +                       return -EINVAL;
> > +               }
> > 
> >         }
> >         
> >         switch(f->type) {
> > 
> > @@ -391,6 +409,7 @@ static int audit_field_valid(struct audit_entry
> > *entry, struct audit_field *f)> 
> >                         return -EINVAL;
> >         
> >         /* FALL THROUGH */
> > 
> >         case AUDIT_ARCH:
> > +       case AUDIT_FSTYPE:
> >                 if (f->op != Audit_not_equal && f->op != Audit_equal)
> >                 
> >                         return -EINVAL;
> >                 
> >                 break;
> > 
> > @@ -910,10 +929,13 @@ static inline int audit_add_rule(struct audit_entry
> > *entry)> 
> >  #ifdef CONFIG_AUDITSYSCALL
> >  
> >         int dont_count = 0;
> > 
> > -       /* If either of these, don't count towards total */
> > -       if (entry->rule.listnr == AUDIT_FILTER_USER ||
> > -               entry->rule.listnr == AUDIT_FILTER_TYPE)
> > +       /* If any of these, don't count towards total */
> > +       switch(entry->rule.listnr) {
> > +       case AUDIT_FILTER_USER:
> > +       case AUDIT_FILTER_TYPE:
> > 
> > +       case AUDIT_FILTER_FS:
> >                 dont_count = 1;
> > 
> > +       }
> > 
> >  #endif
> >  
> >         mutex_lock(&audit_filter_mutex);
> > 
> > @@ -989,10 +1011,13 @@ int audit_del_rule(struct audit_entry *entry)
> > 
> >  #ifdef CONFIG_AUDITSYSCALL
> >  
> >         int dont_count = 0;
> > 
> > -       /* If either of these, don't count towards total */
> > -       if (entry->rule.listnr == AUDIT_FILTER_USER ||
> > -               entry->rule.listnr == AUDIT_FILTER_TYPE)
> > +       /* If any of these, don't count towards total */
> > +       switch(entry->rule.listnr) {
> > +       case AUDIT_FILTER_USER:
> > +       case AUDIT_FILTER_TYPE:
> > 
> > +       case AUDIT_FILTER_FS:
> >                 dont_count = 1;
> > 
> > +       }
> > 
> >  #endif
> >  
> >         mutex_lock(&audit_filter_mutex);
> > 
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 11848df..ce6cbda 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1871,10 +1871,33 @@ void __audit_inode_child(struct inode *parent,
> > 
> >         struct inode *inode = d_backing_inode(dentry);
> >         const char *dname = dentry->d_name.name;
> >         struct audit_names *n, *found_parent = NULL, *found_child = NULL;
> > 
> > +       struct audit_entry *e;
> > +       struct list_head *list = &audit_filter_list[AUDIT_FILTER_FS];
> > +       int i;
> > 
> >         if (!context->in_syscall)
> >         
> >                 return;
> > 
> > +        rcu_read_lock();
> > +       if (!list_empty(list)) {
> > +               list_for_each_entry_rcu(e, list, list) {
> > +                       for (i = 0; i < e->rule.field_count; i++) {
> > +                               struct audit_field *f =
> > &e->rule.fields[i];
> > +
> > +                               if (f->type == AUDIT_FSTYPE) {
> > +                                       if
> > (audit_comparator(parent->i_sb->s_magic, +                               
> >            f->op, f->val)) {
> > +                                               if (e->rule.action ==
> > AUDIT_NEVER) { +                                                      
> > rcu_read_unlock(); +                                                     
> >  return;
> > +                                               }
> > +                                       }
> > +                               }
> > +                       }
> > +               }
> > +       }
> > +        rcu_read_unlock();
> > +
> > 
> >         if (inode)
> >         
> >                 handle_one(inode);
> > 
> > --
> > 1.7.1
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://www.redhat.com/mailman/listinfo/linux-audit

  parent reply	other threads:[~2017-10-10  0:13 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-23 11:03 [PATCH ALT4 V3 1/2] audit: show fstype:pathname for entries with anonymous parents Richard Guy Briggs
2017-08-23 11:03 ` [PATCH ALT4 V3 2/2] audit: filter PATH records keyed on filesystem magic Richard Guy Briggs
2017-09-07 22:36   ` Paul Moore
2017-09-07 22:40     ` Steven Rostedt
2017-09-07 23:05       ` Paul Moore
2017-09-07 23:07         ` Steven Rostedt
2017-10-10  0:13     ` Steve Grubb [this message]
2017-10-19 19:58   ` Paul Moore
2017-10-19 20:10     ` Richard Guy Briggs
2017-09-20 16:52 ` [PATCH ALT4 V3 1/2] audit: show fstype:pathname for entries with anonymous parents Paul Moore
2017-09-21 14:57   ` Richard Guy Briggs
2017-10-12  1:36   ` Richard Guy Briggs
2017-11-08 23:29   ` Steve Grubb
2017-11-09 15:18     ` Paul Moore
2017-11-09 15:31       ` Steve Grubb
2017-11-09 15:59         ` Paul Moore
2017-11-09 20:52           ` Richard Guy Briggs
2017-11-09 21:47             ` Paul Moore
2017-11-09 21:56               ` Richard Guy Briggs
2017-11-13 18:30             ` Steve Grubb
2017-11-13 19:01               ` Paul Moore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5615140.bAjOPuJM3L@x2 \
    --to=sgrubb@redhat.com \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=rgb@redhat.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox