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

Find trust boundaries in VPC endpoint services #511

Conversation

renuez
Copy link

@renuez renuez commented Mar 4, 2020

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

It's my first proposal on a series of potential new checks that find trust boundaries in VPC endpoint services (whitelisted principals and connections).

I do also have one for VPC peerings and I currently work on another one for IAM roles.
I think there are many interesting use cases to compare a known whitelist of accounts against KMS usage/admin policies, S3 bucket policies or ACLs, ...

Maybe it's only interesting for multi-account setups with density and cohesion of AWS account trusts - what do you think of a dedicated new check group?

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.

Thanks @renuez, let me add here some comments and changes I would suggest to make before merging your code:

  • $ACCOUNT_NUM is a variable already set that you can use instead of $THIS_ACCOUNT_ID.
  • Could you comment the code? Some parts are self explanatory but others are less easy to understand, at least for me. That will help others to improve it and create other checks based on these ones.
  • It would be also helpful to add information about the use case in the comments as well as the syntax for the whitelist files. Btw, why not to have comma separated values in a variable inside the check file? or an array?

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.

also, each aws command requries the variable $PROFILE_OPT that includes options if different profile is loaded.

@renuez
Copy link
Author

renuez commented Mar 31, 2020

@toniblyx Thanks for your recent feedback and code review support.
All amendments build into feature branch now.
Ready for you final review.

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

toniblyx added a commit that referenced this pull request Apr 8, 2020
@toniblyx
Copy link
Member

This pull request has been already applied, I had to do a workaround with conflicts. Thanks @renuez

@toniblyx toniblyx closed this Apr 13, 2020
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.

2 participants