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(extra7141): Error handling and include missing policy #1024

Merged
merged 4 commits into from Feb 9, 2022

Conversation

lazize
Copy link
Contributor

@lazize lazize commented Feb 3, 2022

Context

Missing permission was exposing AccessDenied error message.
Also missing check was generating false positive.

Description

Add check to validate access denied when get document from SSM.
Add missing action permission to allow ssm:GetDocument.

Policy actions sorted to keep them in alphabetical order.

Issue #1016

License

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

Add check to validate access denied when get document from SSM.
Add missing action permission to allow ssm:GetDocument.
@lazize
Copy link
Contributor Author

lazize commented Feb 3, 2022

$ ./prowler -p default -f sa-east-1 -c extra7141
                          _
  _ __  _ __ _____      _| | ___ _ __
 | '_ \| '__/ _ \ \ /\ / / |/ _ \ '__|
 | |_) | | | (_) \ V  V /| |  __/ |
 | .__/|_|  \___/ \_/\_/ |_|\___|_|v2.7.0-24January2022
 |_| the handy cloud security tool

 Date: Wed Feb  2 22:05:13 -03 2022

 Color code for results:
 -  INFO (Information)
 -  PASS (Recommended value)
 -  WARNING (Ignored by whitelist)
 -  FAIL (Fix required)

 This report is being generated using credentials below:

 AWS-CLI Profile: [default] AWS API Region: [sa-east-1] AWS Filter Region: [sa-east-1]
 AWS Account: [xxxx5262] UserId: [AROAxxxx:xxxxxx]
 Caller Identity ARN: [arn:aws:sts::xxxx5262:assumed-role/xxxxx/xxxxxx]

7.141 [extra7141] Find secrets in SSM Documents - ssm [Critical]
       INFO! sa-east-1: Access Denied trying to get document
$ ./prowler -p default -f sa-east-1 -c extra7141
                          _
  _ __  _ __ _____      _| | ___ _ __
 | '_ \| '__/ _ \ \ /\ / / |/ _ \ '__|
 | |_) | | | (_) \ V  V /| |  __/ |
 | .__/|_|  \___/ \_/\_/ |_|\___|_|v2.7.0-24January2022
 |_| the handy cloud security tool

 Date: Wed Feb  2 22:05:46 -03 2022

 Color code for results:
 -  INFO (Information)
 -  PASS (Recommended value)
 -  WARNING (Ignored by whitelist)
 -  FAIL (Fix required)

 This report is being generated using credentials below:

 AWS-CLI Profile: [default] AWS API Region: [sa-east-1] AWS Filter Region: [sa-east-1]
 AWS Account: [xxxx5262] UserId: [AROAxxxx:xxxxx]
 Caller Identity ARN: [arn:aws:sts::xxxx5262:assumed-role/xxxx/xxxxx]

7.141 [extra7141] Find secrets in SSM Documents - ssm [Critical]
       PASS! sa-east-1: No secrets found in SSM Document test-prowler-leo

@lazize
Copy link
Contributor Author

lazize commented Feb 3, 2022

I couldn't make it FAIL. How it check for secret?

My test document is the one below

{
  "schemaVersion": "2.2",
  "description": "Command Document Example JSON Template",
  "parameters": {
    "Message": {
      "type": "String",
      "description": "Example",
      "default": "Hello World"
    },
    "dbpass": {
      "type": "String",
      "description": "Example",
      "default": "temp123"
    }
  },
  "mainSteps": [
    {
      "action": "aws:runPowerShellScript",
      "name": "example",
      "inputs": {
        "runCommand": [
          "Write-Output {{Message}}"
        ]
      }
    }
  ]
}

@lazize lazize changed the title [extra7141] Fix AccessDenied issue when ssm:GetDocument Fix(extra7141): Error handling and include missing policy Feb 3, 2022
Copy link
Member

@toniblyx toniblyx left a comment

Choose a reason for hiding this comment

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

This PR needs to be updated since we already added some new actions.

@lazize lazize requested a review from a team as a code owner February 4, 2022 17:54
toniblyx
toniblyx previously approved these changes Feb 4, 2022
Copy link
Member

@toniblyx toniblyx left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Thanks @lazize, please check above comments in order to merge this PR.

if [[ $SSM_DOCS ]];then
for ssmdoc in $SSM_DOCS; do
SSM_DOC_FILE="$SECRETS_TEMP_FOLDER/extra7141-$ssmdoc-$regx-content.txt"
$AWSCLI $PROFILE_OPT --region $regx ssm get-document --name $ssmdoc --output text --document-format JSON > $SSM_DOC_FILE
$AWSCLI $PROFILE_OPT --region $regx ssm get-document --name $ssmdoc --output text --document-format JSON > $SSM_DOC_FILE 2>&1
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
$AWSCLI $PROFILE_OPT --region $regx ssm get-document --name $ssmdoc --output text --document-format JSON > $SSM_DOC_FILE 2>&1
"${AWSCLI}" $PROFILE_OPT --region "${regx ssm}" get-document --name "${ssmdoc}" --output text --document-format JSON > "${SSM_DOC_FILE}" 2>&1

if [[ $SSM_DOCS ]];then
for ssmdoc in $SSM_DOCS; do
SSM_DOC_FILE="$SECRETS_TEMP_FOLDER/extra7141-$ssmdoc-$regx-content.txt"
$AWSCLI $PROFILE_OPT --region $regx ssm get-document --name $ssmdoc --output text --document-format JSON > $SSM_DOC_FILE
$AWSCLI $PROFILE_OPT --region $regx ssm get-document --name $ssmdoc --output text --document-format JSON > $SSM_DOC_FILE 2>&1
if [[ $(grep -E 'AccessDenied|UnauthorizedOperation|AuthorizationError' "$SSM_DOC_FILE") ]]; then
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
if [[ $(grep -E 'AccessDenied|UnauthorizedOperation|AuthorizationError' "$SSM_DOC_FILE") ]]; then
if [[ $(grep -E 'AccessDenied|UnauthorizedOperation|AuthorizationError' "${SSM_DOC_FILE}") ]]; then

$AWSCLI $PROFILE_OPT --region $regx ssm get-document --name $ssmdoc --output text --document-format JSON > $SSM_DOC_FILE
$AWSCLI $PROFILE_OPT --region $regx ssm get-document --name $ssmdoc --output text --document-format JSON > $SSM_DOC_FILE 2>&1
if [[ $(grep -E 'AccessDenied|UnauthorizedOperation|AuthorizationError' "$SSM_DOC_FILE") ]]; then
textInfo "$regx: Access Denied trying to get document" "$regx"
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
textInfo "$regx: Access Denied trying to get document" "$regx"
textInfo "${regx}: Access Denied trying to get document" "${regx}"

@jfagoagas
Copy link
Member

$PROFILE_OPT shouldn't be enclosed within "${}" because it contains spaces and we need to extract both values.

@jfagoagas jfagoagas added the status/awaiting-reponse Waiting response from Issue owner label Feb 8, 2022
@jfagoagas jfagoagas removed the status/awaiting-reponse Waiting response from Issue owner label Feb 8, 2022
@jfagoagas jfagoagas self-requested a review February 8, 2022 09:57
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.

Thank you @lazize !!

@jfagoagas jfagoagas added the status/waiting-for-revision Waiting for maintainer's revision label Feb 8, 2022
@toniblyx toniblyx merged commit 9b772a7 into prowler-cloud:master Feb 9, 2022
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.

None yet

3 participants