[PATCH ghak64 V1] audit: add saddr_fam filter field

[Richard Guy Briggs]

On 2019-04-27 10:09, Paul Moore wrote:
> On Fri, Apr 26, 2019 at 1:00 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 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>
> > ---
> >  include/uapi/linux/audit.h | 1 +
> >  kernel/auditfilter.c       | 6 ++----
> >  kernel/auditsc.c           | 5 +++++
> >  3 files changed, 8 insertions(+), 4 deletions(-)
> 
> In general -rc6 is getting late for things that touch include/uapi,
> but that shouldn't be news.  I also don't see any references here, or
> in the GitHub issue, regarding new/modified tests, but I'm sure you
> are also aware of that and are working on something (I hope anyway).

Please don't let this distract you from other patchsets already posted.

I have a test procedure that I used to verify this is in fact working,
but I'm still thinking about how to automate it (to remove the rule
add/delete events) to check it is properly filtering out any other
events which is the whole purpose of this feature.

> Beyond that, looking at the patch below it seems like there is an
> obvious omission regarding validating the address families; some
> updates to audit_field_valid() to verify that the specified address
> family is greater than AF_UNSPEC and less than AF_MAX would be good to
> have.

I thought of that and as you can see had added it to the userspace code
that accompanies it.  There isn't really any harm to allow it to go
outside those address family limits if someone really wants to do that.

> > 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 2c3c2f349b23..f4bb8e61a54b 100644
> > --- a/kernel/auditfilter.c
> > +++ b/kernel/auditfilter.c
> > @@ -410,6 +410,8 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f)
> >         /* FALL THROUGH */
> >         case AUDIT_ARCH:
> >         case AUDIT_FSTYPE:
> > +       case AUDIT_EXE:
> > +       case AUDIT_SADDR_FAM:
> >                 if (f->op != Audit_not_equal && f->op != Audit_equal)
> >                         return -EINVAL;
> >                 break;
> > @@ -425,10 +427,6 @@ 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_EXE:
> > -               if (f->op != Audit_not_equal && f->op != Audit_equal)
> > -                       return -EINVAL;
> > -               break;
> >         }
> >         return 0;
> >  }
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 5371b59bde36..0a830f67ca7a 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -615,6 +615,11 @@ static int audit_filter_rules(struct task_struct *tsk,
> >                 case AUDIT_LOGINUID_SET:
> >                         result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val);
> >                         break;
> > +               case AUDIT_SADDR_FAM:
> > +                       if (ctx->sockaddr)
> > +                               result = audit_comparator(ctx->sockaddr->ss_family,
> > +                                                         f->op, f->val);
> > +                       break;
> >                 case AUDIT_SUBJ_USER:
> >                 case AUDIT_SUBJ_ROLE:
> >                 case AUDIT_SUBJ_TYPE:
> > --
> > 1.8.3.1
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb at 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