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.

Sign up for GitHub

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

Jump to bottom

[RFC] Stage 0: Introducing new field in rule namespace #2330

Merged
merged 11 commits into from
Sep 10, 2024
+16 −28
Merged

[RFC] Stage 0: Introducing new field in rule namespace #2330

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8f880e8
Update 0000-rfc-template.md
smriti0321 Apr 5, 2024
0bee3ac
Update 0000-rfc-template.md
smriti0321 Apr 25, 2024
8ee34ce
Merge branch 'main' into rule-fields
ebeahan May 29, 2024
0dce6ad
Renaming the template file with recommended name
smriti0321 Jun 27, 2024
6a66ed1
Resolving conflicts
smriti0321 Jun 27, 2024
af356f6
Removing Tag Field
smriti0321 Jun 27, 2024
8fcb9b7
Resolving comments from @trisch-me
smriti0321 Jul 4, 2024
63e9f39
Moving file to rfcs/text folder as per @trisch-me comment. using next…
smriti0321 Jul 4, 2024
9f2c005
I saw number 44 was used in a recent RFC, using next number in series
smriti0321 Jul 4, 2024
660404c
Merge branch 'main' into rule-fields
trisch-me Jul 23, 2024
a8244ca
Merge branch 'main' into rule-fields
smriti0321 Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion rfcs/0000-rfc-template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Feel free to remove these comments as you go along.
<!--
Stage 0: Provide a high level summary of the premise of these changes. Briefly describe the nature, purpose, and impact of the changes. ~2-5 sentences.
-->
This RFC proposes addition of 2 new fields (rule.tags and rule.remediation) in rule fieldset to the Elastic Common Schema (ECS). The goal of these fields is to provide more context to the users in the rule fieldset, rule.tags will be used to track the set of tags applied to the rule, customers can use it to indicate metadata about the rule, and rule.remediation will be used to capture the remediation instructions associated with rules, it is generally provided by the benchmark or framework from which the rule is published.

<!--
Stage 1: If the changes include field additions or modifications, please create a folder titled as the RFC number under rfcs/text/. This will be where proposed schema changes as standalone YAML files or extended example mappings and larger source documents will go as the RFC is iterated upon.
Expand All @@ -23,6 +24,13 @@ Stage X: Provide a brief explanation of why the proposal is being marked as aban

## Fields

The `rule` fields being proposed are as follows:

Field | Type | Description /Usage
-- | -- | -- | -- | --
trisch-me marked this conversation as resolved.
Show resolved Hide resolved
rule.tags | array | Used to track the set of tags applied to a rule | Customers can use it to indicate: author, benchmark partial name, rule number, rule category etc. It will be useful when we extend the capability to add more rules
Copy link
Member

Choose a reason for hiding this comment

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

ECS already provides a top-level tags field: https://www.elastic.co/guide/en/ecs/current/ecs-base.html#field-tags.

Could the existing tags field be used for the same purpose without creating another tags field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be the right approach to reuse top-level tags to include rules tag.

rule.remediation | array | Used to capture remediation instructions that come from the benchmark / framework the rule is from
Copy link
Member

Choose a reason for hiding this comment

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

How would this value be used as an array? From the description, I'd expect a block of text with the instructions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is possibility to have multiple remediation options for same misconfiguration, hence an array makes more sense. For example, we provide remediation manual steps, cli commands for CSPM misconfigurations.

Copy link

@tinnytintin10 tinnytintin10 May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@kfirpeled @maxcold thoughts on this? As @smriti0321 mentioned, from a product perspective, if there are multiple remediation options, we'd love to capture them all and present them in our flyout. From a technical perspective we can group all of these into one field we render as markdown, or as an array of values, etc. Can you help decide what the best path is here from a data type perspective? Thx!

Copy link

Choose a reason for hiding this comment

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

Right now it's just a markdown text for our native integration, for Wiz it's also just a string. Unless we want to have a more complex structure for each remediation instruction, text should be enough as a type in my opinion.

Copy link

@kfirpeled kfirpeled Jul 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There is possibility to have multiple remediation options for same misconfiguration, hence an array makes more sense. For example, we provide remediation manual steps, cli commands for CSPM misconfigurations.

I'm not sure I followed, currently, we don't have any examples of multiple remediation instructions. Can you please provide one?

and another FYI, it introduces a breaking change. so we need to think it through from a technical perspective as well


<!--
Stage 1: Describe at a high level how this change affects fields. Include new or updated yml field definitions for all of the essential fields in this draft. While not exhaustive, the fields documented here should be comprehensive enough to deeply evaluate the technical considerations of this change. The goal here is to validate the technical details for all essential fields and to provide a basis for adding experimental field definitions to the schema. Use GitHub code blocks with yml syntax formatting, and add them to the corresponding RFC folder.
-->
Expand Down Expand Up @@ -79,7 +87,10 @@ Stage 3: Document resolutions for all existing concerns. Any new concerns should

The following are the people that consulted on the contents of this RFC.

* TBD | author
* @smriti0321 | author
* @tinnytintin10 | Product Manager Cloud Security
* @oren-zohar | Engineering Manager Cloud Security
* @orouz | Engineer

<!--
Who will be or has been consulted on the contents of this RFC? Identify authorship and sponsorship, and optionally identify the nature of involvement of others. Link to GitHub aliases where possible. This list will likely change or grow stage after stage.
Expand All @@ -97,6 +108,7 @@ e.g.:
## References

<!-- Insert any links appropriate to this RFC in this section. -->
EPIC with detailed discussion on addition of these fields - https://github.com/elastic/security-team/issues/7658
Copy link
Member

Choose a reason for hiding this comment

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

I recommend removing any references to Elastic-only discussions or references. For anyone outside Elastic, these links will just return a 404 error.

If you want to link the two discussions, add a link on the private to the public.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ACK


### RFC Pull Requests

Expand Down
Loading