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

[SIEM][Detection Engine] Adds of risk score, output index, rule copying, and more #51190

Merged
merged 11 commits into from
Nov 21, 2019

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Nov 20, 2019

Summary

  • risk_score now required on a POST to the rules
  • output_index now required on a POST to the rules
  • Enabled a mechanism to deploy using environment variables a way to turn signals on for testing
  • Removed SIGNALS_REINDEX algorithm now
  • Added an optional meta object for misc storage of UI information on a POST
  • Added status field for the signal document for the signals data grid viewer
  • Added default signals output index to ui settings of siem:defaultSignalsIndex
  • Removed revision from signals as we are not doing revisioning
  • Updated schema to utilize newer rules with slightly different structure
  • Updated the copying of rule meta data into signals to have latest fields
  • Added ability for saved searches to save state so if a saved search is deleted you can have a fallback
  • Updated README.md with new instructions on how to use the system

Screen shot of the advanced setting for the siem signals output index.
Screen Shot 2019-11-19 at 9 08 40 PM

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11

- [ ] 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

- [ ] This was checked for keyboard-only and screenreader accessibility

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@FrankHassanabad FrankHassanabad changed the title Risk score [SIEM][Detection Engine] Addition of risk score, output index, and rule copying Nov 20, 2019
@FrankHassanabad FrankHassanabad self-assigned this Nov 20, 2019
@FrankHassanabad FrankHassanabad changed the title [SIEM][Detection Engine] Addition of risk score, output index, and rule copying [SIEM][Detection Engine] Addition of risk score, output index, rule copying, and more Nov 20, 2019
@FrankHassanabad FrankHassanabad changed the title [SIEM][Detection Engine] Addition of risk score, output index, rule copying, and more [SIEM][Detection Engine] Adds of risk score, output index, rule copying, and more Nov 20, 2019
@FrankHassanabad FrankHassanabad marked this pull request as ready for review November 20, 2019 23:36
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

ruleId: 'rule-1',
description: 'Detecting root and admin users',
falsePositives: [],
immutable: false,
index: ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*'],
interval: '5m',
name: 'Detect Root/Admin Users',
Copy link
Contributor

@XavierM XavierM Nov 21, 2019

Choose a reason for hiding this comment

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

I thought that the name and interval were required.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those two are omitted because they are part of the main base object of alerting and are no longer alert parameters. This fixes some typing issues and bugs we had around our TypeScript types to keep things accurate.

@@ -121,12 +124,15 @@ export const getResult = (): SignalAlertType => ({
index: ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*'],
falsePositives: [],
from: 'now-6m',
filter: undefined,
filter: null,
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the difference between filter and filters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The difference is that filter does not require any kql at all. A user can just push a regular filter they happen to have not based on any query. It's a pure filter only.

However, per our conversations, before shipping I will more than likely remove it and only keep the query + filter to keep the API from having use cases no one is asking for.

@@ -37,50 +76,36 @@ source your .zhsrc/.bashrc or open a new terminal to ensure you get the new valu
Optional env var when set to true will utilize `reindex` api for reindexing
instead of the scroll and bulk index combination.

```
```sh
export USE_REINDEX_API=true
```
Copy link
Contributor

@dhurley14 dhurley14 Nov 21, 2019

Choose a reason for hiding this comment

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

I don't think this code exists anymore so probably don't need the env var in the doc.

@@ -30,10 +28,13 @@ export const signalsAlertType = ({ logger }: { logger: Logger }): SignalAlertTyp
immutable: schema.boolean({ defaultValue: false }),
index: schema.arrayOf(schema.string()),
language: schema.nullable(schema.string()),
outputIndex: schema.string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

the less env vars the better 👍 This is nice to have.

alertId
);
const bulkIndexResult = await searchAfterAndBulkIndex({
someResult: noReIndexResult,
Copy link
Contributor

Choose a reason for hiding this comment

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

I should rename this parameter to be more descriptive.. I'll add that in my open PR.

sortId,
params,
service,
logger
Copy link
Contributor

Choose a reason for hiding this comment

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

funny my linter didn't catch the missing dangling comma..

Copy link
Contributor

@dhurley14 dhurley14 left a comment

Choose a reason for hiding this comment

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

Ran and tested locally 👍 looks great! Thanks for these updates.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@FrankHassanabad FrankHassanabad merged commit acac80c into elastic:master Nov 21, 2019
@FrankHassanabad FrankHassanabad deleted the risk-score branch November 21, 2019 20:00
FrankHassanabad added a commit to FrankHassanabad/kibana that referenced this pull request Nov 21, 2019
…ng, and more (elastic#51190)

## Summary

- `risk_score` now required on a POST to the rules 
- `output_index` now required on a POST to the rules
- Enabled a mechanism to deploy using environment variables a way to turn signals on for testing
- Removed `SIGNALS_REINDEX` algorithm now
- Added an optional `meta` object for misc storage of UI information on a POST
- Added `status` field for the signal document for the signals data grid viewer
- Added default signals output index to ui settings of `siem:defaultSignalsIndex`
- Removed revision from signals as we are not doing revisioning
- Updated schema to utilize newer rules with slightly different structure
- Updated the copying of rule meta data into signals to have latest fields
- Added ability for saved searches to save state so if a saved search is deleted you can have a fallback
- Updated `README.md` with new instructions on how to use the system

Screen shot of the advanced setting for the siem signals output index.
<img width="677" alt="Screen Shot 2019-11-19 at 9 08 40 PM" src="https://user-images.githubusercontent.com/1151048/69287461-9b40fb00-0bb3-11ea-9761-9e0c6df69bb9.png">


### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~

~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

- [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
FrankHassanabad added a commit that referenced this pull request Nov 21, 2019
…ng, and more (#51190) (#51367)

## Summary

- `risk_score` now required on a POST to the rules 
- `output_index` now required on a POST to the rules
- Enabled a mechanism to deploy using environment variables a way to turn signals on for testing
- Removed `SIGNALS_REINDEX` algorithm now
- Added an optional `meta` object for misc storage of UI information on a POST
- Added `status` field for the signal document for the signals data grid viewer
- Added default signals output index to ui settings of `siem:defaultSignalsIndex`
- Removed revision from signals as we are not doing revisioning
- Updated schema to utilize newer rules with slightly different structure
- Updated the copying of rule meta data into signals to have latest fields
- Added ability for saved searches to save state so if a saved search is deleted you can have a fallback
- Updated `README.md` with new instructions on how to use the system

Screen shot of the advanced setting for the siem signals output index.
<img width="677" alt="Screen Shot 2019-11-19 at 9 08 40 PM" src="https://user-images.githubusercontent.com/1151048/69287461-9b40fb00-0bb3-11ea-9761-9e0c6df69bb9.png">


### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~

~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~

~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~

- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios

~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~

- [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants