On 2019-05-08 18:05, Paul Moore wrote: > On Wed, May 8, 2019 at 12:46 PM Richard Guy Briggs <[email protected]> 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 <[email protected]> > > --- > > 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?) Subject: Re: [PATCH ghak64 V1] audit: add saddr_fam filter field On Tue, 30 Apr 2019 13:37:41 -0400, Paul Moore <[email protected]> wrote: > On Tue, Apr 30, 2019 at 1:01 PM Richard Guy Briggs <[email protected]> wrote: > > On 2019-04-27 10:09, Paul Moore wrote: > > > 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. > > I see it as a usability issue. In general terms, we shouldn't allow > admins to add a nonsense filter rule to the kernel, and we shouldn't > rely on the userspace to catch everything. > paul moore - RGB -- Richard Guy Briggs <[email protected]> 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 -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
