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(a)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(a)cn.fujitsu.com>
> * Steve Grubb <sgrubb(a)redhat.com>
> + * Richard Guy Briggs <rgb(a)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(a)redhat.com>
> + * Richard Guy Briggs <rgb(a)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(a)redhat.com>
> + * Richard Guy Briggs <rgb(a)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(a)redhat.com>
> * Rickard E. (Rik) Faith <faith(a)redhat.com>
> + * Richard Guy Briggs <rgb(a)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(a)redhat.com>
> * Rickard E. (Rik) Faith <faith(a)redhat.com>
> + * Richard Guy Briggs <rgb(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rgb(a)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