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

[Steve Grubb <sgrubb@redhat.com>]

Hello,

On Monday, June 12, 2017 10:45:50 PM EDT Richard Guy Briggs wrote:
> On 2017-06-12 20:28, Steve Grubb wrote:
> > 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.)

And now that the other error message macros have been applied...


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

What else could this filter have its hands on? Could it audit mounting/
unmounting of certain file systems?

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

I don't like having path as a filter and path as a field option. Path is 
getting too overloaded.

> At the moment it is only active on audit_inode_child, but I could
> potentially see it being active on audit_inode as well.

I'd lean towards filesystem for the filter name.

-Steve

> > > 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.
> 
> Agreed.  How about "Apply rule when adding PATH auxiliary records to SYSCALL
> events."

I updated this man page. It needs to say when its applicable, whether events 
are normally accepted or blocked with no rule applied, and what the typical 
use case is.

-Steve