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

[RAC][Security Solution] Add base Security Rule Type #105096

Merged
merged 100 commits into from
Aug 3, 2021

Conversation

madirey
Copy link
Contributor

@madirey madirey commented Jul 9, 2021

Summary

This PR adds a base security rule type, which wraps the persistence rule type to provide Detections-specific logic.

Summary of changes

  • Add base security rule type
    • Implement gap detection
    • Do privilege checks
    • Handle exception lists
    • Map event fields to new alerts-as-data schema (outlined here)
    • End-to-end query rule execution
    • Static typing for new rule executors

How to test this implementation

  1. Enable experimental feature flag: echo "xpack.securitySolution.enableExperimental: ['ruleRegistryEnabled']" >> ./config/kibana.dev.yml
  2. Enable rule registry writing: echo "xpack.ruleRegistry.write.enabled: true" >> ./config/kibana.dev.yml
  3. Create a rule by running ./x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/scripts/create_rule_query.sh

It creates a rule that generates up to 10 alerts every minute or so. The created rule is not visible in our UI and is not accessible through most of our API endpoints.

To be addressed in future PRs

  • Implementations for the remaining 4 rule types
  • Remove threshold rule's reliance on outputIndex by utilizing state

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@madirey madirey added Feature:Custom Query Rule Security Solution Custom Query Rule Feature:Detection Rules Anything related to Security Solution's Detection Rules Feature:Detection Alerts Security Solution Detection Alerts Feature Feature:Event Correlation (EQL) Rule Security Solution Event Correlation (EQL) Rule feature Feature:Indicator Match Rule Security Solution Indicator Match Rule feature Feature:RAC label obsolete Feature:Threshold Rule Security Solution Threshold Rule feature release_note:skip Skip the PR/issue when compiling release notes v7.15.0 labels Jul 9, 2021
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

Uptime changes look good to me. Only did a code review.

dateRangeStart: moment(new Date(fields['kibana.rac.alert.start']!))
.subtract('5', 'm')
.toISOString(),
dateRangeEnd: fields[ALERT_STATUS] === 'open' ? 'now' : fields[ALERT_END]!,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for doing this. I actually have a branch up getting ready to update this. Much appreciated.

Copy link
Contributor

@xcrzx xcrzx left a comment

Choose a reason for hiding this comment

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

Checked changes in regards to rule execution logging, LGTM 👍
It would be great to merge this PR sooner so that we can start integration with the new Exec log.

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.

In regards to the comment in your PR description around The created rule is not visible in our UI and is not accessible through most of our API endpoints. I think we just need a small update so we can still manage these rules through the security solution detections page by updating the routes to include the new QUERY_ALERT_TYPE_ID.

One example is in the find_rules route

export const getFilter = (filter: string | null | undefined) => {
if (filter == null) {
return `alert.attributes.alertTypeId: ${SIGNALS_ID}`;
} else {
return `alert.attributes.alertTypeId: ${SIGNALS_ID} AND ${filter}`;
}
};

where we can update this filter to include the new QUERY_ALERT_TYPE_ID

If you agree with the above, I think there are other places where this change should propagate as well, like in the import rules route.

Copy link
Contributor

@ecezalp ecezalp left a comment

Choose a reason for hiding this comment

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

Wow! The changes look fantastic. I think that the overall modularization efforts really came together, and everything became easier to understand! Thank you so much for doing this work!

