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 the rules publication Terraform configuration #742

Merged
merged 5 commits into from
Dec 29, 2023

Conversation

mcdonnnj
Copy link
Member

🗣 Description

This pull request updates the configuration in terraform_egress_pub/ that publishes the rules site listing the IP ranges we publish that are in use for assessments. This includes:

  • Removing the S3 static website configuration.
  • Adding an OAC (Origin Access Control) for the CloudFront distribution.
  • Adding a policy to the rules S3 bucket to allow read access from the OAC now associated with the CloudFront distribution.

💭 Motivation and context

When #654 was merged I apparently neglected to apply the changes in the terraform_egress_pub/ configuration. As a result a bug went unnoticed until @jsf9k applied his changes for #738 and suddenly the rules site was no longer working. I took a look and realized that the problem was due to the fact that although the bucket was set to use the private canned ACL, the script that uploads objects to the bucket would set each uploaded object to use the public-read canned ACL. With the addition of a aws_s3_bucket_public_access_block for the rules S3 bucket that disallowed any public access it broke access to the objects in the bucket. Adding an OAC and appropriate policy to the bucket allows the CloudFront distribution to access the S3 bucket even if it is completely private.

The static website configuration was removed because

  1. The bucket is now completely private.
  2. The CloudFront distribution handles the same functionality making it superfluous.

🧪 Testing

Automated tests pass. I applied this in production and verified that the rules site works once more.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All new and existing tests pass.

@mcdonnnj mcdonnnj added bug This issue or pull request addresses broken functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use terraform Pull requests that update Terraform code labels Dec 28, 2023
@mcdonnnj mcdonnnj self-assigned this Dec 28, 2023
Copy link
Member

@jsf9k jsf9k 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 for fixing this!

Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

👍 👍 Thanks for dragging us into the future, kicking and screaming all the way!

Since we are serving this content with a CloudFront distribution now we
no longer need the S3 bucket configured as a static website.
Create an Origin Access Identity and configure the CloudFront
distribution for the rules bucket to use the newly created OAI.
Add a policy that will allow the CloudFront distribution to read from
the rules bucket. This should allow hte distribution to correctly
access the bucket's contents even though it is completely private.
The bucket that is storing these lists is now set to disallow any ACLs
or policies that allow public access. Attempting to set an object that
has been uploaded to `public-read` will throw an authorization error as
a result.
Origin Access Control is the newer, improved way to secure S3 origins
when using CloudFront. Since it effectively replaces Origin Access
Identity it makes sense to use it instead.
@mcdonnnj mcdonnnj force-pushed the bug/adjust_rules_bucket_access branch from 47fc163 to 0904ddd Compare December 29, 2023 03:59
@mcdonnnj mcdonnnj merged commit bd817ec into develop Dec 29, 2023
8 checks passed
@mcdonnnj mcdonnnj deleted the bug/adjust_rules_bucket_access branch December 29, 2023 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue or pull request addresses broken functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use terraform Pull requests that update Terraform code
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants