Re: [PATCH V3] filter: add filesystem filter with fstype

[Steve Grubb <sgrubb@redhat.com>]

On Tuesday, June 27, 2017 2:46:29 PM 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 "filesystem" 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,filesystem -F fstype=0x74726163 -F key=ignore_tracefs
> 	-a never,filesystem -F fstype=0x64626720 -F key=ignore_debugfs
> 
> Note: "always,filesystem" will log the PATH record anyways and add latency.
> 
> See: https://github.com/linux-audit/audit-kernel/issues/8
> See: https://github.com/linux-audit/audit-userspace/issues/15
> Test case: https://github.com/linux-audit/audit-testsuite/issues/42
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>

Applied. However, I don't like the idea of making people type in magic 
numbers. I'll start a fs name to magic translator and just add a few file 
systems.

-Steve

> ---
> v3:
> 	Update feature bitmap macros to reflect filter name change.
> v2:
> 	Change filter name from "path" to "filesystem".
> 	Rebase onto other patches accepted upstream.
> 
>  docs/audit_add_rule_data.3 |    3 +++
>  lib/errormsg.h             |    5 +++++
>  lib/fieldtab.h             |    2 ++
>  lib/flagtab.h              |   10 ++++++----
>  lib/libaudit.c             |   26 ++++++++++++++++++++++++--
>  lib/libaudit.h             |   10 ++++++++++
>  lib/private.h              |    1 +
>  src/auditctl-listing.c     |    6 ++++--
>  src/auditctl.c             |   16 ++++++++++++++--
>  9 files changed, 69 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/audit_add_rule_data.3 b/docs/audit_add_rule_data.3
> index a0802c0..1e7540c 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. This is
> the main filter that is .TP
>  \(bu
>  AUDIT_FILTER_TYPE - Apply rule at audit_log_start. This is the exclude
> filter which discards any records that match. +.TP
> +\(bu
> +AUDIT_FILTER_FS - Apply rule when adding PATH auxiliary records to SYSCALL
> events. This is the filesystem filter. This is used to ignore PATH records
> that are not of interest. .LP
> 
>  .PP
> diff --git a/lib/errormsg.h b/lib/errormsg.h
> index 91d8252..ef54589 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 {
> @@ -66,6 +67,8 @@ struct msg_tab {
>  #define EAU_FIELDNOFILTER	31
>  #define EAU_FILTERMISSING	32
>  #define EAU_COMPINCOMPAT	33
> +#define EAU_FIELDUNAVAIL	34
> +#define EAU_FILTERNOSUPPORT	35
>  static const struct msg_tab err_msgtab[] = {
>      { -EAU_OPMISSING,		2, "-F missing operation for" },
>      { -EAU_FIELDUNKNOWN,	2, "-F unknown field:" },
> @@ -100,5 +103,7 @@ static const struct msg_tab err_msgtab[] = {
>      { -EAU_FIELDNOFILTER,	1, "must be used with exclude, user, or exit
> filter" }, { -EAU_FILTERMISSING,	0, "filter is missing from rule" },
>      { -EAU_COMPINCOMPAT,	2, "-C incompatible comparison" },
> +    { -EAU_FIELDUNAVAIL,	1, "field is not valid for the filter" },
> +    { -EAU_FILTERNOSUPPORT,	1, "filter is not supported ty kernel" },
>  };
>  #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..7a618e0 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_TASK,    "task"      )
> +_S(AUDIT_FILTER_EXIT,    "exit"      )
> +_S(AUDIT_FILTER_USER,    "user"      )
> +_S(AUDIT_FILTER_EXCLUDE, "exclude"   )
> +_S(AUDIT_FILTER_FS,      "filesystem")
> diff --git a/lib/libaudit.c b/lib/libaudit.c
> index 18cd384..58134a2 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"
> @@ -85,6 +86,7 @@ int _audit_permadded = 0;
>  int _audit_archadded = 0;
>  int _audit_syscalladded = 0;
>  int _audit_exeadded = 0;
> +int _audit_filterfsadded = 0;
>  unsigned int _audit_elf = 0U;
>  static struct libaudit_conf config;
> 
> @@ -1466,6 +1468,23 @@ int audit_rule_fieldpair_data(struct audit_rule_data
> **rulep, const char *pair, }
>  	}
> 
> +	/* FS filter can be used only with FSTYPE field */
> +	if (flags == AUDIT_FILTER_FS) {
> +		uint32_t features = audit_get_features();
> +		if ((features & AUDIT_FEATURE_BITMAP_FILTER_FS) == 0) {
> +			return -EAU_FILTERNOSUPPORT;
> +		} else {
> +			switch(field) {
> +				case AUDIT_FSTYPE:
> +					_audit_filterfsadded = 1;
> +				case AUDIT_FILTERKEY:
> +					break;
> +				default:
> +					return -EAU_FIELDUNAVAIL;
> +			}
> +		}
> +	}
> +
>  	rule->fields[rule->field_count] = field;
>  	rule->fieldflags[rule->field_count] = op;
>  	switch (field)
> @@ -1580,7 +1599,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_exeadded ||
> +				_audit_filterfsadded))
>                                  return -EAU_KEYDEP;
>  			vlen = strlen(v);
>  			if (field == AUDIT_FILTERKEY &&
> @@ -1715,7 +1735,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;
> @@ -1727,6 +1747,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_FS)
> +				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..70646cd 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_FS
> +#define AUDIT_FILTER_FS		0x06 /* FS 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_FS
> +#define AUDIT_FEATURE_BITMAP_FILTER_FS		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 cde1906..bd5e8b3 100644
> --- a/lib/private.h
> +++ b/lib/private.h
> @@ -139,6 +139,7 @@ extern int _audit_permadded;
>  extern int _audit_archadded;
>  extern int _audit_syscalladded;
>  extern int _audit_exeadded;
> +extern int _audit_filterfsadded;
>  extern unsigned int _audit_elf;
> 
>  #ifdef __cplusplus
> diff --git a/src/auditctl-listing.c b/src/auditctl-listing.c
> index 3bc8e71..50bc0b8 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_FS)) {
>  		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_FS))
>  		return 0;
> 
>  	/* See if its all or specific syscalls */
> diff --git a/src/auditctl.c b/src/auditctl.c
> index 04765f4..b99c957 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"
> @@ -74,6 +75,7 @@ static int reset_vars(void)
>  	_audit_permadded = 0;
>  	_audit_archadded = 0;
>  	_audit_exeadded = 0;
> +	_audit_filterfsadded = 0;
>  	_audit_elf = 0;
>  	add = AUDIT_FILTER_UNSET;
>  	del = AUDIT_FILTER_UNSET;
> @@ -151,6 +153,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, "filesystem") == 0)
> +		*filter = AUDIT_FILTER_FS;
>  	else if (strcmp(str, "exclude") == 0) {
>  		*filter = AUDIT_FILTER_EXCLUDE;
>  		exclude = 1;
> @@ -760,6 +764,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_FS || (del &
> +				(AUDIT_FILTER_MASK|AUDIT_FILTER_UNSET)) ==
> +				AUDIT_FILTER_FS)) {
> +			audit_msg(LOG_ERR,
> +			  "Error: syscall auditing being added to filesystem list");
> +			return -1;
>  		} else if (exclude) {
>  			audit_msg(LOG_ERR,
>  		    "Error: syscall auditing cannot be put on exclude list");
> @@ -936,8 +947,9 @@ static int setopt(int count, int lineno, char *vars[])
>  		break;
>  	case 'k':
>  		if (!(_audit_syscalladded || _audit_permadded ||
> -			     _audit_exeadded) || (add==AUDIT_FILTER_UNSET &&
> -					del==AUDIT_FILTER_UNSET)) {
> +		      _audit_exeadded ||
> +		      _audit_filterfsadded) ||
> +		    (add==AUDIT_FILTER_UNSET && del==AUDIT_FILTER_UNSET)) {
>  			audit_msg(LOG_ERR,
>  		    "key option needs a watch or syscall given prior to it");
>  			retval = -1;