One comment though - I flipped on the ruleRegistryFlag (by adding xpack.securitySolution.enableExperimental: ['ruleRegistryEnabled'] to my kibana.dev.yml, but I wasn't able to get any alerts on the Custom Query Rule. It seemed like a new alerting index didn't get created, I wasn't able to catch any suspicious errors / logs in my terminal. I was wondering if I am missing a step or if I should try again. Please let me know! just noticed the instructions above. will try again!

array: false,
required: true,
},
'kibana.alert.threat': {
Copy link
Contributor

Choose a reason for hiding this comment

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

making a personal note to add kibana.alert.reason 😄

@madirey
Copy link
Contributor Author

madirey commented Aug 2, 2021

@elasticmachine merge upstream

@madirey
Copy link
Contributor Author

madirey commented Aug 2, 2021

@elasticmachine merge upstream

Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

APM changes look good.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2338 2341 +3
uptime 563 566 +3
total +6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 4.3MB 4.3MB +13.7KB
observability 490.9KB 507.4KB +16.5KB
securitySolution 6.4MB 6.4MB +5.8KB
uptime 954.5KB 954.5KB +8.0B
total +36.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
apm 41.6KB 44.4KB +2.8KB
infra 146.4KB 149.2KB +2.8KB
uptime 29.3KB 34.7KB +5.4KB
total +10.9KB
Unknown metric groups

API count

id before after diff
ruleRegistry 82 89 +7

API count missing comments

id before after diff
ruleRegistry 82 89 +7

References to deprecated APIs

id before after diff
securitySolution 836 832 -4

History

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

@madirey madirey merged commit 8f9086b into elastic:master Aug 3, 2021
madirey added a commit to madirey/kibana that referenced this pull request Aug 5, 2021
* injects bulkCreate and wrapHits to individual rule executors

* WIP create_security_rule_type_factory based on Marshall's work in #d3076ca54526ea0e61a9a99e1c1bce854806977e

* removes ruleStatusService from old rule executors, fixes executor unit tests

* fixes rebase

* Rename reference_rules to rule_types

* Fix type errors

* Fix type errors in base security rule factory

* Additional improvements to types and interfaces

* More type alignment

* Fix remaining type errors in query rule

* Add validation / inject lists plugin

* Formatting

* Improvements to typing

* Static typing on executors

* cleanup

* Hook up params for query/threshold rules... includes exceptionsList and daterange tuple

* Scaffolding for wrapHits and bulkCreate

* Add error handling / status reporting

* Fixup alert type state

* Begin threshold

* Begin work on threshold state

* Organize rule types

* Export base security rule types

* Fixup lifecycle static typing

* WrapHits / bulk changes

* Field mappings (partial)

* whoops

* Remove redundant params

* More flexibile implementation of bulkCreateFactory

* Add mappings

* Finish query rule

* Revert "Remove redundant params"

This reverts commit 87aff9c.

* Revert "whoops"

This reverts commit a7771bd.

* Fixup return types

* Use alertWithPersistence

* Fix import

* End-to-end rule mostly working

* Fix bulkCreate

* Bug fixes

* Bug fixes and mapping changes

* Fix indexing

* cleanup

* Fix type errors

* Test fixes

* Fix query tests

* cleanup / rename kibana.rac to kibana

* Remove eql/threshold (for now)

* Move technical fields to package

* Add indexAlias and buildRuleMessageFactory

* imports

* type errors

* Change 'kibana.rac.*' to 'kibana.*'

* Fix lifecycle tests

* Single alert instance

* fix import

* Fix type error

* Fix more type errors

* Fix query rule type test

* revert to previous ts-expect-error

* type errors again

* types / linting

* General readability improvements

* Add invariant function from Dmitrii's branch

* Use invariant and constants

* Improvements to field mappings

* More test failure fixes

* Add refresh param for bulk create

* Update more field refs

* Actually use refresh param

* cleanup

* test fixes

* changes to rule creation script

* Fix created signals count

* Use ruleId

* Updates to bulk indexing

* Mapping updates

* Cannot use 'strict' for dynamic setting

Co-authored-by: Marshall Main <marshall.main@elastic.co>
Co-authored-by: Ece Ozalp <ozale272@newschool.edu>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
madirey added a commit that referenced this pull request Aug 5, 2021
* injects bulkCreate and wrapHits to individual rule executors

* WIP create_security_rule_type_factory based on Marshall's work in #d3076ca54526ea0e61a9a99e1c1bce854806977e

* removes ruleStatusService from old rule executors, fixes executor unit tests

* fixes rebase

* Rename reference_rules to rule_types

* Fix type errors

* Fix type errors in base security rule factory

* Additional improvements to types and interfaces

* More type alignment

* Fix remaining type errors in query rule

* Add validation / inject lists plugin

* Formatting

* Improvements to typing

* Static typing on executors

* cleanup

* Hook up params for query/threshold rules... includes exceptionsList and daterange tuple

* Scaffolding for wrapHits and bulkCreate

* Add error handling / status reporting

* Fixup alert type state

* Begin threshold

* Begin work on threshold state

* Organize rule types

* Export base security rule types

* Fixup lifecycle static typing

* WrapHits / bulk changes

* Field mappings (partial)

* whoops

* Remove redundant params

* More flexibile implementation of bulkCreateFactory

* Add mappings

* Finish query rule

* Revert "Remove redundant params"

This reverts commit 87aff9c.

* Revert "whoops"

This reverts commit a7771bd.

* Fixup return types

* Use alertWithPersistence

* Fix import

* End-to-end rule mostly working

* Fix bulkCreate

* Bug fixes

* Bug fixes and mapping changes

* Fix indexing

* cleanup

* Fix type errors

* Test fixes

* Fix query tests

* cleanup / rename kibana.rac to kibana

* Remove eql/threshold (for now)

* Move technical fields to package

* Add indexAlias and buildRuleMessageFactory

* imports

* type errors

* Change 'kibana.rac.*' to 'kibana.*'

* Fix lifecycle tests

* Single alert instance

* fix import

* Fix type error

* Fix more type errors

* Fix query rule type test

* revert to previous ts-expect-error

* type errors again

* types / linting

* General readability improvements

* Add invariant function from Dmitrii's branch

* Use invariant and constants

* Improvements to field mappings

* More test failure fixes

* Add refresh param for bulk create

* Update more field refs

* Actually use refresh param

* cleanup

* test fixes

* changes to rule creation script

* Fix created signals count

* Use ruleId

* Updates to bulk indexing

* Mapping updates

* Cannot use 'strict' for dynamic setting

Co-authored-by: Marshall Main <marshall.main@elastic.co>
Co-authored-by: Ece Ozalp <ozale272@newschool.edu>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Marshall Main <marshall.main@elastic.co>
Co-authored-by: Ece Ozalp <ozale272@newschool.edu>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
streamich pushed a commit to vadimkibana/kibana that referenced this pull request Aug 8, 2021
* injects bulkCreate and wrapHits to individual rule executors

* WIP create_security_rule_type_factory based on Marshall's work in #d3076ca54526ea0e61a9a99e1c1bce854806977e

* removes ruleStatusService from old rule executors, fixes executor unit tests

* fixes rebase

* Rename reference_rules to rule_types

* Fix type errors

* Fix type errors in base security rule factory

* Additional improvements to types and interfaces

* More type alignment

* Fix remaining type errors in query rule

* Add validation / inject lists plugin

* Formatting

* Improvements to typing

* Static typing on executors

* cleanup

* Hook up params for query/threshold rules... includes exceptionsList and daterange tuple

* Scaffolding for wrapHits and bulkCreate

* Add error handling / status reporting

* Fixup alert type state

* Begin threshold

* Begin work on threshold state

* Organize rule types

* Export base security rule types

* Fixup lifecycle static typing

* WrapHits / bulk changes

* Field mappings (partial)

* whoops

* Remove redundant params

* More flexibile implementation of bulkCreateFactory

* Add mappings

* Finish query rule

* Revert "Remove redundant params"

This reverts commit 87aff9c.

* Revert "whoops"

This reverts commit a7771bd.

* Fixup return types

* Use alertWithPersistence

* Fix import

* End-to-end rule mostly working

* Fix bulkCreate

* Bug fixes

* Bug fixes and mapping changes

* Fix indexing

* cleanup

* Fix type errors

* Test fixes

* Fix query tests

* cleanup / rename kibana.rac to kibana

* Remove eql/threshold (for now)

* Move technical fields to package

* Add indexAlias and buildRuleMessageFactory

* imports

* type errors

* Change 'kibana.rac.*' to 'kibana.*'

* Fix lifecycle tests

* Single alert instance

* fix import

* Fix type error

* Fix more type errors

* Fix query rule type test

* revert to previous ts-expect-error

* type errors again

* types / linting

* General readability improvements

* Add invariant function from Dmitrii's branch

* Use invariant and constants

* Improvements to field mappings

* More test failure fixes

* Add refresh param for bulk create

* Update more field refs

* Actually use refresh param

* cleanup

* test fixes

* changes to rule creation script

* Fix created signals count

* Use ruleId

* Updates to bulk indexing

* Mapping updates

* Cannot use 'strict' for dynamic setting

Co-authored-by: Marshall Main <marshall.main@elastic.co>
Co-authored-by: Ece Ozalp <ozale272@newschool.edu>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Custom Query Rule Security Solution Custom Query Rule Feature:Detection Alerts Security Solution Detection Alerts Feature Feature:Detection Rules Anything related to Security Solution's Detection Rules Feature:Event Correlation (EQL) Rule Security Solution Event Correlation (EQL) Rule feature Feature:Indicator Match Rule Security Solution Indicator Match Rule feature Feature:RAC label obsolete Feature:Threshold Rule Security Solution Threshold Rule feature release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants