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

Should the docs note that ELBs can mitigate this? #21

Open
mlissner opened this issue May 12, 2022 · 4 comments
Open

Should the docs note that ELBs can mitigate this? #21

mlissner opened this issue May 12, 2022 · 4 comments

Comments

@mlissner
Copy link

Hi, thanks for the great package. I arrived here today after running into health check errors from my ELB and from my k8s cluster, and I was about to add this package to my app when I realized that you can do HOST checking at the ELB.

If a lot of people are coming here to allow health checks, would it be reasonable to add something to the docs that says, "If you're using an ELB, one solution is to only forward traffic for a particular HOST, and to set ALLOWED_HOSTS to ["*"]"?

If that'd be helpful, I'm happy to make that contribution to the docs. It seems like an easier and probably more performant way to do what this package does?

@n1ngu
Copy link
Contributor

n1ngu commented Jun 14, 2022

Maybe I am overthinking it, but the approach you suggest could still lead to insecure deployments if the application accidentally became the default site for your load balancer, or if the backend happened to feature a reachable public IP, to name some simple threat scenarios that came to my mind.

I think the only alternative that is worth considering is configuring your health probes, or whatever system that requests via IP, to craft an HTTP Host header with any of the whitelisted domains. E.g. #12 (comment) if you are using k8s. This does remove the need for django-allow-cidr if you can set it up.

@mlissner
Copy link
Author

That's definitely a better solution. I'll check that out and see if a contribution to the docs for that makes sense! Thanks for the thoughts and reference.

@mlissner
Copy link
Author

OK, took a deeper look at this today, and realized that setting the HOST header on k8s's healthchecks isn't going to be enough because ELBs send health check probes too, which can't have the header set. I think that means I'm back to a wide open ALLOWED_HOST setting with the ELB only forwarding traffic that matches the correct HOST.

I appreciate defense in depth and this removes a layer of it, but I think it's safe provided that your application servers are only ever accessible from the ELB (which is a pretty normal state of affairs).

So I think it still might be useful to add a note as above that says:

  • You may not need this if:
  • You are using an ELB, and
  • You never allow access to your application servers, and
  • You set your ALLOWED_HOST to *.

What do you think?

@mlissner
Copy link
Author

Oh, and FWIW, when I researched this back in May, I made some notes on it here: freelawproject/courtlistener#2059 (probably worth tying these together for others that land here).

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

No branches or pull requests

2 participants