tillmo / two_tiered_quiz

A multiple choice quiz with justifications for the answers, written in Django
GNU Affero General Public License v3.0
0 stars 0 forks source link

ALLOWED_HOSTS seems too broad #76

Closed MGlauer closed 4 years ago

MGlauer commented 4 years ago

ALLOWED_HOSTS is currently ['*'] - This is not the names of the client hosts that can be served, but the host names under which the application will be available. The django documentation says: "A value of '*' will match anything; in this case you are responsible to provide your own validation of the Host header". Has this been done?

MGlauer commented 4 years ago

Proposed solution: set ALLOWED_HOSTS to [] in localsettins.py and remove the missleading comment.

jelmd commented 4 years ago

Well, set to intended HOST_NAMEs that serve this application is misleading, too.

The setting contains the IPs (or FQDNs?) of the clients, which are allowed to access this service. [] implies localhost, only.

So in standalone mode [*] is usually required, if run via apache httpd [] is sufficient.

tillmo commented 4 years ago

Well, set to intended HOST_NAMEs that serve this application is misleading, too.

The setting contains the IPs (or FQDNs?) of the clients, which are allowed to access this service. [] implies localhost, only.

So in standalone mode [*] is usually required, if run via apache httpd [] is sufficient.

that is also what I thought initially, but no. This concerns the http host header sent by the client (necessarily as FQDN, IP does not make sense here, except perhaps 127.0.0.1) and not the IP of the client. Moreover, this field should be set even when using Apache, see here.

MGlauer commented 4 years ago

Oh, yes, HTTP_HOST is what I meant. The production settings of django do not allow an empty ALLOWED_HOSTS. But does 127.0.0.1 do the trick here? I am not sure how React works, but as far as I understand, it serves HTML/JS that then in turn fetches the data from the backend. Therefore, the API is accessed from the client side and not from localhost. Or does react use a data interface that I am unaware of? So, the production setting of ALLOWED_HOSTS should be the FQDNs and the IP of the distributing server.

To make it more clear: This setting is checked against the HTTP_HOST of each request processed by django. If the request's HTTP_HOST is not in ALLOWED_HOSTS, it will cause an error.

tillmo commented 4 years ago

@MGlauer as far as I understand, only FQDNs make sense here, so I would revert the addition of IPs. (Only the IP 127.0.0.1 might make sense in a development environment - but this is not what local_settings.py.template is about.)

MGlauer commented 4 years ago

Yes, this was also my approach for the OEP. Unfortunately, some bots will still try to access the server via it's IP, which causes regular server errors. I talked with @jelmd about this issue and the cleanest approach is apparently to include the IPs in this setting.

tillmo commented 4 years ago

OK, understood. (But note that in case of several vhosts, a bot using IPs will not work properly.)

jelmd commented 4 years ago

Thanx for the pointers. Django uses the Host header provided by the client to construct URLs in certain cases. - Ohhh my goodness. No more questions. That's so ill, I'm speechless.