Re: Sycall Rules vs Watch Rules

From: Amjad Gabbar <amjadgabbar11@gmail.com>
To: Steve Grubb <sgrubb@redhat.com>
Cc: Richard Guy Briggs <rgb@redhat.com>, linux-audit@redhat.com
Subject: Re: Sycall Rules vs Watch Rules
Date: Thu, 21 Sep 2023 15:02:49 -0500	[thread overview]
Message-ID: <CAJcJf=RKAqp0PPQ2EmOZ5jJ6KGZ4rAvmabdDsg+MvkbmcomChA@mail.gmail.com> (raw)
In-Reply-To: <8320136.T7Z3S40VBb@x2>

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

>But I am surprised my concept of how it works doesn't match the
implementation
I wouldn't worry too much about it. The important thing is we caught it and
can move forward and make the necessary corrections.

>The best solution would be a kernel modification so that there are no
mismatched lists.
I agree as well....This would be the cleanest solution. This would also
solve the userspace problem of maintaining different lists which can get
out of hand fairly quickly.

> I guess we can warn on that to rewrite in syscall notation.
We certainly should.
I think the user should know that there is a performance cost associated
with watches and we should explicitly mention how it can be optimized in
the manpages also.
The reason being I am pretty sure, numerous users/repos still do make use
of the -w notation and we do want to let them know the issue here.
We also need to make quite a few changes to the manpages also regarding
this.
Because, initially even I was very confused when reading the man pages and
seeing the actual implementation of and results were not quite in sync.

Regards
Ali Adnan

On Wed, Sep 20, 2023 at 6:33 PM Steve Grubb <sgrubb@redhat.com> wrote:

> On Wednesday, September 20, 2023 2:45:26 PM EDT Steve Grubb wrote:
> > On Tuesday, September 19, 2023 8:26:04 PM EDT Amjad Gabbar wrote:
> > > > The perm fields select the right system calls
> > > > that should be reported on.
> > >
> > > That is accurate from a functional perspective. There is no change in
> the
> > > events logged. But there is a difference in performance. This is most
> > > evident for syscalls not part of the perm fields.
> >
> > <snip>
> >
> > > If we look at the performance numbers for the file rules as is, the
> > > auditing percentage is about 14%.
> > >
> > > Now if we were to just add the specific syscalls that the perm fields
> > > filter on in the rules file, the auditing percentage would drop to
> around
> > > 2%.
> >
> > I think I am mis-remembering something, or there was a change way back in
> > the beginning. The plan was that we could optimize access by letting the
> > kernel pick the relevant syscalls based on the permissions. User space
> > would just define the permissions and the kernel would make it so.
> >
> > But there were several redesigns of the file auditing. I looked back as
> far
> > as the 3.1 kernel and it always follows lookup the syscall, if it's
> > relevant, then check the rest of the fields in the rule. This eventually
> > checks the set of syscalls selected by the perms.
> >
> > The way that it should have worked is when perms is given, throw away any
> > syscalls and set the mask based on the perms selected. That would have
> been
> > optimal and it was what Al Viro and I talked about long ago. However, it
> > went through several redesigns.
>
> I did some digging. This is the original patch:
> https://listman.redhat.com/archives/linux-audit/2006-August/003593.html
>
> Al does mention that syscalls taking a descriptor should not be included.
> I
> guess that can be cleaned up in the include/asm-generic/audit_*.h files.
>
> I think that patch would have landed in the 2.6.18 kernel. I found a
> 2.6.21
> kernel and the path taken is different:
>
> audit_syscall_exit
> audit_get_context
> audit_filter_inodes   <--- this is where it differs
> audit_filter_rules
> audit_match_perm
>
> In the old kernel, it still called the syscall filter. But I think that
> was
> optimized later. But the whole point of making the perms field was so that
> user space could just tell the kernel what it needs and the kernel would
> select exactly the syscalls needed. There was no other reason to have it.
>
> Now, what to do about it? A watch was biarch. There were 2 tables for 32 &
> 64
> bits and it would swing between them based on the syscall's arch. To fix
> this
> in user space would mean a watch would have to create 2 rules to cover
> biarch. And some systems conceivably may not have 32 bit enabled or vice
> versa. There would be no way for user space to know and work around a
> missing
> arch.
>
> The  -w notation really can't be optimized. It doesn't specify an arch so
> it
> has to be "all". I guess we can warn on that to rewrite in syscall
> notation.
>
> -Steve
>
> > The problem now is that user space has no list of syscalls that match
> each
> > permission. And then, there's no good way to sync based on mixing and
> > matching kernels and user space. The kernel may have an updated syscall
> > list user space doesn't know about and vice versa.
> >
> > I think you are on to something important. But I am surprised my concept
> of
> > how it works doesn't match the implementation. (Al Viro did the original
> > implementation way back around 2006/7.) The best solution would be a
> > kernel modification so that there are no mismatched lists. A suboptimal
> > solution would be to maintain 2 lists and hope they don't change. Which
> by
> > the way, I think the kernel lists are outdated again. (Syscalls keep
> > getting added - quotactl_fd for example)
> >
> > -Steve
> >
> >
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://listman.redhat.com/mailman/listinfo/linux-audit
>
>
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 6479 bytes --]

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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit