Skip to content
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

Improve audit events handling in avc check #3562

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

martinhoyer
Copy link
Collaborator

When troubleshooting ausearch --checkpoint failing tests in certain environments, I've observed changes in avc check behaviour when many events are being created for processing. For example sshd from tmt ssh connections or sudo usage.
I haven't found a way a better check to find if all events are 'synced' before running ausearch commands, so I'm parsing auditctl -s output, which includes: "backlog field tells how many event records are currently queued waiting for auditd to read them" and make sure it is 0 before proceeding.
Unfortunatelly that's not enough to make it work reliably.

Using --just-one was a bad idea as ausearch always starts from beginning of the log, with no way to start with the last one. Redirecting stdout to /dev/null seems to be just as fast.
I've also tried to get the event id with tail -1 audit.log, and use auseach -a $id, but that also didn't solve race conditions(I think) that arise from running all this through ssh. Neither does various --format and --escape options...or any other options for that matter.
Using -ts checkpoint is ultimately the only way I've found in this sunk-cost-fallacy-rabbit-black-hole. Well, unless we want to have dedicated log file for just avc failures, just for the duration of the test.

Needs tests and I'm not quite happy with the bash script and report. Would mark is as draft, but that would not trigger pipelines.

Pull Request Checklist

  • implement the feature
  • extend the test coverage

@martinhoyer martinhoyer added the area | check Test check implementation label Feb 28, 2025
@martinhoyer martinhoyer self-assigned this Feb 28, 2025
@happz happz added the ci | full test Pull request is ready for the full test execution label Mar 2, 2025
- Wait for audit backlog to clear before running. ausearch commands.
- Use 1s sleep only as a fallback if unable to check backlog status.
- Change default method from timestamp to checkpoint.
@happz happz force-pushed the ausearch-checkpoint-mystery branch from 487856a to cded522 Compare March 2, 2025 13:06
@happz
Copy link
Collaborator

happz commented Mar 2, 2025

Needs tests and I'm not quite happy with the bash script and report. Would mark is as draft, but that would not trigger pipelines.

I believe draft isn't the issue, it's the lack of ci | full test label. With it, all checks should run even for drafts.

{% endif %}
""" # noqa: E501
)
textwrap.dedent(f"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only comment so far, the mixture of Jinja template and f-strings. I'd pick one of them and stick to it, most likely Jinja. How about passing the "library" variable to render() below, or we could have just one script, doing both things in if/else branches, and just set a variable for it when rendering it as before and after test (ACTION=prepare vs ACTION=check + if [ "$ACTION" = "prepare" ]; ...). Both should let us avoid using two template flavors.

@martinhoyer martinhoyer marked this pull request as draft March 3, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area | check Test check implementation ci | full test Pull request is ready for the full test execution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants