Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Detections] Add rule overrides for single event EQL rules #78876

Merged
merged 4 commits into from
Sep 30, 2020

Conversation

marshallmain
Copy link
Contributor

@marshallmain marshallmain commented Sep 29, 2020

Adds buildRuleWithOverrides function which is essentially equivalent to buildRule, but implemented as a composition of buildRuleWithoutOverrides (which is used for sequences, and might be useful for other "synthetic" signals where overrides from an event don't make sense) and applyRuleOverrides. These functions take the rule SavedObject or RuleTypeParams as a parameter and extract values from it rather than passing in all the necessary values individually.

I'd like to refactor soon and use buildRuleWithOverrides in place of buildRule so we can pass the SavedObject as a single parameter through the layers of functions instead of a longer list of parameters. It's not refactored here since there are multiple layers throughout different rule types with 15-20+ parameters being passed in so the diff will be relatively large. After the refactor we can remove buildRule so we don't have to maintain both implementations.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@marshallmain marshallmain added Team:SIEM v8.0.0 v7.10.0 Feature:Detection Rules Anything related to Security Solution's Detection Rules labels Sep 29, 2020
@marshallmain marshallmain requested review from a team as code owners September 29, 2020 21:35
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@marshallmain marshallmain added the release_note:skip Skip the PR/issue when compiling release notes label Sep 29, 2020
Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked out, tested locally, and performed code review -- LGTM! 👍

Tested Severity override, Risk score override, Rule name override, and Timestamp override and each functioned as expected for non-sequence EQL rules. Appreciate the cleanup in (and added!) tests, and abstraction with eventSource. And ++ to further refactoring with buildRuleWithOverrides in place of buildRule, thanks all the cleanup @marshallmain! 🙂

@spong
Copy link
Member

spong commented Sep 30, 2020

Just a note, in testing sequences, the overrides do function with the child sequence alerts (and as expected, not parents).

Curious if we want to just disable overrides when it's a sequence query? @MikePaquette / @paulewing, is there any use in the overrides functioning with sequence alerts?

Override config

Resulting Alerts (w/ building block sequence alerts)

(Note: `event.sequence` isn't showing in the table view (@XavierM has a fix :), but value for 2nd and 5th row is as the override is set)

@marshallmain
Copy link
Contributor Author

@elasticmachine merge upstream

@marshallmain
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@marshallmain marshallmain merged commit e41f436 into elastic:master Sep 30, 2020
@marshallmain marshallmain deleted the eql-rules-round-2 branch September 30, 2020 20:17
marshallmain added a commit to marshallmain/kibana that referenced this pull request Sep 30, 2020
…QL rules (elastic#78876)

* Add buildRuleWithOverrides function for single event EQL queries

* Disable rule overrides for all sequence signals

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
phillipb added a commit to phillipb/kibana that referenced this pull request Sep 30, 2020
…aly-detection-partition-field

* 'master' of github.com:elastic/kibana: (37 commits)
  Fixes for the Ticket 78375 (elastic#79004)
  [Security] Alert Telemetry for the Security app (elastic#77200)
  [Search bar] Remove duplicate `popoverProps` (elastic#79025)
  [Security Solution][Detections] Add rule overrides for single event EQL rules (elastic#78876)
  [SECURITY_SOLUTION][ENDPOINT] Improve Endpoint Host data generator to also integrate with Ingest (elastic#74305)
  remove file accidentally checked in (elastic#79005)
  [ML] DF Analytics creation wizard: replace select input with job type cards with icons (elastic#78872)
  [Design] A couple fixes for 7.10 (elastic#78801)
  Fix KQL autocomplete value suggestions (elastic#78676)
  [Security Solution][Resolver] New mock with cursor (elastic#78863)
  Embeddables: basic documentation (elastic#78900)
  [security solution] only import beat_schema when needed (elastic#78708)
  [Reporting] API Integration tests: fix flaky tests for Spaces CSV formatting (elastic#78849)
  [Actions] Adds a "Test Connector" button on the Connectors List to make discovery of the Test tab easier (elastic#78746)
  [Discover] Fix functional time picker test permissions (elastic#78564)
  [ML] Fixing module datafeed overrides (elastic#78925)
  Adds some missing licenses to the CSV export (elastic#78719)
  [dev/cli] ensure plugins/ and all watch source dirs exist (elastic#78973)
  [Lens] Stop using scripted metric to collect telemetry (elastic#78687)
  [Lens] fix wrong message in fields accordion (elastic#78924)
  ...
marshallmain added a commit that referenced this pull request Sep 30, 2020
…QL rules (#78876) (#79033)

* Add buildRuleWithOverrides function for single event EQL queries

* Disable rule overrides for all sequence signals

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@elasticmachine
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Detection Rules Anything related to Security Solution's Detection Rules release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants