aws-samples / experimental-programmatic-access-ccft

Experimental Programmatic Access to the AWS Customer Carbon Footprint Tool data
MIT No Attribution
28 stars 8 forks source link

Add S3 Versioning - Closes #12 #13

Closed xnick123 closed 3 months ago

xnick123 commented 3 months ago

07/08/2024 corrected from [S3.10] S3 general purpose buckets with versioning enabled should have Lifecycle configurations to [S3.14] [S3 general purpose buckets should have versioning enabled](https://docs.aws.amazon.com/securityhub/latest/userguide/s3-controls.html#s3-14)

Issue #12 :

Description of changes:

Adds versioning to the S3 Buckets to resolve the AWS Securityhub finding: 07/08/2024 corrected from [S3.10] S3 general purpose buckets with versioning enabled should have Lifecycle configurations to [S3.14] [S3 general purpose buckets should have versioning enabled]

07/08/2024 readded: By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

steffeng commented 3 months ago

Thanks @xnick123! The permanent deletion of the Amazon Athena query results bucket is intended for cost reduction reasons.

After you enabled versioning, what did you do about the "[S3.10] S3 general purpose buckets with versioning enabled should have Lifecycle configurations" message mentioned above in your case?

You must keep this in the PR description (refers to your second PR as well):

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

xnick123 commented 3 months ago

Hi Steffen,

my fault, this was intended to cover: [S3.14] S3 general purpose buckets should have versioning enabled I adjusted the description, hope that is ok. I'd also add that legal term which i deleted.

This was just the initial step, in my queue i had planned to add

steffeng commented 3 months ago

Thanks @xnick123, see my comment in the corresponding issue. We don't want to make a decision for versioning and retention on behalf of customers - or move CFN configuration to optional parameters. We however implemented your suggestions of S3.5/8.

xnick123 commented 3 months ago

Hi @steffeng thank you for putting much effort to implement the feedback, even implement static application security testing. You certainly raise the bar! I appreciate that you both shared your professional work due to the missing official Api.

Securityhub is still reporting the issues though. For me raising the bar and increasing the customer focus would include letting the customer decide whether she wants to enable both versioning and access logs. I know that adding more parameters increases complexity, and that you should decide the best initial set of parameters. This is why default parameters exist.

I now have to clone the repo,make amendments through scripts and keep it in sync with any of your great future improvements, without being able to pull from the repo easily. If it would be parametrized, I could do it with ease by overwriting the parameter file.

Hence I think your arguments are valid but focused on your current scope. I do know this is experimental and best effort, hence I'll live with your decision.

I am big advocate of AWS Securityhub, but wonder if/how it’s being used at AWS extensively, as situations like this make me doubt. The solution can’t be me writing rules to suppress this finding..

steffeng commented 3 months ago

Thanks @xnick123 for the feedback and sorry for the inconvenience. For context, why don't you suppress the findings in AWS Security Hub that don't make sense (like versioning of the Athena results bucket)? Will you implement any of the findings we suppressed in CFN guard for any other reason than satisfying the security hub control?

xnick123 commented 3 months ago

@steffeng I'll share my thoughts, thank you for your interested in! I understand your perspective now. I am not sure how you experience customers using AWS Securityhub. Securityhub's capabilities are built for either standalone or centralized usage. We are in a centralized environment and as account owner i have to request any suppression from a central security team, I cannot suppress them on my own.

This is why I prefer to fix the underlying root cause, to comply with both AWS default suggestion and with my company's guardrails.

I agree that we could debate if one or the other security control is really required in each use case.

Anyway no hard feelings, I always appreciate having good debates about how far we raise the bar.