mozmeao / django-allow-cidr

A Django Middleware to enable use of CIDR IP ranges in ALLOWED_HOSTS.
Other
97 stars 15 forks source link

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

Open mlissner opened 2 years ago

mlissner commented 2 years ago

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 commented 2 years ago

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. https://github.com/mozmeao/django-allow-cidr/issues/12#issuecomment-607409299 if you are using k8s. This does remove the need for django-allow-cidr if you can set it up.

mlissner commented 2 years ago

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 commented 2 years ago

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:

What do you think?

mlissner commented 2 years ago

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