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

sentinel_one_cloud_funnel: improve detection rules support #9120

Merged
merged 4 commits into from
Mar 5, 2024

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Feb 12, 2024

Proposed commit message

See title.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

How to test this PR locally

First

$ export AWS_DEFAULT_PROFILE=elastic-siem
$ aws-mfa --profile=elastic-siem
$ eval $(grep ^aws ~/.aws/credentials | gsed -r 's/^(aws[^ ]+) = (.*)$/export \U\1\E=\2/g')

(gsed is GNU sed so if on macos you will need to install that, if on linux s/gsed/sed/ in the command above)

then test with elastic-package as normal.

Related issues

Screenshots

@efd6 efd6 added enhancement New feature or request Integration:sentinel_one_cloud_funnel SentinelOne Cloud Funnel Team:Security-Service Integrations Security Service Integrations Team labels Feb 12, 2024
@efd6 efd6 self-assigned this Feb 12, 2024
@efd6 efd6 force-pushed the 9086-sentinelone_cloud_funnel branch 2 times, most recently from 72e0edf to 4d9ffde Compare February 12, 2024 20:23
@elasticmachine
Copy link

elasticmachine commented Feb 12, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@efd6 efd6 force-pushed the 9086-sentinelone_cloud_funnel branch 4 times, most recently from 5fe3280 to 1e0b467 Compare February 15, 2024 01:05
@efd6 efd6 marked this pull request as ready for review February 15, 2024 01:24
@efd6 efd6 requested a review from a team as a code owner February 15, 2024 01:24
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@efd6 efd6 requested a review from w0rk3r February 15, 2024 01:24
Copy link
Contributor

@w0rk3r w0rk3r left a comment

Choose a reason for hiding this comment

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

Awesome work 🧙🏼 🔥 🧙🏼 🔥 🧙🏼 🔥 🧙🏼 🔥

field: process.code_signature.trusted
value: false
override: false
if: ctx.process?.code_signature?.exists == true
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if I understood this one, this will set process.code_signature.trusted to false whenever the signature exists? Or only if the ctx.sentinel_one_cloud_funnel?.event?.src?.process?.verified_status?.contains('verified') == true returns false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the previous set sets the process.code_signature.trusted value to true if sentinel_one_cloud_funnel.event.src.process.verified_status contains "verified". This one sets it to false if a signature exists but process.code_signature.trusted is not already set (override: false). So if the previous set has not fired, and we have a signature then we have not verified it, so set to false.

copy_from: sentinel_one_cloud_funnel.event.registry.key.path
ignore_empty_value: true
description: Implements Windows-like SplitCommandLine
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change this description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that is copy/paste error.

Copy link
Contributor

@tomsonpl tomsonpl left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the good work! I left one question :)

@@ -6,7 +6,16 @@ processors:
value: [process]
- set:
field: event.type
value: [info]
value: [start]
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀

@@ -229,6 +240,10 @@ processors:
field: json.src.process.image.path
target_field: sentinel_one_cloud_funnel.event.src.process.image.path
ignore_missing: true
- set:
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering if this should be set from tgt instead of src in pipeline-process.yml, where I think we overwrite the 'hierarchy'

- set: 
  field: process.executable
  copy_from: sentinel_one_cloud_funnel.event.tgt.process.image.path
  ignore_empty_value: true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The choice of src here was purely because of the local context; I have no expertise in the semantics of sentinel one fields, so if we have some expertise in this, that would be helpful.

Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

Few minor suggestions. LGTM 👍🏼

@@ -2,6 +2,9 @@
"expected": [
{
"@timestamp": "2022-10-03T15:32:29.475Z",
"destination": {
"address": "www.asdf.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also capture into destination.domain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me

Copy link
Contributor

Choose a reason for hiding this comment

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

No opinion, sorry!

@efd6 efd6 force-pushed the 9086-sentinelone_cloud_funnel branch from 47ac63c to 1e79d02 Compare March 5, 2024 20:22
@efd6 efd6 requested a review from kcreddy March 5, 2024 20:24
@elasticmachine
Copy link

💚 Build Succeeded

History

cc @efd6

Copy link

Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

LGTM

@efd6 efd6 merged commit e42f1fb into elastic:main Mar 5, 2024
5 checks passed
@elasticmachine
Copy link

Package sentinel_one_cloud_funnel - 0.12.0 containing this change is available at https://epr.elastic.co/search?package=sentinel_one_cloud_funnel

1 similar comment
@elasticmachine
Copy link

Package sentinel_one_cloud_funnel - 0.12.0 containing this change is available at https://epr.elastic.co/search?package=sentinel_one_cloud_funnel

gizas pushed a commit that referenced this pull request Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: 3rd EDR enhancement New feature or request Integration:sentinel_one_cloud_funnel SentinelOne Cloud Funnel Team:Security-Service Integrations Security Service Integrations Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SentinelOne Cloud Funnel] Adjust mappings for detection rules
6 participants