linux-audit / audit-testsuite

A simple, self-contained regression test suite for the Linux Kernel's audit subsystem
GNU General Public License v2.0
21 stars 25 forks source link

tests: test also exclude filter by executable name #68

Closed WOnder93 closed 5 years ago

WOnder93 commented 6 years ago

Requires the following kernel patch: https://www.redhat.com/archives/linux-audit/2018-April/msg00114.html

Signed-off-by: Ondrej Mosnacek omosnace@redhat.com

WOnder93 commented 6 years ago

This also depends on the following userspace patch: https://github.com/linux-audit/audit-userspace/pull/48

Related GHAK: https://github.com/linux-audit/audit-kernel/issues/54

rgbriggs commented 6 years ago

On 2018-05-03 02:23, Ondrej Mosnáček wrote:

-do_test("exe=$exec"); -do_test("exe!=$exec2"); +do_test(["-F exe=$exec1"], [], 1, 0, 0); +do_test(["-F exe!=$exec2"], [], 1, 0, 1); +do_test([""], ["-F exe!=$exec1"], 1, 0, 0); +do_test([""], ["-F exe=$exec2"], 1, 0, 1); +do_test(["-F exe=$exec1", "-F exe=$exec2"], [], 1, 1, 0); +do_test([""], ["-F exe=$exec1", "-F exe=$exec2"], 0, 0, 1);

Adding Perl magic to improve readability of the do_test calls seems like overengineering to me... I would prefer the keep-it-simple approach, even if it is a bit crude. Right now the whole test can be read and understood quickly (IMHO). If we add too much "magic" inside do_test then people will have to carefully read and understand that magic to understand what's going on.

I did have some trouble reading it, but did succeed.

I think the $f[1-3] expected values should be given explicitly - these are basically test vectors so it should be clearly visible what is the expected result. I will at least move the numeric arguments to the beginning since like that they will align naturally.

I had trouble spotting the f[1-3] parameters used inside the function, so I'd make them a bit longer (4-6 chars?) to make it more descriptive what they are and easier to spot.

WOnder93 commented 6 years ago

I had trouble spotting the f[1-3] parameters used inside the function, so I'd make them a bit longer (4-6 chars?) to make it more descriptive what they are and easier to spot.

Good idea. I went with exp_found[1-3] ("Is exec[1-3] record expected to be found?"). It's not perfect, but I guess it's better than just f...

pcmoore commented 6 years ago

Adding Perl magic to improve readability of the do_test calls seems like overengineering to me... I would prefer the keep-it-simple approach, even if it is a bit crude. Right now the whole test can be read and understood quickly (IMHO). If we add too much "magic" inside do_test then people will have to carefully read and understand that magic to understand what's going on.

You might be able to read the code quickly, but both Richard and I had some difficulty. Please understand, it isn't that we couldn't read it, it was just more painful that it needed to be. You may not like my example, feel free to come up with something else, but we need something better than the combination of ones and zeros assigned to an arbitrary array.

WOnder93 commented 6 years ago

I pushed another version which computes the expected "found" values directly from the rules. The rules are now represented by a hash containing the executable index and the comparison operator.

I don't know if it is what you wanted, but I wasn't able to come up with anything better.

I also fixed the formatting to pass make check-syntax.

WOnder93 commented 5 years ago

Squashed and rebased on top of current master.

WOnder93 commented 5 years ago

...and replaced kernel patch link with commit ID in the commit message.

pcmoore commented 5 years ago

Merged, with heavy merge fixups, via 57d0261b4d0aacec7fc6eacc3f7e08865174858e, thanks.