[FEATURE] Support selective rule overrides / merging

[jonny-wg2]

Motivation

Falco has around 60 default rules configured. However, I am only interested in say configuring 15 rules with an alert policy because of my setup. Falcosidekick is a great tool for managing the alerting of falco rules with setting a default priority on what it should fire.

At current, there are priorities of notice, informational, error (which I find a weird name), but also the priorities alert, critical and emergency. As part of the default rules, alert and emergency are not used. Weird.

My main goal is to change 15 alerts to the alert priority.

The only way to do this is copy the entire rule into my rules.local.yaml. There has to be a smarter and better way for changing this.

Feature

- rule: xyz
  append: priority
  priority: alert

Alternatives

My alternative is copying the entire rule from the default rules into my rules.local

Additional context

Others have had this problem: https://kubernetes.slack.com/archives/CMWH3EH32/p1596015898189000

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Issues labeled "cncf", "roadmap" and "help wanted" will not be automatically closed. Please refer to a maintainer to get such label added if you think this should be kept open.

[ossie-git]

This issue was closed but was it every resolved? I find that some priorities need modification but I'd rather just add a line or two instead of having to copy over the entire rule to my local rules file. Thanks

[bsod90]

So, copying is the only way for now?

[jasondellaluce]

/reopen

This is still not addressed.

[poiana]

@jasondellaluce: Reopened this issue.

In response to this:

/reopen

This is still not addressed.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jonny-wg2]

As of now, we still have to copy the entire alert...

…

On Tue, May 23, 2023 at 10:30 AM poiana ***@***.***> wrote: @jasondellaluce <https://github.com/jasondellaluce>: Reopened this issue. In response to this <#1340 (comment)> : /reopen This is still not addressed. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. — Reply to this email directly, view it on GitHub <#1340 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJP4OTANYCNRU3OWQ277KGLXHRYS3ANCNFSM4PRQXE2Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[incertum]

Assigning myself and will take care of it as it is related to other similar asks I opened up PRs for.

priority is a string, therefore instead of append ability we need either (option 1) a new override: true that allows overriding just for example the priority without needing to define/re-define the entire rule or (option 2) support this feature silently, that is, if we see the same rule name and for example just the priority field is re-defined we only override that field and keep the other old fields?

Which option is the preferred one from a user experience perspective? My vote goes for option 2 and code changes needed should be minimal.

[tspearconquest]

Option 2

[jonny-wg2]

option 2 works for me. What ever is easiest to implement.

…

On Thu, Jul 6, 2023 at 12:23 AM tspearconquest ***@***.***> wrote: Option 2 — Reply to this email directly, view it on GitHub <#1340 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJP4OTDD6HV6DWGWABD2B5LXOXSNTANCNFSM4PRQXE2Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[incertum]

@jasondellaluce and I have synced and will prioritize any items related to rules loading and managements which fits in nicely with the upcoming revamp according to a new rules maturity framework. There will be a more official announcement before the Falco 0.36 release.

/milestone 0.36.0

[incertum]

@jonny-wg2 in addition I renamed the issue title, please let me know if the new version is ok with you. Thanks a bunch!

[incertum]

@ericjulien-okta and @tspearconquest I have closed your other 2 related issues to have one consolidated issue, hope this is ok with you, thanks!

[tspearconquest]

Yes, that's fine. Get Outlook for Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Melissa Kilby ***@***.***> Sent: Monday, July 10, 2023 9:01:47 PM To: falcosecurity/falco ***@***.***> Cc: Thomas Spear ***@***.***>; Mention ***@***.***> Subject: Re: [falcosecurity/falco] Support partial overrides / merging for existing rule (e.g. `priority` field) to complement the append functionality (#1340) CAUTION: This email originated from outside the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe. @ericjulien-okta<https://github.com/ericjulien-okta> and @tspearconquest<https://github.com/tspearconquest> I have closed your other 2 related issues to have on consolidated issue, hope this is ok with you, thanks! — Reply to this email directly, view it on GitHub<#1340 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ATRTFZ76JF3A5R2UX6FSDLLXPSXYXANCNFSM4PRQXE2Q>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[incertum]

I have a complete PR up https://github.com/falcosecurity/falco/pull/2671/files

Falco would now allow

(1) partial overrides of any yaml keys when re-defining
(2) append to output and tags keys in append mode
(3) override for enabled and priority fields in append mode as well

The PR includes a new unit test suite and a test yaml, @tspearconquest @jonny-wg2 @ericjulien-okta, would you mind taking a look at the test conditions and letting me know if there are more edge cases I could test? And of course please confirm that those are the capabilities that would help you.

Thank you!

There are still some other fixes we need for tags related filtering and enabling that @jasondellaluce wanted to resolve.

[jonny-wg2]

@incertum I don't see the following below in your test. Would just appending the priority without having to also include the condition work? E.g.

- rule: Dummy Rule 5
  append: true
  priority: CRITICAL

[incertum]

@jonny-wg2 understood, added this feature in as well, see latest commit.
Mind checking again if we cover all requested scenarios?

[jonny-wg2]

Looks GTG @incertum - Thanks again for taking on this endeavour. Much appreciated!! 🤗

[incertum]

Posted in the PR #2671 (comment), we may need another round of discussion so that everyone is on board so there is a chance for these chances to be approved.

[jasondellaluce]

@incertum I don't see the following below in your test. Would just appending the priority without having to also include the condition work? E.g.

- rule: Dummy Rule 5
  append: true
  priority: CRITICAL

@jonny-wg2 Hi! Just so that I understand better, what's your use case for this? What are you trying to accomplish by appending to a priority?

[jonny-wg2]

@jasondellaluce described in my iniatial ticket perhaps is the best explanation, but I can reword it.

Out of 60 rules, I only want to receive slack notifications on 15 rules. I configured falco + falcosidekick to forward all alerts with priority alert to send to falcosidekick, and then sent to slack.

Falco provides a lot of insight into what is happening on the host that is beneficial for investigations but perhaps not the best for trigger oncall alerts.

So the aim of the game is to have a list of 15 rules that are deemed as oncall and change the priority of these alerts.

Right now, we have a custom script that will copy these rules + logic to just change the priority and really messy.

As a user, you would assume that the priority field could be easily changed as ALERT and EMERGENCY are not used by any of the default rules.

[Andreagit97]

If we implement this logic #2671 we should resolve this issue 🤔

[leogr]

In PR #2671, multiple approaches were discussed, though a consensus on implementation was not reached. IMO, the main challenge here is the cumbersome nature of overriding or merging specific fields in Falco rules without replicating the entire rule in the local configuration.

With that in mind, and after having rethought the entire issue, below is my proposal. I'm keen to hear feedback and suggestions for improvements.

Also, since we are too late to include this in 0.36, I propose to implement this in the 0.37 dev cycle. I apologize that this took so long. The rules syntax is a complex matter, and we know well from our experience that introducing features too hastily causes more problems than benefits.

cc @falcosecurity/falco-maintainers

[PROPOSAL] Selective rule ovveride

I'd like to suggest the introduction of an override field, structured as:

override:
  [key]: [append|replace]

Where:

• [key] targets the field we wish to override (e.g., condition, priority).
• [append|replace] specifies the operation on the targeted field.

This feature should be supported by rule, macro, and list constructs.

Examples:

- rule: Rule 1
  condition: evt.type=openat
  
- rule: Rule 1
  condition: and evt.type=openat
  priority: CRITICAL
  override:
    condition: append
    priority: replace

The above example would append to the existing condition and replace the priority.

Backward Compatibility

To ensure backward compatibility, the existing append shortcut will remain:

- macro: xxx
  condition: yyy
  append: true

Equivalent to:

- macro: xxx
  condition: yyy
  override:
    condition: append

Using both append and override together would result in an error.

When ovveride is not specified, we will retain its current behavior (e.g., the enabled special case).

Benefits

1. Backward Compatibility: Existing configurations remain unaffected.
2. Extensibility: The proposal is future-proof and can be extended with new operations, e.g., prepend.

TBD

• Decide which keys should be supported by ovveride (good examples in this comment)

[FedeDP]

I like the new design @leogr and i think it is also good because it allows us to retain backward compatibility; moreover, it's really fine-grained since it allows to specifcy which keys should [append,replace] the eventually existing ones from the original rule.

Also, since we are too late to include this in 0.36, I propose to implement this in the 0.37 dev cycle.

+1 also for this. Don't mess up with rules syntax 25d from a release!

[Andreagit97]

Let's say I am not a big fan of postponing it, but since we have not reached an agreement on the design yet and since this feature will require not only lines of code but also a great documentation effort, I'm not sure one of us has time to deliver it in time... I agree with moving it to the next release, but we should definitely implement it in the next release cycle.

I tend to agree with the proposed design because with a single block, we can replace/append all the fields we want.

Decide which keys should be supported by ovveride (good examples in #2671 (comment))

I would say all the ones that bring a value, so condition, output, tags and desc. Not sure about exceptions... first I would like to understand how this feature is related to exceptions and if in some way we can do what exceptions do with this new feature

Moreover, I would propose to deprecate the append: field since it will be replace by this feature

[incertum]

@leogr +1 re the design.

@Andreagit97 +1 re starting the deprecation of append: add a warning for Falco 0.37 and deprecate in Falco 0.38.

re exceptions I would like to discuss the faith of exceptions in general separately in the next dev cycle.

We should support all fields, however priority and enabled can really only be replaced and not appended to, else both options should be supported.

re postponing to Falco 0.37, what @Andreagit97 said about no one of us having the time right now is true. Therefore, sadly we almost have no choice other than postponing.

[incertum]

We brought this up again in today's core maintainer meeting.

@jasondellaluce will open the new PR according to above's agreed upon design plus new tests (possibly recovering some unit tests from the old PR, creating new tests and / or add tests to the Falco regression testing suite).

Milestone remains Falco 0.37

[LucaGuerra]

/assign

[incertum]

Hi 👋 @jonny-wg2 @ossie-git @bsod90 @ericjulien-okta @atmosx and @tspearconquest

This feature is now available in the master build for testing. Thanks in advance for your feedback!

Happy Holidays!