Re: [PATCH] filter: add path filter with fstype

[Steve Grubb <sgrubb@redhat.com>]

Hello,

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?

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?


> 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@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.

>  .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@cn.fujitsu.com>
>   *      Steve Grubb <sgrubb@redhat.com>
> + *      Richard Guy Briggs <rgb@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.

-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@redhat.com>
> + *      Richard Guy Briggs <rgb@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@redhat.com>
> + *      Richard Guy Briggs <rgb@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@redhat.com>
>   *      Rickard E. (Rik) Faith <faith@redhat.com>
> + *      Richard Guy Briggs <rgb@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@redhat.com>
>   *     Rickard E. (Rik) Faith <faith@redhat.com>
> + *     Richard Guy Briggs <rgb@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");