libs

libs copied to clipboard

What type of PR is this?

/kind cleanup

Any specific area of the project related to this PR?

/area driver-modern-bpf

/area libsinsp

/area tests

Does this PR require a change in the driver versions?

No

What this PR does / why we need it:

This PR does the followings:

• add a test for UNIX sockets in sinsp
• add some tests for evt.arg/evt.rawarg/avt.args fields
• enforce some runtime checks check_param_name_exists/check_param_id_exists on evt.arg this should prevent our users from using non-existent fields with evt.arg.* . I noticed it in one of our default rules Non sudo setuid, see it here https://github.com/falcosecurity/libs/issues/1630. we use exe_flags=%evt.arg.flags when the event is setuid but setuid doesn't have a flags arg. Please note this is a BREAKING CHANGE since now some broken rules will stop Falco at runtime... it would be amazing to stop it when parsing the filter-checks names but at the moment we don't have any event information at that point so we need to do that during extraction :/ BTW there is room for improvements in the future

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

cleanup(sinsp)!: enforce some runtime checks and remove an extra space from `evt.args`

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [Andreagit97]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

Oh this is a real bummer, I wasn't aware Falco would just crash in such cases, re specifically the exe_flags=%evt.arg.flags besides renaming it to flags we should adjust all upstream rules to not cause crashes and only use fields that are valid. We should still fix it here, but at least that way we fix things for Falco 0.37 and the next rules release. I can take care of that PR.

Edit @Andreagit97 you already opened the rules PR, on it!

at the moment Falco doesn't crash if a field evt.arg.* doesn't exist for an event, so we are fine. The 2 checks implemented here check_param_name_exists/check_param_id_exists cannot work with actual Falco because we have several rules in which we use evt.arg.* but the field is not defined for all the events in the condition (see https://github.com/falcosecurity/rules/issues/214) so it would cause random crashes at runtime :/ I will detach the tests in another PR and probably close this one! until we solve the above issue (if we want to solve it) we cannot implement this strict validation unfortunately

/hold

Thanks for the clarification and also thanks for looking into it!

@Andreagit97 could you rebase? I think we could start getting this over the finish line if you would like, I'll help as reviewer, thank you!

i will try to address it this week!

close in favor of https://github.com/falcosecurity/libs/pull/1791