[Security Solution][Endpoint] Guided onboarding configures events only for endpoint policy

[gergoabraham]

Summary

When Endpoint integration is added via the new guided onboarding, every protection is disabled in the endpoint policy. This does not have an effect on the [ NGAV, EDRComplete, EDREssential] configs.

To try out, add useMultiPageLayout query param to the URL when adding Endpoint integration:
localhost:5601/app/fleet/integrations/endpoint/add-integration?useMultiPageLayout

See config result:

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

[elasticmachine]

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

[opauloh]

I was thinking about your comment on slack that getCloudPolicyConfig does not have all protections disabled, and I'm wondering how we can enforce that all protections will be disabled in case more protections are added in the future since protections are not included under a specific object.

So I think, instead of a disabledProtections function, we could create a policyFactoryWithDisabledProtections, then Typescript would ensure every new field added on PolicyConfig is explicitly set on the policy:

For example (in policy_config.ts):

/**
 * Return a policyFactory with all protections disabled`
 */
export const policyFactoryWithDisabledProtections = (): ReturnType<typeof policyFactory> => {
  const policy = policyFactory();
  return {
    windows: {
      events: {
        ...policy.windows.events
      },
      malware: {
        mode: ProtectionModes.off,
        blocklist: true,
      },
      ransomware: {
        mode: ProtectionModes.off,
        supported: false,
      },
      memory_protection: {
        mode: ProtectionModes.off,
        supported: false,
      },
      behavior_protection: {
        mode: ProtectionModes.off,
        supported: false,
      },
      popup: {
        malware: {
          message: '',
          enabled: false,
        },
        ransomware: {
          message: '',
          enabled: false,
        },
        memory_protection: {
          message: '',
          enabled: false,
        },
        behavior_protection: {
          message: '',
          enabled: false,
        },
      },
      logging: {
        ...policy.windows.logging
      },
      antivirus_registration: {
        enabled: false,
      },
      attack_surface_reduction: {
        credential_hardening: {
          enabled: false,
        },
      },
    },
    mac: {
      events: {
        ...policy.mac.events
      },
      malware: {
        mode: ProtectionModes.off,
        blocklist: true,
      },
      behavior_protection: {
        mode: ProtectionModes.off,
        supported: true,
      },
      memory_protection: {
        mode: ProtectionModes.off,
        supported: true,
      },
      popup: {
        malware: {
          message: '',
          enabled: false,
        },
        behavior_protection: {
          message: '',
          enabled: false,
        },
        memory_protection: {
          message: '',
          enabled: false,
        },
      },
      logging: {
        ...policy.mac.logging
      },
    },
    linux: {
      events: {
        ...policy.linux.events
      },
      malware: {
        mode: ProtectionModes.off,
        blocklist: false,
      },
      behavior_protection: {
        mode: ProtectionModes.off,
        supported: false,
      },
      memory_protection: {
        mode: ProtectionModes.off,
        supported: false,
      },
      popup: {
        malware: {
          message: '',
          enabled: false,
        },
        behavior_protection: {
          message: '',
          enabled: false,
        },
        memory_protection: {
          message: '',
          enabled: false,
        },
      },
      logging: {
        ...policy.linux.logging
      },
    },
  };
};

This way, we would still spread over specific fields, but new fields on the root would need to be manually added, and that would enforce the addition of new protection fields to the policy into this function.

Also, this way, the getCloudPolicyConfig() function would be able to reuse the policy.

[gergoabraham]

That was my biggest question, too: how to ensure that we won't forget to add new protections to disableProtections?

Actually, it is enforced: I added a PolicyConfig constant to the tests that will throw a type error if PolicyConfig is ever modified. See it here.

Why did I add it to the test? Mostly because I didn't want to interfere with the current logic. Now there is only one default config (all ON). And there are 3 modifiers: one for stripping of paid features, one for enabling the support for paid features, and the new one for disabling protections.

I think we can go with either having a constant config for every scenario, or having these modifiers that can be applied after each other. As subsets of the config are independent from each other (like events from protections), I think using modifiers is a better choice.

This means, that getCloudPolicyConfig() can reuse disableProtections() right now, because it only disables protections and does not touch anything else.

What do you think? Would you like me to add disableProtections() to getCloudPolicyConfig()?

[opauloh]

Hey, @gergoabraham just tested adding new protection and the test works great, nicely done! 🙌

[opauloh]

What do you think? Would you like me to add disableProtections() to getCloudPolicyConfig()?

That makes sense to me, getCloudPolicyConfig inherits all policy defaults, then needs all protections off, and has policy.linux.events.session_data as true. If you can make that change that would be awesome!

[opauloh]

Left a couple of suggestions, let me know what do you think

[kevinlog]

@gergoabraham we should still not change this when we are looking at an unpaid license because part of the advantage of the license is that users can customize the message and toggle it on/off.

This is what the UI looks like with a basic license:

[gergoabraham]

@kevinlog thanks for the explanation, it makes sense now!

I've reverted the modification with an explanatory comment: 5b30c69

And switched the order of the logic, so license limitations are applied in the final step, so disableProtections won't override and disable malware popup: 570c16d

(Disabling malware popup when disabling protections shouldn't be a problem in practice though, I think, because after enabling the malware protection, the popup will be automatically enabled, but still, I think it is clearer to apply the limitations at the end.)

[kevinlog]

Thanks @gergoabraham ! The logic makes sense to me, we should just be sure to test the cases using the existing NGAV, EDR Essential, and EDR Complete with a basic license and ensure that everything is still working.

[gergoabraham]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 7218b30
• Cloud Deployment

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	57	63	+6
osquery	103	108	+5
securitySolution	439	443	+4
total			+17

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	20	22	+2
fleet	65	71	+6
osquery	104	110	+6
securitySolution	516	520	+4
total			+18

History

• 💔 Build #83500 failed 4747431
• 💔 Build #83083 failed a260a80

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

cc @gergoabraham

[kevinlog]

@gergoabraham

I checked it out and tried it, it looks good!

When I step through the onboarding flow, the policy is created just with eventing options enabled ✅

The existing pre-set options NGAV, Essential, and Complete still create the policies correctly ✅

NGAV

Essential

Complete

When I downgrade to a basic license, the policy configs are still properly downgraded ✅

Before, I had some paid protections enabled. After downgrade, they are automatically turned off and sent to Endpoints

When I use the pre-set NGAV to make a new policy with a basic license, the paid features are still OFF ✅

[kevinlog]

Functionality LGTM - we should still get approvals from @opauloh after the requested changes and someone else from our team.

[opauloh]

Thanks for this comprehensive test @kevinlog - I also tested how it handles when new protections are added and everything LGTM

[opauloh]

everything LGTM!

If you can make getCloudPolicyConfig use the disableProtections that would be great, feel free to add in a subsequent PR if you want.

[gergoabraham]

If you can make getCloudPolicyConfig use the disableProtections that would be great, feel free to add in a subsequent PR if you want.

@opauloh thanks for the review! I'll open a new PR for this ☝️

[parkiino]

🐑 this lgtm