-
Notifications
You must be signed in to change notification settings - Fork 905
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 correctness tests #83
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mstemm
force-pushed
the
add-correctness-tests
branch
from
May 24, 2016 20:56
738ad22
to
5e742d6
Compare
Add signal handlers for SIGINT/SIGTERM that set a shutdown flag. Initialize the live inspector with a timeout so the main loop can watch the flag set by the signal handlers.
At shutdown, print stats on the number of rules triggered by severity and rule name. This is done by a lua function print_stats and the associated table rule_output_counts. When passing rules to outputs, update the counts in rule_output_counts.
Start using the Avocado framework for automated regression testing. Create a test FalcoTest in falco_test.py which can run on a collection of trace files. The script test/run_regression_tests.sh is responsible for pulling zip files containing the positive (falco should detect) and negative (falco should not detect) trace files, creating a Avocado multiplex file that defines all the tests (one for each trace file), running avocado on all the trace files, and showing full logs for any test that didn't pass. The old regression script, which simply ran falco, has been removed. Modify falco's stats output to show the total number of events detected for use in the tests. In travis.yml, pull a known stable version of avocado and build it, including installing any dependencies, as a part of the build process.
// | ||
// Helper functions | ||
// | ||
static void signal_callback(int signal) |
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.
ha, i had this initially then took it out as it wasn't necessary at the time...
🎉 we have tests! |
Do another round of rule cleanups now that we have a larger set of positive and negative trace files to work with. Outside of this commit, there are now trace files for all the positive rules, a docker-compose startup and teardown, and some trace files from the sysdig cloud staging environment. Also add a script that runs sysdig with a filter that removes all the syscalls not handled by falco as well as a few other high-volume, low-information syscalls. This script was used to create the staging environment trace files. Notable rule changes: - The direction for write_binary_dir/write_etc needs to be exit instead of enter, as the bin_dir clause works on the file descriptor returned by the open/openat call. - Add login as a trusted binary that can read sensitive files (occurs for direct console logins). - sshd can read sensitive files well after startup, so exclude it from the set of binaries that can trigger read_sensitive_file_trusted_after_startup. - limit run_shell_untrusted to non-containers. - Disable the ssh_error_syslog rule for now. With the current restriction on system calls (no read/write/sendto/recvfrom/etc), you won't see the ssh error messages. Nevertheless, add a string to look for to indicate ssh errors and add systemd's true location for the syslog device. - Sshd attemps to setuid even when it's not running as root, so exclude it from the set of binaries to monitor for now. - Let programs that are direct decendants of systemd spawn user management tasks for now. - Temporarily disable the EACCESS rule. This rule is exposing a bug in sysdig in debug mode, draios/sysdig#598. The rule is also pretty noisy so I'll keep it disabled until the sysdig bug is fixed. - The etc_dir and bin_dir macros both have the problem that they match pathnames with /etc/, /bin/, etc in the middle of the path, as sysdig doesn't have a "begins with" comparison. Add notes for that. - Change spawn_process to spawned_process to indicate that it's for the exit side of the execve. Also use it in a few places that were looking for the same conditions without any macro. - Get rid of adduser_binaries and fold any programs not already present into shadowutils_binaries. - Add new groups sysdigcloud_binaries and sysdigcloud_binaries_parent and add them as exceptions for write_etc/write_binary_dir. - Add yum as a package management binary and add it as an exception to write_etc/write_binary_dir. - Change how db_program_spawned_process works. Since all of the useful information is on the exit side of the event, you can't really add a condition based on the process being new. Isntead, have the rule check for a non-database-related program being spawned by a database-related program. - Allow dragent to run shells. - Add sendmail, sendmail-msp as a program that attempts to setuid. - Some of the *_binaries macros that were based on dpkg -L accidentally contained directories in addition to end files. Trim those. - Add systemd-logind as a login_binary. - Add unix_chkpwd as a shadowutils_binary. - Add parentheses around any macros that group items using or. I found this necessary when the macro is used in the middle of a list of and conditions. - Break out system_binaries into a new subset user_mgmt_binaries containing login_, passwd_, and shadowutils_ binaries. That way you don't have to pull in all of system_binaries when looking for sensisitive files or user management activity. - Rename fs-bash to fbash, thinking ahead to its more likely name.
mstemm
force-pushed
the
add-correctness-tests
branch
from
May 26, 2016 00:51
f6320bb
to
0f4b378
Compare
Exclude trace directories.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add overall statistics reporting to falco to count the number of rules that triggered and add correctness tests using the test running framework Avocado. A set of positive and negative trace files are available on s3 and are pulled down by travis and run against falco, which simply verifies the positive traces have a detected event and the negative traces do not detect anything.
Based on these tests, do a pass of rule cleanups to improve fn/fp coverage.
This is somewhat blocked on a sysdig bug I ran into (draios/sysdig#598), although I'd be ok merging this and re-enabling the rule once the sysdig bug is fixed.
@henridf