BUG: import the ausearch-test code into a new repo on github.com/linux-audit

[pcmoore]

Source: https://people.redhat.com/sgrubb/audit/ausearch-test-0.5.tar.gz

[WOnder93]

I could take on this and #11, but I don't have permissions to create a repo in linux-audit. I think only @pcmoore can do that.

[pcmoore]

I think a good first step would be to unpack the ausearch-test into a personal repository for review before inclusion in the linux-audit organization. Discussions on the mailing list have raised some questions in my mind about these tests.

The same applies to #11 and the audit-validation tests.

[WOnder93]

That's a good idea. I imported the two available versions (0.5 and 0.6) of ausearch-test here:

https://github.com/WOnder93/ausearch-test

What discussion and questions do you have in mind?

[pcmoore]

Thanks for unpacking the tests @WOnder93.

As far as questions are concerned, this isn't a complete list, but this is what comes to mind quickly:

• How does one install the test (e.g. what dependencies are there, if any)?
• It looks like the entire test (suite?) is a single C file, are there multiple tests in that file? If so, is there any documentation about what is tested, or does one need to read the C source?
• Once the tests are described somewhere, do we actually agree that these tests are correct and useful?
• If the tests are correct and useful, does the tests need any work before we add it to the linux-audit GH organization?

[WOnder93]

• How does one install the test (e.g. what dependencies are there, if any)?
• It looks like the entire test (suite?) is a single C file, are there multiple tests in that file? If so, is there any documentation about what is tested, or does one need to read the C source?

ausearch-test itself is just a single C program and a couple helper bash scripts. The usage of the test is described quite nicely in the included README file (which I now slightly edited and converted to Markdown).

• Once the tests are described somewhere, do we actually agree that these tests are correct and useful?

I haven't looked closely at the audit-validation test yet, but ausearch-test is definitely useful. It can be used for regression-testing the ausearch program as well as for making sure that ausearch can parse newly introduced record types.

Correctness is not something I would worry about, since it is just a tool for discovering possible problems. Think of it as a kind of fuzzer for ausearch. It is a really simple tool - the whole code is smaller than the file containing the license :)

• If the tests are correct and useful, does the tests need any work before we add it to the linux-audit GH organization?

Well, there is one thing - ausearch-test requires an audit.log file containing a collection of sample records (ideally acquired from real running systems). The tool's documentation expects each user to maintain their own database of records, but I think it would be much better if we have a single 'official' collection maintained inside the repository (then we could e.g. set up Travis CI on audit-userspace to check ausearch for regressions on each commit). I'm sure @stevegrubb has a personal collection somewhere, maybe he would be willing to share it? One possible problem could be that the records may contain sensitive information so we may either need to either generate some 'safe' collection or somehow filter/sanitize Steve's file if he is collecting the records from some live system(s).

@stevegrubb, do you have a record collection we could publish? Or how difficult do you think it would be to produce a reasonably complete collection that we could have in the public repository?

[rgbriggs]

On 2018-05-03 03:33, Ondrej Mosnáček wrote: > * Once the tests are described somewhere, do we actually agree that these tests are correct and useful? I haven't looked closely at the `audit-validation` test yet, but `ausearch-test` is definitely useful. It can be used for regression-testing the `ausearch` program as well as for making sure that ausearch can parse newly introduced record types.

Wait, is ausearch-test for testing ausearch, or for testing the current state of record formats?

> * If the tests are correct and useful, does the tests need any work before we add it to the linux-audit GH organization? Well, there is one thing - `ausearch-test` requires an `audit.log` file containing a collection of sample records (ideally acquired from real running systems). The tool's documentation expects each user to maintain their own database of records, but I think it would be much better if we have a single 'official' collection maintained inside the repository (then we could e.g. set up Travis CI on `audit-userspace` to check `ausearch` for regressions on each commit). I'm sure @stevegrubb has a personal collection somewhere, maybe he would be willing to share it? One possible problem could be that the records may contain sensitive information so we may either need to either generate some 'safe' collection or somehow filter/sanitize Steve's file if he is collecting the records from some live system(s). @stevegrubb, do you have a record collection we could publish? Or how difficult do you think it would be to produce a reasonably complete collection that we could have in the public repository?

This would give much more consistent results, I agree, but again, wait, I realize this is a much bigger job, but a stock test file is going to go stale fairly quickly, but is a start. A script/program to generate all possible events to test them for current compliance would actually test the records rather than ausearch. Understanding the goal of the test would be a good starting point. So, if the test fails, does it point to the test itself, ausearch, or new records as the cause?

[WOnder93]

Well, the idea is (I think):

