Skip to content

[Security Solutions][Detection Engine] Adds missing mappings to the signals for the indicator rules#92928

Merged
FrankHassanabad merged 1 commit intoelastic:masterfrom
FrankHassanabad:add-indicator-rule-to-mappings
Mar 1, 2021
Merged

[Security Solutions][Detection Engine] Adds missing mappings to the signals for the indicator rules#92928
FrankHassanabad merged 1 commit intoelastic:masterfrom
FrankHassanabad:add-indicator-rule-to-mappings

Conversation

@FrankHassanabad
Copy link
Copy Markdown
Contributor

@FrankHassanabad FrankHassanabad commented Feb 25, 2021 •
edited
Loading

Summary

Indicator rules were missing the mappings and the copy code for when they were being created. This fixes that.

Manual testing instructions

Add an indicator rule:
Screen Shot 2021-02-25 at 3 33 05 PM

Then after it fires check that it shows up in the timeline and tables and also check that it is queryable:
Screen Shot 2021-02-25 at 3 31 48 PM
Screen Shot 2021-02-25 at 3 32 12 PM
Screen Shot 2021-02-25 at 3 39 20 PM

Checklist

@FrankHassanabad FrankHassanabad requested a review from a team as a code owner February 25, 2021 22:44
@FrankHassanabad FrankHassanabad self-assigned this Feb 25, 2021
@FrankHassanabad FrankHassanabad added v8.0.0 v7.13.0 v7.12.0 Feature:Detection Alerts Security Solution Detection Alerts Feature Feature:Indicator Match Rule Security Solution Indicator Match rule type release_note:fix bug Fixes for quality problems that affect the customer experience labels Feb 25, 2021
@FrankHassanabad FrankHassanabad added the auto-backport Deprecated - use backport:version if exact versions are needed label Feb 25, 2021
@FrankHassanabad FrankHassanabad enabled auto-merge (squash) February 25, 2021 22:46
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 26, 2021

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

cc @FrankHassanabad

@spong spong linked an issue Feb 26, 2021 that may be closed by this pull request
[Security Solution] Indicator Match/Threat Match does not copy over its rule settings when creating a signal #82529
Closed
marshallmain
marshallmain reviewed Feb 26, 2021
View reviewed changes
Comment thread x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_rule.ts
created_by: createdBy,
updated_by: updatedBy,
threat: ruleParams.threat ?? [],
threat_mapping: ruleParams.threatMapping ?? [],
Copy link
Copy Markdown
Contributor

@marshallmain marshallmain Feb 26, 2021

Choose a reason for hiding this comment

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

wouldn't we expect these to be left off of signal.rule when they are undefined for other rule types rather than defaulted to []?

Copy link
Copy Markdown
Contributor Author

@FrankHassanabad FrankHassanabad Mar 1, 2021

Choose a reason for hiding this comment

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

I followed the other examples, above this one. We can change this behavior with a follow up if we don't want empty arrays.

Copy link
Copy Markdown
Contributor Author

@FrankHassanabad FrankHassanabad Mar 1, 2021

Choose a reason for hiding this comment

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

Follow up:
#93063

rylnd
rylnd approved these changes Mar 1, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

I agree with @marshallmain that the defaulting in build_rule should probably be removed for consistency, but everything else looks great!

