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][Endpoint] Update warning text for event filter matches operator #127958

Conversation

ashokaditya
Copy link
Member

@ashokaditya ashokaditya commented Mar 17, 2022

Summary

Show a more descriptive warning when the filename has a wildcard when using matches operator in event filters.

Screenshot 2022-03-17 at 10 12 18

related
/pull/127432
/pull/125202

Checklist

Delete any items that are not applicable to this PR.

@ashokaditya ashokaditya added release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team:Defend Workflows “EDR Workflows” sub-team of Security Solution auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 labels Mar 17, 2022
@ashokaditya ashokaditya self-assigned this Mar 17, 2022
@ashokaditya ashokaditya force-pushed the task/olm-3199-update-warning-event-filter-matches-operator branch from 30262a5 to 4683816 Compare March 17, 2022 10:37
@ashokaditya ashokaditya marked this pull request as ready for review March 17, 2022 10:37
@ashokaditya ashokaditya requested a review from a team as a code owner March 17, 2022 10:37
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

Copy link
Contributor

@paul-tavares paul-tavares left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

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

Left a comment but it LGTM!

review changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

LGTM, looks like good standard stuff. I read through it quickly

@ashokaditya ashokaditya enabled auto-merge (squash) March 17, 2022 16:37
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Mar 17, 2022
…ists

This works out of the box and we don't add endpoint related `file.name` or `process.name` entry when it already is added by the user

refs
elastic/pull/127958#discussion_r829086447
elastic/security-team/issues/3199
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Security Solution Tests / Marking alerts as acknowledged with read only role Mark one alert as acknowledged when more than one open alerts are selected

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
lists 146.9KB 147.4KB +494.0B
securitySolution 4.8MB 4.8MB -13.0B
total +481.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
lists 3.5KB 3.6KB +56.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ashokaditya

@ashokaditya ashokaditya merged commit 0d0ff4e into elastic:main Mar 17, 2022
@kibanamachine
Copy link
Contributor

⚪ Backport skipped

The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.

Manual backport

To create the backport manually run:

node scripts/backport --pr 127958

Questions ?

Please refer to the Backport tool documentation

@ashokaditya ashokaditya deleted the task/olm-3199-update-warning-event-filter-matches-operator branch March 18, 2022 10:17
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Mar 18, 2022
Add `file.name` and `process.name` entry for wildcard path values only when file.name and process.name entries do not already exist.

The earlier commit 8a516ae was mistakenly labeled as this worked out of the box. In the same commit we notice that the test data had a wildcard file path that did not add a `file.name` or `process.name` entry.

For more see:
elastic/pull/127958#discussion_r829086447
elastic/security-team/issues/3199
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 21, 2022
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 127958 or prevent reminders by adding the backport:skip label.

@kevinlog kevinlog added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 21, 2022
ashokaditya added a commit that referenced this pull request Mar 23, 2022
…wildcard) in wildcard-ed event filter `file.path.text` (#127432)

* update filename regex to include multiple hyphens and periods

Uses a much simpler pattern that covers a whole gamut file name patterns.
fixes elastic/security-team/issues/3294

* remove duplicated code

* add tests for `process.name` entry for filenames with wildcard path

refs
/pull/120349
/pull/125202

* Add file.name optimized entry when wildcard filepath in file.path.text has a filename

fixes elastic/security-team/issues/3294

* update regex to include unicode chars

review changes

* add tests for `file.name` and `process.name` entries if it already exists

This works out of the box and we don't add endpoint related `file.name` or `process.name` entry when it already is added by the user

refs
/pull/127958#discussion_r829086447
elastic/security-team/issues/3199

* fix `file.name` and `file.path.text` entries for linux and mac/linux

refs /pull/127098

* do not add endpoint optimized entry

Add `file.name` and `process.name` entry for wildcard path values only when file.name and process.name entries do not already exist.

The earlier commit 8a516ae was mistakenly labeled as this worked out of the box. In the same commit we notice that the test data had a wildcard file path that did not add a `file.name` or `process.name` entry.

For more see:
/pull/127958#discussion_r829086447
elastic/security-team/issues/3199

* update regex to include gamut of unicode characters

review suggestions

* remove regex altogether

simplifies the logic to check if path is without wildcard characters. This way it includes all other strings as valid filenames that do not have * or ?

* update artifact creation for `file.path.text` entries

Similar to when we normalize `file.path.caseless` entries, except that the `type` is `*_cased` for linux and `*_caseless` for non-linux
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Detections and Resp Security Detection Response Team v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants