[Security Solution][Detections] Reduce detection engine reliance on _source #89371

marshallmain · 2021-01-26T21:59:43Z

Summary

Changes by rule type:

KQL
- Runtime fields in source documents will be evaluated against value list exceptions
Threshold
- Timestamp for generated alert is now extracted from fields instead of _source, so if the timestamp override is a runtime field then @timestamp will be correctly populated
- threshold_result.value now uses the bucket key rather than extracting the value from _source - so if the key is a runtime field it will be correctly populated. This also fixes a bug where if the field being aggregated on was an array of values then threshold_result.value would contain the full array rather than the single value from the array that was actually being used as the key
ML
- Runtime fields in source documents will be evaluated against value list exceptions
Threat match
- Runtime fields can now be used in the indicator match indices
- Runtime fields in source documents will be evaluated against value list exceptions
EQL
- No changes as runtime fields are not accessible in the EQL search API yet (https://github.com/elastic/dev/issues/1600)

Remaining dependencies on _source

Building the alert document that goes into .siem-signals
- We rely on the parent and ancestor information for signals on signals and this info is only available in _source - fields flattens arrays of objects which makes it impossible to reconstruct the original array of objects
- Merging _source and fields does not work for arrays of objects because not all objects in the array have the same set of fields. For example,

[{
  field1: 1
},
{
  field1: 2,
  field2: 3
}]

would flatten to

fields: {
  field1: [1, 2]
  field2: [3]
}

If field2 is overwritten by a runtime field, then we don't have enough information to merge the field2 from fields with the array of objects from _source.

It's also possible to have runtime fields that can't be represented by a JSON structure at all.

fields: {
  field1: [1],
  field1.sub_field: [2]
}

This situation is possible to create with field1 coming from the _source of a document and field1.sub_field defined as a runtime field, however, these fields and values can't be represented as a single object.

One solution would be to only merge fields from fields that don't conflict with existing fields in _source
- However, this may not store all runtime fields
Another solution would be to ask the ES team for the ability to retrieve only runtime fields from fields and store those in their own section of the alert document
- Maybe possible to identify the runtime fields by comparing with _source, but it's much easier if ES can specify whether each field is runtime in the response
Hybrid approach
- Get runtime fields from fields response and then merge the runtime fields that do NOT conflict into _source
- Store all runtime fields in a separate section as well for completeness

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2021-01-27T16:54:06Z

Pinging @elastic/security-solution (Team: SecuritySolution)

elasticmachine · 2021-01-28T14:34:30Z

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

marshallmain · 2021-01-28T22:11:15Z

@elasticmachine merge upstream

marshallmain · 2021-01-29T18:02:57Z

@elasticmachine merge upstream

marshallmain · 2021-02-01T18:33:01Z

@elasticmachine merge upstream

marshallmain · 2021-02-02T19:43:31Z

jenkins test this

marshallmain · 2021-02-02T22:59:01Z

@elasticmachine merge upstream

yctercero · 2021-02-03T20:12:39Z

x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.ts

-        if (eventItem == null) {
-          return true;
-        } else if (tuple.operator === 'included') {
+        const eventItem = item.fields ? item.fields[tuple.field] : undefined;


Super nit: wonder if we're gonna be doing this a lot if it's worth just creating a tiny util to extract x field from fields.

Yeah if it becomes a common pattern I'd support pulling it out into a function

yctercero · 2021-02-03T20:48:54Z

x-pack/plugins/security_solution/server/lib/detection_engine/signals/filters/filter_events.ts

          // only create a signal if the event is not in the value list
          return !tuple.matchedSet.has(JSON.stringify(eventItem));
        } else if (tuple.operator === 'excluded') {
+          if (eventItem == null) {
+            return false;
+          }
          // only create a signal if the event is in the value list
          return tuple.matchedSet.has(JSON.stringify(eventItem));
        } else {


Since there's only two operators, not sure this else path ever hits.

It shouldn't be hit, but since the operator is a string it's possible for it to get into an invalid state so it's good to handle that possibility. Looking at it again we'd probably want to return true in that case so an invalid exception operator doesn't allowlist everything, but I think that's an issue for a separate PR.

👍 Wonder if we'd want to log there too. Worry about just letting through invalid states.

yctercero

LGTM - really interesting reading up on runtime fields! I pulled down and tested creating various rule types. Looks great, thanks!

marshallmain · 2021-02-04T03:47:39Z

@elasticmachine merge upstream

kibanamachine · 2021-02-04T05:33:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9479bfa

Metrics [docs]

✅ unchanged

History

💚 Build #103234 succeeded 9b9d1b6
💔 Build #103144 failed 6420cff
💚 Build #102702 succeeded 6420cff
💚 Build #102261 succeeded 47297b6
💔 Build #101977 failed 286e8d5

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

…source (#89371) (#90287) * First pass at switching rules to depend on fields instead of _source * Fix tests * Change operator: excluded logic so missing fields are allowlisted Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: (244 commits) [maps] Top hits per entity--change to title to use recent, minor edits (elastic#89254) [DOCS] Update installation details (elastic#90354) RFC for automatically generated typescript API documentation for every plugins public services, types, and functionality (elastic#86704) Elastic Maps Server config is `host` not `hostname` (elastic#90234) Use doc link services in index pattern management (elastic#89937) [Fleet] Managed Agent Policy (elastic#88688) [Workplace Search] Fix Source Settings bug (elastic#90242) [Enterprise Search] Refactor MockRouter test helper to not store payload (elastic#90206) Use doc link service in more Stack Monitoring pages (elastic#89050) [App Search] Relevance Tuning logic - actions and selectors only, no listeners (elastic#89313) Remove UI filters from UI (elastic#89793) Use newfeed.service config for all newsfeeds (elastic#90252) skip flaky suite (elastic#85086) Add readme to geo containment alert covering test alert setup (elastic#89625) [APM] Enabling yesterday option when 24 hours is selected (elastic#90017) Test user for maps tests under import geoJSON tests (elastic#86015) [Lens] Hide column in table (elastic#88680) [Security Solution][Detections] Reduce detection engine reliance on _source (elastic#89371) [Discover] Minor cleanup (elastic#90260) [Search Session][Management] Rename "cancel" button and delete "Reload" button (elastic#90015) ...

marshallmain added 4 commits January 26, 2021 16:37

First pass at switching rules to depend on fields instead of _source

1a0a6ed

Merge branch 'master' into drop-source

084ba5e

Fix tests

2ff44e1

Change operator: excluded logic so missing fields are allowlisted

8d8b62a

marshallmain added Feature:Detection Rules Anything related to Security Solution's Detection Rules Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.12.0 v8.0.0 labels Jan 27, 2021

marshallmain marked this pull request as ready for review January 27, 2021 16:54

marshallmain requested review from a team as code owners January 27, 2021 16:54

marshallmain added release_note:enhancement Team:Detections and Resp Security Detection Response Team labels Jan 27, 2021

Merge branch 'master' into drop-source

286e8d5

Merge branch 'master' into drop-source

47297b6

Merge branch 'master' into drop-source

6420cff

Merge branch 'master' into drop-source

9b9d1b6

spong requested a review from a team February 2, 2021 23:08

yctercero reviewed Feb 3, 2021

View reviewed changes

yctercero approved these changes Feb 3, 2021

View reviewed changes

Merge branch 'master' into drop-source

9479bfa

marshallmain merged commit e013389 into elastic:master Feb 4, 2021

marshallmain deleted the drop-source branch February 4, 2021 15:20

marshallmain mentioned this pull request Feb 4, 2021

[7.x] [Security Solution][Detections] Reduce detection engine reliance on _source (#89371) #90287

Merged

marshallmain mentioned this pull request Feb 24, 2021

[Security Solution] Re-validate rule exceptions when updating pre-packaged rules #84385

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Detections] Reduce detection engine reliance on _source #89371

[Security Solution][Detections] Reduce detection engine reliance on _source #89371

marshallmain commented Jan 26, 2021 •

edited

Loading

elasticmachine commented Jan 27, 2021

elasticmachine commented Jan 28, 2021

marshallmain commented Jan 28, 2021

marshallmain commented Jan 29, 2021

marshallmain commented Feb 1, 2021

marshallmain commented Feb 2, 2021

marshallmain commented Feb 2, 2021

yctercero Feb 3, 2021

marshallmain Feb 4, 2021

yctercero Feb 3, 2021

marshallmain Feb 4, 2021

yctercero Feb 5, 2021

yctercero left a comment

marshallmain commented Feb 4, 2021

kibanamachine commented Feb 4, 2021

[Security Solution][Detections] Reduce detection engine reliance on _source #89371

[Security Solution][Detections] Reduce detection engine reliance on _source #89371

Conversation

marshallmain commented Jan 26, 2021 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jan 27, 2021

elasticmachine commented Jan 28, 2021

marshallmain commented Jan 28, 2021

marshallmain commented Jan 29, 2021

marshallmain commented Feb 1, 2021

marshallmain commented Feb 2, 2021

marshallmain commented Feb 2, 2021

yctercero Feb 3, 2021

Choose a reason for hiding this comment

marshallmain Feb 4, 2021

Choose a reason for hiding this comment

yctercero Feb 3, 2021

Choose a reason for hiding this comment

marshallmain Feb 4, 2021

Choose a reason for hiding this comment

yctercero Feb 5, 2021

Choose a reason for hiding this comment

yctercero left a comment

Choose a reason for hiding this comment

marshallmain commented Feb 4, 2021

kibanamachine commented Feb 4, 2021

💚 Build Succeeded

Metrics [docs]

History

marshallmain commented Jan 26, 2021 •

edited

Loading