[Security Solution][Detections] Threshold rules can generate signals with overridden fields

[rylnd]

Summary

Due to the way threshold rules generate signals (quoted from an SDH ticket):

Because the threshold query itself is a relatively simple terms aggregation, most document-level information is lost, and consequently the generated signal would be similarly empty. To make the signal more useful, we promote some information about the rule into the signal. This includes things like the timestamp and index of the search, but more importantly it includes:

• the fields/values involved in the rule query, if exact (e.g. user.name.keyword/test)
• the field used in the aggregation, with values from the resulting buckets

combined with a lack of validation on those fields, it's possible for a rule to generate signals that have overridden signal.* fields, which can cause various data integrity issues.

Examples

1. A user wishes to generate a high-priority signal when another low-priority rule generates multiple signals. A natural way to accomplish this would be with a threshold rule against the signals index, with a query of signal.rule.name: "my-low-priority-rule".

   • Because of this issue, the resulting signals will have a signal.rule.name of "my-low-priority-rule" along with a signal.rule.id of the threshold rule.

2. A user wishes to generate a signal when N building block signals are generated. A natural implementation would be a threshold rule of threshold N and a query of signal.rule.building_block_type: "default"

• Because of this issue, the resulting signals will have a signal.rule.building_block_type of "default" and will thus also be building block signals and hidden by default.

[elasticmachine]

Pinging @elastic/siem (Team:SIEM)

[madirey]

As we discussed in chat, I believe we need to start being more clear about the semantics when dealing with signal properties. signal.rule.* should contain properties about the rule... these should be immutable and contain a snapshot of the rule data that generated the signal. Properties of the signal itself should be a level higher: signal.* ... we may need to move building_block_type up a level to accommodate this? Or duplicate it?

The above is a related issue, but doesn't fix the problem, as we still use one of the matching documents to generate the signal. And thus, this document will still also be a building block signal. We'll need to add some logic to overwrite building_block_type before creating these signals.

Finally, we may want another place to dump the actual matching values (the matching signal.rule.id for example).

If we do those 3 things before generating the signal, I think it fixes the issue:

1. Overwrite signal.rule with rule snapshot before writing signal.
2. Set signal.building_block_type (or signal.rule.building_block_type if we leave it) to undefined before writing signal.
3. Add matching values to a separate object in document to avoid collisions (threshold.result.matches?).

[rylnd]

Some prior art: #82191 fixed a similar issue where the triggering document contained signal.* fields. However, because threshold rules generate these fields in dot notation as opposed to nested, the generated signal ultimately evaluates to e.g.

{
  "signal.rule.building_block_type": "default",
  "signal": {
    "rule": {
      "building_block_type": null
    }
  }
}

which causes the value of "default" to win after the signal has been ingested. I believe this is due to the behavior of the dot expand processor, namely:

If there is already a bar field nested under foo then this processor merges the foo.bar field into it.

but this has not been verified as the cause.

It seems that this bug affects any signal documents with signal fields in dotted notation.

[rylnd]

we may need to move building_block_type up a level to accommodate this? Or duplicate it?

@madirey I can envision a future where we have some user-defined, post-trigger transformations on signals; having both signal.rule.building_block_type and signal.building_block_type would allow us to distinguish between the rule's configuration and the signal's ultimate type 👍

[rylnd]

3. Add matching values to a separate object in document to avoid collisions (threshold.result.matches?).

Yep, this makes sense to me 👍. We won't be able to search/aggregate on those values once they're scoped within a "matches" field, but for me that's far outweighed by the ability to distinguish the conditions of the match from the other signal data.