Sycall Rules vs Watch Rules

Sycall Rules vs Watch Rules

[Amjad Gabbar]

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

Hi,

I have done some analysis and digging into how both the watch rules and
syscall rules are translated.

From my understanding, in terms of logging, both the below rules are
similar. There is no difference in either of the rules.

1. -w /etc -p wa -k ETC_WATCH

2. -a always,exit -F arch=b64 -S <all syscalls part of the write and attr
classes> -F dir=/etc  -F perm=wa -k ETC_WATCH

The write and attr classes consist of syscalls in
“include/asm-generic/audit_*.h“.

 The perm flag is needed in the second case for including open/openat
syscalls which are not a part of the write and attr syscall list.

I'd like to verify if what I mentioned earlier is accurate, and I have an
additional point but depends on whether this is accurate.

Ali

[-- Attachment #1.2: Type: text/html, Size: 4064 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

Re: Sycall Rules vs Watch Rules

[Richard Guy Briggs]

On 2023-09-06 10:56, Amjad Gabbar wrote:
> Hi,
> 
> I have done some analysis and digging into how both the watch rules and
> syscall rules are translated.
> 
> From my understanding, in terms of logging, both the below rules are
> similar. There is no difference in either of the rules.
> 
> 1. -w /etc -p wa -k ETC_WATCH

They are similar in this case.
-w behaves differently depending on the existance of the watched entity
and the presence of a trailing "/".  This is why the form above is
deprecated.

> 2. -a always,exit -F arch=b64 -S <all syscalls part of the write and attr
> classes> -F dir=/etc  -F perm=wa -k ETC_WATCH
> 
> The write and attr classes consist of syscalls in
> “include/asm-generic/audit_*.h“.
> 
>  The perm flag is needed in the second case for including open/openat
> syscalls which are not a part of the write and attr syscall list.
> 
> I'd like to verify if what I mentioned earlier is accurate, and I have an
> additional point but depends on whether this is accurate.
> 
> Ali

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: Sycall Rules vs Watch Rules

[Amjad Gabbar]

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

So,

Based on this and some experiments I have been performing, I would suggest
changing how a lot of the FileSystem rules are written and illustrated.
Ex -
https://github.com/linux-audit/audit-userspace/blob/master/rules/30-pci-dss-v31.rules#L34-L35

The rule in the repository is
-a always,exit -F path=/etc/sudoers -F perm=wa -F
key=10.2.2-priv-config-changes

My suggestion is to instead change the rule based on the permissions
defined. The above rule would change to the following based on the kernel
being used.
-a always,exit -S <list of syscalls in audit_write.h and audit_read.h
+open,openat> -F path=/etc/sudoers -F perm=wa -F
key=10.2.2-priv-config-changes

This is higher performance because we are limiting the syscalls instead of
making use of -S all which has more paths of evaluation for each and every
syscall.

Same thing for watches. Watches are inherently -S all rules which are very
performance intensive.
https://github.com/linux-audit/audit-userspace/blob/1482cec74f2d9472f81dd4f0533484bd0c26decd/lib/libaudit.c#L805

Ideally we should limit the syscalls based on the permissions being used.

I have implemented the same in my environment rules and have noticed a
massive performance difference with no difference in the events being
logged since we anyways filter eventually based on the permissions.

Let me know what you all think.

Ali Adnan.





On Wed, Sep 6, 2023 at 2:58 PM Richard Guy Briggs <rgb@redhat.com> wrote:

> On 2023-09-06 10:56, Amjad Gabbar wrote:
> > Hi,
> >
> > I have done some analysis and digging into how both the watch rules and
> > syscall rules are translated.
> >
> > From my understanding, in terms of logging, both the below rules are
> > similar. There is no difference in either of the rules.
> >
> > 1. -w /etc -p wa -k ETC_WATCH
>
> They are similar in this case.
> -w behaves differently depending on the existance of the watched entity
> and the presence of a trailing "/".  This is why the form above is
> deprecated.
>
> > 2. -a always,exit -F arch=b64 -S <all syscalls part of the write and attr
> > classes> -F dir=/etc  -F perm=wa -k ETC_WATCH
> >
> > The write and attr classes consist of syscalls in
> > “include/asm-generic/audit_*.h“.
> >
> >  The perm flag is needed in the second case for including open/openat
> > syscalls which are not a part of the write and attr syscall list.
> >
> > I'd like to verify if what I mentioned earlier is accurate, and I have an
> > additional point but depends on whether this is accurate.
> >
> > Ali
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> Upstream IRC: SunRaycer
> Voice: +1.613.860 2354 SMS: +1.613.518.6570
>
>

[-- Attachment #1.2: Type: text/html, Size: 3929 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

Re: Sycall Rules vs Watch Rules

[Amjad Gabbar]

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

Hi,

Just wanted to follow up wrt the previous findings and experiments and what
some of your thoughts are on the suggested optimizations.

Regards
Ali

On Tue, Sep 12, 2023 at 4:20 PM Amjad Gabbar <amjadgabbar11@gmail.com>
wrote:

> So,
>
> Based on this and some experiments I have been performing, I would suggest
> changing how a lot of the FileSystem rules are written and illustrated.
> Ex -
> https://github.com/linux-audit/audit-userspace/blob/master/rules/30-pci-dss-v31.rules#L34-L35
>
> The rule in the repository is
> -a always,exit -F path=/etc/sudoers -F perm=wa -F
> key=10.2.2-priv-config-changes
>
> My suggestion is to instead change the rule based on the permissions
> defined. The above rule would change to the following based on the kernel
> being used.
> -a always,exit -S <list of syscalls in audit_write.h and audit_read.h
> +open,openat> -F path=/etc/sudoers -F perm=wa -F
> key=10.2.2-priv-config-changes
>
> This is higher performance because we are limiting the syscalls instead of
> making use of -S all which has more paths of evaluation for each and every
> syscall.
>
> Same thing for watches. Watches are inherently -S all rules which are very
> performance intensive.
>
> https://github.com/linux-audit/audit-userspace/blob/1482cec74f2d9472f81dd4f0533484bd0c26decd/lib/libaudit.c#L805
>
> Ideally we should limit the syscalls based on the permissions being used.
>
> I have implemented the same in my environment rules and have noticed a
> massive performance difference with no difference in the events being
> logged since we anyways filter eventually based on the permissions.
>
> Let me know what you all think.
>
> Ali Adnan.
>
>
>
>
>
> On Wed, Sep 6, 2023 at 2:58 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
>> On 2023-09-06 10:56, Amjad Gabbar wrote:
>> > Hi,
>> >
>> > I have done some analysis and digging into how both the watch rules and
>> > syscall rules are translated.
>> >
>> > From my understanding, in terms of logging, both the below rules are
>> > similar. There is no difference in either of the rules.
>> >
>> > 1. -w /etc -p wa -k ETC_WATCH
>>
>> They are similar in this case.
>> -w behaves differently depending on the existance of the watched entity
>> and the presence of a trailing "/".  This is why the form above is
>> deprecated.
>>
>> > 2. -a always,exit -F arch=b64 -S <all syscalls part of the write and
>> attr
>> > classes> -F dir=/etc  -F perm=wa -k ETC_WATCH
>> >
>> > The write and attr classes consist of syscalls in
>> > “include/asm-generic/audit_*.h“.
>> >
>> >  The perm flag is needed in the second case for including open/openat
>> > syscalls which are not a part of the write and attr syscall list.
>> >
>> > I'd like to verify if what I mentioned earlier is accurate, and I have
>> an
>> > additional point but depends on whether this is accurate.
>> >
>> > Ali
>>
>> - RGB
>>
>> --
>> Richard Guy Briggs <rgb@redhat.com>
>> Sr. S/W Engineer, Kernel Security, Base Operating Systems
>> Remote, Ottawa, Red Hat Canada
>> Upstream IRC: SunRaycer
>> Voice: +1.613.860 2354 SMS: +1.613.518.6570
>>
>>

[-- Attachment #1.2: Type: text/html, Size: 4582 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

Re: Sycall Rules vs Watch Rules

[Steve Grubb]

Hello,

On Tuesday, September 12, 2023 5:20:54 PM EDT Amjad Gabbar wrote:
> Based on this and some experiments I have been performing, I would suggest
> changing how a lot of the FileSystem rules are written and illustrated.
> Ex -
> https://github.com/linux-audit/audit-userspace/blob/master/rules/30-pci-dss
> -v31.rules#L34-L35
> 
> The rule in the repository is
> -a always,exit -F path=/etc/sudoers -F perm=wa -F
> key=10.2.2-priv-config-changes
> 
> My suggestion is to instead change the rule based on the permissions
> defined. The above rule would change to the following based on the kernel
> being used.
> -a always,exit -S <list of syscalls in audit_write.h and audit_read.h
> +open,openat> -F path=/etc/sudoers -F perm=wa -F
> key=10.2.2-priv-config-changes

That should be exactly what the kernel does with the perm fields. The perm 
fields select the right system calls that should be reported on.
 
> This is higher performance because we are limiting the syscalls instead of
> making use of -S all which has more paths of evaluation for each and every
> syscall.
> 
> Same thing for watches. Watches are inherently -S all rules which are very
> performance intensive.
> https://github.com/linux-audit/audit-userspace/blob/1482cec74f2d9472f81dd4f
> 0533484bd0c26decd/lib/libaudit.c#L805

There should be no difference in performance between watches and syscall 
based file auditing.

> Ideally we should limit the syscalls based on the permissions being used.
> 
> I have implemented the same in my environment rules and have noticed a
> massive performance difference with no difference in the events being
> logged since we anyways filter eventually based on the permissions.
> 
> Let me know what you all think.

I'm looking into this more. I see a 1 line change that I am testing.

-Steve

> On Wed, Sep 6, 2023 at 2:58 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2023-09-06 10:56, Amjad Gabbar wrote:
> > > Hi,
> > > 
> > > I have done some analysis and digging into how both the watch rules and
> > > syscall rules are translated.
> > > 
> > > From my understanding, in terms of logging, both the below rules are
> > > similar. There is no difference in either of the rules.
> > > 
> > > 1. -w /etc -p wa -k ETC_WATCH
> > 
> > They are similar in this case.
> > -w behaves differently depending on the existance of the watched entity
> > and the presence of a trailing "/".  This is why the form above is
> > deprecated.
> > 
> > > 2. -a always,exit -F arch=b64 -S <all syscalls part of the write and
> > > attr
> > > classes> -F dir=/etc  -F perm=wa -k ETC_WATCH
> > > 
> > > The write and attr classes consist of syscalls in
> > > “include/asm-generic/audit_*.h“.
> > > 
> > >  The perm flag is needed in the second case for including open/openat
> > > 
> > > syscalls which are not a part of the write and attr syscall list.
> > > 
> > > I'd like to verify if what I mentioned earlier is accurate, and I have
> > > an
> > > additional point but depends on whether this is accurate.
> > > 
> > > Ali
> > 
> > - RGB
> > 
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Sr. S/W Engineer, Kernel Security, Base Operating Systems
> > Remote, Ottawa, Red Hat Canada
> > Upstream IRC: SunRaycer
> > Voice: +1.613.860 2354 SMS: +1.613.518.6570




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

Re: Sycall Rules vs Watch Rules

[Amjad Gabbar]

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

Hi,

> 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.

Futex is a syscall that I see called fairly often in my system, which is
not part of the perm fields.
As an example, I selected the ospp rules file to measure  performance via a
synthetic test-
https://github.com/linux-audit/audit-userspace/blob/master/rules/30-ospp-v42.rules

stress-ng —futex 1 —futex-ops 1000000

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%.

Again this synthetic test is just for demonstration purposes but helps
explain the point. Basically for syscalls not part of the perm fields we
filter them at a much later stage in the AUDIT_PERM case(due to -S all)
whereas if we use specific syscalls within the rule itself, we would exit
the processing in audit_filter_syscall itself for uninteresting syscalls,
hence improving the performance.

>I see a 1 line change that I am testing.
Let me know if you need any help. I did have a partial PR ready for
submission but wanted to get your opinions before submitting anything.

Regards
Ali Adnan

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

> Hello,
>
> On Tuesday, September 12, 2023 5:20:54 PM EDT Amjad Gabbar wrote:
> > Based on this and some experiments I have been performing, I would
> suggest
> > changing how a lot of the FileSystem rules are written and illustrated.
> > Ex -
> >
> https://github.com/linux-audit/audit-userspace/blob/master/rules/30-pci-dss
> > -v31.rules#L34-L35
> >
> > The rule in the repository is
> > -a always,exit -F path=/etc/sudoers -F perm=wa -F
> > key=10.2.2-priv-config-changes
> >
> > My suggestion is to instead change the rule based on the permissions
> > defined. The above rule would change to the following based on the kernel
> > being used.
> > -a always,exit -S <list of syscalls in audit_write.h and audit_read.h
> > +open,openat> -F path=/etc/sudoers -F perm=wa -F
> > key=10.2.2-priv-config-changes
>
> That should be exactly what the kernel does with the perm fields. The perm
> fields select the right system calls that should be reported on.
>
> > This is higher performance because we are limiting the syscalls instead
> of
> > making use of -S all which has more paths of evaluation for each and
> every
> > syscall.
> >
> > Same thing for watches. Watches are inherently -S all rules which are
> very
> > performance intensive.
> >
> https://github.com/linux-audit/audit-userspace/blob/1482cec74f2d9472f81dd4f
> > 0533484bd0c26decd/lib/libaudit.c#L805
>
> There should be no difference in performance between watches and syscall
> based file auditing.
>
> > Ideally we should limit the syscalls based on the permissions being used.
> >
> > I have implemented the same in my environment rules and have noticed a
> > massive performance difference with no difference in the events being
> > logged since we anyways filter eventually based on the permissions.
> >
> > Let me know what you all think.
>
> I'm looking into this more. I see a 1 line change that I am testing.
>
> -Steve
>
> > On Wed, Sep 6, 2023 at 2:58 PM Richard Guy Briggs <rgb@redhat.com>
> wrote:
> > > On 2023-09-06 10:56, Amjad Gabbar wrote:
> > > > Hi,
> > > >
> > > > I have done some analysis and digging into how both the watch rules
> and
> > > > syscall rules are translated.
> > > >
> > > > From my understanding, in terms of logging, both the below rules are
> > > > similar. There is no difference in either of the rules.
> > > >
> > > > 1. -w /etc -p wa -k ETC_WATCH
> > >
> > > They are similar in this case.
> > > -w behaves differently depending on the existance of the watched entity
> > > and the presence of a trailing "/".  This is why the form above is
> > > deprecated.
> > >
> > > > 2. -a always,exit -F arch=b64 -S <all syscalls part of the write and
> > > > attr
> > > > classes> -F dir=/etc  -F perm=wa -k ETC_WATCH
> > > >
> > > > The write and attr classes consist of syscalls in
> > > > “include/asm-generic/audit_*.h“.
> > > >
> > > >  The perm flag is needed in the second case for including open/openat
> > > >
> > > > syscalls which are not a part of the write and attr syscall list.
> > > >
> > > > I'd like to verify if what I mentioned earlier is accurate, and I
> have
> > > > an
> > > > additional point but depends on whether this is accurate.
> > > >
> > > > Ali
> > >
> > > - RGB
> > >
> > > --
> > > Richard Guy Briggs <rgb@redhat.com>
> > > Sr. S/W Engineer, Kernel Security, Base Operating Systems
> > > Remote, Ottawa, Red Hat Canada
> > > Upstream IRC: SunRaycer
> > > Voice: +1.613.860 2354 SMS: +1.613.518.6570
>
>
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 7715 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

Re: Sycall Rules vs Watch Rules

[Steve Grubb]

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.

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

Re: Sycall Rules vs Watch Rules

[Steve Grubb]

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




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

Re: Sycall Rules vs Watch Rules

[Amjad Gabbar]

[-- 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

Re: Sycall Rules vs Watch Rules

[Steve Grubb]

On Thursday, September 21, 2023 4:02:49 PM EDT Amjad Gabbar wrote:
> > 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.

After looking into this, a kernel patch would also not work well. It has to 
be arch specific
 
> > 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.

I have made the changes to the master and audit-3.1-maint branches. Please 
everyone concerned give them tests. The short of it is that if you use the '-
w' notation for watches, it will remain the same and slower. If you use the 
syscall notation without "-F arch", you will get a warning that it cannot be 
optimized without adding "-Farch". If you add "-F arch", you will possibly 
need one for both arches which means doubling the rules. If you do not want 
to double the rules, you might place a syscall rule for any 32 system call 
(21-no32bit.rules). Or you can leave it as is and not care. The sample rules 
and all man pages have been updated.

Please, let me know if this works out better.

-Steve


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

Re: Sycall Rules vs Watch Rules

[Steve Grubb]

On Thursday, September 28, 2023 11:53:26 AM EDT Steve Grubb wrote:
> On Thursday, September 21, 2023 4:02:49 PM EDT Amjad Gabbar wrote:
> > > 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.
> 
> After looking into this, a kernel patch would also not work well. It has to
> be arch specific
> 
> > > 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.
> 
> I have made the changes to the master and audit-3.1-maint branches. Please
> everyone concerned give them tests. The short of it is that if you use the
> '- w' notation for watches, it will remain the same and slower.

Actually, ths is the one that draws the warning to urge people to migrate.

> If you use
> the syscall notation without "-F arch", you will get a warning that it
> cannot be optimized without adding "-Farch".

Actually, you won't in order to preserve intentional behavior.

> If you add "-F arch", you
> will possibly need one for both arches which means doubling the rules. If
> you do not want to double the rules, you might place a syscall rule for
> any 32 system call (21-no32bit.rules). Or you can leave it as is and not
> care. The sample rules and all man pages have been updated.

I should have provided an example of what this means. If you have this kind
of rule:

-w /etc/shadow -p wa -k shadow

And when applied draws a warning:

# auditctl -w /etc/shadow -p wa -k shadow
Old style watch rules are slower

It should be rewritten as

-a always,exit -F arch=b64 -F path=/etc/shadow -F perm=wa -F key=shadow

Then it looks like this when loaded:

#auditctl -l
-a always,exit -F arch=b64 -S open,bind,truncate,ftruncate,rename,mkdir,rmdir,creat,link,unlink,symlink,chmod,fchmod,chown,fchown,lchown,mknod,acct,swapon,quotactl,setxattr,lsetxattr,fsetxattr,removexattr,lremovexattr,fremovexattr,openat,mkdirat,mknodat,fchownat,unlinkat,renameat,linkat,symlinkat,fchmodat,fallocate,renameat2,openat2 -F path=/etc/shadow -F perm=wa -F key=shadow

And to delete  the rule, 
auditctl -d always,exit -F arch=b64 -F path=/etc/shadow -F perm=wa -F key=shadow

or the long way

auditctl -d always,exit -F arch=b64 -S open,bind,truncate,ftruncate,rename,mkdir,rmdir,creat,link,unlink,symlink,chmod,fchmod,chown,fchown,lchown,mknod,acct,swapon,quotactl,setxattr,lsetxattr,fsetxattr,removexattr,lremovexattr,fremovexattr,openat,mkdirat,mknodat,fchownat,unlinkat,renameat,linkat,symlinkat,fchmodat,fallocate,renameat2,openat2 -F path=/etc/shadow -F perm=wa -F key=shadow

Hopefully this is clearer what the change is.
 
-Steve



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

Re: Sycall Rules vs Watch Rules

[Amjad Gabbar]

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

Sounds good. I will test this out.

Regards
Ali Adnan

On Thu, Sep 28, 2023 at 11:30 AM Steve Grubb <sgrubb@redhat.com> wrote:

> On Thursday, September 28, 2023 11:53:26 AM EDT Steve Grubb wrote:
> > On Thursday, September 21, 2023 4:02:49 PM EDT Amjad Gabbar wrote:
> > > > 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.
> >
> > After looking into this, a kernel patch would also not work well. It has
> to
> > be arch specific
> >
> > > > 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.
> >
> > I have made the changes to the master and audit-3.1-maint branches.
> Please
> > everyone concerned give them tests. The short of it is that if you use
> the
> > '- w' notation for watches, it will remain the same and slower.
>
> Actually, ths is the one that draws the warning to urge people to migrate.
>
> > If you use
> > the syscall notation without "-F arch", you will get a warning that it
> > cannot be optimized without adding "-Farch".
>
> Actually, you won't in order to preserve intentional behavior.
>
> > If you add "-F arch", you
> > will possibly need one for both arches which means doubling the rules. If
> > you do not want to double the rules, you might place a syscall rule for
> > any 32 system call (21-no32bit.rules). Or you can leave it as is and not
> > care. The sample rules and all man pages have been updated.
>
> I should have provided an example of what this means. If you have this kind
> of rule:
>
> -w /etc/shadow -p wa -k shadow
>
> And when applied draws a warning:
>
> # auditctl -w /etc/shadow -p wa -k shadow
> Old style watch rules are slower
>
> It should be rewritten as
>
> -a always,exit -F arch=b64 -F path=/etc/shadow -F perm=wa -F key=shadow
>
> Then it looks like this when loaded:
>
> #auditctl -l
> -a always,exit -F arch=b64 -S
> open,bind,truncate,ftruncate,rename,mkdir,rmdir,creat,link,unlink,symlink,chmod,fchmod,chown,fchown,lchown,mknod,acct,swapon,quotactl,setxattr,lsetxattr,fsetxattr,removexattr,lremovexattr,fremovexattr,openat,mkdirat,mknodat,fchownat,unlinkat,renameat,linkat,symlinkat,fchmodat,fallocate,renameat2,openat2
> -F path=/etc/shadow -F perm=wa -F key=shadow
>
> And to delete  the rule,
> auditctl -d always,exit -F arch=b64 -F path=/etc/shadow -F perm=wa -F
> key=shadow
>
> or the long way
>
> auditctl -d always,exit -F arch=b64 -S
> open,bind,truncate,ftruncate,rename,mkdir,rmdir,creat,link,unlink,symlink,chmod,fchmod,chown,fchown,lchown,mknod,acct,swapon,quotactl,setxattr,lsetxattr,fsetxattr,removexattr,lremovexattr,fremovexattr,openat,mkdirat,mknodat,fchownat,unlinkat,renameat,linkat,symlinkat,fchmodat,fallocate,renameat2,openat2
> -F path=/etc/shadow -F perm=wa -F key=shadow
>
> Hopefully this is clearer what the change is.
>
> -Steve
>
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 4362 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

Re: Sycall Rules vs Watch Rules

[Amjad Gabbar]

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

Tested out all different combinations and performed performance experiments
and tests using different permutations and combinations of rules.

Can confirm the changes work as expected.

1. The old -w rule format is slower since it encompasses 'all' syscalls. A
warning is emitted on using the -w notation that 'Old style watch rules are
slower'.

2. On making use of the syscall format but without specifying the arch, a
warning is emitted - 'perm used without an arch is slower`.
The rules are similar to the old style -w watch rules encompassing 'all'
syscalls and hampering performance significantly.

3. On specifying an arch with the syscall format, the respective syscalls
are added based on the permissions field. Tested all different permissions
to ensure that the respective syscalls are added.
Works as expected and massively improves performance as well.

Thanks for working together on this. Hopefully the end users are able to
see the boost in performance post these changes.

Regards
Ali Adnan

On Fri, Sep 29, 2023 at 11:39 AM Amjad Gabbar <amjadgabbar11@gmail.com>
wrote:

> Sounds good. I will test this out.
>
> Regards
> Ali Adnan
>
> On Thu, Sep 28, 2023 at 11:30 AM Steve Grubb <sgrubb@redhat.com> wrote:
>
>> On Thursday, September 28, 2023 11:53:26 AM EDT Steve Grubb wrote:
>> > On Thursday, September 21, 2023 4:02:49 PM EDT Amjad Gabbar wrote:
>> > > > 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.
>> >
>> > After looking into this, a kernel patch would also not work well. It
>> has to
>> > be arch specific
>> >
>> > > > 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.
>> >
>> > I have made the changes to the master and audit-3.1-maint branches.
>> Please
>> > everyone concerned give them tests. The short of it is that if you use
>> the
>> > '- w' notation for watches, it will remain the same and slower.
>>
>> Actually, ths is the one that draws the warning to urge people to migrate.
>>
>> > If you use
>> > the syscall notation without "-F arch", you will get a warning that it
>> > cannot be optimized without adding "-Farch".
>>
>> Actually, you won't in order to preserve intentional behavior.
>>
>> > If you add "-F arch", you
>> > will possibly need one for both arches which means doubling the rules.
>> If
>> > you do not want to double the rules, you might place a syscall rule for
>> > any 32 system call (21-no32bit.rules). Or you can leave it as is and not
>> > care. The sample rules and all man pages have been updated.
>>
>> I should have provided an example of what this means. If you have this
>> kind
>> of rule:
>>
>> -w /etc/shadow -p wa -k shadow
>>
>> And when applied draws a warning:
>>
>> # auditctl -w /etc/shadow -p wa -k shadow
>> Old style watch rules are slower
>>
>> It should be rewritten as
>>
>> -a always,exit -F arch=b64 -F path=/etc/shadow -F perm=wa -F key=shadow
>>
>> Then it looks like this when loaded:
>>
>> #auditctl -l
>> -a always,exit -F arch=b64 -S
>> open,bind,truncate,ftruncate,rename,mkdir,rmdir,creat,link,unlink,symlink,chmod,fchmod,chown,fchown,lchown,mknod,acct,swapon,quotactl,setxattr,lsetxattr,fsetxattr,removexattr,lremovexattr,fremovexattr,openat,mkdirat,mknodat,fchownat,unlinkat,renameat,linkat,symlinkat,fchmodat,fallocate,renameat2,openat2
>> -F path=/etc/shadow -F perm=wa -F key=shadow
>>
>> And to delete  the rule,
>> auditctl -d always,exit -F arch=b64 -F path=/etc/shadow -F perm=wa -F
>> key=shadow
>>
>> or the long way
>>
>> auditctl -d always,exit -F arch=b64 -S
>> open,bind,truncate,ftruncate,rename,mkdir,rmdir,creat,link,unlink,symlink,chmod,fchmod,chown,fchown,lchown,mknod,acct,swapon,quotactl,setxattr,lsetxattr,fsetxattr,removexattr,lremovexattr,fremovexattr,openat,mkdirat,mknodat,fchownat,unlinkat,renameat,linkat,symlinkat,fchmodat,fallocate,renameat2,openat2
>> -F path=/etc/shadow -F perm=wa -F key=shadow
>>
>> Hopefully this is clearer what the change is.
>>
>> -Steve
>>
>>
>>
>>

[-- Attachment #1.2: Type: text/html, Size: 5847 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