awslabs / landing-zone-accelerator-on-aws

Deploy a multi-account cloud foundation to support highly-regulated workloads and complex compliance requirements.
https://aws.amazon.com/solutions/implementations/landing-zone-accelerator-on-aws/
Apache License 2.0
556 stars 438 forks source link

Unable to configure deploymentTargets for securityHub without CIS AWS Foundations Benchmark v1.2.0 being enabled #537

Open gr-georgeshort opened 2 months ago

gr-georgeshort commented 2 months ago

Describe the bug When attempting to configure securityHub with deploymentTargets, we receive the error: 'securityHub standard (CIS AWS Foundations Benchmark v1.2.0) "deploymentTargets" is required when "deploymentTargets" for securityHub is defined'

This is not mentioned within the documentation for LZA.

To Reproduce Something like:

securityHub:
  enable: true
  regionAggregation: true
  excludeRegions: []
  deploymentTargets:
    organizationalUnits:
      - Root
  standards:
    - name: AWS Foundational Security Best Practices v1.0.0
      deploymentTargets:
      organizationalUnits:
        -  Root
      enable: true
      controlsToDisable:
        # Refer to the document for the controls
        # https://docs.aws.amazon.com/securityhub/latest/userguide/fsbp-standard.html
        - Control1
        - Control2
  logging:
    cloudWatch:
      enable: true
      logLevel: MEDIUM

Expected behavior Security Hub to be enabled in provided OUs under deployment targets without the requirement for CIS standard to be enabled.

Please complete the following information about the solution:

erwaxler commented 2 months ago

Hi @gr-georgeshort , thanks for reaching out to the team. The indentation of your configuration appears to be incorrect, please try adding an extra indentation to organizationalUnits and try again to see if that resolves your issue:

    deploymentTargets:
      organizationalUnits:
        -  Root
gr-georgeshort commented 2 months ago

Hi @erwaxler, Sorry I made up the example from the github pages as do not have access to the real world example atm. Our indentation is correct 👍 I copied it from here by the way - https://awslabs.github.io/landing-zone-accelerator-on-aws/latest/typedocs/latest/classes/_aws_accelerator_config.SecurityHubConfig.html

erwaxler commented 2 months ago

@gr-georgeshort thanks for confirming! I'll create an issue in the team's backlog to update our documentation and fix the indentation. I'll add that the deploymentTargets property on the top-level securityHub configuration is optional - if you are targeting the entire AWS Organization through Root, there's no need to specify deploymentTargets there.

gr-georgeshort commented 2 months ago

@erwaxler Sorry should have waited to provide a real world example. We are however not targeting Root, we are targeting specific OUs as we do not want to currently enable securityHub / best practices standard across the whole Org.

Our end goal is to have a subset of OUs with security hub configured and only the AWS best practices standard enabled. When trying to implement this is when we received the error. I will now test configuring deploymentTarget within the securityHub configuration and remove the deploymentTargets from best practices standard if you believe this will work/meet these requirements?

The error returned though would imply this is not possible as we do not require the CIS standard.

erwaxler commented 2 months ago

Hi @gr-georgeshort no worries, thanks for the additional information. I believe the validation logic could be more clearly described as:

If deploymentTargets are defined for securityHub, deploymentTargets must be defined for each standard that is enabled.

The logic here is that we need to validate that the deploymentTargets defined for each standard are a subset of the deploymentTargets defined for all of Security Hub. That way, we can validate that a certain standard's deploymentTargets does not include accounts where Security Hub is not enabled.

There should be no need to use specific Security Hub standards when using deploymentTargets - please let me know if you are seeing that behavior as that would indicate a bug. Based on your error message, it seems like the CIS AWS Foundations Benchmark v1.2.0 standard exists in your security-config.yaml file without defined deploymentTargets. If you don't want that standard applied, I would remove it entirely from the configuration.

gr-georgeshort commented 2 months ago

Hey @erwaxler, I believe the issue we originally had was that CIS was listed but with enabled: false so your answer above now corresponds to this validation. We have removed the CIS reference completely now, the pipeline did not fail but it didn't actually disable the standard within Security Hub. We have manually disabled it, rerun LZA and it hasn't re-enabled. Real world example now:

  securityHub:
    enable: true
    regionAggregation: true
    autoEnableOrgMembers: false
    deploymentTargets:
      organizationalUnits:
        - Security
        - Infrastructure
        - Sandbox
    standards:
      - name: AWS Foundational Security Best Practices v1.0.0
        deploymentTargets:
          organizationalUnits:
            - Security
            - Infrastructure
            - Sandbox
        enable: true
gr-georgeshort commented 2 months ago

Hey @erwaxler I'm happy that we can close this off as just missing documentation around validation? Shall I create a new issue for the new problem that removing CIS standard from LZA does not disable it within the applied OUs?

erwaxler commented 2 months ago

Hey @gr-georgeshort , thanks for confirming that. I'd like to keep this issue open to track the documentation changes, as well as the validation code - if enable is set to false for a Security Hub standard, we should not be validating the deploymentTargets property. If you don't mind creating a new issue for removing a Security Hub standard I think that would be best to track separately. Thanks again for bringing this to the team's attention!

gr-georgeshort commented 2 months ago

Hey @erwaxler, have done some testing and believe the removal of security standard may also be missing documentation too. If you have a standard which is enabled, and then go ahead and remove the configuration block for it, LZA will not disable it. You have to first set enable to false and run the pipeline prior to this.

erwaxler commented 2 months ago

Hey @gr-georgeshort thanks for diving into that, I've updated the internal issue to include that as well.