Re: [PATCH ALT4 V2 2/2] audit: filter PATH records keyed on filesystem magic

From: Richard Guy Briggs
Date: Tue Jun 27 2017 - 16:46:00 EST

Next message: Matthias Schiffer: "[PATCH net-next 2/2] vxlan: add back error messages to vxlan_config_validate() as extended netlink acks"
Previous message: Grygorii Strashko: "Re: [RFC] gpio: about the need to manage irq mapping dynamically."
Next in thread: Paul Moore: "Re: [PATCH ALT4 V2 2/2] audit: filter PATH records keyed on filesystem magic"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 2017-05-30 17:30, Paul Moore wrote:
> On Tue, Apr 4, 2017 at 5:21 AM, Richard Guy Briggs <rgb@xxxxxxxxxx> 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_PATH", 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,path -F fstype=0x74726163 -F key=ignore_tracefs
> > -a never,path -F fstype=0x64626720 -F key=ignore_debugfs
>
> Trying to look into the future I wonder if we are ever going to need
> to expand the "path" filtering to regular inode lookups, e.g.
> audit_inode()?

That thought had occurred to me. Do you see any concern with that that
would affect this patch in terms of naming?
>
I could see expanding this filter to include other filter fields though
nothing specific comes to mind now.

> > Arguably the better way to address this issue is to disable tracefs and
> > debugfs on boot from production systems.
>
> Agreed. A good goal whenever possible, but that is a larger topic
> beyond the scope of this patchset.
>
> > See: https://github.com/linux-audit/audit-kernel/issues/16
> > Test case: https://github.com/linux-audit/audit-testsuite/issues/42
> >
> > Signed-off-by: Richard Guy Briggs <rgb@xxxxxxxxxx>
> > ---
> > include/uapi/linux/audit.h | 8 ++++++--
> > kernel/auditfilter.c | 39 ++++++++++++++++++++++++++++++++-------
> > kernel/auditsc.c | 23 +++++++++++++++++++++++
> > 3 files changed, 61 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 3c02bb2..0464910 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_PATH 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). */
> > @@ -334,12 +336,14 @@ enum {
> > #define AUDIT_FEATURE_BITMAP_EXECUTABLE_PATH 0x00000004
> > #define AUDIT_FEATURE_BITMAP_SESSIONID_FILTER 0x00000010
> > #define AUDIT_FEATURE_BITMAP_LOST_RESET 0x00000020
> > +#define AUDIT_FEATURE_BITMAP_FILTER_PATH 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_SESSIONID_FILTER | \
> > - AUDIT_FEATURE_BITMAP_LOST_RESET)
> > + AUDIT_FEATURE_BITMAP_LOST_RESET | \
> > + AUDIT_FEATURE_BITMAP_FILTER_PATH)
> >
> > /* deprecated: AUDIT_VERSION_* */
> > #define AUDIT_VERSION_LATEST AUDIT_FEATURE_BITMAP_ALL
> > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > index 239d11c..3e0ccf2 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 @@ static struct list_head audit_rules_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 inline struct audit_entry *audit_to_entry_common(struct audit_rule_data *
> > #endif
> > case AUDIT_FILTER_USER:
> > case AUDIT_FILTER_TYPE:
> > + case AUDIT_FILTER_PATH:
> > ;
> > }
> > 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_PATH)
> > + return -EINVAL;
> > + break;
> > + }
> > +
> > + switch(entry->rule.listnr) {
> > + case AUDIT_FILTER_PATH:
> > + 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_PATH:
> > 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_PATH:
> > dont_count = 1;
> > + }
> > #endif
> >
> > mutex_lock(&audit_filter_mutex);
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index b3797c7..a12531f 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1868,10 +1868,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_PATH];
> > + 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@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/linux-audit
>
>
>
> --
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@xxxxxxxxxx>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Next message: Matthias Schiffer: "[PATCH net-next 2/2] vxlan: add back error messages to vxlan_config_validate() as extended netlink acks"
Previous message: Grygorii Strashko: "Re: [RFC] gpio: about the need to manage irq mapping dynamically."
Next in thread: Paul Moore: "Re: [PATCH ALT4 V2 2/2] audit: filter PATH records keyed on filesystem magic"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]