Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(terraform): setting namespace check for CKV_AWS_312 #6027

Closed

Conversation

avazula
Copy link

@avazula avazula commented Feb 12, 2024

User description

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Description

CKV_AWS_312 setting for checking that HealthStreaming is enabled does not exist in the AWS docs. It is instead found in the aws:elasticbeanstalk:cloudwatch:logs:health namespace.

Fixes # 6007

New/Edited policies (Delete if not relevant)

Description

Edited check to focus on the new property instead of the obsolete one from the healthreporting namespace.

Fix

  setting {
    namespace = "aws:elasticbeanstalk:cloudwatch:logs:health"
    name      = "HealthStreamingEnabled"
    value     = true
  }

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

PLEASE NOTE: This PR most likely still needs work. I just wanted to help after finding this mistake while checking for flaws in my code, but I unfortunately do not have the time to get to know the project structure, how tests are written and run, and so on. I apologize for this, but this is the best I could do in the short amount of time I have.

Updated the docs here


Generated description

Dear maintainer, below is a concise technical summary of the changes proposed in this PR:

Update the ElasticBeanstalkUseEnhancedHealthChecks class to check for the SystemType setting in the aws:elasticbeanstalk:healthreporting:system namespace, ensuring it is set to enhanced. Modify test cases to reflect this change by removing obsolete settings and adjusting expected values.

TopicDetails
Enhanced Health Check Update the ElasticBeanstalkUseEnhancedHealthChecks class to check for the SystemType setting in the aws:elasticbeanstalk:healthreporting:system namespace, ensuring it is set to enhanced.
Modified files (1)
  • checkov/terraform/checks/resource/aws/ElasticBeanstalkUseEnhancedHealthChecks.py
Latest Contributors(1)
EmailCommitDate
james.woolfenden@gmail...feat-terraform-Elastic...May 04, 2023
Test Adjustments Modify test cases to reflect the change in health check settings by removing obsolete settings and adjusting expected values.
Modified files (1)
  • tests/terraform/checks/resource/aws/example_ElasticBeanstalkUseEnhancedHealthChecks/main.tf
Latest Contributors(1)
EmailCommitDate
james.woolfenden@gmail...feat-terraform-Elastic...May 04, 2023
This pull request is reviewed by Baz. Join @avazula and the rest of your team on (Baz).

@avazula
Copy link
Author

avazula commented Feb 26, 2024

Would it be possible to get a review/adjustments to get it merged? This error forces me to issue a custom security report and come back to this PR regularly to see if I can adjust my audit to go back to normal.

@gruebel
Copy link
Contributor

gruebel commented Feb 27, 2024

hey @avazula

your fix doesn't fit the intention of the check, it should be

  setting {
    namespace = "aws:elasticbeanstalk:healthreporting:system"
    name      = "SystemType"
    value     = "enhanced"
  }

and you laos need to adjust the tests https://github.com/bridgecrewio/checkov/blob/main/tests/terraform/checks/resource/aws/example_ElasticBeanstalkUseEnhancedHealthChecks/main.tf

@avazula
Copy link
Author

avazula commented Feb 27, 2024

Hi @gruebel

If healthreporting should be modified instead of cloudwatch:logs:health, doesn't it mean that we should also have EnhancedHealthAuthEnabled set to true?
Looking at the docs it feels that this goes hand in hand with health logs security, but I might be wrong: https://docs.aws.amazon.com/elasticbeanstalk/latest/dg/command-options-general.html#command-options-general-elasticbeanstalkhealthreporting

Pending your answer I will adjust to match your suggested change. I will also try to update the tests.

@avazula
Copy link
Author

avazula commented Mar 7, 2024

Could someone please approve the workflow? I'm not fluent in GitHub but I assume this means running the tests.

@pazbechor
Copy link
Contributor

Hey @avazula, Thanks for your contributing!
According the docs, why not checking EnhancedHealthAuthEnabled value as well?
Anyway, please add more tests with multiple failures & pass examples :)

@pazbechor pazbechor changed the title changed setting namespace check for CKV_AWS_312 feat(terraform): setting namespace check for CKV_AWS_312 Jul 30, 2024
@avazula
Copy link
Author

avazula commented Aug 7, 2024

Hi @pazbechor thanks for checking out this PR.
Unfortunately I don't currently have the time to work more on this. Feel free to take it from here (or anyone else) if you can and wish to. Otherwise I'll get back to it as soon as I can. Cheers!

@pazbechor
Copy link
Contributor

@avazula Hey!
Closing this one because it's been a while, if it's relevant - please open new one (and make changees according @gruebel requests 😄 )
Thanks for the contribution!

@pazbechor pazbechor closed this Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants