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

fix(check32): filterName base64encoded to avoid space problems in filter names #1020

Conversation

n4ch04
Copy link
Contributor

@n4ch04 n4ch04 commented Feb 2, 2022

Context

Prowler fails in check32 when metric filter names include spaces, interpreting each single word of the name as a different metric filter name. This generates false positives from non existing metric filter names - #745

Description

To avoid this metric filter name is base64 encoded in check3x file, decoding it when its needed ( aws api calls and display messages)

License

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

@n4ch04 n4ch04 self-assigned this Feb 2, 2022
Copy link
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!!

Please, check below suggestions/comments to simplify commands and quote variable names

include/check3x Outdated
@@ -39,11 +39,12 @@ check3x(){
if [ "$CLOUDWATCH_LOGGROUP_ACCOUNT" == "$CURRENT_ACCOUNT_ID" ];then
# Filter control and whitespace from .metricFilters[*].filterPattern for easier matching later
METRICFILTER_CACHE=$($AWSCLI logs describe-metric-filters --log-group-name "$CLOUDWATCH_LOGGROUP_NAME" $PROFILE_OPT --region "$CLOUDWATCH_LOGGROUP_REGION"|jq '.metricFilters|=map(.filterPattern|=gsub("[[:space:]]+"; " "))')
METRICFILTER_SET=$(echo $METRICFILTER_CACHE | jq -r --arg re "$grep_filter" '.metricFilters[]|select(.filterPattern|test($re))|.filterName')
METRICFILTER_SET=$(echo $METRICFILTER_CACHE | jq -r --arg re "$grep_filter" '.metricFilters[]|select(.filterPattern|test($re))|.filterName|@base64')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
METRICFILTER_SET=$(echo $METRICFILTER_CACHE | jq -r --arg re "$grep_filter" '.metricFilters[]|select(.filterPattern|test($re))|.filterName|@base64')
METRICFILTER_SET=$(echo "${METRICFILTER_CACHE}" | jq -r --arg re "${grep_filter}" '.metricFilters[]|select(.filterPattern|test("${re}"))|.filterName|@base64')

include/check3x Outdated
fi
if [[ $METRICFILTER_SET ]];then
for metric in $METRICFILTER_SET; do
metric_name=$(echo $METRICFILTER_CACHE | jq -r --arg name $metric '.metricFilters[]|select(.filterName==$name)|.metricTransformations[0].metricName')
metric_decode=$(echo $metric | base64 -d)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
metric_decode=$(echo $metric | base64 -d)
metric_decode=$(base64 -d <<< "${metric}")

include/check3x Outdated
fi
if [[ $METRICFILTER_SET ]];then
for metric in $METRICFILTER_SET; do
metric_name=$(echo $METRICFILTER_CACHE | jq -r --arg name $metric '.metricFilters[]|select(.filterName==$name)|.metricTransformations[0].metricName')
metric_decode=$(echo $metric | base64 -d)
metric_name=$(echo $METRICFILTER_CACHE | jq -r --arg name "${metric_decode}" '.metricFilters[]|select(.filterName==$name)|.metricTransformations[0].metricName')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
metric_name=$(echo $METRICFILTER_CACHE | jq -r --arg name "${metric_decode}" '.metricFilters[]|select(.filterName==$name)|.metricTransformations[0].metricName')
metric_name=$(echo "${METRICFILTER_CACHE}" | jq -r --arg name "${metric_decode}" '.metricFilters[]|select(.filterName=="${name}")|.metricTransformations[0].metricName')

include/check3x Outdated
@@ -61,15 +62,15 @@ check3x(){

if [[ $CHECK_OK ]]; then
for group in $CHECK_OK; do
metric=${group#*:}
metric=$(echo ${group#*:} | base64 -d)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
metric=$(echo ${group#*:} | base64 -d)
metric=$(base64 -d <<< "${group#*:}")

include/check3x Outdated
group=${group%:*}
textPass "$REGION: CloudWatch group $group found with metric filter $metric and alarms set" "$REGION" "$group"
done
fi
if [[ $CHECK_WARN ]]; then
for group in $CHECK_WARN; do
case $group in
*:*) metric=${group#*:}
*:*) metric=$(echo ${group#*:} | base64 -d)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*:*) metric=$(echo ${group#*:} | base64 -d)
*:*) metric=$(base64 -d <<< "${group#*:}")

@n4ch04 n4ch04 force-pushed the PRWLR-26-fix-check-32-fails-when-metric-name-contain-spaces-in-it-745 branch from 16b22c7 to 9a84dd6 Compare February 2, 2022 10:58
@jfagoagas jfagoagas linked an issue Feb 2, 2022 that may be closed by this pull request
@jfagoagas jfagoagas added the status/waiting-for-revision Waiting for maintainer's revision label Feb 2, 2022
@toniblyx toniblyx merged commit d9561d5 into master Feb 2, 2022
@toniblyx toniblyx deleted the PRWLR-26-fix-check-32-fails-when-metric-name-contain-spaces-in-it-745 branch February 2, 2022 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/waiting-for-revision Waiting for maintainer's revision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix check32: fails when metric name contain spaces in it
3 participants