-
Notifications
You must be signed in to change notification settings - Fork 687
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
Add initial automated tests for OSSEC and whitelist overloaded Tor guard log event #2137
Conversation
Hmm, I would expect the |
Hmm so this isn't
in |
A Tor log event indicating that a Tor guard in use is overloaded currently produces an OSSEC alert. While this alert is an excellent candidate to be sent upstream to FPF for analysis, there is no action that a SecureDrop administrator is expected to take, making this a spurious OSSEC alert. This test reproduces this spurious alert and is a regression test for an OSSEC rule patch.
0bf8027
to
32b18ad
Compare
Spinning up VMs to test out these changes. |
We have alerting tests! 😍 Thanks @redshiftzero! I'll note now that we're definitely going to want to DRY out the alerting config tests by writing a reusable function (should technically be a fixture in pytest/testinfra nomenclature, but a helper function will work too), and reading in the alert metadata via the YAML testinfra vars files. Won't block merge for that, since we have several pending alert test PRs (e.g. #2143, #2152). Will need to circle back on the dev docs anyway to explain the alert testing workflow, we can handle DRY cleanup then. |
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.
If you want to be super thorough
I did and was. Confirmed that the new test failed on develop, but passed after reprovisioning staging machines with the code changes in this PR (via the rebuilt deb package).
Nice, thanks for reviewing this @conorsch - I'm with you on DRYing up the alerting config tests and adding developer docs to explain that people should be writing regression tests for OSSEC rule changes (to address in a followup). |
Status
Ready for review
Description of Changes
Fixes #1670 and begins work towards #2134
Changes proposed in this pull request:
(Happy to break this up into separate PRs to aid in review)
Testing
At minimum, verify the new tests run and pass in the CI staging environment.
If you want to be super thorough:
test_overloaded_tor_guard_does_not_produce_alert
fails ("Alert to be generated" appears in the output ofossec-logtest
)test_overloaded_tor_guard_does_not_produce_alert
now passes ("Alert to be generated" does not appear in the output ofossec-logtest
)Deployment
This just updates the OSSEC rules, so there should be no issues in deployment.
Checklist
If you made changes to the system configuration: