-
Notifications
You must be signed in to change notification settings - Fork 387
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
[amazon_security_lake] Added support for all the OCSF Classes #8579
[amazon_security_lake] Added support for all the OCSF Classes #8579
Conversation
🌐 Coverage report
|
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+52,228
😮
What can you tell us reviewers about these changes to help us? Is there any duplication between the fields.yml files such that we could review the common fields only once? Was any of the fields.yml content generated from OCSF? If so, how?
- source_dataset: amazon_security_lake.event | ||
rules: | ||
- target_dataset: amazon_security_lake.system_activity | ||
if: ctx.ocsf?.category_uid != null && ctx.ocsf.category_uid == '1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if: ctx.ocsf?.category_uid != null && ctx.ocsf.category_uid == '1' | |
if: ctx.ocsf?.category_uid == '1' |
The null check should not be necessary if you use the null-safe operator. Applies to the other conditions in this file too.
@@ -0,0 +1,20 @@ | |||
- name: data_stream.type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field (any many others here) is declared in ECS, and therefore should be using the external: ecs
definition for consistency.
My recommendation for fixing is to use
go run github.com/andrewkroh/fydler@main --fix packages/amazon_security_lake/**/fields/*.yml
then review the automated changes for correctness and commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewkroh The mentioned fields are automatically included in the base-fields.yml file during package creation, and we've opted to maintain consistency across all integrations by keeping them unchanged. Let me know your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are indeed defined manually for other packages too.
@andrewkroh We've structured the fields based on OCSF categories, assigning them to the relevant datastream's fields.yml files. For instance, fields linked to the System Activity category are gathered in the system_activity datastream's fields.yml file. Similarly, we have followed this approach for each data-stream. Notably, there's no predefined list or method for identifying duplicates within the fields.yml files. Reviewers can optimize their process by bypassing the review of an actor object in one data-stream if it has already been reviewed in another. |
@janvi-elastic Did you do this without any automation? Given that OSCF is itself a schema as JSON, it seems that generation of fields definitions for each specific data stream could largely be mechanized. The benefits would be a drastic reduction is reviewable material and a simple manner to keep these fields up to date when OCSF releases new versions. The reduction in reviewable material results from being able to review the "tool" used for the code generation and trusting its output. |
Yes we have not used automation for creating fields.yml. In first phase we have covered maximum objects and in this phase we have reused that objects. |
🚀 Benchmarks reportTo see the full report comment with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, approving to unblock, but please wait for @andrewkroh to give the final approval before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reduction in reviewable material results from being able to review the "tool" used for the code generation and trusting its output.
Yes we have not used automation for creating fields.yml. In first phase we have covered maximum objects and in this phase we have reused that objects.
It might be worth creating automation tool and review it if we are going to have more use cases with OCSF. For now, I have reviewed few datasets like Discovery
and Application Activity
and it looks good to me. Proceeding with approval to unblock 👍🏼
Package amazon_security_lake - 0.9.0 containing this change is available at https://epr.elastic.co/search?package=amazon_security_lake |
* upstream/main: (117 commits) [TI MISP] Add IOC expiration support (#8639) Add CSPM Rules 6.2, 6.3 and 6.4 (#8778) [Infoblox NIOS] Update timestamp parsing logic (#8767) [Rapid7 InsightVM] Split vulnerability categories into array (#8768) [Exchange Online Message Trace] Add Additional Look-back Time & Fix Cursor Value (#8717) [Buildkite] Update bucket settings (#8765) Remove Jenkins .ci folder (#8766) First part of removal of Jenkins jobs (#8763) misp: parse URIs for URI type threats (#8760) [amazon_security_lake] Added support for all the OCSF Classes (#8579) [Buildkite] Update settings for integrations pipeline (#8758) [TI ThreatQ] Add IOC expiration support (#8691) [ti_opencti] Support OpenCTI 5.12 by removing filters parameter (#8744) [Cribl] Updating setup guidance for Cribl field (#8746) crowdstrike: add userinfo enrichment support and map fields to ECS (#8742) [etcd] Enable TSDB for metrics datastream (#8649) Bump golang.org/x/crypto from 0.16.0 to 0.17.0 (#8749) auditd: relax field_split pattern and handle AVC header (#8748) Update cloud packages codeowner (#8672) [O11Y] [AWS Billing] Convert "Total Estimated Charges" visualization to new metric (#8509) ...
Type of change
What does this PR do?
Checklist
changelog.yml
file.How to test this PR locally
Automated Test