Skip to content

[7.x] [Security Solution][Detections]Indicator Match Enrichment (#89899)#91264

Merged
rylnd merged 1 commit intoelastic:7.xfrom
rylnd:backport/7.x/pr-89899
Feb 12, 2021
Merged

[7.x] [Security Solution][Detections]Indicator Match Enrichment (#89899)#91264
rylnd merged 1 commit intoelastic:7.xfrom
rylnd:backport/7.x/pr-89899

Conversation

@rylnd
Copy link
Copy Markdown
Contributor

@rylnd rylnd commented Feb 12, 2021

Backports the following commits to 7.x:

)

* Adds basic integration test for threat enrichment

* Update signals mappings with indicator fields

* Simplify some ternaries with Math.min

* Remove outdated comments

* Add notes from walkthrough with devin

* Add an enrichment hook to the current signal creation pipeline

When this moves to individual rule-specific data transformations this
will be a little more explicit/configurable; for now to keep changes
minimal, we're using dependency injection to pass a function, which will
default to the identity function (e.g. a no-op).

* Add utility functions for encoding/decoding our threat query

This is what allows us to enrich the threat match signals using only the
signal search response.

* Add a name to each threat match filter clause

This gives us the information we need to enrich our signals after
they've been queried without having to perform a complicated reverse
query.

* Adds functions for signal enrichment of threat indicators

* Wire up threat enrichment to threat match rules

* Fleshes out threat match integration tests

Adds assertions to the existing test, and fleshes out another test for a
multi-match signal.

* Add more test cases to indicator match integration tests

* single indicator matching multiple events
* multiple indicators matching a single event
* multiple indicators, multiple events
* placeholder for deduplication logic

This also adds some descriptions to our threat intel documents, to give
a little context around how they're meant to function within the tests,
particularly as relates to the auditbeat/hosts data on which it is meant
to function.

* Implement signal deduplification

This handles the situation where the indicator match search has returned
the same signal multiple times due to the source event matching
different indicators in different query batches. In this case, we want
to generate a single signal with all matched indicators.

* Move default indicator path to constant

* Testing some edge cases with signal enrichment

* Cover and test edge cases with threat enrichment generation

* Fix logical error in TI enrichment

We were previously adding the indicator's field to matched.field,
instead of the corresponding event field that matched the indicator.

In the normal case, the expectation is that the indicator field is
self-evident, and thus we want to know the other side of the match on
the event itself.

Updates tests accordingly.

* Document behavior when an indicator matched but is absent on enrichment

This could occur if the indicator index is updated while a rule is being
run.

* Add followup note

* Add basic unit test for our enrichment function

This just verifies that the enrichment function gets invoked with search
results.

* Update license headers for new files

* Remove unused threatintel archive

I made both of these before we were clear on the direction we were
taking here.

* Bump signals version to allows some updates in patch releases

* Fix typings of threat list item

We were conflating the type of the underlying document with the type of
the search response for that document. This is now addressed with two
types: ThreatListDoc and ThreatListItem, respectively.

ThreatListDoc isn't the most distinguishing name but it avoids a lot of
unnecessary renaming for the existing concept of ThreatListItem.

* Update test mock to be aware of (but not care about) named queries

* Remove/update outdated comments

This code was modified to perform two searches instead of one; at that
time, a lot of this code was duplicated and modified slightly, and these
misleading comments were a result. I removed the ones that were no
longer relevant, but left a TODO for one that could be a bug.

* Remove outdated comment

Documents will always have _id.

* Update enriched signals' total to account for deduplication

If a given signal matched on multiple indicators in different loops of
our indicator query, it may appear multiple times. Our enrichment
performs the merging of those duplicated results, but did not previously
update the response's total field to account for this.

I don't believe that anything downstream is actually using this field and that we
are instead operating on the length of hits and the response from the
bulk create request, but this keeps things consistent in case that
changes.

* Remove development comments

* Add JSDoc for our special template version constant

* Remove outdated comments

* Add an additional test permutation for error cases

Ensure that we throw an error if the indicator field is either a
primitive or an array of primitives.

* Remove unnecessary coalescing

These values are already defaulted in the parent, and the types are
correct in that these cannot be undefined.

* Move logic to build threat enrichment function into helper

* Refactor code to allow typescript to infer our type narrowing

existingSignalHit could not be undefined on line 30 here, but typescript
could not infer this from the !acc.has() call.

* Use a POJO over a Map

We were using a map previously in order to use .has() for a predicate,
but code has since been refactored to make that unnecessary.

* Explicitly type our enriched signals

These are being typed implicitly and verified against SignalSourceHit[]
on the assignment below, but this makes the types explicit and surfaces
a type error here instead of the subsequent assignment.

* Add an explanatory note about these test results

* Remove unused imports

These references were moved into buildThreatEnrichment

* Remove threat mappings accidentally brought in with indicator work

I copied the entirety of the `threat` mappings in order to get the
`threat.indicator` ones, but it looks like these were added at some
point too.

I'd rather these not be added incidentally. If we need them, we should
do so explicitly.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@rylnd rylnd added the backport This PR is a backport of another PR label Feb 12, 2021
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@rylnd rylnd merged commit 60bdf52 into elastic:7.x Feb 12, 2021
@rylnd rylnd deleted the backport/7.x/pr-89899 branch February 12, 2021 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants