cleanup(app_actions): finalize base_syscalls.repair option

Co-authored-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>
This commit is contained in:
Melissa Kilby 2023-03-29 05:08:42 +00:00 committed by poiana
parent 2b93a79521
commit 78daafb56c
2 changed files with 33 additions and 21 deletions

View File

@ -185,7 +185,7 @@ TEST(ConfigureInterestingSets, selection_not_allevents)
// also check if a warning has been printed in stderr
// check that the final selected set is the one expected
ASSERT_NE(s.selected_sc_set.size(), 0);
ASSERT_GT(s.selected_sc_set.size(), 1);
auto selected_sc_names = libsinsp::events::sc_set_to_names(s.selected_sc_set);
auto expected_sc_names = strset_t({
// note: we expect the "read" syscall to have been erased
@ -228,7 +228,7 @@ TEST(ConfigureInterestingSets, selection_allevents)
// also check if a warning has not been printed in stderr
// check that the final selected set is the one expected
ASSERT_NE(s.selected_sc_set.size(), 0);
ASSERT_GT(s.selected_sc_set.size(), 1);
auto selected_sc_names = libsinsp::events::sc_set_to_names(s.selected_sc_set);
auto expected_sc_names = strset_t({
// note: we expect the "read" syscall to not be erased
@ -260,7 +260,7 @@ TEST(ConfigureInterestingSets, selection_generic_evts)
ASSERT_EQ(result.errstr, "");
// check that the final selected set is the one expected
ASSERT_NE(s.selected_sc_set.size(), 0);
ASSERT_GT(s.selected_sc_set.size(), 1);
auto selected_sc_names = libsinsp::events::sc_set_to_names(s.selected_sc_set);
auto expected_sc_names = strset_t({
// note: we expect the "read" syscall to not be erased

View File

@ -71,18 +71,18 @@ static void select_event_set(falco::app::state& s, const libsinsp::events::set<p
}
/* DEFAULT OPTION:
* Current sinsp_state_sc_set() approach includes multiple steps:
* Current `sinsp_state_sc_set()` approach includes multiple steps:
* (1) Enforce all positive syscalls from each Falco rule
* (2) Enforce static `libsinsp` state set (non-adaptive, not conditioned by rules,
* (2) Enforce static Falco state set (non-adaptive, not conditioned by rules,
* but based on PPME event table flags indicating generic sinsp state modifications)
* -> Final set is union of (1) and (2)
*
* Fall-back if no valid positive syscalls in "base_syscalls",
* e.g. when using "base_syscalls" only for negative syscalls.
* Fall-back if no valid positive syscalls in `base_syscalls.custom_set`,
* e.g. when using `base_syscalls.custom_set` only for negative syscalls.
*/
auto base_sc_set = libsinsp::events::sinsp_state_sc_set();
/* USER OVERRIDE INPUT OPTION "base_syscalls". */
/* USER OVERRIDE INPUT OPTION `base_syscalls.custom_set` etc. */
std::unordered_set<std::string> user_positive_names = {};
std::unordered_set<std::string> user_negative_names = {};
extract_base_syscalls_names(s.config->m_base_syscalls_custom_set, user_positive_names, user_negative_names);
@ -112,12 +112,6 @@ static void select_event_set(falco::app::state& s, const libsinsp::events::set<p
// base events set (either the default or the user-defined one)
s.selected_sc_set = rules_sc_set.merge(base_sc_set);
/* Hidden safety enforcement for `base_syscalls.custom_set` user input
* override option -> sched_process_exit trace point activation
* (procexit event) is necessary for continuous state engine cleanup,
* else memory would grow rapidly and linearly over time. */
s.selected_sc_set.insert(ppm_sc_code::PPM_SC_SCHED_PROCESS_EXIT);
if (!user_negative_sc_set.empty())
{
/* Remove negative base_syscalls events. */
@ -137,6 +131,19 @@ static void select_event_set(falco::app::state& s, const libsinsp::events::set<p
}
}
/* REPLACE DEFAULT STATE, nothing else. */
if (s.config->m_base_syscalls_repair && s.config->m_base_syscalls_custom_set.empty())
{
/* If `base_syscalls.repair` is specified, but `base_syscalls.custom_set` is empty we are replacing
* the default `sinsp_state_sc_set()` enforcement with the alternative `sinsp_repair_state_sc_set`.
* This approach only activates additional syscalls Falco needs beyond the
* syscalls defined in each Falco rule that are absolutely necessary based
* on the current rules configuration. */
// returned set already has rules_sc_set merged
s.selected_sc_set = libsinsp::events::sinsp_repair_state_sc_set(rules_sc_set);
}
/* Derive the diff between the additional syscalls added via libsinsp state
enforcement and the syscalls from each Falco rule. We avoid printing
this in case the user specified a custom set of base syscalls */
@ -168,16 +175,14 @@ static void select_event_set(falco::app::state& s, const libsinsp::events::set<p
}
}
/* if a custom set is specified (positive, negative, or both), we attempt
/* If a custom set is specified (positive, negative, or both), we attempt
* to repair it if configured to do so. */
if (s.config->m_base_syscalls_repair && !s.config->m_base_syscalls_custom_set.empty())
{
/* If base_syscalls.repair set enforce `libsinsp` state based on rules set
* and merge with user supplied base_syscalls.custom_set
*
* Also applies when base_syscalls.custom_set empty but base_syscalls.repair set
* effectively bypassing the default `libsinsp` state enforcement and using
* `sinsp_repair_state_sc_set` instead. */
/* If base_syscalls.repair is specified enforce state using `sinsp_repair_state_sc_set`.
* This approach is an alternative to the default `sinsp_state_sc_set()` state enforcement
* and only activates additional syscalls Falco needs beyond the syscalls defined in the
* Falco rules that are absolutely necessary based on the current rules configuration. */
auto selected_sc_set = s.selected_sc_set;
s.selected_sc_set = libsinsp::events::sinsp_repair_state_sc_set(s.selected_sc_set);
auto repaired_sc_set = s.selected_sc_set.diff(selected_sc_set);
@ -189,6 +194,13 @@ static void select_event_set(falco::app::state& s, const libsinsp::events::set<p
}
}
/* Hidden safety enforcement for `base_syscalls.custom_set` user
* input override option (but keep as general safety enforcement)
* -> sched_process_exit trace point activation (procexit event)
* is necessary for continuous state engine cleanup,
* else memory would grow rapidly and linearly over time. */
s.selected_sc_set.insert(ppm_sc_code::PPM_SC_SCHED_PROCESS_EXIT);
if (!s.selected_sc_set.empty())
{
auto selected_sc_set_names = libsinsp::events::sc_set_to_names(s.selected_sc_set);