-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(aws): Add new CodeBuild check to validate environment variables #4632
feat(aws): Add new CodeBuild check to validate environment variables #4632
Conversation
report.resource_arn = project.arn | ||
report.status = "PASS" | ||
report.status_extended = f"CodeBuild project {project.name} does not have sensitive environment plaintext credentials." | ||
sensitive_vars = ["AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about using the audit_config
to allow users to add/remove sensitive environment variables? It'd be a great addition!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I've just added it.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4632 +/- ##
==========================================
+ Coverage 89.06% 89.11% +0.04%
==========================================
Files 912 915 +3
Lines 27783 28016 +233
==========================================
+ Hits 24745 24966 +221
- Misses 3038 3050 +12 ☔ View full report in Codecov by Sentry. |
Please use the detect secrets library for this check like in |
But what is the point of the check? I think is not to have these sensitive environment variables, as per https://docs.aws.amazon.com/securityhub/latest/userguide/codebuild-controls.html#codebuild-2 |
We don't have to limit Prowler checks to CIS or SecurityHub controls, if we detect all type of secrets, the check would be more advance and we are also covering the SecurityHub check. It makes no sense to make the user setting in the |
@@ -0,0 +1,32 @@ | |||
{ | |||
"Provider": "aws", | |||
"CheckID": "codebuild_project_no_plaintext_credentials", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"CheckID": "codebuild_project_no_plaintext_credentials", | |
"CheckID": "codebuild_project_no_secrets_in_variables", |
@danibarranqueroo I apologise because I explained my point just the opposite as I wanted. What I wanted to say is that it could be helpful for our users to be able to exclude some environment variables via |
Finally, after following all your advises, I've completed this check using Detect Secrets to find credentials/secrets in the value of the environmental variables, but only when they are stored as plaintext. Additionally, I've used Audit Config to allow users to exclude specific environment variables from the check by adding their names to the config.yaml. Take a look at it when you can! 😄 🚀 |
"Code": { | ||
"CLI": "", | ||
"NativeIaC": "", | ||
"Other": "https://docs.aws.amazon.com/securityhub/latest/userguide/codebuild-controls.html", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Other": "https://docs.aws.amazon.com/securityhub/latest/userguide/codebuild-controls.html", | |
"Other": "https://docs.aws.amazon.com/securityhub/latest/userguide/codebuild-controls.html#codebuild-2", |
"Description": "This check ensures that AWS CodeBuild projects do not use plaintext AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY as environment variables.", | ||
"Risk": "Using plaintext AWS credentials in CodeBuild project environment variables can expose these sensitive keys, leading to unauthorized access and potential security breaches.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do this description generic, this check is not only flagging AWS credentials.
{ | ||
"Provider": "aws", | ||
"CheckID": "codebuild_project_no_secrets_in_variables", | ||
"CheckTitle": "Ensure CodeBuild projects do not contain plaintext AWS credentials", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do this title generic, this check is not only flagging AWS credentials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @danibarranqueroo ! Please, update the metadata of the check according to the new changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🏼
Context
To enhance security and follow best practices for handling sensitive information, we are adding a new check within the CodeBuild framework. This check ensures that sensitive environment variables (AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY) in CodeBuild projects are not stored as clear text credentials.
To facilitate this check, modifications were necessary in the CodeBuild service because the
batch_get_projects
method previously did not utilize this parameter. Additionally, I have renamed all the methods according to the Python naming convention.In addition, I did not use Moto for testing because the way it creates a new CodeBuild project seems not to accept environment variables, or something similar, because when I debug, they are always empty. I have tested the check using the AWS CLI and it seems to be working great, so for the test file, I had use MagicMock instead of Moto.
Description
I have added this new check called
codebuild_project_no_plaintext_credentials
with his respective test to prove that it works.License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.