Skip to content

[Security Solution][Endpoint] Improve endpoint performance with TA wildcard paths#120349

Merged
ashokaditya merged 20 commits intoelastic:mainfrom
ashokaditya:fix/olm-improve_endpoint_perf_with_TA_wildcards-2293
Dec 16, 2021
Merged

[Security Solution][Endpoint] Improve endpoint performance with TA wildcard paths#120349
ashokaditya merged 20 commits intoelastic:mainfrom
ashokaditya:fix/olm-improve_endpoint_perf_with_TA_wildcards-2293

Conversation

@ashokaditya
Copy link
Member

@ashokaditya ashokaditya commented Dec 3, 2021 •
edited
Loading

Summary

Displays a soft warning when the matching path has wildcard executable name. If the wildcard path has a proper executable in the path then it adds a process.name value to the TA artifact.

image

Linux entries
image

Mac entries
image

Windows entries
iamge

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

@ashokaditya ashokaditya self-assigned this Dec 3, 2021
@ashokaditya ashokaditya added bug Fixes for quality problems that affect the customer experience 8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed release_note:fix v7.16.1 v7.16.2 Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.0.0 v8.1.0 and removed 8.0.0 labels Dec 3, 2021
@ashokaditya ashokaditya force-pushed the fix/olm-improve_endpoint_perf_with_TA_wildcards-2293 branch 3 times, most recently from 8d7340e to 06fb215 Compare December 6, 2021 19:15
@ashokaditya ashokaditya force-pushed the fix/olm-improve_endpoint_perf_with_TA_wildcards-2293 branch from 2e5a11d to 27fa87c Compare December 9, 2021 10:21
@ashokaditya ashokaditya force-pushed the fix/olm-improve_endpoint_perf_with_TA_wildcards-2293 branch from 27fa87c to da1d1be Compare December 9, 2021 13:41
@ashokaditya ashokaditya force-pushed the fix/olm-improve_endpoint_perf_with_TA_wildcards-2293 branch from da1d1be to 5757522 Compare December 9, 2021 14:01
@ashokaditya ashokaditya marked this pull request as ready for review December 9, 2021 14:01
@ashokaditya ashokaditya requested review from a team as code owners December 9, 2021 14:01
@ashokaditya ashokaditya requested review from joeypoon and pzl December 9, 2021 14:01
@elasticmachine
Copy link
Contributor

elasticmachine commented Dec 9, 2021

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

@ashokaditya
Copy link
Member Author

ashokaditya commented Dec 16, 2021

@elasticmachine merge upstream

kibanamachine and others added 4 commits December 16, 2021 03:14
@ashokaditya ashokaditya enabled auto-merge (squash) December 16, 2021 09:30
@ashokaditya ashokaditya disabled auto-merge December 16, 2021 09:35
@ashokaditya
Copy link
Member Author

ashokaditya commented Dec 16, 2021 •
edited
Loading

@dasansol92

Tested this with v7.15.3 and upgraded to the current changes and all works as it should.
The artifacts are cleaned up correctly to reflect process.name in entries.

artifacts (7.15.3 on left)

linux
image

macox
image

windows
image

@ashokaditya ashokaditya enabled auto-merge (squash) December 16, 2021 16:49
@kibana-ci
Copy link

kibana-ci commented Dec 16, 2021

💚 Build Succeeded

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
securitySolution 4.6MB 4.6MB +597.0B

History

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

cc @ashokaditya

@ashokaditya ashokaditya merged commit 1dd669c into elastic:main Dec 16, 2021
@kibanamachine
Copy link
Contributor

kibanamachine commented Dec 16, 2021

💔 Backport failed

Status Branch Result
❌ 8.0 Commit could not be cherrypicked due to conflicts
❌ 7.17 Commit could not be cherrypicked due to conflicts
❌ 8.2 The branch "8.2" is invalid or doesn't exist

To backport manually run:
node scripts/backport --pr 120349

@ashokaditya ashokaditya mentioned this pull request Dec 16, 2021
Merged
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Dec 16, 2021
@ashokaditya @kibanamachine
[Security Solution][Endpoint] Improve endpoint performance with TA wi…
f0c5e5b 
…ldcard paths (elastic#120349)

* Show full executable names in placeholder for wildcard paths

fixes elastic/security-team/issues/2293

* Show soft warning when wildcard also in executable names

fixes elastic/security-team/issues/2293

* add wildcard path entries to fake TA list

refs elastic/security-team/issues/2293

* Append a process.name entry when executable name in wildcard path

fixes elastic/security-team/issues/2293

* commit using ashokaditya@elastic.co

* linux should always use  `_cased` types

review changes

* use better TS

* use matcher functions to compute operator value for linux

review suggestions

* use path to extract filenames on server side

review suggestions

* improve regex for windows and unix filepaths

review suggestions

* update test mocks

review changes

* update regex to match multi spaces and single chars with spaces in filenames

* add comment to explain

review suggestions

* update copy

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	x-pack/plugins/security_solution/public/management/pages/policy/view/trusted_apps/layout/policy_trusted_apps_layout.test.tsx
@ashokaditya ashokaditya deleted the fix/olm-improve_endpoint_perf_with_TA_wildcards-2293 branch December 16, 2021 19:06
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Dec 16, 2021
@ashokaditya @kibanamachine
[Security Solution][Endpoint] Improve endpoint performance with TA wi…
ef1fb2b 
…ldcard paths (elastic#120349)

* Show full executable names in placeholder for wildcard paths

fixes elastic/security-team/issues/2293

* Show soft warning when wildcard also in executable names

fixes elastic/security-team/issues/2293

* add wildcard path entries to fake TA list

refs elastic/security-team/issues/2293

* Append a process.name entry when executable name in wildcard path

fixes elastic/security-team/issues/2293

* commit using ashokaditya@elastic.co

* linux should always use  `_cased` types

review changes

* use better TS

* use matcher functions to compute operator value for linux

review suggestions

* use path to extract filenames on server side

review suggestions

* improve regex for windows and unix filepaths

review suggestions

* update test mocks

review changes

* update regex to match multi spaces and single chars with spaces in filenames

* add comment to explain

review suggestions

* update copy

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	x-pack/plugins/security_solution/common/endpoint/data_generators/exceptions_list_item_generator.ts
#	x-pack/plugins/security_solution/public/management/pages/mocks/trusted_apps_http_mocks.ts
#	x-pack/plugins/security_solution/public/management/pages/policy/view/trusted_apps/layout/policy_trusted_apps_layout.test.tsx
#	x-pack/plugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.test.tsx
@ashokaditya ashokaditya mentioned this pull request Dec 16, 2021
Merged
ashokaditya added a commit that referenced this pull request Dec 16, 2021
@ashokaditya @kibanamachine
[Security Solution][Endpoint] Improve endpoint performance with TA wi…
48e248a 
…ldcard paths (#120349) (#121443)

* Show full executable names in placeholder for wildcard paths

fixes elastic/security-team/issues/2293

* Show soft warning when wildcard also in executable names

fixes elastic/security-team/issues/2293

* add wildcard path entries to fake TA list

refs elastic/security-team/issues/2293

* Append a process.name entry when executable name in wildcard path

fixes elastic/security-team/issues/2293

* commit using ashokaditya@elastic.co

* linux should always use  `_cased` types

review changes

* use better TS

* use matcher functions to compute operator value for linux

review suggestions

* use path to extract filenames on server side

review suggestions

* improve regex for windows and unix filepaths

review suggestions

* update test mocks

review changes

* update regex to match multi spaces and single chars with spaces in filenames

* add comment to explain

review suggestions

* update copy

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	x-pack/plugins/security_solution/public/management/pages/policy/view/trusted_apps/layout/policy_trusted_apps_layout.test.tsx
ashokaditya added a commit that referenced this pull request Dec 16, 2021
@ashokaditya @kibanamachine
[Security Solution][Endpoint] Improve endpoint performance with TA wi…
3448697 
…ldcard paths (#120349) (#121448)

* Show full executable names in placeholder for wildcard paths

fixes elastic/security-team/issues/2293

* Show soft warning when wildcard also in executable names

fixes elastic/security-team/issues/2293

* add wildcard path entries to fake TA list

refs elastic/security-team/issues/2293

* Append a process.name entry when executable name in wildcard path

fixes elastic/security-team/issues/2293

* commit using ashokaditya@elastic.co

* linux should always use  `_cased` types

review changes

* use better TS

* use matcher functions to compute operator value for linux

review suggestions

* use path to extract filenames on server side

review suggestions

* improve regex for windows and unix filepaths

review suggestions

* update test mocks

review changes

* update regex to match multi spaces and single chars with spaces in filenames

* add comment to explain

review suggestions

* update copy

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	x-pack/plugins/security_solution/common/endpoint/data_generators/exceptions_list_item_generator.ts
#	x-pack/plugins/security_solution/public/management/pages/mocks/trusted_apps_http_mocks.ts
#	x-pack/plugins/security_solution/public/management/pages/policy/view/trusted_apps/layout/policy_trusted_apps_layout.test.tsx
#	x-pack/plugins/security_solution/public/management/pages/trusted_apps/view/trusted_apps_page.test.tsx
@ashokaditya ashokaditya mentioned this pull request Feb 17, 2022
Closed
2 tasks
ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Mar 17, 2022
@ashokaditya
add tests for process.name entry for filenames with wildcard path
2b7f459 
refs
elastic/pull/120349
elastic/pull/125202
ashokaditya added a commit that referenced this pull request Mar 23, 2022
@ashokaditya
[Security Solution][Endpoint] Accept all kinds of filenames (without …
98300c2 
…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
@gergoabraham gergoabraham mentioned this pull request Nov 13, 2023
Merged
8 tasks
gergoabraham added a commit that referenced this pull request Dec 5, 2023
@gergoabraham
[EDR Workflows] Add matches capabilities to Endpoint Exception creati…
44d7c0a 
…on (#166002)

## Summary

This PR adds `matches` (`wildcard include`) and `does not match`
(`wildcard exclude`) to fields which support them when creating an
Endpoint exception.

For backwards compatibility with Endpoints < 8.2.0, Manifest Manager
adds the following entry to Endpoint Exceptions containing _only_
wildcards:
```json
{
  "field": "event.module",
  "operator": "included",
  "type": "exact_cased",
  "value": "endpoint"
}
```

> [!Note]
> Warnings for wrongly formatted wildcards don't seem to work correctly
at the moment. #170495 will bring some changes in the related functions,
so this PR is waiting on that to be merged.


<img width="1465" alt="image"
src="https://github.com/elastic/kibana/assets/39014407/db04fe0b-4cb3-4cba-a6d7-622a2239f059">

## Sample manifests
### Linux
⚠️ On Linux, the type is always `wildcard_cased`, see the following
comment for details:
#120349 (comment)
```json
{
  "entries": [
    {
      "type": "simple",
      "entries": [
        {
          "field": "file.path",
          "operator": "included",
          "type": "wildcard_cased",
          "value": "*/test/*"
        },
        {
          "field": "event.module",
          "operator": "included",
          "type": "exact_cased",
          "value": "endpoint"
        }
      ]
    }
  ]
}
```

### Windows
```json
{
  "entries": [
    {
      "type": "simple",
      "entries": [
        {
          "field": "file.path",
          "operator": "included",
          "type": "wildcard_caseless",
          "value": "*/test/*"
        },
        {
          "field": "event.module",
          "operator": "included",
          "type": "exact_cased",
          "value": "endpoint"
        }
      ]
    }
  ]
}
```

### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dasansol92 dasansol92 dasansol92 left review comments

@paul-tavares paul-tavares paul-tavares approved these changes

@ferullo ferullo Awaiting requested review from ferullo

@intxgo intxgo Awaiting requested review from intxgo

@gabriellandau gabriellandau Awaiting requested review from gabriellandau

+2 more reviewers

@kevinlog kevinlog kevinlog left review comments

@academo academo academo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ashokaditya ashokaditya

Labels

auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience release_note:fix Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Detections and Resp Security Detection Response Team v7.17.0 v8.0.0 v8.1.0 v8.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@ashokaditya @elasticmachine @kevinlog @ferullo @academo @kibana-ci @kibanamachine @dasansol92 @gabriellandau @paul-tavares