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

Added real IP and block IP functionality. #714

Merged
merged 2 commits into from
Jul 15, 2020
Merged

Conversation

auspicacious
Copy link
Contributor

Related to issue #648

Based on research from @ZacharyCChang0828

This will enable us to block IP addresses or CIDR ranges by modifying the web.config file and redeploying.

I have manually applied these changes in dev, so I know that the config works, but I'm not sure the .ebextensions syntax is correct.

Risks:

  • Listing these IP address publicly could theoretically aid an attacker, but I'm not sure how.
  • Using the real_ip module will change our logs; the AWS load balancer IP address will no longer show up in them. Instead, the request IP address will be the real one.

Follow-up tasks:

  • We need to review and document what steps need to be taken to deploy an .ebextensions change. I'm not sure an ordinary deploy will do it.
  • We should make the same changes to ingest.

@matschaffer
Copy link
Contributor

matschaffer commented Jul 14, 2020

We could merge like this but I'm fairly certain nginx needs a reload signal to pick up the changes. So a normal deploy probably wouldn't do the trick.

We could add a command step after this (similar to match_nginx_timeout_to_elb_timeout) to hup nginx, or look to see if there's some way to have it notice the file changes.

@matschaffer
Copy link
Contributor

If we ever wanted to hide the IPs or share with ingest we could put an s3 object somewhere and build the list from that.

Copy link
Contributor

@matschaffer matschaffer left a comment

Choose a reason for hiding this comment

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

Would be fine merging this. As you noted, it won't be perfect w/o some plan on reloading or config sharing. But we can iterate.

@auspicacious
Copy link
Contributor Author

auspicacious commented Jul 14, 2020

@matschaffer web.config already contains service nginx restart in match_nginx_timeout_to_elb_timeout, will that execute again?

@matschaffer
Copy link
Contributor

matschaffer commented Jul 14, 2020 via email

Copy link
Collaborator

@sasharevzin sasharevzin left a comment

Choose a reason for hiding this comment

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

I don’t understand much in it, so I’ll just go ahead and approve, hehe

@auspicacious
Copy link
Contributor Author

I'm going to make another build with my own IP address in the block list and deploy that to make sure it works, then will merge.

@auspicacious
Copy link
Contributor Author

Bit of confusion, I was pushing to the wrong config directory, but after that's fixed I've confirmed that a redeploy to the same environment should update the blocklist.

@auspicacious auspicacious merged commit 1ab8790 into master Jul 15, 2020
@auspicacious auspicacious deleted the block-ips-nginx branch July 15, 2020 02:32
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.

4 participants