[Security Solution] [Detections] Provide field-specific suppression configuration to all rule types

[dhurley14]

Edit: After evaluating the options outlined below for supporting building block alert suppression the team decided we could utilize this same (new) data structure to also provide a path for configuring suppression settings for an individual field for all rule types. I will try to outline the outcome of our discussion below, which will be implemented at a later date.

Considering what the needs are for building block suppression, we determined the data structure that would support building block suppression also provided an avenue to further customize suppression fields for all rule types.

In order to do this the team has determined the best path forward includes adding a new field, for this issue we will name it group_by_v2, and "deprecating" usage of the current group_by field. This would be in part supporting a new UI that would allow for further customization and configuration of suppression settings per field for all rule types. The expected format will look something like this:

group_by_v2: Array<{field: string; sequence_index?: number; duration?: number etc... >;

and will live alongside the current group_by field until we are able to determine a safe route for deprecation and removal. We do have a history of handling similar situations by "migrating" fields whenever a CRUD operation is performed on a rule, despite this not being the preference.

This work for developing building block suppression will fold into a different epic for providing field-specific suppression functionality to all rule types, which will make suppressing building block alerts not just a special case (as would be the situation if implemented today) but a new foundation for security rule suppression.

---

Original Issue Description

EQL sequence alert suppression was split into two phases. The first phase implements suppression for the sequence alert and keeps only the building block alerts associated with the non-suppressed sequence alert. The suppression implementation focuses solely on suppressing the values in the sequence alert. Sequence-based alert suppression has some drawbacks which we hope to mitigate via this second phase. This ticket will discuss implementation options and other considerations for event-based (building block alert) suppression.

The second phase will implement alert suppression based on the building block alerts generated by a given EQL rule's sequence query. Suppressing on specific fields in a sequences' events, rather than the sequence as a whole, can provide customers more flexibility when determining which fields / values within a given sequence should be suppressed. We plan to accomplish this event-based suppression utilizing the building block alerts as the basis for the suppression.

The aim of this ticket is to provide context for reviewing changes introduced for sequence-based suppression (draft pr) which will determine whether the sequence-based implementation should include additional changes for preparation of event-based suppression, specifically any changes to the api that might be considered breaking or identifying areas of improvement in the sequence-based suppression pr.

In order to do this we want to discuss the two options we have (so-far, not precluding any other ideas so please discuss below) for representing the suppressed fields on building block alerts via the rule's properties.

Currently the suppression fields for a given rule are stored as such:

"alert_suppression": {
    "duration": {
      "unit": "h",
      "value": 5
    },
    "group_by": [
      "agent.name"
    ],
    "missing_fields_strategy": "suppress"
  },

Or

"alert_suppression": {
    "group_by": [
      "agent.name"
    ],
    "missing_fields_strategy": "suppress"
  },

When the duration property is omitted we execute suppression on a per-rule-execution basis.

The property worth focusing on listed above is the group_by property.

In order to instruct the EQL rule executor on which alert to suppress by (either sequence alert suppression or building block suppression, or maybe both?) we need to modify that group_by array OR introduce another field. The representation of event-based suppression with the group_by property will be the focus for the discussion.

The first option is to keep group_by as an array of strings and introduce a dotted-numeric notation to indicate which event in the sequence the field will suppress on. So for group_by: ['0.agent.name', ‘0.agent.version’, '1.host.name'] we are asking to suppress by the agent name for the first event in the sequence, the agent version for the first event in the sequence, and then suppress on host name values for the second event in the sequence. This representation does not require us to introduce new fields but will require us to parse these values more carefully than we are currently, as we could have multiple fields suppressed for multiple events in the sequence.

The second option is to provide an array of objects to group_by where the position of each individual object corresponds to the position of the event in the sequence which will be suppressed by the fields in that object. Something like group_by: [{"fields": ["agent.name", "host.name"]}, {"fields": ["network.application", "user.domain"]}]

The objective here is to determine potential integration points for EQL alert suppression and whether those points will need addressing in work that is underway or if changes can be postponed until it's time to implement event-based suppression.

The following are a few potential points of interest I briefly came across. I hope these can help provide better context for reviewing changes introduced in the EQL sequence alert suppression PR:

Schema validation

CRUD operations will need updated schemas to include an array of objects if we decide to go with that option for event suppression.

Link here:

kibana/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.gen.ts

Line 578 in ceb1b1a

export const AlertSuppressionGroupBy = z.array(z.string()).min(1).max(3);

If we go with the dotted-numeric representation we will also need to update the min and max values for the given array, and do the validation of the minimum / maximum amount of alert suppression fields in another area of the routes, moving the validation away from the zod representation. So that would be a potential downfall of the dotted-numeric representation.

Executor logic

Another area of possible discussion interest is during the actual creation of the suppressed alerts. We will need to transform the dotted-numeric array items into some other usable data structure (custom string parsing for 0.agent.name etc..) I won’t begin to hypothesize what that would look like.

I believe determining which type of suppression (either sequence based or event based) to apply to the given EQL rule will most likely be done based on the absence of certain properties (i.e. if the array of strings is agent.name and not 0.agent.name we know we are doing sequence-based suppression, or if the group_by array is made up of strings and not objects.) I believe we can mitigate breaking changes here.

Saved object schema

Since these are seemingly additive properties / changes, I believe it is not required to develop another saved object migration. But if another implementation outside of the two proposed above arises or preferred, it is worth keeping in mind what the impact this change might have on the saved object schema.

[elasticmachine]

Pinging @elastic/security-detections-response (Team:Detections and Resp)

[dhurley14]

cc: @marshallmain @vitaliidm @yctercero

[vitaliidm]

Since these are seemingly additive properties / changes, I believe it is not required to develop another saved object migration. But if another implementation outside of the two proposed above arises or preferred, it is worth keeping in mind what the impact this change might have on the saved object schema.

If we would go with the option array of objects and keeping same name of property, change won't be additive anymore since mapping of group_by is keyword in existing alerts. So, we probably need to introduce another field for this purpose.

Also if we roll out implementation in current draft PR in the same state is is right now, I think in future would be easier to adapt EQL rule suppression for both sequence and non-sequence to support both formats, rather than depending on query.
In this way, when user would change query through API, suppression config won't need to be changed

[dhurley14]

Yeah so I see the predicament with the params in the rule SO. Since we don't have available migrations for rule params we would need to support (theoretically) both arrays of strings and arrays of objects (if we go with the objects for the building block alert suppression implementation.) Or do what we've done in the past which is migrate those suppression params anytime a rule is updated.

[yctercero]

Gonna just brain 🧠 dump here. I'm trying to think of a way in which we 1) only need additive changes for phase 2 and 2) don't need to tie the structure to the query type (sequence/no sequence).

So let's say right now I have a non-sequence rule with the following:

"alert_suppression": {
    "duration": {
      "unit": "h",
      "value": 5
    },
    "group_by": [
      "agent.name"
    ],
    "missing_fields_strategy": "suppress"
  },

I update my query to include sequence and now want to update my suppression settings to mean that I want to suppress where agent.name and host.name in event 1 and foo.bar in event 2 match:

"alert_suppression": {
    "duration": {
      "unit": "h",
      "value": 5
    },
    "group_by": [
      "agent.name"
    ],
   sequence_order: [{ 0: ["agent.name", "host.name"] }, { 1: ["foo.bar"] }]
    "missing_fields_strategy": "suppress"
  },

In the UI we could have a checkbox for if they want to suppress by the shell alert (phase 1) or specify building block sequence logic (phase 2). The checkbox would be greyed out if we detect it's not a sequence alert. If it is a sequence alert, then we determine which logic to use based on whether sequence_order (or whatever better naming we find) exists or not).

Let's say I change my mind and now I want to go back to non-sequence, then:

• If via UI: we need to clear sequence_order and update the UI to the greyed out checkbox with probably a message that informs why it's greyed out.
• API: we need to include validation in any routes in which we allow query modification to check that sequence_order only exists if query is sequence type.

So I failed at #2 which was trying hard not to tie query logic to the structure, but I think it's unescapable here. We could in theory, not make the checks at the API boundaries and maintain the logic within the executor of checking which fields to use based on if the query includes sequence or not - but I think that's a bit messy to let users have say sequence_order around when it's irrelevant and could add a lot of confusion.

[marshallmain]

If we would go with the option array of objects and keeping same name of property, change won't be additive anymore since mapping of group_by is keyword in existing alerts. So, we probably need to introduce another field for this purpose.

@vitaliidm can you expand on this? Are we explicitly mapping the group_by fields in the alerts index somewhere or is it only stored flattened in kibana.alert.rule.parameters? If we're only storing it in parameters then I think it would be ok to add an additional format option to group_by, e.g. group_by: string[] | object[].

If we add an extra format option to group_by, my proposal would be to use a structure like

group_by: Array<{field: string; sequence_index?: number >;

so effectively we're just turning each string from the current schema into an object where we can specify options for that field. We'd still want validation to ensure that sequence_index is only used with sequence queries, but we could add other properties within this object for future configuration on a field by field basis (e.g. maybe configuring how array values are handled for each field?)

[vitaliidm]

@vitaliidm can you expand on this? Are we explicitly mapping the group_by fields in the alerts index somewhere or is it only stored flattened in kibana.alert.rule.parameters? If we're only storing it in parameters then I think it would be ok to add an additional format option to group_by, e.g. group_by: string[] | object[].

params are flattened, so both types would actually be stored in ES without errors.
But I was thing accessing and search might become complicated as 2 different data structures need to be accounted for

For example, search would become

POST suppression_test/_search
{
  "query": {
    "term": {"params.suppression.group_by": "agent.name"}
  }
}

POST suppression_test/_search
{
  "query": {
    "term": {"params.suppression.group_by.fields": "user.domain"}
  }
}

[dhurley14]

Summary from the tech-time meeting today:

No new changes needed for sequence alert suppression pr. We are now looking to add an additional field to migrate group_by array of strings into a new field group_by_with_objects (name to be determined) which will allow us to do field-specific suppression configuration (like setting a duration for a specific field rather than on the rule as a whole) for all rule types, and this work will also be used for building block alert suppression. I will update this ticket summary to reflect this discussion too.