-
Notifications
You must be signed in to change notification settings - Fork 25
RFE: add test support for session ID user filter #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFE: add test support for session ID user filter #22
Conversation
while ($line = <$fh_out>) { | ||
# test if we generate a PATH record | ||
if ($line =~ m?^type=PATH ? and | ||
$line =~ m? name=/tmp/$key ? and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to matching on the key we should also match on session ID given the nature of the test ... and yes, I realize you are passing the session ID to ausearch already.
Since the key is only in the SYSCALL message type and not in the PATH, CWD or PROCTITLE message types, I'd have to match on two messages from the same record, which is doable, but more complex than I figured was necessary since we are already requesting only records with a matching session ID, PID and key. How will this modification improve on what is already coded? Would adding "-ts recent" to the ausearch command be appropriate? |
Simply match on the SYSCALL record, this particular test does not need to match on the pathname; I would say that they key and session ID are the important fields for this test.
It would ensure a positive match on the session ID, which is the point of this test.
I don't think it matters, I would assume leave it out as passing the key filter to ausearch should help limit the results. |
43d47b0
to
1a30291
Compare
Ok, pushed an update that adds an explicit check of the session ID in the SYSCALL message. |
Wait, why are we still testing for a PATH record? A SYSCALL record with the correct session ID should be sufficient, yes? |
@rgbriggs See above. Why do we care about the PATH record in this test? |
Ok, strictly speaking that PATH record test doesn't belong in this one, but should be moved/retained for a dedicated test... |
We already have several tests that check for the presence and correctness of PATH records, see the file_* tests. |
There's a new version of the test script available here: https://github.com/rgbriggs/audit-testsuite/tree/ghak4-test-for-sessionID-user-filter |
There's a new version of the test script that now addresses SESSIONID_SET. |
And yet another newer version of this test script at rgbriggs@c265e9f It isn't tracking properly because I renamed the branch... I suppose I could push it to the lagh4... branch. |
No worries on the tracking, links to the mailing list posts are probably more helpful anyway as they capture the discussion. |
Commit rgbriggs@13f5d6f looks reasonable, although I would prefer if we renamed the directory from "sessionid_filter" to "filter_sessionid". Does that sound reasonable @rgbriggs ? |
I added one comment to the rgbriggs@c265e9f patch. |
test: RFE: add a session ID filter to the kernel's user filter linux-audit/audit-kernel#4 Signed-off-by: Richard Guy Briggs <[email protected]>
I've already argued against flipping the name around, but I don't care enough to press it... This pull request is stale since my branch got renamed on my local machine to reflect the new naming convention. The up to date branch is: https://github.com/rgbriggs/audit-testsuite/tree/ghak4-test-for-sessionID-user-filter |
1a30291
to
2584bcc
Compare
On 2016-08-30 12:41, Paul Moore wrote:
This is also stale.
Richard Guy Briggs [email protected] |
For reference, the latest draft from @rgbriggs appears to be here: rgbriggs@2584bcc |
I made one small comment on this revision, once we resolve this I think we are good to go as soon as the kernel/userspace patches are merged. |
@pcmoore ok grab rgbriggs@ec6d30f |
I'll trust that the perl expression matching is correct. Looks like we are just waiting on Steve now. |
Adjusting the pending label, it looks like we are just waiting on a rework for the tests to remove the set flag. Also, a reminder to @rgbriggs that we need this test and RFE wiki page done before I send the kernel patches to Linus. |
Both the test update and push and the RFE update were done last week. |
@rgbriggs great, I'll take a look at them today. For future reference, when you update a PR I think it would be good practice to comment on the PR/thread as GitHub doesn't appear to send notifications that you updated the code in your PR. |
The test looks good to me, but since we don't have a released version of the audit userspace that supports the session ID filter (waiting on audit userspace v2.7) I'm not going to merge this just yet as I don't want to keep seeing failures when I run the testsuite. @stevegrubb has mentioned that he intends to release the audit userspace v2.7 this month (December 2016) so we shouldn't have to wait long. |
Merged via b378225. |
Signed-off-by: Richard Guy Briggs [email protected]