[PATCH] filter: add path filter with fstype

Richard Guy Briggs rgb at redhat.com
Tue Jun 13 02:45:50 UTC 2017


On 2017-06-12 20:28, Steve Grubb wrote:
> Hello,

Hi (swapping in this task after > 2 months...)

> This patch needs to be refactored to match the current count of error messages 
> in err_msgtab.
> 
> What error message is emitted when run on a kernel that does not support the 
> new filter?

-36 (which needs re-checking now that ghau12/ghau21pr has been reworked.)

> On Tuesday, April 4, 2017 6:40:18 AM EDT Richard Guy Briggs 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
> > 
> > Add the new "path" filter list anchored in __audit_inode_child() to
> > filter out PATH records from uninteresting filesystem types, "fstype",
> > keying on their kernel hexadecimal 4-octet magic identifier.
> > 
> > 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
> 
> Are we sure path is the best name for this filter? Is there something more 
> precise like filesystem?

It is filesystem type that we are filtering, but there may be a use case
to filter on another factor later, so like the "type" filter that really
is the "exclude" filter, let's not make that mistake again.

I wrestled with that for a while and kept coming back to "path" filter
due to the fact that it was a path record that was affected.  At the
moment it is only active on audit_inode_child, but I could potentially
see it being active on audit_inode as well.

