falcosecurity
FR: Configuration flag support to disable enter event generation
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
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?
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 ?
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: https://github.com/falcosecurity/libs/blob/5a1faed1581c74b5a4adb42b7ec28333b95ea3c7/driver/modern_bpf/maps/maps.h#L16-L64 As the proposal said, we can use then ad-hoc tracepoints to handle TOCTOU attacks, and provide additional global variables to selectively handle them.
I tend to agree that a solution which enables pruning would be preferable.
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!
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.
While working on https://github.com/falcosecurity/libs/issues/2427 and supporting activities such as https://github.com/falcosecurity/libs/pull/2545 in order to remove the current implementation's dependency of information in the enter events, I started thinking that there are events for which the exit event is not expected/generated:
driver/ppm_events_public.h: PPME_PROCEXIT_X = 17, /* This should never be called */
driver/ppm_events_public.h: PPME_SCHEDSWITCH_1_X = 147, /* This should never be called */
driver/ppm_events_public.h: PPME_SCHEDSWITCH_6_X = 153, /* This should never be called */
driver/ppm_events_public.h: PPME_PROCEXIT_1_X = 187, /* This should never be called */
driver/ppm_events_public.h: PPME_SYSCALL_SENDFILE_X = 189, /* This should never be called */
driver/ppm_events_public.h: PPME_SCAPEVENT_X = 197, /* This should never be called */
driver/ppm_events_public.h: PPME_SIGNALDELIVER_X = 233, /* This should never be called */
driver/ppm_events_public.h: PPME_PROCINFO_X = 235, /* This should never be called */
driver/ppm_events_public.h: PPME_CPU_HOTPLUG_X = 245, /* This should never be called */
how should we handle these events?
While working on #2427 and supporting activities such as #2545 in order to remove the current implementation's dependency of information in the enter events, I started thinking that there are events for which the exit event is not expected/generated:
driver/ppm_events_public.h: PPME_PROCEXIT_X = 17, /* This should never be called */ driver/ppm_events_public.h: PPME_SCHEDSWITCH_1_X = 147, /* This should never be called */ driver/ppm_events_public.h: PPME_SCHEDSWITCH_6_X = 153, /* This should never be called */ driver/ppm_events_public.h: PPME_PROCEXIT_1_X = 187, /* This should never be called */ driver/ppm_events_public.h: PPME_SYSCALL_SENDFILE_X = 189, /* This should never be called */ driver/ppm_events_public.h: PPME_SCAPEVENT_X = 197, /* This should never be called */ driver/ppm_events_public.h: PPME_SIGNALDELIVER_X = 233, /* This should never be called */ driver/ppm_events_public.h: PPME_PROCINFO_X = 235, /* This should never be called */ driver/ppm_events_public.h: PPME_CPU_HOTPLUG_X = 245, /* This should never be called */how should we handle these events?
Hey @terror96 . Those are meta event/internal event types that don't fit well with the enter/exit split, as they only require a single event type. However, in order to be able to make some assumptions on the event types defined in the event table (mainly that enter events have an associated even number, and exit events have an associated odd number), a couple has been introduced for each of them... But as you can see, only the "enter" event type is used, and we can keep it as is.
While working on #2427 and supporting activities such as #2545 in order to remove the current implementation's dependency of information in the enter events, I started thinking that there are events for which the exit event is not expected/generated:
driver/ppm_events_public.h: PPME_PROCEXIT_X = 17, /* This should never be called */ driver/ppm_events_public.h: PPME_SCHEDSWITCH_1_X = 147, /* This should never be called */ driver/ppm_events_public.h: PPME_SCHEDSWITCH_6_X = 153, /* This should never be called */ driver/ppm_events_public.h: PPME_PROCEXIT_1_X = 187, /* This should never be called */ driver/ppm_events_public.h: PPME_SYSCALL_SENDFILE_X = 189, /* This should never be called */ driver/ppm_events_public.h: PPME_SCAPEVENT_X = 197, /* This should never be called */ driver/ppm_events_public.h: PPME_SIGNALDELIVER_X = 233, /* This should never be called */ driver/ppm_events_public.h: PPME_PROCINFO_X = 235, /* This should never be called */ driver/ppm_events_public.h: PPME_CPU_HOTPLUG_X = 245, /* This should never be called */how should we handle these events?
Hey @terror96 . Those are meta event/internal event types that don't fit well with the enter/exit split, as they only require a single event type. However, in order to be able to make some assumptions on the event types defined in the event table (mainly that enter events have an associated even number, and exit events have an associated odd number), a couple has been introduced for each of them... But as you can see, only the "enter" event type is used, and we can keep it as is.
Okay, makes sense. I was that thinking that disable enter event generation will ultimately not completely eliminate all events defined as enter which I guess is worth noting at some point. For the above case it is merely semantics for the reasons you explained.
Based on https://github.com/falcosecurity/libs/issues/2427#issuecomment-2948799557, 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.
I agree with this. Since we already removed the drivers support for some enter event, it would not make much sense to add this flag now. If you agree, we can proceed on this direction and avoid the flag implementation.
Based on #2427 (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.
I agree with this. Since we already removed the drivers support for some enter event, it would not make much sense to add this flag now. If you agree, we can proceed on this direction and avoid the flag implementation.
Yup, I see no further use case for this flag. I see work is already on-going to cleanup the enter events, good work!
Thank you very much! I'm gonna wait till the end of the day in order to let other people add their opinions on this, and if nobody complains, I'll close this!