Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filterchecks flags cleanup #1109

Merged
merged 3 commits into from
May 3, 2018
Merged

Filterchecks flags cleanup #1109

merged 3 commits into from
May 3, 2018

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented May 2, 2018

Several changes to update the flags used for filterchecks to make them accurately reflect how they can be used.

This is related to some falco changes in falcosecurity/falco#364.

@mattpag can you double-check the changes?

evt.dir was flagged as EPF_PRINT_ONLY, but it can be used in filters as
well. Fix the flag.
@mstemm mstemm requested a review from mattpag May 2, 2018 17:34
Many filters required an argument in the form .arg or [arg], but they
didn't have the flag EPF_REQUIRES_ARGUMENT. This flag wasn't used
anywhere, but we plan on using it in falco soon, so it needs to be
accurate.

Do a pass over all filterchecks that call extract_arg and add
EPF_REQUIRES_ARGUMENT as needed.
@mstemm mstemm force-pushed the filterchecks-flags-cleanup branch from c3a1fd8 to 51d9d65 Compare May 2, 2018 17:39
Copy link
Contributor

@mattpag mattpag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've gone through all the parse_field_name implementations and look for the fields that use extract_arg. It seems to me you're missing:

  • proc.apid, proc.aname
  • evt.type.is
  • span.count.fortag, span.childcount.fortag
  • evtin.span.p.arg

Missed some in the previous commit.
@mstemm
Copy link
Contributor Author

mstemm commented May 3, 2018

Thanks for double checking. proc.aname and proc.apid are a bit different in that they can work with arguments, but they do not require arguments. Without arguments, they will check all process ancestors until one with the desired name/pid is found. That's why I didn't flag them.

I added the others though.

@mattpag
Copy link
Contributor

mattpag commented May 3, 2018

Yeah, I added them because I supposed you wanted to use that flag in falco as an indicator of a possible argument, whether mandatory or not.
Even better then, I had noticed the inconsistency too :)

@mstemm mstemm merged commit 85f88ab into dev May 3, 2018
@mstemm mstemm deleted the filterchecks-flags-cleanup branch May 3, 2018 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants