-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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(Kafka): New Kafka AWS checks #4021
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4021 +/- ##
==========================================
- Coverage 86.51% 86.33% -0.18%
==========================================
Files 776 789 +13
Lines 24163 24707 +544
==========================================
+ Hits 20904 21332 +428
- Misses 3259 3375 +116 ☔ View full report in Codecov by Sentry. |
report.resource_arn = arn_cluster | ||
report.resource_tags = cluster.tags | ||
report.status = "FAIL" | ||
report.status_extended = f"Kafka cluster '{cluster.name}' does not have encryption at rest enabled with a CMK." |
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.
Would it be interesting to put a FAIL if it is encrypted with another type of 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.
There is only two types of managers or AWS or CUSTOMER, if the manager is AWS it FAIL and if is CUSTOMER like is reflected on check it will PASS. https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/kms/client/describe_key.html
f"Kafka cluster '{cluster.name}' has enhanced monitoring enabled." | ||
) | ||
|
||
if cluster.enhanced_monitoring == "DEFAULT": |
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.
Which are the possible values here?
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.
'DEFAULT'|'PER_BROKER'|'PER_TOPIC_PER_BROKER'|'PER_TOPIC_PER_PARTITION'
I only put DEFAULT as FAIL because every other method is an enhance over default monitoring.
"RelatedUrl": "https://docs.aws.amazon.com/msk/latest/developerguide/msk-encryption.html", | ||
"Remediation": { | ||
"Code": { | ||
"CLI": "", |
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.
"CLI": "", | |
"CLI": "https://docs.prowler.com/checks/aws/general-policies/bc_aws_general_32/#cli", |
report.resource_arn = arn_cluster | ||
report.resource_tags = cluster.tags | ||
report.status = "FAIL" | ||
report.status_extended = f"Kafka cluster '{cluster.name}' does not have encryption in transit enabled" |
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.
Please add a final dot in all the statuses extended.
report.resource_tags = cluster.tags | ||
report.status = "FAIL" | ||
report.status_extended = ( | ||
f"Kafka cluster '{cluster.name}' does not have public access disabled." |
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.
f"Kafka cluster '{cluster.name}' does not have public access disabled." | |
f"Kafka cluster '{cluster.name}' is publicly accessible." |
{ | ||
"Provider": "aws", | ||
"CheckID": "kafka_cluster_unrestricted_access_disabled", | ||
"CheckTitle": "Ensure Kafka Cluster is not publicly accessible", |
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.
Is not this confusing with the check kafka_cluster_is_public
?
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 job @puchy22 !!
Context
New checks to cover basic security best practices for Apache Kafka service managed by AWS.
Description
Added new checks with metadata and respective unit testing
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.