[Security Solution] User can make Exceptions for Memory protection alerts #102196

academo · 2021-06-15T12:20:56Z

Summary

Adds a feature to allow users make Exceptions for Memory protection alerts
Adds memory protection alert generation to the security data generator

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests 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)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)

add tests for the memory exception default fields

elasticmachine · 2021-06-28T08:14:49Z

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

dplumlee

Looks good to me besides a couple comments, thanks for refactoring some of this to make it easier adding more exception types in the future!

dplumlee · 2021-06-29T04:42:27Z

x-pack/plugins/security_solution/common/ecs/memory_protection/index.ts

+ */
+export interface MemoryProtection {
+  cross_session?: boolean;
+  // should this be a enum with 'signature' and '  shellcode_thread' ? are there more options?


im not sure if this is more a question for the pr or leftover comments, but we'll wanna remove it before merge. Idk much about the context for the field itself

leftover comment. Thanks for pointing it.

dplumlee · 2021-06-29T04:59:30Z

...ins/security_solution/public/common/components/exceptions/exceptionable_endpoint_fields.json

+  "Memory_protection.feature",
+  "Memory_protection.self_injection"


Just double checking, get @gogochan or someone from the endpoint team to sign off on these fields being accessed by users, I don't know enough about them to say they'd be prone to unknowingly making super permissive exceptions

They were explicitly mentioned in the acceptance criteria, but I summon @kevinlog to help me confirm.

These seem to be memory protection plug in feature. @gabriellandau, can you check the fields?

CC @jdu2600 for user-facing shellcode exception fields

TL;DR - Memory_protection.feature and Memory_protection.self_injection are required for user exceptions.

Hey folks,

I'm super new - and not familiar with the typical user exception writing process.
And, in particular, how we balance simplicity and flexibility.

These two fields are absolutely necessary for writing user Memory Protection exceptions - however, if used on their own, they will produce super permissive exceptions (i.e. basically the equivalent of disabling the feature). But I assume that this is the case for all other exception types? Or do we typically provide guardrails - e.g. mandatory rule fields/complexity?

My expectation is that users will predominately be writing simple rules based on acting process + Memory_protection.feature + Memory_protection.self_injection [+ optionally target process].
This is broader that the global exception list rules that we write internally - but that's reasonable since the deployment scope for user rules is not global.

TBH - I'm personally not sure that the typical user needs to see the above Target.process.thread.Ext.start_address* fields, but suspect that 'power users' will be more comfortable having them there. If they use them though they'll like write rules that are too specific - and end up having to tune the rules over time...

@jdu2600 this is how the add exception UI looks in kibana:

Note: this is from a malware alert. just an example.

The user will have pre-filled the value that came with the alert itself. For Memory_protection.feature the value could be shellcode_thread. The user has the option to remove the rule or to put a custom value on the rule.

If they're required for the Memory exceptions themselves, it shouldn't be a problem. And yeah, technically a user could create an exception for "@timestamp is not 1" which would be pretty permissive. I think the goal is just to make it less likely for users to unknowingly do that by not adding unnecessary fields, but these don't fit in that category so should be all good 👍

paul-tavares

left a few comments. Let me know if you have any questions

paul-tavares · 2021-07-01T13:36:44Z

x-pack/plugins/security_solution/common/ecs/file/index.ts

+  integrity_level_name: string;
+}
+
+export interface MemoryPe {


what does Pe suffix mean? Can we spell it out instead of shorten it up (if possible)? (same for other areas below where this suffix is used)

I am not sure what it means, maybe "portable executable". Everywhere inside the indexes is mentioned as PE. @kevinlog do you know?

Yes it's a Portable Executable file that's found in memory.

@gabriellandau should we change the keys to reflect the fullname?

@paul-tavares perhaps if we use the abbreviation everywhere it might come more confusing if we use the fullname in some place and the short in other. Perhaps better add a comment to explain the full meaning?

should we change the keys to reflect the fullname?

I don't think it's necessary. pe is an ECS field. I was following that convention with these other field names.

paul-tavares · 2021-07-01T13:39:44Z

x-pack/plugins/security_solution/common/ecs/index.ts

@@ -63,4 +65,7 @@ export interface Ecs {
  // This should be temporary
  eql?: { parentId: string; sequenceNumber: string };
  Ransomware?: Ransomware;
+  // eslint-disable-next-line @typescript-eslint/naming-convention


Curious: did this really error? I see keys in other areas here starting with Capital letter and including underscores with no eslint disable rules on them

Memory_protection fails to match this rule. memory_protection would work, but not with the upper M

@paul-tavares can you point me to places where there are keys like this without eslint errors?

x-pack/plugins/security_solution/common/endpoint/generate_data.ts

gabriellandau · 2021-07-01T18:55:02Z

x-pack/plugins/security_solution/common/ecs/file/index.ts

+export interface StartAddressDetails {
+  allocation_type?: string;
+  allocation_size?: number;
+  region_size?: number;
+  region_protection?: string;
+  memory_pe?: MemoryPe;
+}


Do we want the rest of the fields from here too? https://github.com/elastic/endpoint-package/blob/1c983ab3fe1ebaa62171b38cb37c3dc420d9b16b/custom_schemas/custom_memory_region.yml

@gabriellandau they are not used for the functionality in this PR so I left them out. I can add them.

gabriellandau · 2021-07-01T19:00:46Z

...ins/security_solution/public/common/components/exceptions/exceptionable_endpoint_fields.json

+  "Target.process.thread.Ext.start_address_details.allocation_size",
+  "Target.process.thread.Ext.start_address_details.region_size",
+  "Target.process.thread.Ext.start_address_details.region_protection",
+  "Target.process.thread.Ext.start_address_details.memory_pe.imphash",


Suggested change

"Target.process.thread.Ext.start_address_details.memory_pe.imphash",

"Target.process.thread.Ext.start_address_details.memory_pe.imphash",

"Target.process.thread.Ext.start_address_bytes",

"process.executable",

"process.name",

"Target.process.executable",

"Target.process.name",

"process.Ext.code_signature", /* can we allow nested only? */

@gabriellandau can you clarify what do you mean with /* can we allow nested only? */ ?

code_signature should be a nested comparison so "trusted": true can be included, keeping users from accidentally trusting untrusted signatures:

"code_signature": [ { "exists": true, "status": "trusted", "subject_name": "Microsoft Windows", "trusted": true } ],

Untrusted:

"code_signature": [ { "exists": true, "status": "errorCode_80092003", "subject_name": "LogMeIn, Inc.", "trusted": false } ],

dasansol92 · 2021-07-12T08:54:06Z

x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx

+        field: 'Memory_protection.feature',
+        operator: 'included',
+        type: 'match',
+        value: alertEcsData.Memory_protection?.feature ?? '',


Q: Do we have to add an entry with empty value or should we just skip the entry if we don't have the specific value?

This should always come with a value.

Then I'm wondering why this key can be undefined alertEcsData.Memory_protection?.feature ?? ''
That seems .feature can be null and then value will be empty.
If this is ok, forget this comment 🙂

@dasansol92 there's not really a good reason for this type to allow undefined. But I saw that every single type inside the ECS is marked as possible undefined and I followed that pattern. Based on the real data I got for this kind of alarm, they should not be undefined and the other exception implementations do the same as me here.

It actually makes more sense to conditionally add fields only when they are populated rather than this than this nullish coalescing.

I'll keep it like this to follow the pattern and I'll reevaluate it when I start working in the behaviour protection that should also touch this area.

kibanamachine · 2021-07-12T09:59:13Z

💚 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`	6.3MB	6.3MB	+7.7KB

History

💚 Build #136018 succeeded 1efeb8e
💚 Build #135503 succeeded 25a6e0e
💚 Build #134952 succeeded 55800eb
💚 Build #134483 succeeded e39e7b2
💚 Build #133014 succeeded 3e590c4

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

dasansol92

LGTM!

kibanamachine · 2021-07-13T08:24:12Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…erts (elastic#102196)

…erts (#102196) (#105397) Co-authored-by: Esteban Beltran <academo@users.noreply.github.com>

…-png-pdf-report-type * 'master' of github.com:elastic/kibana: (292 commits) bring back KQL autocomplete in timeline + fix last updated (elastic#105380) [Maps] Change TOC pop-up wording to reflect filter change, not search bar change (elastic#105163) Updating urls to upstream elastic repo (elastic#105250) [Maps] Move new vector layer wizard card down (elastic#104797) Exclude registering the cases feature if not enabled (elastic#105292) [Uptime] Alerts - Monitor status alert - check monitor status by monitor.timespan (elastic#104541) updated UI copy (elastic#105184) Log a warning when documents of unknown types are detected during migration (elastic#105213) [Logs UI] Register log threshold rule as lifecycle rule (elastic#104341) [Ingest pipelines] add network direction processor (elastic#103436) [Console] Autocomplete definitions (manual backport) (elastic#105086) [Security Solution] User can make Exceptions for Memory protection alerts (elastic#102196) [Lens] Formula: add validation for multiple field/metrics (elastic#104092) Removing async from file upload and data visualizer plugins start lifecycle (elastic#105197) Fix error when validating the form with non blocking validations (elastic#103629) [ML] Fix "View by" swim lane with applied filter and sorting by score (elastic#105217) Update dependency @elastic/charts to v32 (elastic#104625) [CTI] shortens large numbers on Dashboard Link Panel (elastic#105269) [Security Solution][Endpoint][Host Isolation] Fixes bug to remove excess host metadata status toasts on non user initiated errors (elastic#105331) [Cases] Fix pushing alerts count on every push to external service (elastic#105030) ... # Conflicts: # x-pack/plugins/reporting/common/types.ts

academo added 6 commits June 15, 2021 14:17

Add generation of memory alerts to generate data script

906d856

Set the correct event code for memory alerts generation

fd6558a

Add exceptions fields for memory alerts

7f64258

Update generator to use malware as the memory alerts category

d3819a0

Merge branch 'master' into feature/memory_protection_exceptions-1214

ba3307e

Add missing memory shellcode_thread fields

708355c

add tests for the memory exception default fields

academo requested review from dplumlee, marshallmain and yctercero June 17, 2021 15:25

academo added 5 commits June 18, 2021 10:07

Merge branch 'master' into feature/memory_protection_exceptions-1214

9cdc535

Change the target folder to target_type

c050bca

Merge branch 'master' into feature/memory_protection_exceptions-1214

e3091e8

Add Target.process.thread fields to excepctionable fields

5d0484e

Merge branch 'master' into feature/memory_protection_exceptions-1214

3e590c4

academo marked this pull request as ready for review June 28, 2021 07:12

academo requested a review from a team as a code owner June 28, 2021 07:12

academo added v7.15.0 Team:Defend Workflows “EDR Workflows” sub-team of Security Solution labels Jun 28, 2021

Merge branch 'master' into feature/memory_protection_exceptions-1214

e39e7b2

academo added the release_note:feature Makes this part of the condensed release notes label Jun 28, 2021

dplumlee approved these changes Jun 29, 2021

View reviewed changes

dplumlee reviewed Jun 29, 2021

View reviewed changes

dplumlee added the v8.0.0 label Jun 29, 2021

Remove unused comment

55800eb

academo changed the title ~~User can make Exceptions for Memory protection alerts~~ [Security Solution] User can make Exceptions for Memory protection alerts Jun 29, 2021

Merge branch 'master' into feature/memory_protection_exceptions-1214

25a6e0e

academo added the auto-backport Deprecated - use backport:version if exact versions are needed label Jul 1, 2021

paul-tavares reviewed Jul 1, 2021

View reviewed changes

Use a function to return defaultDlls instead of mutable data

1efeb8e

gabriellandau reviewed Jul 1, 2021

View reviewed changes

academo added 3 commits July 12, 2021 09:39

Merge branch 'master' into feature/memory_protection_exceptions-1214

2e4b21d

Add missing types for StartAddressDetails

f1f968f

Add additional exceptionable_endpoint_fields as suggested

571902e

dasansol92 reviewed Jul 12, 2021

View reviewed changes

dasansol92 approved these changes Jul 13, 2021

View reviewed changes

academo merged commit a4a7253 into elastic:master Jul 13, 2021

academo deleted the feature/memory_protection_exceptions-1214 branch July 13, 2021 08:21

kibanamachine mentioned this pull request Jul 13, 2021

[7.x] [Security Solution] User can make Exceptions for Memory protection alerts (#102196) #105397

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 13, 2021

[Security Solution] User can make Exceptions for Memory protection al…

916aa55

…erts (elastic#102196)

kibanamachine added a commit that referenced this pull request Jul 13, 2021

[Security Solution] User can make Exceptions for Memory protection al…

0bec0e0

…erts (#102196) (#105397) Co-authored-by: Esteban Beltran <academo@users.noreply.github.com>

academo mentioned this pull request Jul 26, 2021

Filter out empty values for exceptions #106685

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] User can make Exceptions for Memory protection alerts #102196

[Security Solution] User can make Exceptions for Memory protection alerts #102196

academo commented Jun 15, 2021 •

edited

Loading

elasticmachine commented Jun 28, 2021

dplumlee left a comment

dplumlee Jun 29, 2021

academo Jun 29, 2021

dplumlee Jun 29, 2021

academo Jun 29, 2021

gogochan Jun 29, 2021

gabriellandau Jun 29, 2021

jdu2600 Jun 30, 2021

academo Jul 1, 2021 •

edited

Loading

dplumlee Jul 4, 2021

paul-tavares left a comment

paul-tavares Jul 1, 2021

academo Jul 1, 2021

gabriellandau Jul 1, 2021

academo Jul 1, 2021

gabriellandau Jul 6, 2021

paul-tavares Jul 1, 2021

academo Jul 1, 2021

academo Jul 1, 2021

gabriellandau Jul 1, 2021

academo Jul 12, 2021

gabriellandau Jul 1, 2021

academo Jul 12, 2021

gabriellandau Jul 13, 2021

dasansol92 Jul 12, 2021 •

edited

Loading

academo Jul 12, 2021

dasansol92 Jul 12, 2021

academo Jul 13, 2021

kibanamachine commented Jul 12, 2021

dasansol92 left a comment

kibanamachine commented Jul 13, 2021

		"Memory_protection.feature",
		"Memory_protection.self_injection"

-  "Target.process.thread.Ext.start_address_details.memory_pe.imphash",
+  "Target.process.thread.Ext.start_address_details.memory_pe.imphash",
+  "Target.process.thread.Ext.start_address_bytes",
+  "process.executable",
+  "process.name",
+  "Target.process.executable",
+  "Target.process.name",
+  "process.Ext.code_signature", /* can we allow nested only? */

[Security Solution] User can make Exceptions for Memory protection alerts #102196

[Security Solution] User can make Exceptions for Memory protection alerts #102196

Conversation

academo commented Jun 15, 2021 • edited Loading

Summary

Checklist

elasticmachine commented Jun 28, 2021

dplumlee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

academo Jul 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasansol92 Jul 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Jul 12, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

dasansol92 left a comment

Choose a reason for hiding this comment

kibanamachine commented Jul 13, 2021

💚 Backport successful

academo commented Jun 15, 2021 •

edited

Loading

academo Jul 1, 2021 •

edited

Loading

dasansol92 Jul 12, 2021 •

edited

Loading