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] Calculate and save ruleSource.isCustomized in API endpoint handlers #180145

Closed
6 tasks
Tracked by #174168
jpdjere opened this issue Apr 5, 2024 · 4 comments · Fixed by #186988
Closed
6 tasks
Tracked by #174168
Assignees
Labels
8.16 candidate Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Feature:Rule Management Security Solution Detection Rule Management area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0

Comments

@jpdjere
Copy link
Contributor

jpdjere commented Apr 5, 2024

Epics: https://github.com/elastic/security-team/issues/1974 (internal), #174168

Summary

Implement calculation of ruleSource.isCustomized in all necessary endpoints that write the ruleSource field to prebuilt rules, and where fields can be customized, diverging from the base version from the Rule Asset.

The calculation and saving of the field should be done in the following endpoints:

  • Update Rule - PUT /rules
  • Patch Rule - PATCH /rules
  • Bulk Update Rules - PUT /rules/_bulk_update
  • Bulk Patch Rules - PATCH /rules/_bulk_update
  • Import Rules - POST /rules/_import
  • Perform Rule Upgrade - POST /prebuilt_rules/upgrade/_perform (Internal)

Extracted to other tickets:

Background

Context from RFC:

#### Updating and upgrading rules
Updating rules can currently be performed via five endpoints:
- **Update Rule** - `PUT /rules`
- **Patch Rule** - `PATCH /rules`
- **Bulk Update Rules** - `PUT /rules/_bulk_update`
- **Bulk Patch Rules** - `PATCH /rules/_bulk_update`
- **Bulk Actions** - `POST /rules/_bulk_action`: with **bulk edit** action
Upgrading prebuilt rules to their newer version is done by two endpoints:
- **(LEGACY) Install Prebuilt Rules And Timelines** - `PUT /rules/prepackaged`
- **Perform Rule Upgrade** - `POST /prebuilt_rules/upgrade/_perform` (Internal)
The legacy endpoint does not allow for customization of fields during the upgrade, but the new rule upgrade customization endpoint does.
Additionally:
- **Bulk Actions** - `POST /rules/_bulk_action`: with **duplicate** action
will perform migration but does not allow for customization during the duplication process.
So we can analyze the expected outputs of the migration of all these 8 endpoints together.
The resulting values for `immutable` and `rule_source` when calling these endpoints, and the migration being performed in the background, should be as follows:

Secondly, in order to migrate the `is_customized` value for rule edits, we can follow two approaches:
1. Calculate it in the `paramsModifier` callback that is passed to the `rulesClient.bulkEdit` method. This will need to modify the parameters of the callback to take as parameter the whole rule and the operations on the attributes, in order to have access to the values of the rule's field before and after of the edit.
2. Calculate it as part of the `validateMutatedRuleTypeParams` method in `x-pack/plugins/alerting/server/lib/validate_mutated_rule_type_params.ts` where we have access to the original params and the modified params.

@jpdjere jpdjere added triage_needed Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Rule Management Security Solution Detection Rule Management area Team:Detection Rule Management Security Detection Rule Management Team labels Apr 5, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@banderror banderror changed the title [Security Solution] Implement migration on write for rule update API and calculation of is_customized [Security Solution] Implement migration on write for rule update API and calculation of is_customized (DRAFT) Apr 17, 2024
@banderror banderror added Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area 8.15 candidate and removed triage_needed labels May 12, 2024
@jpdjere jpdjere changed the title [Security Solution] Implement migration on write for rule update API and calculation of is_customized (DRAFT) [Security Solution] Calculate ruleSource.isCustomized field for rule update API (DRAFT) May 23, 2024
@jpdjere jpdjere changed the title [Security Solution] Calculate ruleSource.isCustomized field for rule update API (DRAFT) [Security Solution] Calculate ruleSource.isCustomized field for rule update and bulk rule update API (DRAFT) May 23, 2024
@jpdjere jpdjere changed the title [Security Solution] Calculate ruleSource.isCustomized field for rule update and bulk rule update API (DRAFT) [Security Solution] Calculate ruleSource.isCustomized field for rule update and bulk rule update APIs (DRAFT) May 23, 2024
@jpdjere jpdjere changed the title [Security Solution] Calculate ruleSource.isCustomized field for rule update and bulk rule update APIs (DRAFT) [Security Solution] Calculate ruleSource.isCustomized in API endpoint handlers (DRAFT) May 23, 2024
@jpdjere jpdjere changed the title [Security Solution] Calculate ruleSource.isCustomized in API endpoint handlers (DRAFT) [Security Solution] Calculate and save ruleSource.isCustomized in API endpoint handlers (DRAFT) May 23, 2024
@jpdjere jpdjere changed the title [Security Solution] Calculate and save ruleSource.isCustomized in API endpoint handlers (DRAFT) [Security Solution] Calculate and save ruleSource.isCustomized in API endpoint handlers May 24, 2024
@banderror
Copy link
Contributor

@jpdjere @xcrzx Extracted bulk editing into #187706 and updated the description.

@xcrzx xcrzx closed this as completed in 045aafc Jul 9, 2024
xcrzx added a commit that referenced this issue Aug 29, 2024
…190041)

**Resolves: #187706

## Summary

Added the `isCustomized` field recalculation after a bulk edit operation
on rules as part of the [rules customization
epic](elastic/security-team#1974).

**Background**
The `isCustomized` field is a rule parameter indicating if a prebuilt
Elastic rule has been modified by a user. This field is extensively used
in the prebuilt rule upgrade workflow. It's essential to ensure any rule
modification operation recalculates this field to keep its value in sync
with the rule content. Most of the rule CRUD operations were already
covered in a previous PR: [Calculate and save ruleSource.isCustomized in
API endpoint handlers](#180145).
This PR addresses the remaining bulk rule modification operations
performed using the `rulesClient.bulkEdit` method.

**`rulesClient.bulkEdit` changes**

The `isCustomized` calculation is based on the entire rule object (i.e.,
rule params and attributes) and should be performed after all bulk
operations have been applied to the rule - after `operations` and
`paramsModifier`. To support this, I changed the `paramsModifier` to
accept entire rule object:

```diff
export type ParamsModifier<Params extends RuleParams> = (
-  params: Params
+  rule: Rule<Params>
) => Promise<ParamsModifierResult<Params>>;
```

**Security Solution Bulk Endpoint changes**

The `/api/detection_engine/rules/_bulk_action` endpoint now handles bulk
edit actions a bit differently. Previously, most of the bulk action was
delegated to the rules client. Now, we need to do some preparatory work:

1. Fetch the affected rules in memory first, regardless of whether we
received a query or rule IDs as input (previously delegated to
Alerting).
2. Identify all prebuilt rules among the fetched rules.
3. Fetch base versions of the prebuilt rules.
4. Provide base versions to `ruleModifier` for the `isCustomized`
calculation.

These changes add a few extra roundtrips to Elasticsearch and make the
bulk edit endpoint less efficient than before. However, this seems
justified given the added functionality of the customization epic. In
the future, we might consider optimizing to reduce the number of
database requests. Ideally, for Security Solution use cases, we would
need a more generic method than `bulkEdit`, such as `bulkUpdate`,
allowing us to implement any required rule update logic fully on the
solution side.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.16 candidate Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Feature:Rule Management Security Solution Detection Rule Management area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants