FR: Configuration flag support to disable enter event generation

[tuminoid]

Motivation

As part of the disabling enter events proposal, we need to have a configuration flag that allows disabling generation of the enter events.

We need to expose a flag from sinsp to avoid the generation of enter events. The consumer can choose to receive or not enter events.

This issue is opened to discuss the implementation design. Proposal suggests it to be done after all the conversion work has happened, but most of the rulesets could use just the exit events much earlier than that, so we'd like to have it implemented sooner. It would also make measuring the real performance impact easier and earlier.

Feature

Per the proposal, we need to have configation option at libsinsp, that'll propagate through to all supported drivers, and their respective event generation methods.

Alternatives

Instead of a runtime configuration flag, we could have a compile time flag to remove enter events completely.

Additional context

This issue is opened to formulate the implementation plan for at least the following open items:

• how is the configuration enabled (besides debug, there isn't much configuration available in libsinsp)
• correct propagation path to drivers

Also relevant here:

• TOCTOU mitigation on the kernel side - separate FR for that

[tuminoid]

Ping @terror96, @leogr, @ekoops

[ekoops]

Proposal suggests it to be done after all the conversion work has happened, but most of the rulesets could use just the exit events much earlier than that, so we'd like to have it implemented sooner. It would also make measuring the real performance impact easier and earlier.

The conversion is only relevant in the context of scap files. For all the other drivers, we can augment the configuration related to the syscall of interest to support filtering the event direction. For the modern probe, for instance, we can change the value type here: https://github.com/falcosecurity/libs/blob/master/driver/modern_bpf/maps/maps.h#L128-L133 and use an enum like the following:

enum ppm_syscall_support {
  PPM_SYSCALL_SUPPORT_DISABLED = 0,
  PPM_SYSCALL_SUPPORT_SYSENTER = 1 << 1,
  PPM_SYSCALL_SUPPORT_SYSEXIT = 1 << 2,
}

The value can be set to PPM_SYSCALL_SUPPORT_DISABLED or any of the combination of the other two.
Then we could modify https://github.com/falcosecurity/libs/blob/master/driver/modern_bpf/helpers/interfaces/syscalls_dispatcher.h#L16-L18 to support querying for the specific event direction.
EDIT:
alternatively, another approach would be disabling all of them using a global variable. This has the advantage of enabling code pruning for the two eBPF probes
WDYT?

[terror96]

For the modern probe, for instance, we can change the value type here: https://github.com/falcosecurity/libs/blob/master/> driver/modern_bpf/maps/maps.h#L128-L133 and use an enum like the following:

Would https://github.com/falcosecurity/libs/blob/master/driver/bpf/plumbing_helpers.h#L419 be a candidate to take into account a similar kind of enum for the "older" (e)bpf? Just to put me on the same page...

EDIT: and possibly for kmod the logic in https://github.com/falcosecurity/libs/blob/master/driver/main.c#L1764 ?

[ekoops]

Yes both of them are correct. However, as I said, I think the boolean global variable solution will work better, especially because it will enable the eBPF verifier to perform dead code pruning. By taking again the modern probe as an example, we can add a read-only boolean global variable in this section:

libs/driver/modern_bpf/maps/maps.h

Lines 16 to 64 in 5a1faed

	/*=============================== BPF READ-ONLY GLOBAL VARIABLES ===============================*/
	/* The `volatile` qualifier is necessary to make sure Clang doesn't optimize away the read-only
	* global variables, ignoring user-space provided value. Without it, Clang is free to
	* just assume 0 and remove the variable completely.
	*
	* These read-only global variables need to be set before BPF skeleton is loaded into the
	* kernel by the user-space. These maps don't change after loading phase. They are initialized by
	* userspace before loading phase and they can no longer be modified neither
	* on the userspace-side nor on the bpf-side.
	*/
	/**
	* @brief Take as input the `ppm_event_code` enum and returns the number
	* of parameters for that event.
	*/
	__weak const volatile uint8_t g_event_params_table[PPM_EVENT_MAX];
	/**
	* @brief Take as input the `syscall_id` and returns the PPM_SC_CODE
	* associated with the syscall.
	*/
	__weak const volatile uint16_t g_ppm_sc_table[SYSCALL_TABLE_SIZE];
	/**
	* @brief Actual probe API version
	*/
	__weak const volatile uint64_t probe_api_ver = PPM_API_CURRENT_VERSION;
	/**
	* @brief Actual probe schema version
	*/
	__weak const volatile uint64_t probe_schema_var = PPM_SCHEMA_CURRENT_VERSION;
	/**
	* @brief Given the syscall id on 64-bit-architectures returns:
	* - `UF_NEVER_DROP` if the syscall must not be dropped in the sampling logic.
	* - `UF_ALWAYS_DROP` if the syscall must always be dropped in the sampling logic.
	* - `UF_NONE` if we drop the syscall depends on the sampling ratio.
	*/
	__weak const volatile uint8_t g_64bit_sampling_syscall_table[SYSCALL_TABLE_SIZE];
	/**
	* @brief Given the syscall id on 32-bit x86 arch returns
	* its x64 value. Used to support ia32 syscall emulation.
	*/
	__weak const volatile uint32_t g_ia32_to_64_table[SYSCALL_TABLE_SIZE];
	/*=============================== BPF READ-ONLY GLOBAL VARIABLES ===============================*/

As the proposal said, we can use then ad-hoc tracepoints to handle TOCTOU attacks, and provide additional global variables to selectively handle them.

[terror96]

I tend to agree that a solution which enables pruning would be preferable.

[tuminoid]

I'm drafting a PR based on these inputs, I'll ask further clarifications if I run into something unclear during implementations. Thanks for the input!

[tuminoid]

Based on @FedeDP comment, ultimately we will remove also implementation of this as well when removing enter event support, so I'm actually questioning myself if this is worth implementing at all. We have good momentum in syscall fixing right now and it looks like the config flag wouldn't be live for very long.

[leogr]

/milestone 0.22.0

[ekoops]

/assign

[terror96]

/assign