• You start with an initial collection of (real-world) records that you expect ausearch to handle correctly. You run the test - if it reports a problem, you investigate it and fix ausearch to work correctly.
• When you make a modification to ausearch, you run the test again to see if it still processes the collection correctly.
• When the syntax of the records changes in the kernel (or when a new type of record is added), you first modify ausearch to recognize the new syntax, then add sample(s) of the new records, and run the test to verify that the new ausearch handles both existing and new records correctly.
• When you come across a record that ausearch handles incorrectly, you fix ausearch and then add the record to the collection so that the same bug doesn't happen again (it would be discovered by running the test).

It may also happen that a test failure uncovers a problem in the kernel or the test, but it is always the responsibility of the one who ran the test to investigate why it happened and what needs to be fixed.

[pcmoore]

• How does one install the test (e.g. what dependencies are there, if any)?
• It looks like the entire test (suite?) is a single C file, are there multiple tests in that file? If so, is there any documentation about what is tested, or does one need to read the C source?

ausearch-test itself is just a single C program and a couple helper bash scripts. The usage of the test is described quite nicely in the included README file (which I now slightly edited and converted to Markdown).

I don't see any mention of dependencies in the README. A quick inspection of the header files and Makefile would seem to indicate that audit-libs-devel is required on Fedora/RHEL systems; that and any other non-standard packages should be listed as a dependency for the test.

Correctness is not something I would worry about, since it is just a tool for discovering possible problems. Think of it as a kind of fuzzer for ausearch. It is a really simple tool - the whole code is smaller than the file containing the license :)

Correctness is very important to me; we need to be able to verify that the test is correct if we are going to include this in the linux-audit org here on GH. If we can't trust the test, how do we trust the test results?

You bring up another point that @rgbriggs touched upon, and I mentioned too (see above): what is this actually testing? Based on your last comment it looks like this is only testing ausearch's parsing of audit records, not the record format directly?

[WOnder93]

I don't see any mention of dependencies in the README. A quick inspection of the header files and Makefile would seem to indicate that audit-libs-devel is required on Fedora/RHEL systems; that and any other non-standard packages should be listed as a dependency for the test.

I didn't list the packages explicitly, but I did have the dependencies listed in the README (maybe you were looking at na older version). Anyway, I updated the README with the list of Fedora/RHEL packages to install.

Correctness is very important to me; we need to be able to verify that the test is correct if we are going to include this in the linux-audit org here on GH. If we can't trust the test, how do we trust the test results?

OK, I will take a closer look at the code and do a proper review, but then you will still heave to trust me that I did it right :)

You bring up another point that @rgbriggs touched upon, and I mentioned too (see above): what is this actually testing? Based on your last comment it looks like this is only testing ausearch's parsing of audit records, not the record format directly?

Exactly, it is supposed to test whether ausearch parses the records correctly and whether it filters them correctly based on the command line arguments. I guess it could also be used to test regressions in the kernel output, it depends on what you take as the frame of reference.

[WOnder93]

So I went through the code and I found no apparent issues. I did try to play with it a bit and it was able to detect a problem when I intentionally broke something in the ausearch parser.

Some notes:

• The tool doesn't only test the ausearch application, but also tests the built-in searching functionality provided by the libauparse library (which provides similar functionality to ausearch, but is implemented independently).
• There is a condition that skips testing of AUDIT_USER_AVC records when testing ausearch:

  // [...]
  				if (type == AUDIT_USER_AVC)
  					// USER AVCs are a mess - skip
  					continue;
  // [...]

  I tried removing the condition and the test passed fine on such records (at least on the ones generated by my F28 kernel). I guess @stevegrubb added the condition some time ago when there were known problems with these records and then forgot about it. I suggest to remove it, since it doesn't seem to be useful any more (and might hide future bugs from us).

[stevegrubb]

To see the reason for the conditional, you would need to trigger USER_AVC events which is difficult to do. Examples events would come from passwd or dbus. Additionally, there seems to be no format enforced which means they really can't be searched.

[WOnder93]

To see the reason for the conditional, you would need to trigger USER_AVC events which is difficult to do. Examples events would come from passwd or dbus.

Well, I do get some on my machine even without doing anything :) Although they are not many and do not vary very much...

# ausearch --raw -m USER_AVC | wc -l
95
# ausearch --raw | wc -l
78800

Additionally, there seems to be no format enforced which means they really can't be searched.

I see... In fact I just realized I had tried to run the current ausearch just over one of the events I found in my local trail and now that I ran it over all of them I do get some problems.

So for the time being, I'll leave the condition there. I can't see any other quick solution...

[stevegrubb]

If you do put this in a repo, I have 2 patches to apply. One fixes some shell script warnings and the other adds a new test that checks single shot search options and then cumulative search options. It currently does cumulative.

[WOnder93]

@stevegrubb Right now it is in my personal repo (https://github.com/WOnder93/ausearch-test). The plan is it will eventually get cloned into a repo under @linux-audit. I will give you commit rights to the temporary repos so you can work on them until @pcmoore decides to do the transfer.