> > Note: "always,path" will log the PATH record anyways and add latency.
> > 
> > See: https://github.com/linux-audit/audit-userspace/issues/15
> > See: https://github.com/linux-audit/audit-kernel/issues/8
> > Test case: https://github.com/linux-audit/audit-testsuite/issues/42
> > 
> > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
> > ---
> >  docs/audit_add_rule_data.3 |    3 +++
> >  lib/errormsg.h             |    5 +++++
> >  lib/fieldtab.h             |    2 ++
> >  lib/flagtab.h              |    2 ++
> >  lib/libaudit.c             |   26 ++++++++++++++++++++++++--
> >  lib/libaudit.h             |   10 ++++++++++
> >  lib/private.h              |    1 +
> >  src/auditctl-listing.c     |    6 ++++--
> >  src/auditctl.c             |   14 +++++++++++++-
> >  9 files changed, 64 insertions(+), 5 deletions(-)
> > 
> > diff --git a/docs/audit_add_rule_data.3 b/docs/audit_add_rule_data.3
> > index 2321f39..4867e8c 100644
> > --- a/docs/audit_add_rule_data.3
> > +++ b/docs/audit_add_rule_data.3
> > @@ -22,6 +22,9 @@ AUDIT_FILTER_EXIT - Apply rule at syscall exit.
> >  .TP
> >  \(bu
> >  AUDIT_FILTER_TYPE - Apply rule at audit_log_start.
> > +.TP
> > +\(bu
> > +AUDIT_FILTER_PATH - Apply rule at __audit_inode_child.
> 
> I don't think this is real clear. Maybe some others need touching up here as 
> well. But we should say something someone with a casual knowledge of audit 
> would understand.

Agreed.  How about "Apply rule when adding PATH auxiliary records to SYSCALL events."
> 
> >  .LP
> > 
> >  .PP
> > diff --git a/lib/errormsg.h b/lib/errormsg.h
> > index 50c7d50..2a6e4d6 100644
> > --- a/lib/errormsg.h
> > +++ b/lib/errormsg.h
> > @@ -20,6 +20,7 @@
> >   * Authors:
> >   *      Zhang Xiliang <zhangxiliang at cn.fujitsu.com>
> >   *      Steve Grubb <sgrubb at redhat.com>
> > + *      Richard Guy Briggs <rgb at redhat.com>
> >   */
> > 
> >  struct msg_tab {
> > @@ -70,6 +71,8 @@ static const struct msg_tab err_msgtab[] = {
> >      { -32,    0,    "field data is missing" },
> >      { -33,    2,    "-C field incompatible" },
> >      { -34,    2,    "-C value incompatible" },
> > +    { -35,    1,    "field is not valid for the filter" },
> > +    { -36,    1,    "filter is not supported ty kernel" },
> >  };
> 
> Numbers need re-aligning.

Probably...

> -Steve
> 
> >  #define EAU_OPMISSING		1
> >  #define EAU_FIELDUNKNOWN	2
> > @@ -103,4 +106,6 @@ static const struct msg_tab err_msgtab[] = {
> >  #define EAU_DATAMISSING		32
> >  #define EAU_COMPFIELDINCOMPAT	33
> >  #define EAU_COMPVALINCOMPAT	34
> > +#define EAU_FIELDUNAVAIL	35
> > +#define EAU_FILTERNOSUPPORT	36
> >  #endif
> > diff --git a/lib/fieldtab.h b/lib/fieldtab.h
> > index 0c5e39d..c425d5b 100644
> > --- a/lib/fieldtab.h
> > +++ b/lib/fieldtab.h
> > @@ -18,6 +18,7 @@
> >   *
> >   * Authors:
> >   *      Steve Grubb <sgrubb at redhat.com>
> > + *      Richard Guy Briggs <rgb at redhat.com>
> >   */
> > 
> >  _S(AUDIT_PID,          "pid"          )
> > @@ -56,6 +57,7 @@ _S(AUDIT_WATCH,        "path"         )
> >  _S(AUDIT_PERM,         "perm"         )
> >  _S(AUDIT_DIR,          "dir"          )
> >  _S(AUDIT_FILETYPE,     "filetype"     )
> > +_S(AUDIT_FSTYPE,       "fstype"       )
> >  _S(AUDIT_OBJ_UID,      "obj_uid"      )
> >  _S(AUDIT_OBJ_GID,      "obj_gid"      )
> >  _S(AUDIT_FIELD_COMPARE, "field_compare" )
> > diff --git a/lib/flagtab.h b/lib/flagtab.h
> > index 4b04692..ed3e729 100644
> > --- a/lib/flagtab.h
> > +++ b/lib/flagtab.h
> > @@ -18,8 +18,10 @@
> >   *
> >   * Authors:
> >   *      Steve Grubb <sgrubb at redhat.com>
> > + *      Richard Guy Briggs <rgb at redhat.com>
> >   */
> >  _S(AUDIT_FILTER_TASK,    "task"     )
> >  _S(AUDIT_FILTER_EXIT,    "exit"     )
> >  _S(AUDIT_FILTER_USER,    "user"     )
> >  _S(AUDIT_FILTER_EXCLUDE, "exclude"  )
> > +_S(AUDIT_FILTER_PATH,    "path"     )
> > diff --git a/lib/libaudit.c b/lib/libaudit.c
> > index 028483d..f28238a 100644
> > --- a/lib/libaudit.c
> > +++ b/lib/libaudit.c
> > @@ -19,6 +19,7 @@
> >   * Authors:
> >   *      Steve Grubb <sgrubb at redhat.com>
> >   *      Rickard E. (Rik) Faith <faith at redhat.com>
> > + *      Richard Guy Briggs <rgb at redhat.com>
> >   */
> > 
> >  #include "config.h"
> > @@ -86,6 +87,7 @@ int _audit_archadded = 0;
> >  int _audit_syscalladded = 0;
> >  int _audit_exeadded = 0;
> >  int _audit_filterexcladded = 0;
> > +int _audit_filterpathadded = 0;
> >  unsigned int _audit_elf = 0U;
> >  static struct libaudit_conf config;
> > 
> > @@ -1475,6 +1477,23 @@ int audit_rule_fieldpair_data(struct audit_rule_data
> > **rulep, const char *pair, }
> >  	}
> > 
> > +	/* PATH filter can be used only with FSTYPE field */
> > +	if (flags == AUDIT_FILTER_PATH) {
> > +		uint32_t features = audit_get_features();
> > +		if ((features & AUDIT_FEATURE_BITMAP_FILTER_PATH) == 0) {
> > +			return -EAU_FILTERNOSUPPORT;
> > +		} else {
> > +			switch(field) {
> > +				case AUDIT_FSTYPE:
> > +					_audit_filterpathadded = 1;
> > +				case AUDIT_FILTERKEY:
> > +					break;
> > +				default:
> > +					return -EAU_FIELDUNAVAIL;
> > +			}
> > +		}
> > +	}
> > +
> >  	rule->fields[rule->field_count] = field;
> >  	rule->fieldflags[rule->field_count] = op;
> >  	switch (field)
> > @@ -1589,7 +1608,8 @@ int audit_rule_fieldpair_data(struct audit_rule_data
> > **rulep, const char *pair, }
> >  			if (field == AUDIT_FILTERKEY &&
> >  				!(_audit_syscalladded || _audit_permadded ||
> > -				_audit_exeadded || _audit_filterexcladded))
> > +				_audit_exeadded || _audit_filterexcladded ||
> > +				_audit_filterpathadded))
> >                                  return -EAU_KEYDEP;
> >  			vlen = strlen(v);
> >  			if (field == AUDIT_FILTERKEY &&
> > @@ -1724,7 +1744,7 @@ int audit_rule_fieldpair_data(struct audit_rule_data
> > **rulep, const char *pair, return -EAU_EXITONLY;
> >  			/* fallthrough */
> >  		default:
> > -			if (field == AUDIT_INODE) {
> > +			if (field == AUDIT_INODE || field == AUDIT_FSTYPE) {
> >  				if (!(op == AUDIT_NOT_EQUAL ||
> >  							op == AUDIT_EQUAL))
> >  					return -EAU_OPEQNOTEQ;
> > @@ -1736,6 +1756,8 @@ int audit_rule_fieldpair_data(struct audit_rule_data
> > **rulep, const char *pair, if (!isdigit((char)*(v)))
> >  				return -EAU_FIELDVALNUM;
> > 
> > +			if (field == AUDIT_FSTYPE && flags != AUDIT_FILTER_PATH)
> > +				return -EAU_FIELDUNAVAIL;
> >  			if (field == AUDIT_INODE)
> >  				rule->values[rule->field_count] =
> >  					strtoul(v, NULL, 0);
> > diff --git a/lib/libaudit.h b/lib/libaudit.h
> > index e5c7a4d..e9c4973 100644
> > --- a/lib/libaudit.h
> > +++ b/lib/libaudit.h
> > @@ -277,6 +277,9 @@ extern "C" {
> >  #define AUDIT_KEY_SEPARATOR 0x01
> > 
> >  /* These are used in filter control */
> > +#ifndef AUDIT_FILTER_PATH
> > +#define AUDIT_FILTER_PATH	0x06 /* PATH record filter in 
> __audit_inode_child
> > */ +#endif
> >  #define AUDIT_FILTER_EXCLUDE	AUDIT_FILTER_TYPE
> >  #define AUDIT_FILTER_MASK	0x07	/* Mask to get actual filter */
> >  #define AUDIT_FILTER_UNSET	0x80	/* This value means filter is unset */
> > @@ -305,6 +308,9 @@ extern "C" {
> >  #ifndef AUDIT_FEATURE_BITMAP_LOST_RESET
> >  #define AUDIT_FEATURE_BITMAP_LOST_RESET		0x00000020
> >  #endif
> > +#ifndef AUDIT_FEATURE_BITMAP_FILTER_PATH
> > +#define AUDIT_FEATURE_BITMAP_FILTER_PATH	0x00000040
> > +#endif
> > 
> >  /* Defines for interfield comparison update */
> >  #ifndef AUDIT_OBJ_UID
> > @@ -324,6 +330,10 @@ extern "C" {
> >  #define AUDIT_SESSIONID 25
> >  #endif
> > 
> > +#ifndef AUDIT_FSTYPE
> > +#define AUDIT_FSTYPE 26
> > +#endif
> > +
> >  #ifndef AUDIT_COMPARE_UID_TO_OBJ_UID
> >  #define AUDIT_COMPARE_UID_TO_OBJ_UID   1
> >  #endif
> > diff --git a/lib/private.h b/lib/private.h
> > index 855187b..117d6e3 100644
> > --- a/lib/private.h
> > +++ b/lib/private.h
> > @@ -140,6 +140,7 @@ extern int _audit_archadded;
> >  extern int _audit_syscalladded;
> >  extern int _audit_exeadded;
> >  extern int _audit_filterexcladded;
> > +extern int _audit_filterpathadded;
> >  extern unsigned int _audit_elf;
> > 
> >  #ifdef __cplusplus
> > diff --git a/src/auditctl-listing.c b/src/auditctl-listing.c
> > index 3bc8e71..e8640dd 100644
> > --- a/src/auditctl-listing.c
> > +++ b/src/auditctl-listing.c
> > @@ -91,7 +91,8 @@ static int is_watch(const struct audit_rule_data *r)
> > 
> >  	if (((r->flags & AUDIT_FILTER_MASK) != AUDIT_FILTER_USER) &&
> >  		((r->flags & AUDIT_FILTER_MASK) != AUDIT_FILTER_TASK) &&
> > -		((r->flags & AUDIT_FILTER_MASK) != AUDIT_FILTER_EXCLUDE)) {
> > +		((r->flags & AUDIT_FILTER_MASK) != AUDIT_FILTER_EXCLUDE) &&
> > +		((r->flags & AUDIT_FILTER_MASK) != AUDIT_FILTER_PATH)) {
> >  		for (i = 0; i < (AUDIT_BITMASK_SIZE-1); i++) {
> >  			if (r->mask[i] != (uint32_t)~0) {
> >  				all = 0;
> > @@ -139,7 +140,8 @@ static int print_syscall(const struct audit_rule_data
> > *r, unsigned int *sc) /* Rules on the following filters do not take a
> > syscall */
> >  	if (((r->flags & AUDIT_FILTER_MASK) == AUDIT_FILTER_USER) ||
> >  	    ((r->flags & AUDIT_FILTER_MASK) == AUDIT_FILTER_TASK) ||
> > -	    ((r->flags &AUDIT_FILTER_MASK) == AUDIT_FILTER_EXCLUDE))
> > +	    ((r->flags &AUDIT_FILTER_MASK) == AUDIT_FILTER_EXCLUDE) ||
> > +	    ((r->flags &AUDIT_FILTER_MASK) == AUDIT_FILTER_PATH))
> >  		return 0;
> > 
> >  	/* See if its all or specific syscalls */
> > diff --git a/src/auditctl.c b/src/auditctl.c
> > index c785087..c7e8f0f 100644
> > --- a/src/auditctl.c
> > +++ b/src/auditctl.c
> > @@ -19,6 +19,7 @@
> >   * Authors:
> >   *     Steve Grubb <sgrubb at redhat.com>
> >   *     Rickard E. (Rik) Faith <faith at redhat.com>
> > + *     Richard Guy Briggs <rgb at redhat.com>
> >   */
> > 
> >  #include "config.h"
> > @@ -75,6 +76,7 @@ static int reset_vars(void)
> >  	_audit_archadded = 0;
> >  	_audit_exeadded = 0;
> >  	_audit_filterexcladded = 0;
> > +	_audit_filterpathadded = 0;
> >  	_audit_elf = 0;
> >  	add = AUDIT_FILTER_UNSET;
> >  	del = AUDIT_FILTER_UNSET;
> > @@ -152,6 +154,8 @@ static int lookup_filter(const char *str, int *filter)
> >  		*filter = AUDIT_FILTER_EXIT;
> >  	else if (strcmp(str, "user") == 0)
> >  		*filter = AUDIT_FILTER_USER;
> > +	else if (strcmp(str, "path") == 0)
> > +		*filter = AUDIT_FILTER_PATH;
> >  	else if (strcmp(str, "exclude") == 0) {
> >  		*filter = AUDIT_FILTER_EXCLUDE;
> >  		exclude = 1;
> > @@ -761,6 +765,13 @@ static int setopt(int count, int lineno, char *vars[])
> >  			audit_msg(LOG_ERR,
> >  			  "Error: syscall auditing being added to user list");
> >  			return -1;
> > +		} else if (((add & (AUDIT_FILTER_MASK|AUDIT_FILTER_UNSET)) ==
> > +				AUDIT_FILTER_PATH || (del &
> > +				(AUDIT_FILTER_MASK|AUDIT_FILTER_UNSET)) ==
> > +				AUDIT_FILTER_PATH)) {
> > +			audit_msg(LOG_ERR,
> > +			  "Error: syscall auditing being added to path list");
> > +			return -1;
> >  		} else if (exclude) {
> >  			audit_msg(LOG_ERR,
> >  		    "Error: syscall auditing cannot be put on exclude list");
> > @@ -937,7 +948,8 @@ static int setopt(int count, int lineno, char *vars[])
> >  		break;
> >  	case 'k':
> >  		if (!(_audit_syscalladded || _audit_permadded ||
> > -		      _audit_exeadded || _audit_filterexcladded) ||
> > +		      _audit_exeadded || _audit_filterexcladded ||
> > +		      _audit_filterpathadded) ||
> >  		    (add==AUDIT_FILTER_UNSET && del==AUDIT_FILTER_UNSET)) {
> >  			audit_msg(LOG_ERR,
> >  		    "key option needs a watch or syscall given prior to it");
> 
> 
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb at redhat.com>
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




More information about the Linux-audit mailing list