[PATCH ghak64 V2] audit: add saddr_fam filter field

[Paul Moore]

On Wed, May 8, 2019 at 9:52 PM Richard Guy Briggs <rgb at redhat.com> wrote:
> On 2019-05-08 18:05, Paul Moore wrote:
> > On Wed, May 8, 2019 at 12:46 PM Richard Guy Briggs <rgb at redhat.com> wrote:
> > >
> > > Provide a method to filter out sockaddr and bind calls by network
> > > address family.
> > >
> > > Existing SOCKADDR records are listed for any network activity.
> > > Implement the AUDIT_SADDR_FAM field selector to be able to classify or
> > > limit records to specific network address families, such as AF_INET or
> > > AF_INET6.
> > >
> > > An example of a network record that is unlikely to be useful and flood
> > > the logs:
> > >
> > > type=SOCKADDR msg=audit(07/27/2017 12:18:27.019:845) : saddr={ fam=local
> > > path=/var/run/nscd/socket }
> > > type=SYSCALL msg=audit(07/27/2017 12:18:27.019:845) : arch=x86_64
> > > syscall=connect success=no exit=ENOENT(No such file or directory) a0=0x3
> > > a1=0x7fff229c4980 a2=0x6e a3=0x6 items=1 ppid=3301 pid=6145 auid=sgrubb
> > > uid=sgrubb gid=sgrubb euid=sgrubb suid=sgrubb fsuid=sgrubb egid=sgrubb
> > > sgid=sgrubb fsgid=sgrubb tty=pts3 ses=4 comm=bash exe=/usr/bin/bash
> > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> > > key=network-test
> > >
> > > Please see the audit-testsuite PR at
> > > https://github.com/linux-audit/audit-testsuite/pull/87
> > > Please see the github issue
> > > https://github.com/linux-audit/audit-kernel/issues/64
> > > Please see the github issue for the accompanying userspace support
> > > https://github.com/linux-audit/audit-userspace/issues/93
> > >
> > > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
> > > ---
> > > Changelog:
> > > v2:
> > > - rebase on ghak73 v2
> > > - check for valid range of saddr_fam value
> > >
> > >  include/uapi/linux/audit.h | 1 +
> > >  kernel/auditfilter.c       | 5 +++++
> > >  kernel/auditsc.c           | 5 +++++
> > >  3 files changed, 11 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > index a1280af20336..c89c6495983d 100644
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -281,6 +281,7 @@
> > >  #define AUDIT_OBJ_GID  110
> > >  #define AUDIT_FIELD_COMPARE    111
> > >  #define AUDIT_EXE      112
> > > +#define AUDIT_SADDR_FAM        113
> > >
> > >  #define AUDIT_ARG0      200
> > >  #define AUDIT_ARG1      (AUDIT_ARG0+1)
> > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > > index 5beb2244d5ba..4c897281beb8 100644
> > > --- a/kernel/auditfilter.c
> > > +++ b/kernel/auditfilter.c
> > > @@ -387,6 +387,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f)
> > >         case AUDIT_SUCCESS:
> > >         case AUDIT_INODE:
> > >         case AUDIT_SESSIONID:
> > > +       case AUDIT_SADDR_FAM:
> > >                 /* bit ops are only useful on syscall args */
> > >                 if (f->op == Audit_bitmask || f->op == Audit_bittest)
> > >                         return -EINVAL;
> > > @@ -438,6 +439,10 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f)
> > >                 if (f->val > AUDIT_MAX_FIELD_COMPARE)
> > >                         return -EINVAL;
> > >                 break;
> > > +       case AUDIT_SADDR_FAM:
> > > +               if (f->val <= AF_UNSPEC || f->val >= AF_MAX)
> >
> > AF_UNSPEC is a valid address family for some operations, and while I'm
> > not sure what value there is in an auditing these events, we should
> > allow it.  It's also worth noting that f->val is an unsigned value so
> > we are never going to see a value less than AF_UNSPEC/0.  This is why
> > on your earlier patch I only talked about AF_MAX and not AF_UNSPEC.
>
> On reflection, I agree about the unsigned nature of f->val.  I'm also prepared
> to allow AF_UNSPEC/0.
>
> However, I'm having trouble reconciling your last sentence with the
> previous thread in which you explicitly mention AF_UNSPEC and sanity
> checks related to it:  (Am I losing it, or misunderstanding?)

I'm not prepared to comment on your mental condition, but it appears
that I did mistakenly say that we should exclude AF_UNSPEC.  My
apologies.

-- 
paul moore
www.paul-moore.com