@FrankHassanabad FrankHassanabad merged commit 4b42574 into elastic:master Mar 1, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 1, 2021
@FrankHassanabad @kibanamachine
Adds mapping to the signals for the indicator rules that were missing (…
3744cc5 
…elastic#92928)

## Summary

Indicator rules were missing the mappings and the copy code for when they were being created. This fixes that.

**Manual testing instructions**

Add an indicator rule:
<img width="1075" alt="Screen Shot 2021-02-25 at 3 33 05 PM" src="https://user-images.githubusercontent.com/1151048/109229217-37c35700-7780-11eb-9988-573d53f2c076.png">

Then after it fires check that it shows up in the timeline and tables and also check that it is queryable:
<img width="473" alt="Screen Shot 2021-02-25 at 3 31 48 PM" src="https://user-images.githubusercontent.com/1151048/109229261-4a3d9080-7780-11eb-808b-06fb0e9e4099.png">
<img width="509" alt="Screen Shot 2021-02-25 at 3 32 12 PM" src="https://user-images.githubusercontent.com/1151048/109229269-4c075400-7780-11eb-96bd-2464a7ac555e.png">
<img width="1390" alt="Screen Shot 2021-02-25 at 3 39 20 PM" src="https://user-images.githubusercontent.com/1151048/109229290-545f8f00-7780-11eb-982c-4506552973df.png">

### Checklist
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
@kibanamachine kibanamachine mentioned this pull request Mar 1, 2021
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 1, 2021
@FrankHassanabad @kibanamachine
Adds mapping to the signals for the indicator rules that were missing (…
63584f8 
…elastic#92928)

## Summary

Indicator rules were missing the mappings and the copy code for when they were being created. This fixes that.

**Manual testing instructions**

Add an indicator rule:
<img width="1075" alt="Screen Shot 2021-02-25 at 3 33 05 PM" src="https://user-images.githubusercontent.com/1151048/109229217-37c35700-7780-11eb-9988-573d53f2c076.png">

Then after it fires check that it shows up in the timeline and tables and also check that it is queryable:
<img width="473" alt="Screen Shot 2021-02-25 at 3 31 48 PM" src="https://user-images.githubusercontent.com/1151048/109229261-4a3d9080-7780-11eb-808b-06fb0e9e4099.png">
<img width="509" alt="Screen Shot 2021-02-25 at 3 32 12 PM" src="https://user-images.githubusercontent.com/1151048/109229269-4c075400-7780-11eb-96bd-2464a7ac555e.png">
<img width="1390" alt="Screen Shot 2021-02-25 at 3 39 20 PM" src="https://user-images.githubusercontent.com/1151048/109229290-545f8f00-7780-11eb-982c-4506552973df.png">

### Checklist
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
@kibanamachine kibanamachine mentioned this pull request Mar 1, 2021
Merged
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Mar 1, 2021

💚 Backport successful

✅ 7.12 / #93038
✅ 7.x / #93039

Successful backport PRs will be merged automatically after passing CI.

@FrankHassanabad FrankHassanabad deleted the add-indicator-rule-to-mappings branch March 1, 2021 15:49
@FrankHassanabad FrankHassanabad mentioned this pull request Mar 1, 2021
Merged
1 task
kibanamachine added a commit that referenced this pull request Mar 1, 2021
@kibanamachine @FrankHassanabad
Adds mapping to the signals for the indicator rules that were missing (…
c5a9d9b 
…#92928) (#93039)

## Summary

Indicator rules were missing the mappings and the copy code for when they were being created. This fixes that.

**Manual testing instructions**

Add an indicator rule:
<img width="1075" alt="Screen Shot 2021-02-25 at 3 33 05 PM" src="https://user-images.githubusercontent.com/1151048/109229217-37c35700-7780-11eb-9988-573d53f2c076.png">

Then after it fires check that it shows up in the timeline and tables and also check that it is queryable:
<img width="473" alt="Screen Shot 2021-02-25 at 3 31 48 PM" src="https://user-images.githubusercontent.com/1151048/109229261-4a3d9080-7780-11eb-808b-06fb0e9e4099.png">
<img width="509" alt="Screen Shot 2021-02-25 at 3 32 12 PM" src="https://user-images.githubusercontent.com/1151048/109229269-4c075400-7780-11eb-96bd-2464a7ac555e.png">
<img width="1390" alt="Screen Shot 2021-02-25 at 3 39 20 PM" src="https://user-images.githubusercontent.com/1151048/109229290-545f8f00-7780-11eb-982c-4506552973df.png">

### Checklist
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
kibanamachine added a commit that referenced this pull request Mar 1, 2021
@kibanamachine @FrankHassanabad
Adds mapping to the signals for the indicator rules that were missing (…
2d85a2d 
…#92928) (#93038)

## Summary

Indicator rules were missing the mappings and the copy code for when they were being created. This fixes that.

**Manual testing instructions**

Add an indicator rule:
<img width="1075" alt="Screen Shot 2021-02-25 at 3 33 05 PM" src="https://user-images.githubusercontent.com/1151048/109229217-37c35700-7780-11eb-9988-573d53f2c076.png">

Then after it fires check that it shows up in the timeline and tables and also check that it is queryable:
<img width="473" alt="Screen Shot 2021-02-25 at 3 31 48 PM" src="https://user-images.githubusercontent.com/1151048/109229261-4a3d9080-7780-11eb-808b-06fb0e9e4099.png">
<img width="509" alt="Screen Shot 2021-02-25 at 3 32 12 PM" src="https://user-images.githubusercontent.com/1151048/109229269-4c075400-7780-11eb-96bd-2464a7ac555e.png">
<img width="1390" alt="Screen Shot 2021-02-25 at 3 39 20 PM" src="https://user-images.githubusercontent.com/1151048/109229290-545f8f00-7780-11eb-982c-4506552973df.png">

### Checklist
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
FrankHassanabad added a commit that referenced this pull request Mar 1, 2021
@FrankHassanabad
Changes out the default arrays and adds types (#93063)
1a77095 
## Summary

Follow up from: 
#92928

Removes the default arrays and adds typing to the rule schema in order to see which ones require default arrays vs. which ones can/should be defaulted as `undefined`. Updates unit tests.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 1, 2021
@FrankHassanabad @kibanamachine
Changes out the default arrays and adds types (elastic#93063)
e75e3db 
## Summary

Follow up from: 
elastic#92928

Removes the default arrays and adds typing to the rule schema in order to see which ones require default arrays vs. which ones can/should be defaulted as `undefined`. Updates unit tests.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 1, 2021
@FrankHassanabad @kibanamachine
Changes out the default arrays and adds types (elastic#93063)
91ed9bb 
## Summary

Follow up from: 
elastic#92928

Removes the default arrays and adds typing to the rule schema in order to see which ones require default arrays vs. which ones can/should be defaulted as `undefined`. Updates unit tests.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
kibanamachine added a commit that referenced this pull request Mar 1, 2021
@kibanamachine @FrankHassanabad
Changes out the default arrays and adds types (#93063) (#93090)
8235656 
## Summary

Follow up from: 
#92928

Removes the default arrays and adds typing to the rule schema in order to see which ones require default arrays vs. which ones can/should be defaulted as `undefined`. Updates unit tests.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
kibanamachine added a commit that referenced this pull request Mar 1, 2021
@kibanamachine @FrankHassanabad
Changes out the default arrays and adds types (#93063) (#93089)
74a834b 
## Summary

Follow up from: 
#92928

Removes the default arrays and adds typing to the rule schema in order to see which ones require default arrays vs. which ones can/should be defaulted as `undefined`. Updates unit tests.

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
jloleysens added a commit that referenced this pull request Mar 3, 2021
@jloleysens
Merge branch 'ilm/rollup-v2-action' of github.com:elastic/kibana into…
cae0462 
… ilm/rollup-v2-action

* 'ilm/rollup-v2-action' of github.com:elastic/kibana:
  [Security Solution][Case][Bug] Only add rule object for alert comments (#92977)
  [Security Solution][Case] Show the current connector name in case view (#93018)
  [Security Solution] Remove unused mock data (#92357)
  Adds mapping to the signals for the indicator rules that were missing (#92928)
  skip flaky suite (#85208)
  Cleanup spaces plugin (#91976)
  Control round and decimal places in Gauge Visualization when using aggregate functions like average (#91293)
  Added alerting ui mock for jest test (#92604)
  Remove "beta" label from URL Drilldown as it is now GA (#92859)
@timroes timroes added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Mar 16, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 16, 2021

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

rylnd added a commit to rylnd/kibana that referenced this pull request Apr 20, 2021
@rylnd
Update signals template version as per guidelines.
99e7bbf 
The last released mappings update was elastic#92928, which bumped from 24 ->
25. The few unreleased updates since then have increased this by 1, but
since these changes are going out with 7.13 we are bumping by 10 _since
the last release_, in order to give "room" for minor releases.
rylnd added a commit that referenced this pull request Apr 20, 2021
@rylnd @ecezalp
[Security Solution][Detections]Update detection alert mappings to ECS…
538a6c0 
… 1.9 (#97573)

* adds snapshot test for getSignalsTemplate

* [CTI] Extracts non-ecs, non-signal mappings to separate file

* adds updated ECS mappings

* Normalize/clean up various mappings files

* Adds a wrapping "mappings.properties" around our extra mappings
* Spreads our other mappings similarly to ECS mappings
* Moves dynamic: false out of ECS mappings and into our main template
* Ensures we include 'threat.properties.indicator', since that's where
  our 'type: nested' declaration resides

* Update ECS mappings snapshot post-1.9 updates

This updated snapshot reflects the mappings changes that one will receive when
migrating/rolling over to a 7.13 alerts index.

* Update signals template version as per guidelines.

The last released mappings update was #92928, which bumped from 24 ->
25. The few unreleased updates since then have increased this by 1, but
since these changes are going out with 7.13 we are bumping by 10 _since
the last release_, in order to give "room" for minor releases.

* Fix cypress test failure due to updated mappings

This magic number represents "the number of mapped fields that begin
with 'host.geo.c' and, because this PR adds a mapping for
host.geo.continent_code, the test needed to be updated.

Co-authored-by: Ece Ozalp <ozale272@newschool.edu>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 20, 2021
@rylnd @ecezalp @kibanamachine
[Security Solution][Detections]Update detection alert mappings to ECS…
22b8294 
… 1.9 (elastic#97573)

* adds snapshot test for getSignalsTemplate

* [CTI] Extracts non-ecs, non-signal mappings to separate file

* adds updated ECS mappings

* Normalize/clean up various mappings files

* Adds a wrapping "mappings.properties" around our extra mappings
* Spreads our other mappings similarly to ECS mappings
* Moves dynamic: false out of ECS mappings and into our main template
* Ensures we include 'threat.properties.indicator', since that's where
  our 'type: nested' declaration resides

* Update ECS mappings snapshot post-1.9 updates

This updated snapshot reflects the mappings changes that one will receive when
migrating/rolling over to a 7.13 alerts index.

* Update signals template version as per guidelines.

The last released mappings update was elastic#92928, which bumped from 24 ->
25. The few unreleased updates since then have increased this by 1, but
since these changes are going out with 7.13 we are bumping by 10 _since
the last release_, in order to give "room" for minor releases.

* Fix cypress test failure due to updated mappings

This magic number represents "the number of mapped fields that begin
with 'host.geo.c' and, because this PR adds a mapping for
host.geo.continent_code, the test needed to be updated.

Co-authored-by: Ece Ozalp <ozale272@newschool.edu>
kibanamachine added a commit that referenced this pull request Apr 20, 2021
@kibanamachine @rylnd @ecezalp
[Security Solution][Detections]Update detection alert mappings to ECS…
f774556 
… 1.9 (#97573) (#97682)

* adds snapshot test for getSignalsTemplate

* [CTI] Extracts non-ecs, non-signal mappings to separate file

* adds updated ECS mappings

* Normalize/clean up various mappings files

* Adds a wrapping "mappings.properties" around our extra mappings
* Spreads our other mappings similarly to ECS mappings
* Moves dynamic: false out of ECS mappings and into our main template
* Ensures we include 'threat.properties.indicator', since that's where
  our 'type: nested' declaration resides

* Update ECS mappings snapshot post-1.9 updates

This updated snapshot reflects the mappings changes that one will receive when
migrating/rolling over to a 7.13 alerts index.

* Update signals template version as per guidelines.

The last released mappings update was #92928, which bumped from 24 ->
25. The few unreleased updates since then have increased this by 1, but
since these changes are going out with 7.13 we are bumping by 10 _since
the last release_, in order to give "room" for minor releases.

* Fix cypress test failure due to updated mappings

This magic number represents "the number of mapped fields that begin
with 'host.geo.c' and, because this PR adds a mapping for
host.geo.continent_code, the test needed to be updated.

Co-authored-by: Ece Ozalp <ozale272@newschool.edu>

Co-authored-by: Ryland Herrick <ryalnd@gmail.com>
Co-authored-by: Ece Ozalp <ozale272@newschool.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@marshallmain marshallmain marshallmain left review comments

@rylnd rylnd rylnd approved these changes

Assignees

@FrankHassanabad FrankHassanabad

Labels

auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience Feature:Detection Alerts Security Solution Detection Alerts Feature Feature:Indicator Match Rule Security Solution Indicator Match rule type release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.12.0 v7.13.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Security Solution] Indicator Match/Threat Match does not copy over its rule settings when creating a signal

6 participants

@FrankHassanabad @kibanamachine @elasticmachine @rylnd @marshallmain @timroes