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

[Rule Tuning] Add Investigation Fields to Specific AWS Rules #4261

Merged
merged 7 commits into from
Nov 9, 2024
Merged

[Rule Tuning] Add Investigation Fields to Specific AWS Rules #4261

merged 7 commits into from
Nov 9, 2024

Conversation

terrancedejesus
Copy link
Contributor

@terrancedejesus terrancedejesus commented Nov 7, 2024 •
edited
Loading

Pull Request

Issue link(s):

Summary - What I changed

Adds investigation (highlighted) fields to specific AWS rules.

How To Test

Screenshot 2024-11-07 at 2 11 55 PM

Checklist

  • Added a label for the type of pr: bug, enhancement, schema, maintenance, Rule: New, Rule: Deprecation, Rule: Tuning, Hunt: New, or Hunt: Tuning so guidelines can be generated
  • Added the meta:rapid-merge label if planning to merge within 24 hours
  • Secret and sensitive material has been managed correctly
  • Automated testing was updated or added to match the most common scenarios
  • Documentation and comments were added for features that require explanation

Contributor checklist

@terrancedejesus terrancedejesus added Integration: AWS AWS related rules Domain: Cloud Rule: Tuning tweaking or tuning an existing rule labels Nov 7, 2024
@terrancedejesus terrancedejesus self-assigned this Nov 7, 2024
@protectionsmachine
Copy link
Collaborator

protectionsmachine commented Nov 7, 2024 •
edited
Loading

Enhancement - Guidelines

These guidelines serve as a reminder set of considerations when addressing adding a new schema feature to the code.

Documentation and Context

  • Describe the feature enhancement in detail (alternative solutions, description of the solution, etc.) if not already documented in an issue.
  • Include additional context or screenshots.
  • Ensure the enhancement includes necessary updates to the documentation and versioning.

Code Standards and Practices

  • Code follows established design patterns within the repo and avoids duplication.
  • Code changes do not introduce new warnings or errors.
  • Variables and functions are well-named and descriptive.
  • Any unnecessary / commented-out code is removed.
  • Ensure that the code is modular and reusable where applicable.
  • Check for proper exception handling and messaging.

Testing

  • New unit tests have been added to cover the enhancement.
  • Existing unit tests have been updated to reflect the changes.
  • Provide evidence of testing and validating the enhancement (e.g., test logs, screenshots).
  • Validate that any rules affected by the enhancement are correctly updated.
  • Ensure that performance is not negatively impacted by the changes.
  • Verify that any release artifacts are properly generated and tested.

Additional Schema Related Checks

  • Ensure that the enhancement does not break existing functionality. (e.g., run make test-cli)
  • Review the enhancement with a peer or team member for additional insights.
  • Verify that the enhancement works across all relevant environments (e.g., different OS versions).
  • Confirm that all dependencies are up-to-date and compatible with the changes.
  • Link to the relevant Kibana PR or issue provided
  • Exported detection rule(s) from Kibana to showcase the feature(s)
  • Converted the exported ndjson file(s) to toml in the detection-rules repo
  • Re-exported the toml rule(s) to ndjson and re-imported into Kibana
  • Updated necessary unit tests to accommodate the feature
  • Applied min_compat restrictions to limit the feature to a specified minimum stack version
  • Executed all unit tests locally with a test toml rule to confirm passing
  • Included Kibana PR implementer as an optional reviewer for insights on the feature
  • Implemented requisite downgrade functionality
  • Cross-referenced the feature with product documentation for consistency
  • Incorporated a comprehensive test rule in unit tests for full schema coverage
  • Conducted system testing, including fleet, import, and create APIs (e.g., run make test-remote-cli)
  • Confirm that the proper version label is applied to the PR patch, minor, major.

@terrancedejesus terrancedejesus added the documentation Improvements or additions to documentation label Nov 7, 2024
@terrancedejesus terrancedejesus marked this pull request as ready for review November 7, 2024 19:14
@botelastic botelastic bot added the bbr Building Block Rules label Nov 7, 2024
shashank-elastic
shashank-elastic reviewed Nov 8, 2024
View reviewed changes
Copy link
Contributor

@shashank-elastic shashank-elastic left a comment

Choose a reason for hiding this comment

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

Relook at Rule minstack

rules/cross-platform/execution_aws_ssm_sendcommand_with_command_parameters.toml Outdated Show resolved Hide resolved
imays11
imays11 reviewed Nov 8, 2024
View reviewed changes
Copy link
Contributor

@imays11 imays11 left a comment

Choose a reason for hiding this comment

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

Submitting about half of my review now, I will continue to review later today!

rules/integrations/aws/defense_evasion_cloudtrail_logging_deleted.toml Outdated Show resolved Hide resolved
rules/integrations/aws/defense_evasion_cloudtrail_logging_deleted.toml Outdated Show resolved Hide resolved
rules/integrations/aws/defense_evasion_cloudtrail_logging_deleted.toml Outdated Show resolved Hide resolved
rules/integrations/aws/discovery_ec2_multi_region_describe_instances.toml Outdated Show resolved Hide resolved
rules/integrations/aws/discovery_ec2_multiple_discovery_api_calls_via_cli.toml Show resolved Hide resolved
rules/integrations/aws/discovery_new_terms_sts_getcalleridentity.toml Outdated Show resolved Hide resolved
imays11
imays11 approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@imays11 imays11 left a comment

Choose a reason for hiding this comment

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

These look good! I think the order change would be ideal but the fields look good. Added source.address to a few

rules/integrations/aws/execution_ssm_command_document_created_by_rare_user.toml Outdated Show resolved Hide resolved
rules/integrations/aws/impact_aws_s3_bucket_enumeration_or_brute_force.toml Outdated
Comment on lines 101 to 106
field_names = [
"failed_requests",
"tls.client.server_name",
"source.address",
"cloud.account.id"
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have no way of showing the actor for this? Both for the rule and the higlighted fields? Or is this rule only interested in the details of the bucket itself?

Copy link
Contributor Author

@terrancedejesus terrancedejesus Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We can only show the fields available after the aggregation unfortunately. You could add source.address to timeline then add the event.action and find out, but in the alert document, it would not be there.

rules/integrations/aws/persistence_ec2_security_group_configuration_change_detection.toml Outdated Show resolved Hide resolved
rules/integrations/aws/persistence_iam_create_user_via_assumed_role_on_ec2_instance.toml Outdated Show resolved Hide resolved
rules/integrations/aws/persistence_iam_user_created_access_keys_for_another_user.toml Show resolved Hide resolved
rules/integrations/aws/persistence_iam_user_created_access_keys_for_another_user.toml Outdated Show resolved Hide resolved
...s/integrations/aws/privilege_escalation_iam_administratoraccess_policy_attached_to_user.toml Outdated Show resolved Hide resolved
...s/integrations/aws/privilege_escalation_iam_administratoraccess_policy_attached_to_user.toml Outdated Show resolved Hide resolved
rules/integrations/aws/privilege_escalation_iam_customer_managed_policy_attached_to_role.toml Outdated Show resolved Hide resolved
rules/integrations/aws/privilege_escalation_role_assumption_by_service.toml Outdated Show resolved Hide resolved
@terrancedejesus terrancedejesus merged commit ef453d8 into main Nov 9, 2024
12 checks passed
@terrancedejesus terrancedejesus deleted the rule-tuning-aws-add-investigation-fields branch November 9, 2024 04:11
protectionsmachine pushed a commit that referenced this pull request Nov 9, 2024
@terrancedejesus @github-actions
[Rule Tuning] Add Investigation Fields to Specific AWS Rules (#4261)
396e99a 
* adding investigation fields to specific aws rules

* updated patch

* removing min-stack requirements

* removed user.name redundancy

* adjusted order of investigation fields

* adding source address

Removed changes from:
- rules/integrations/aws/discovery_ec2_multiple_discovery_api_calls_via_cli.toml
- rules/integrations/aws/impact_aws_s3_bucket_enumeration_or_brute_force.toml
- rules/integrations/aws/persistence_iam_user_created_access_keys_for_another_user.toml
- rules/integrations/aws/privilege_escalation_iam_administratoraccess_policy_attached_to_user.toml
- rules/integrations/aws_bedrock/aws_bedrock_multiple_attempts_to_use_denied_models_by_user.toml
- rules/integrations/aws_bedrock/aws_bedrock_multiple_validation_exception_errors_by_single_user.toml

(selectively cherry picked from commit ef453d8)
protectionsmachine pushed a commit that referenced this pull request Nov 9, 2024
@terrancedejesus @github-actions
[Rule Tuning] Add Investigation Fields to Specific AWS Rules (#4261)
93ce87c 
* adding investigation fields to specific aws rules

* updated patch

* removing min-stack requirements

* removed user.name redundancy

* adjusted order of investigation fields

* adding source address

Removed changes from:
- rules/integrations/aws/discovery_ec2_multiple_discovery_api_calls_via_cli.toml
- rules/integrations/aws/impact_aws_s3_bucket_enumeration_or_brute_force.toml
- rules/integrations/aws/persistence_iam_user_created_access_keys_for_another_user.toml
- rules/integrations/aws/privilege_escalation_iam_administratoraccess_policy_attached_to_user.toml
- rules/integrations/aws_bedrock/aws_bedrock_multiple_attempts_to_use_denied_models_by_user.toml
- rules/integrations/aws_bedrock/aws_bedrock_multiple_validation_exception_errors_by_single_user.toml

(selectively cherry picked from commit ef453d8)
protectionsmachine pushed a commit that referenced this pull request Nov 9, 2024
@terrancedejesus @github-actions
[Rule Tuning] Add Investigation Fields to Specific AWS Rules (#4261)
8c804e9 
* adding investigation fields to specific aws rules

* updated patch

* removing min-stack requirements

* removed user.name redundancy

* adjusted order of investigation fields

* adding source address

(cherry picked from commit ef453d8)
protectionsmachine pushed a commit that referenced this pull request Nov 9, 2024
@terrancedejesus @github-actions
[Rule Tuning] Add Investigation Fields to Specific AWS Rules (#4261)
05fe7ae 
* adding investigation fields to specific aws rules

* updated patch

* removing min-stack requirements

* removed user.name redundancy

* adjusted order of investigation fields

* adding source address

(cherry picked from commit ef453d8)
protectionsmachine pushed a commit that referenced this pull request Nov 9, 2024
@terrancedejesus @github-actions
[Rule Tuning] Add Investigation Fields to Specific AWS Rules (#4261)
df4ad3b 
* adding investigation fields to specific aws rules

* updated patch

* removing min-stack requirements

* removed user.name redundancy

* adjusted order of investigation fields

* adding source address

(cherry picked from commit ef453d8)
protectionsmachine pushed a commit that referenced this pull request Nov 9, 2024
@terrancedejesus @github-actions
[Rule Tuning] Add Investigation Fields to Specific AWS Rules (#4261)
e6dd57d 
* adding investigation fields to specific aws rules

* updated patch

* removing min-stack requirements

* removed user.name redundancy

* adjusted order of investigation fields

* adding source address

(cherry picked from commit ef453d8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mikaayenson Mikaayenson Mikaayenson approved these changes

@imays11 imays11 imays11 approved these changes

@eric-forte-elastic eric-forte-elastic Awaiting requested review from eric-forte-elastic eric-forte-elastic is a code owner

@w0rk3r w0rk3r Awaiting requested review from w0rk3r

@DefSecSentinel DefSecSentinel Awaiting requested review from DefSecSentinel

@Samirbous Samirbous Awaiting requested review from Samirbous

@Aegrah Aegrah Awaiting requested review from Aegrah

@shashank-elastic shashank-elastic Awaiting requested review from shashank-elastic

Assignees

@terrancedejesus terrancedejesus

Labels
backport: auto bbr Building Block Rules documentation Improvements or additions to documentation Domain: Cloud Integration: AWS AWS related rules patch Rule: Tuning tweaking or tuning an existing rule schema
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@terrancedejesus @protectionsmachine @Mikaayenson @imays11 @shashank-elastic