[PATCH] audit: fix broken class-based syscall audit

[PATCH] audit: fix broken class-based syscall audit

[Klaus Weidner]

Bug description: When I add an audit watch on a file with no arguments, I
get perm=rwxa but on ia64, changes to the mode and context aren't
audited.  I get audit records on i386 and x86_64.
(from https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=239887 )

The sanity check in audit_match_class() is wrong, AUDIT_BITMASK_SIZE is
64, providing space for 2048 syscalls in 64 * 32bit integers. The
comparison only supports 256 syscalls (sizeof __u32 is 4), and silently
returns "no match" for valid higher-numbered syscalls.

This breaks class-based audit for all syscalls on ia64 since on that
architecture syscall numbers start at 1024. It breaks some syscall audit
on other architectures also, for example __NR_fchmodat is 306 on x86.

I'd suggest adding a printk() in addition to returning 0 - you don't want
to silently ignore unknown or unsupported syscalls when auditing.

Signed-off-by: Klaus Weidner <klaus@atsec.com>

Followup discussion was on the linux-audit mailing list:

 https://www.redhat.com/archives/linux-audit/2007-May/msg00030.html

Acked-by: Al Viro <viro@zeniv.linux.org.uk>

--- linux-2.6.18.i686/kernel/auditfilter.c.lspp.80	2007-05-11 17:06:08.000000000 -0500
+++ linux-2.6.18.i686/kernel/auditfilter.c	2007-05-11 17:09:37.000000000 -0500
@@ -306,7 +306,7 @@
 
 int audit_match_class(int class, unsigned syscall)
 {
-	if (unlikely(syscall >= AUDIT_BITMASK_SIZE * sizeof(__u32)))
+	if (unlikely(syscall >= AUDIT_BITMASK_SIZE * 32))
 		return 0;
 	if (unlikely(class >= AUDIT_SYSCALL_CLASSES || !classes[class]))
 		return 0;

Re: [PATCH] audit: fix broken class-based syscall audit

[Valdis.Kletnieks]

[-- Attachment #1.1: Type: text/plain, Size: 1038 bytes --]

On Wed, 16 May 2007 17:45:42 CDT, Klaus Weidner said:
> Bug description: When I add an audit watch on a file with no arguments, I
> get perm=rwxa but on ia64, changes to the mode and context aren't
> audited.  I get audit records on i386 and x86_64.
> (from https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=239887 )
> 
> The sanity check in audit_match_class() is wrong, AUDIT_BITMASK_SIZE is
> 64, providing space for 2048 syscalls in 64 * 32bit integers. The
> comparison only supports 256 syscalls (sizeof __u32 is 4), and silently
> returns "no match" for valid higher-numbered syscalls.
> 
> This breaks class-based audit for all syscalls on ia64 since on that
> architecture syscall numbers start at 1024. It breaks some syscall audit
> on other architectures also, for example __NR_fchmodat is 306 on x86.
> 
> I'd suggest adding a printk() in addition to returning 0 - you don't want
> to silently ignore unknown or unsupported syscalls when auditing.

Make it rate-limited, so a program can't unintentionally spam your logs.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 226 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] audit: fix broken class-based syscall audit

[Steve Grubb]

On Thursday 17 May 2007 09:19, Valdis.Kletnieks@vt.edu wrote:
> > I'd suggest adding a printk() in addition to returning 0 - you don't want
> > to silently ignore unknown or unsupported syscalls when auditing.
>
> Make it rate-limited, so a program can't unintentionally spam your logs.

For this to happen, the syscall would have to be > 2048. I'd almost image 
syscalls out of range in general...whether being auditing by class as in this 
case or with a typical syscall rule is a problem. So, way back over at 
syscall entry would be the time to notice this problem instead of here. If we 
are concerned about this, it might be a general control feature like 
enable/disable, fail mode, or backlog. We could make something to report out 
of range syscalls.

-Steve

Re: [PATCH] audit: fix broken class-based syscall audit

[Klaus Weidner]

On Thu, May 17, 2007 at 09:58:25AM -0400, Steve Grubb wrote:
> On Thursday 17 May 2007 09:19, Valdis.Kletnieks@vt.edu wrote:
> > > I'd suggest adding a printk() in addition to returning 0 - you don't want
> > > to silently ignore unknown or unsupported syscalls when auditing.
> >
> > Make it rate-limited, so a program can't unintentionally spam your logs.
> 
> For this to happen, the syscall would have to be > 2048. I'd almost image 
> syscalls out of range in general...whether being auditing by class as in this 
> case or with a typical syscall rule is a problem. So, way back over at 
> syscall entry would be the time to notice this problem instead of here. If we 
> are concerned about this, it might be a general control feature like 
> enable/disable, fail mode, or backlog. We could make something to report out 
> of range syscalls.

Can we agree to do just the simple fix for this issue for now, and maybe
revisit adding additional sanity checks later if people think they are
helpful?

-Klaus

Re: [PATCH] audit: fix broken class-based syscall audit

[Steve Grubb]

On Thursday 17 May 2007 11:23, Klaus Weidner wrote:
> > So, way back over at syscall entry would be the time to notice this
> > problem instead of here. If we are concerned about this, it might be a
> > general control feature like enable/disable, fail mode, or backlog. We
> > could make something to report out of range syscalls.
>
> Can we agree to do just the simple fix for this issue for now, and maybe
> revisit adding additional sanity checks later if people think they are
> helpful?

Certainly. The patch as submitted is fine and Al ack'ed it. I was thinking we 
should have one more cleanup as a separate patch at some point that catches 
this at syscall entry and allows ignore/printk/panic selection just like the 
fail option for the audit system does. In the case of ignore (which would be 
default), your patch is needed.

-Steve