update

Signed-off-by: Andrea Terzolo <[email protected]>

proposals/20240901-drop-support-for-syscall-enter-events.md → proposals/20240901-disable-support-for-syscall-enter-events.md

@@ -1,8 +1,8 @@
-# Drop support for syscall enter events
+# Disable support for syscall enter events
 
 ## Motivation
 
-This document proposes removing support for syscall enter events in our codebase. The primary reason behind this proposal is to reduce the throughput of events that our userspace needs to process. As the number of syscalls increases, we are no longer able to support them, and we start dropping events. Since enter events do not provide additional information compared to exit events (TOCTOU attacks will be addressed [in this section](#what-about-toctou-attacks)), the idea is to remove them entirely, thereby halving the number of events generated and processed.
+This document proposes disabling support for syscall enter events in our codebase. The primary reason behind this proposal is to reduce the throughput of events that our userspace needs to process. As the number of syscalls increases, we are no longer able to support them, and we start dropping events. Since enter events do not provide additional information compared to exit events (TOCTOU attacks will be addressed [in this section](#implement-toctou-mitigation)), the idea is to disable them entirely, thereby halving the number of events generated and processed.
 
 In summary, the main benefits of this proposal are:
 
@@ -319,33 +319,94 @@ HSET: 41618.11 requests per second, p50=0.623 msec
 
 This activity involves several steps at different levels:
 
-* Update our three drivers to collect all parameters in the exit fillers and remove the enter ones.
-* Update scap-file management to convert all enter events generated over the years into new exit events.
-* Update our userspace engines to support only exit events.
-* Update the sinsp parsing and extraction logic to use only exit events.
-* Update all tests (drivers, libscap, unit, e2e) to use only exit events.
-* Update all documentation.
+### Move all the parameters from the enter event into the exit one
 
-## Bonus point: scap-file management
+The idea is to work without forks, using incremental steps. Each PR will contain the following:
 
-During this workflow, we need to address the issue of supporting old scap-files with enter and exit events. We definitely need to write a converter to transform enter events into exit events. But can we do better? We know that our events are not "optimized"; for example, we send PID and FD on 64 bits instead of 32 bits and send a 24-byte header for each event, even though 16 bytes could suffice. These changes are currently unthinkable as they would completely break compatibility with older scap-files. However, if we could introduce a versioning mechanism, we could finally clean up our event schema without breaking compatibility.
+1. Add enter parameters to the exit event without touching anything else. This would be a minor change in the event schema since we are just adding new parameters to the exit one. This must be done for all drivers (kmod, ebpf, modern ebpf, gvisor). To give you an example
 
-Since this initiative will involve revising the schema for many events, this seems like the right time to clean up the schema by introducing a versioning mechanism for backward compatibility! The title says "Bonus point" because it is not guaranteed that this work will be carried out during this initiative; it depends on the challenges encountered during the process.
+ ```c
+ /////////////////////////// TODAY
+   [PPME_SYSCALL_READ_E] = {"read",
+         EC_IO_READ | EC_SYSCALL,
+         EF_USES_FD | EF_READS_FROM_FD,
+         2,
+         {{"fd", PT_FD, PF_DEC}, {"size", PT_UINT32, PF_DEC}}},
+   [PPME_SYSCALL_READ_X] = {"read",
+         EC_IO_READ | EC_SYSCALL,
+         EF_USES_FD | EF_READS_FROM_FD,
+         2,
+         {{"res", PT_ERRNO, PF_DEC},
+         {"data", PT_BYTEBUF, PF_NA}}},
 
-## What about TOCTOU attacks?
+ /////////////////////////// TOMORROW
+   [PPME_SYSCALL_READ_E] = {"read",
+         EC_IO_READ | EC_SYSCALL,
+         EF_USES_FD | EF_READS_FROM_FD,
+         2,
+         {{"fd", PT_FD, PF_DEC}, {"size", PT_UINT32, PF_DEC}}},
+   [PPME_SYSCALL_READ_X] = {"read",
+         EC_IO_READ | EC_SYSCALL,
+         EF_USES_FD | EF_READS_FROM_FD,
+         4,
+         {{"res", PT_ERRNO, PF_DEC},
+         {"data", PT_BYTEBUF, PF_NA},
+         {"fd", PT_FD, PF_DEC},
+         {"size", PT_UINT32, PF_DEC}}},
+ ```
 
-We currently provide mitigation against TOCTOU attacks for certain syscalls, specifically:
+2. Adapt sinsp state to work just with exit events. Enter events can still be generated but they won't populate the state anymore. The idea is to use a feature flag in sinsp to enable/disable enter events. Please note! This means that old driver versions won't work with new libs versions because they cannot correctly populate the sinsp state since it will work only with exit events. The idea is to bump also the minor schema version required by scap <https://github.com/falcosecurity/libs/blob/2e1c5b68380eb00ab79e61f369a8bf95faab4968/userspace/libscap/scap.h#L105>
+3. Create a scap-file conversion (in a dedicated scap-file converter) to convert ENTER events into merged EXIT ones. Since we are here we will also handle old event versions in this converter, but each PR will add conversion just for one event pair.
+4. Add some tests replaying scap-files and checking that everything is good.
 
-* `connect`
-* `creat`
-* `open`
-* `openat`
-* `openat2`
+Look at some PRs to have an example:
+
+* <https://github.com/falcosecurity/libs/pull/2187>
+* <https://github.com/falcosecurity/libs/pull/2205>
+* <https://github.com/falcosecurity/libs/pull/2206>
+
+### Implement TOCTOU mitigation
+
+We currently provide mitigation against TOCTOU attacks for 5 syscalls:
+
+* connect
+* open
+* creat
+* openat
+* openat2
 
-The mitigation implemented [in this PR](https://github.com/falcosecurity/libs/pull/235) uses enter events. How can we achieve the same result without sending these events to userspace? The idea is to hook into the specific tracepoints for those five syscalls (e.g., `tracepoint/sys_enter_open`), collect the necessary parameters, and save them in a map indexed by thread ID. In the exit event, we can retrieve this information and send only one event to userspace.
+The mitigation implemented in this PR (<https://github.com/falcosecurity/libs/pull/235>) uses enter events. How can we achieve the same result without sending these events to userspace?
+
+For the connect syscall, we can use the tuple parameter in the exit event to avoid issues with TOCTOU. So, the mitigation here is to populate the userspace with information directly from the kernel.
+
+For what concerns the other 4 syscalls, the idea is to hook into the specific tracepoints  (e.g., `tracepoint/sys_enter_open`), collect the necessary parameters, and save them in a map indexed by thread ID. So we use a bpf hash map with thread-id as a key and syscall arguments as a value. This is very similar to what we do today when `RAW_TRACEPOINTS` are not enabled:  <https://github.com/falcosecurity/libs/blob/0.19.0/driver/bpf/maps.h#L103> In the exit tracepoint, we can retrieve this information and send only one event to userspace.  This drastically reduces the instrumentation time with respect to using the generic sys_enter tracepoint like today. Moreover, we won't send the enter event to userspace but we will merge the information directly in the kernel.
 
 Long-term mitigation would involve attaching to internal kernel hooks rather than using the syscall enter hooks, but currently, we only support tracepoints as hook points in our drivers.
 
+### Adapt consumers to use only exit events
+
+Some consumers of the libraries still use enter events for their logic. Now exit events should contain all necessary parameters so it’s time to switch the consumers to use exit events. Consumers we need to update:
+
+* Plugins
+* Falco
+* Sysdig CLI tool
+
+### Adapt rules to use only exit events
+
+Today some rules could use enter events. We need to provide an automatic migration. An idea could be to update our default rulesets and provide a script for users' rulesets' automatic migration.
+
+### Update Documentation
+
+Tell users that by default enter events won’t be generated anymore.
+
+### Make enter event optional in libs
+
+[Preliminary cleanup] During the work, we may have left some todo! to solve at the end of the work. This is probably the right moment to do it and simplify the code. For example, the flag `EF_TMP_CONVERTER_MANAGED` can be removed since we can now mark the enter events as `EF_OLD_VERSION`. We can use `EF_OLD_VERSION` to understand if we need a conversion instead of `EF_TMP_CONVERTER_MANAGED`.
+
+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.
+
+When we add a new syscall we should remember to add an enter event with 0 parameters like we do today. So events will always be added to the event table in pairs.
+
 ## Additional link/resources
 
 * <https://github.com/falcosecurity/falco/issues/2960>