Skip to content

[Security Solution][Endpoint] Reuse disableProtections function for cloud policies#144460

Merged
gergoabraham merged 6 commits into
elastic:mainfrom
gergoabraham:reuse-disable-protections-function-for-cloud-policies
Nov 7, 2022
Merged

[Security Solution][Endpoint] Reuse disableProtections function for cloud policies#144460
gergoabraham merged 6 commits into
elastic:mainfrom
gergoabraham:reuse-disable-protections-function-for-cloud-policies

Conversation

@gergoabraham
Copy link
Copy Markdown
Contributor

Summary

We got two implementations for disabling protections, one for Traditional Endpoints environment and one for Cloud Workloads environment. It would be great to use the same implementation for both, this PR wants to check whether disableProtections() implemented for Traditional Endpoints (added in #144087) can be used for Cloud Workloads, too.

Pros:

  • disables credential_hardening protection on Windows,
  • if a new protection is added PolicyConfig, type error will make sure that disableProtections() will be updated

❓ Questions, issues

There are some differences between the original cloud policy and the one created by disableProtections(), and I'd like to understand the differences, and whether it's worth to use disableProtections() at all, if it needs so many modifications.

  • 1️⃣ policy.*.malware.blocklist are set to false - as malware itself is disabled, this shouldn't be a problem. Or can it be?
  • 2️⃣ policy.*.behavior_protection.supported are set to false - how should the supported fields look like? According to @kevinlog, they should describe the support by license, so I assume they should be untouched by disableProtections(), but in the original cloud policy some are disabled, some are not.
  • 3️⃣ policy.*.popup.*.enabled are set to false - if protections are disabled, these should not matter. Or do they?

@kevinlog, @opauloh could you please help me out with these questions? ☝️

Here is the difference for Windows, the original policy is on the left, the new one is on the right:
image

Mac and Linux are similar:
image

image

Checklist

@gergoabraham gergoabraham added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Defend Workflows “EDR Workflows” sub-team of Security Solution OLM Sprint 8.6 candidate labels Nov 2, 2022
@gergoabraham gergoabraham requested a review from a team as a code owner November 2, 2022 16:58
@gergoabraham gergoabraham self-assigned this Nov 2, 2022
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@opauloh
Copy link
Copy Markdown
Contributor

opauloh commented Nov 2, 2022

Hey @gergoabraham

Thank you for adding disableProtections to getCloudPolicyConfig 🙌

Regarding the questions:

1️⃣ policy..malware.blocklist are set to false - as malware itself is disabled, this shouldn't be a problem. Or can it be?
3️⃣ policy.
.popup.*.enabled are set to false - if protections are disabled, these should not matter. Or do they

You're correct about these ones. That shouldn't be a problem. There is no need to disable the blocklist or the popup.

2️⃣ policy.*.behavior_protection.supported are set to false - how should the supported fields look like? According to @kevinlog, they should describe the support by license, so I assume they should be untouched by disableProtections(), but in the original cloud policy some are disabled, some are not.

Actually, we ended up disabling everything just for safety. If @kevinlog can confirm that the supported field on protections does not impact on the 'mode' field, I think that can simplify things a bit on the PR (since we won't need to have the supportedField parameter on disableProtections). The only two requirements for getCloudPolicyConfig are really to disable all protections and enable linux.events.session_data

@kevinlog
Copy link
Copy Markdown
Contributor

kevinlog commented Nov 2, 2022

@opauloh

Actually, we ended up disabling everything just for safety. If @kevinlog can confirm that the supported field on protections does not impact on the 'mode' field, I think that can simplify things a bit on the PR (since we won't need to have the supportedField parameter on disableProtections)

This is correct. The supported field is tied to the license. We set it to true when we have at least a platinum license and we tell the Endpoint that the protections are supported by the current license for telemetry purposes. It is set to false if the license is below platinum.

You are both correct that disableProtections does not and should not change the supported field as this will be picked up by the license check.

@gergoabraham
Copy link
Copy Markdown
Contributor Author

@kevinlog @opauloh thanks a lot for the thoughts! I've reverted the disabling of the supported fields.

Here is how a cloud policy look like now with the current version:

{
  "windows": {
    "events": {
      "dll_and_driver_load": true,
      "dns": true,
      "file": true,
      "network": true,
      "process": true,
      "registry": true,
      "security": true
    },
    "malware": { "mode": "off", "blocklist": false },
    "ransomware": { "mode": "off", "supported": true },
    "memory_protection": { "mode": "off", "supported": true },
    "behavior_protection": { "mode": "off", "supported": true },
    "popup": {
      "malware": { "message": "", "enabled": false },
      "ransomware": { "message": "", "enabled": false },
      "memory_protection": { "message": "", "enabled": false },
      "behavior_protection": { "message": "", "enabled": false }
    },
    "logging": { "file": "info" },
    "antivirus_registration": { "enabled": false },
    "attack_surface_reduction": { "credential_hardening": { "enabled": false } }
  },
  "mac": {
    "events": { "process": true, "file": true, "network": true },
    "malware": { "mode": "off", "blocklist": false },
    "behavior_protection": { "mode": "off", "supported": true },
    "memory_protection": { "mode": "off", "supported": true },
    "popup": {
      "malware": { "message": "", "enabled": false },
      "behavior_protection": { "message": "", "enabled": false },
      "memory_protection": { "message": "", "enabled": false }
    },
    "logging": { "file": "info" }
  },
  "linux": {
    "events": {
      "process": true,
      "file": true,
      "network": true,
      "session_data": true,
      "tty_io": false
    },
    "malware": { "mode": "off", "blocklist": false },
    "behavior_protection": { "mode": "off", "supported": true },
    "memory_protection": { "mode": "off", "supported": true },
    "popup": {
      "malware": { "message": "", "enabled": false },
      "behavior_protection": { "message": "", "enabled": false },
      "memory_protection": { "message": "", "enabled": false }
    },
    "logging": { "file": "info" }
  }
}

Notes:

  • all protections are disabled, but the support is not modified, but they will be disabled on basic license
  • popups and blocklist are disabled, too, because it should not matter, and this is the original implementation of disableProtections()

Copy link
Copy Markdown
Contributor

@opauloh opauloh left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @gergoabraham !!

Copy link
Copy Markdown
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

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

Checked it out. Works as expected. 🚢 it

@gergoabraham
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@gergoabraham gergoabraham enabled auto-merge (squash) November 7, 2022 09:42
@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #4 / Row renderers Selected renderer can be disabled and enabled
  • [job] [logs] Security Solution Tests #4 / url state sets and reads the url state for timeline by id

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 58 64 +6
osquery 108 113 +5
securitySolution 440 446 +6
total +19

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 66 72 +6
osquery 109 115 +6
securitySolution 517 523 +6
total +20

History

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

cc @gergoabraham

@gergoabraham gergoabraham merged commit 3d82d7e into elastic:main Nov 7, 2022
@gergoabraham gergoabraham deleted the reuse-disable-protections-function-for-cloud-policies branch November 7, 2022 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting OLM Sprint release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants