-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Discuss rule fields in the context of the prebuilt rule upgrade workflow #147239
Comments
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
@jethr0null @peluja1012 Can we please have your input on this?
Feel free to ping us if you have any questions and would like to chat over zoom. |
@approksiu As we agreed over Zoom, please review the ticket's description and especially the "Rule fields to discuss" section. There's a list of rule fields we will need to make some decisions for. There's also a related discussion "ticket" in the form of PR #148913 that gives examples of typical diff situations for a few typical rule fields. We will need to revisit this PR at some point after we start working on Milestone 3. cc @jpdjere |
Some corrections to the tables above:
|
@vitaliidm could you add the fields for es|ql rule type in the comment please? |
This one is tricky.
Arguments for not supporting rule type change on upgrade:
That being said, I think we should not allow rule type updates at the moment. We can revisit this in the future if there are substantial arguments against this decision. cc @brokensound77 |
|
Agree, same as |
We don't allow changes, this field will always be the same. I don't think it is needed. |
We could consider adding UI for this at a later stage. Can we treat no-UI fields with the json diff? |
This could be useful for MSSP use-case. Will investigate further. |
There are no new fields introduced to schema specific to ES|QL rule type, only new value to
And some of the fields are not applicable to ES|QL rule:
Before #167999, sending these fields in request would fail it. But not anymore, they will be dropped after migration to OpenAPI |
We had a discussion on zoom, and decided to support rule type changes on updates. Will look into UI for this. |
ML rules
There is also quite a bit of unit testing and rule sync validation that goes on within the rules repo. To an extent, we should be letting users know that by modifying a prebuilt rule, they take on the risk/responsibility of unsyncing, but also, there may be some opportunity to do some of that validation at the rule and rule set level as well |
Updates:
Open questions:
Fields to add customization for: (WIP)
|
Answers for questions in comment above from Product Meeting of 14/12/2023:
Prevent their update for prebuilt rules. The fields would remain part of the rule schema, but we would add manual checks in the endpoints that reject the update with 400 if the request tries to update those fields.
This field might be subject to change, so handle
Don't create UI to edit them. In the endpoint, always return the user's value. No diff should ever show up for these fields.
|
@jpdjere @approksiu I think we can consider this discussion finished, with decisions made and tickets created. Thank you very much for the work you've done for making it clear. Besides the tickets for missing editing UI for certain rule fields, @jpdjere is working on a ticket #180393 that describes what other changes for rule fields should be introduced to the rule upgrade workflow, rule schemas, and in general. Closing this one as done ✅ |
Epics: https://github.com/elastic/security-team/issues/1974 (internal), #174168
Forked from: #144060
Summary
We need to figure out what rule fields we’re going to show in the rule upgrade flyout.
You can see a detailed breakdown of all the existing rule fields below. They are categorized by multiple dimensions which will hopefully help us with reasoning about each field and making decisions.
Please scroll down to find fields to pay attention to and discuss.
Rule fields
Kind:
Change:
Level:
params
objectPrebuilt?
User editable?
Customizable?
Fields common to all rule types
id
rule_id
created_at
created_by
updated_at
updated_by
immutable
version
revision
enabled
execution_summary
meta
outcome
alias_target_id
alias_purpose
type
name
tags
description
severity
severity_mapping
risk_score
risk_score_mapping
references
- reference URLsfalse_positives
- false positive examplesthreat
- MITRE ATT&CK classificationnote
- investigation guidesetup
- setup guiderelated_integrations
required_fields
author
license
building_block_type
rule_name_override
timestamp_override
timestamp_override_fallback_disabled
timeline_id
timeline_title
interval
from
to
actions
throttle
exceptions_list
max_signals
- cirquit breakeroutput_index
- deprecated and unused?namespace
- unused?Fields specific to Custom Query and Saved Query rules
index
data_view_id
query
language
filters
saved_id
response_actions
alert_suppression
Fields specific to EQL rules
index
data_view_id
query
language
filters
event_category_override
timestamp_field
tiebreaker_field
Fields specific to Indicator Match rules
index
data_view_id
query
language
filters
saved_id
threat_query
threat_mapping
threat_index
threat_filters
threat_indicator_path
threat_language
concurrent_searches
items_per_search
alert_suppression
Fields specific to Threshold rules
index
data_view_id
query
language
filters
saved_id
threshold
Fields specific to Machine Learning rules
machine_learning_job_id
anomaly_threshold
Fields specific to New Terms rules
index
data_view_id
query
language
filters
new_terms_fields
history_window_start
Fields specific to ES|QL Rules
query
language
Rule fields to discuss
Fields common to all rule types:
type
― Confirm that we're not going to support changing the rule type on upgrade.related_integrations
,required_fields
,setup
― These can be specified in https://github.com/elastic/detection-rules but we don't support editing them in the app. Since we don't have UI for editing them, I'm wondering if it's time to finally build it. When we build the UI we will be able to support these fields in the rule upgrade flyout.author
,license
― Are we going to allow the user to customize these? Could there be any legal restrictions on that?actions
,throttle
― We allow the user to specify actions for installed prebuilt rules, but no prebuilt rules actually have actions in https://github.com/elastic/detection-rules. Can we remove support for actions from the prebuilt rule schema and handle the upgrade for these 2 fields under the hood (always take the current user's version)?exceptions_list
― We allow the user to specify exceptions for installed prebuilt rules, but only one prebuilt rule actually have exceptions in https://github.com/elastic/detection-rules. It's the "Endpoint Security" rule. How should we handle potential conflicts between Elastic's and user's versions of this field? Can we use some simple heuristic and handle this under the hood instead of building some kind of UI?max_signals
― This is a circuit breaker value. A few prebuilt rules have it in https://github.com/elastic/detection-rules. Users can modify it via the API. We don't have a UI for it. How do we resolve conflicts in this field?output_index
― This is a name of a concrete target index where the rule is supposed to write alerts. I believe this field is deprecated and unused in the app. No prebuilt rules have it in https://github.com/elastic/detection-rules. Users can still modify it via the API because it's in the schema. I don't think we have a UI for it. What should we do with this field?namespace
― This is a namespace (a suffix) of the alerts-as-data index where the rule is supposed to write alerts. AFAIR this is a replacement foroutput_index
but it looks to be unused in the app. No prebuilt rules have it in https://github.com/elastic/detection-rules. Users can still modify it via the API because it's in the schema. I don't think we have a UI for it. What should we do with this field?Fields specific to Custom Query and Saved Query rules:
response_actions
― This is fully user-editable and supported by the prebuilt rule schema, but no prebuilt rules have it in https://github.com/elastic/detection-rules. Our suggestion would be to remove it from the schema and handle it under the hood of the upgrade workflow.Fields specific to EQL rules:
language
― Specifically for EQL rules, do we allow the user to change this? From the rule schema, it looks like this can accept only one value: "eql".eql
andesql
respectively. The field might change if a rule changes its type from an upstream update, but a user should not be able to manually force this.Fields specific to Indicator Match rules:
threat_indicator_path
― What is this field used for? Do we have a UI for it? A few prebuilt rules have it in https://github.com/elastic/detection-rules.concurrent_searches
― This one is user-editable via API but we have no UI for it. No prebuilt rules have it in https://github.com/elastic/detection-rules. How should we handle it in the upgrade workflow?items_per_search
― This one is user-editable via API but we have no UI for it. No prebuilt rules have it in https://github.com/elastic/detection-rules. How should we handle it in the upgrade workflow?The text was updated successfully, but these errors were encountered: