aquasecurity / tracee

Linux Runtime Security and Forensics using eBPF
https://aquasecurity.github.io/tracee/latest
Apache License 2.0
3.61k stars 416 forks source link

Parse argument in a separate argument instead of inline #3008

Open NDStrahilevitz opened 1 year ago

NDStrahilevitz commented 1 year ago

Currently when running tracee --o option:parse-arguments event arguments will be switched out to a string form where possible. In order to support #2177 in REGO, it's required that rego signatures will be able to access parsed arguments, despite golang signatures being written with the assumption of non parsed arguments.

As such the logic of the flag should change to one of these two:

  1. Add a new event ParsedArgument field which will include a parsed copy of the arguments.
  2. For each argument append a new argument with the name argname_parsed which will have the parsed values.

REGO and cel-go signatures will then implicitly ALWAYS access those instead of the default arguments.

itaysk commented 1 year ago

it's required that rego signatures will be able to access parsed arguments, despite golang signatures being written with the assumption of non parsed arguments

can you explain how did you come to this conclusion? why can't we do the same in rego and go (non-parsed)?

NDStrahilevitz commented 1 year ago

it's required that rego signatures will be able to access parsed arguments, despite golang signatures being written with the assumption of non parsed arguments

can you explain how did you come to this conclusion? why can't we do the same in rego and go (non-parsed)?

Well not absolutely required, but so far rego signatures have used them for readability. We're doing this change in golang with the knowledge we can use the libbpfgo exported consts to keep readability. Exporting these consts to rego will be cumbersome maintainability wise, so either users can refer to the code itself (and write non parsed compatible rego sigs), or keep using the parsed arguments convention. In that case, in order to keep the possibility of running parsed and non parsed signatures together we need to separate the parsed arguments not to be inlined on top of the existing ones.

There are other non signature related reasons to do so as well:

  1. It hides the original event info with no reference back to it
  2. It makes the argument type inconsistent from documentation and it is difficult to anticipate which argument will be parsed and which won't when running with the option.
itaysk commented 1 year ago

Thanks for the explanation, allowing the user/author to use parsed values is also a nice feature, but I don't understand why it's special to rego and not to go. we had a reason for moving to raw args in Go signatures and the same motivation is also relevant for rego signatures, isn't it? If so, I think we should provide the same experience in rego by exporting the helpers as rego functions.

NDStrahilevitz commented 1 year ago

Thanks for the explanation, allowing the user/author to use parsed values is also a nice feature, but I don't understand why it's special to rego and not to go. we had a reason for moving to raw args in Go signatures and the same motivation is also relevant for rego signatures, isn't it? If so, I think we should provide the same experience in rego by exporting the helpers as rego functions.

While REGO signatures could benefit from moving from parsed arguments in terms of performance, they are also already by their own nature not optimal for performance. If a tracee user has its performance in mind for his intended usage, they should probably not use REGO anyway and thus making sure this performance optimization is made as convenient in REGO as it is in golang is less important IMO.

The issue here is providing a more convenient syntax from our own repository. Doing so would require to do a go import -> rego export for ALL the (argparsers) constants in libbpfgo and making sure we are always aligned with libbpfgo on that. Maintenance wise it would be cumbersome. It might be possible to AST parse the helpers module (probably moving the consts to their own package in the module) and then using that to ease REGO exporting. Either way it seems to me like a lot of effort just for improving the performance of a feature which is already not recommended for use when performance matters (at least not for the initial delivery of this optimization in golang signatures).

In any case, rego users can refer to libbpfgo's helpers for the relevant constants for use in their own rego sigs and then disable parse-arguments.

itaysk commented 1 year ago

ok 👍

NDStrahilevitz commented 5 months ago

This solution will not be done as noted in: https://github.com/aquasecurity/tracee/pull/3018#issuecomment-1667481272