hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.18k stars 2.77k forks source link

Option to force SSL #2539

Open lukaspili opened 5 years ago

lukaspili commented 5 years ago

Platforms like heroku require to handle SSL redirect at the application level. It would be nice to have an option on Hasura like HASURA_FORCE_SSL = true which would force all traffic through https.

dsandip commented 5 years ago

@shahidhk Is this similar to what we were discussing for another user recently?

woba-nwa commented 5 years ago

Following a full-scale pen-test of our systems, this as been identified as a problem for us too. Any update on where this issue is in the likely long list of priorities and when (if at all) we could expect this type of functionality to be released? Is there maybe a temporary work-around to this problem?

0x777 commented 5 years ago

@woba-nwa Is your requirement similar to https://github.com/hasura/graphql-engine/issues/2848 or do you expect graphql-engine to terminate ssl connections?

woba-nwa commented 5 years ago

The two scenarios are in my view different solutions to the same problem, so yes. I do find the environment variable to be the most user-friendly solution however.

The basic need is just to prevent unencrypted traffic to the Hasura server when on Heroku, where you cannot control these things on domain level, e.g. via CloudFlare (all servers have the [app name].herokuapp.com subdomain, which cannot be forced SSL nor deleted).

In other apps, we have created a function that generates a 301 redirect to HTTPS if HTTP and domain contains herokuapp.com. Custom domains have forced SSL by CloudFlare.

0x777 commented 5 years ago

Unfortunately 301 redirects and POST requests don't play well together. Recently 308 redirects are introduced to solve this, however all http clients may not handle 308 redirects as expected. Hence the only place where a 301 redirect make sense is with the console assets.

Another issue with redirects is in determining the Location to which the request has to be redirected. We can at best issue a redirect to https://{host}{path} in the request handler. This will be incorrect if you have proxies which are rewriting the url.

I'm thinking of having a configuration option like HTTP_PROTO_CONF which takes two values: disabled and https_redirect. disabled would deny the request and https_redirect would issue a 301 in case of GET requests and 308 in case of POST requests determining the location using Host header and the request path.

Thoughts? @lexi-lambda @shahidhk

lexi-lambda commented 5 years ago

One data point I know of: Ruby on Rails has a force_ssl feature that is quite commonly used, and I imagine it is fairly robust. Its behavior is to simply issue 307 Temporary Redirect responses for non-GET or HEAD requests. (Weirdly, Rails sends 301 Moved Permanently responses for GET/HEAD requests, but 307 Temporary Redirect responses for other requests. Looking into the history of the feature, that decision seems semi-arbitrary, so we should should probably use 301 + 308, instead.)

That feature has existed in Rails for four years, and it existed in rack-ssl for even longer. Browser support for 307/308 responses is good, and for ancient systems that don’t support them, it’s unlikely to matter very much, anyway, since it’s very rare for someone to make a non-HTTPS POST request. (Normally, they navigate to a non-HTTP webpage via an ordinary GET, then get upgraded to HTTPS at that point.)

For the Location header, I believe it’s safe to trust X-Forwarded-Host and X-Forwarded-Proto, falling back to Host otherwise. In general, trusting X-Forwarded-Host can be dangerous, since it can open vulnerabilities to host header attacks. However, in this case, that isn’t a problem, since redirecting someone somewhere else does not, to the best of my knowledge, have any problems. The usual open redirect vulnerability also doesn’t apply because the host isn’t taken from a URL, so it can’t be embedded in a link.

Note that Rails’s force_ssl option also enables Secure cookies and HSTS. I don’t believe graphql-engine sets any cookies, so that part doesn’t matter, but we should set the HSTS header.

shahidhk commented 5 years ago

I think we should only do these changes along with support for SSL/TLS, where one can provide a key-cert and get HTTPS without a reverse proxy. The entire force_ssl story fits together now. Otherwise, unless you're on managed platforms like heroku, you'd still need a reverse proxy.

lexi-lambda commented 5 years ago

I think there’s still a lot of value in providing HTTPS-forcing without doing SSL/TLS termination. “People running Hasura on Heroku” is a pretty valid use case, and in that context, SSL/TLS termination is actually totally unhelpful.

I feel like the people who are most likely to be unable to run a reverse proxy to issue the redirects themselves are people running in managed environments like Heroku, anyway. I have a hard time imagining all that many use cases where people are exposing Hasura directly to the internet and want to use HTTPS, but maybe I just have tunnel vision.

shahidhk commented 5 years ago

Fair point. Heroku asks users to configure redirects are the application end: https://help.heroku.com/J2R1S4T8/can-heroku-force-an-application-to-use-ssl-tls and 308 seems to be a good idea: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308

tovbinm commented 5 years ago

I would be great to have this ;) (especially useful for Heroku - https://help.heroku.com/J2R1S4T8/can-heroku-force-an-application-to-use-ssl-tls)

dionjwa commented 5 years ago

Use case: I serve Hasura on Google Kubernetes, using the new ingress object. It's great, since it manages certificates and routing to services, and terminates the SSL connection. But it does not support automatic redirecting http->https, even though it probably should: https://medium.com/google-cloud/google-cloud-platform-redirecting-http-traffic-to-https-f63c1d7dbc6d

This could potentially handled by the auth webhook, but that is limited in the status code it can return. So I would have to set up an nginx intermediate just for this use case, which is annoying.

tejasmanohar commented 4 years ago

This should at least be supported for the Hasura console

ItsWendell commented 4 years ago

Would love to see support for this too, also hosting on Heroku. https connections work but if I by accident visit a http link it is still allowed, looking for a solution that I can apply to my docker container directly.

marindrag commented 2 years ago

Hi @0x777 and hasura team, thanks for your awesome stuff! It has been quite long since this issue opened. I also have issue. Any update?