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

Misc prowler fixes #407

Merged
merged 1 commit into from
Nov 20, 2019
Merged

Conversation

zfLQ2qx2
Copy link
Contributor

  • Add GetEbsEncryptionByDefault wherever Prowler policies are mentioned
  • Update Extra718 check to be aware of access denied responses
  • Update Extra726 check to be more verbose for non-failure items
  • Update Extra73 check to be aware of access denied responses
  • Update Extra734 check to be aware of access denied responses and parse policies with jq for better accuracy
  • Update Extra742 check for verbiage
  • Update Extra756 check for verbiage and parameter order
  • Update Extra761 check for failure scenarios (requires most recent awscli and addition to Prowler IAM policy)
  • Added Extra763 check to verify that object versioning is enabled on S3 buckets
  • Added Extra764 check to verify that S3 buckets enforce a secure transport policy

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

@zfLQ2qx2
Copy link
Contributor Author

@toniblyx I've opened a new pull request for Prowler

@toniblyx
Copy link
Member

wow! Thanks for this amazing PR @zfLQ2qx2 I´ll review it as soon as I can. I gave it a quickly review and it looks awesome, thanks!

@toniblyx
Copy link
Member

Your suggested extra764 seems to do the same as extra734

@zfLQ2qx2
Copy link
Contributor Author

zfLQ2qx2 commented Nov 15, 2019

Your suggested extra764 seems to do the same as extra734

@toniblyx extra764 checks for a secure transport policy (denying S3 requests made over http) and extra734 is checking for server side encryption policy. What I did for both was instead of the awk stuff in the original extra734 I used jq to parse the bucket policy and do a little stronger validation. For example, in the case of check764, its not enough to have an aws:SecureTransport - false condition, it has to be a Deny policy, for all s3 actions, applying to all principles. Its still just jq - someone could iterate all possible s3 actions individually and while technically correct the check would still report failure, but I think that person can just be unhappy. ;) The check for extra734 is similarly precise but could be toned down to the original just check for the condition if desired.

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.

Awesome!

@toniblyx toniblyx merged commit d737193 into prowler-cloud:master Nov 20, 2019
@zfLQ2qx2 zfLQ2qx2 deleted the prowler_misc_fixes branch November 21, 2019 06:41
@jansepke
Copy link
Contributor

@zfLQ2qx2 I have one stupid question to this PR why is extra734 now enforcing an S3 policy for denying every request without a s3:x-amz-server-side-encryption:true header? I cannot find a documentation stating that you need that header if you have default encryption enabled. Wouldn't it be better to make this a separate check or checking for a policy that denies requests WITH a s3:x-amz-server-side-encryption:false header?

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