ipfs / infra

Tools and systems for the IPFS community
MIT License
133 stars 41 forks source link

gateway: redirect to HTTPS and fix HSTS header #342

Closed ghost closed 7 years ago

ghost commented 7 years ago

Except for requests for deeplinks like /ipfs, /ipns, and /api. We don't want to break existing clients that forgot to use HTTPS. This definitely isn't the last word on redirecting deeplinks.

We also stop sending the HSTS header over plaintext HTTP, and start sending it on vhosts that aren't gateway.ipfs.io or ipfs.io.

Also make h.ipfs.io its own vhost.

cc @Kubuxu @kpcyrd

ghost commented 7 years ago

Already deployed this, leaving open for review

kpcyrd commented 7 years ago

@lgierth awesome!

Could you bump 15768000 (6m) to 31536000 (1y)? The minimum requirement for preloading got increased recently.

ghost commented 7 years ago

Okay will do tomorrow.


On a different note, it looks like this breaks cases where people point only A/AAAA/CNAME records at the gateway, but without setting a dnslink TXT record -- i.e. when they re-label our gateway from ipfs.io to e.g. cdn.example.com and request e.g. http://cdn.example.com/ipfs/QmFoo. They'd get redirected to https://cdn.example.com which we don't have certificates for.

I think I'm okay with that. Thoughts @kyledrake?

kyledrake commented 7 years ago

CNAMEing stuff on the gateway is clever, but I don't think it's a good idea. The only way that works is if the connection stays unencrypted. Also HSTS preload is very important and needed. :+1: on this.

kyledrake commented 7 years ago

I would recommend adding this to nginx.conf:

add_header Upgrade-Insecure-Requests "1";

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Upgrade-Insecure-Requests

That said really this should be done in a CSP, that's where it's being standardized now.

jbenet commented 7 years ago

@lgierth i believe lots of CLI tools use http: and do not have support for https.

I think that http -> https should be mandatory in browsers, but optional in CLI tools.

kyledrake commented 7 years ago

@jbenet HSTS preload requires a forced redirect. https://en.m.wikipedia.org/wiki/HTTP_Strict_Transport_Security

Any http client/cli worth it's salt has an option to "follow redirects", requiring developers to only make a small change to their cli usage or code to make things continue to work. They could also add the missing "s".

Not having HSTS preload is a genuine security concern, and IMHO strongly justifies this change.

jbenet commented 7 years ago

Any http client/cli worth it's salt has an option to "follow redirects", requiring developers to only make a small change to their cli usage or code to make things continue to work.

No! not all clients have https support, even. Do not assume this-- and do not break usage. Do not break userspace!



I think either:

But whatever it is, please be very careful to NOT break userspace. This change broke it and that should be fixed.

ghost commented 7 years ago

@lgierth i believe lots of CLI tools use http: and do not have support for https.

  • does this change break that usage? (breaking lots of links if so)
  • is this break warranted?

I think that http -> https should be mandatory in browsers, but optional in CLI tools.

Deep links to /ipfs /ipns and /api don't get the redirect, that's sufficient imo - browser detection is a road I'd prefer we don't go down, it's very fragile.

As a follow up to the CNAME case I've disabled the redirect for any domains that we don't have certs for.

kpcyrd commented 7 years ago

@jbenet the hsts deployment has rather strict rules compared to traditional https deployments. Those rules are set by mozilla/google and require that the first domain after the public suffix passes some tests (.io being the public suffix, so ipfs.io is going to be tested). All subdomains get the same level of protection afterwards, but don't need to pass those tests (eg. gateway.ipfs.io doesn't need any changes in theory).

In theory the "upgrade anything to https first" should be done for the whole zone, but since only / is tested the solution by @lgierth would nicely work around this, I doubt that cli tools send requestes directly to /. :)

Personally I think local hashing is the exception and the only way to properly protect the 99% is getting hsts preloaded. For the local hashing usecase it might make sense to have an explicit insecure endpoint.

The current deployment fulfills all requirements, only the hsts max-age needs to be increased to a year.

ghost commented 7 years ago

Ooops, this has been sitting here for a while.

The end result is: HTTPS redirects now happen if:

Redirects don't happen if:

kpcyrd commented 7 years ago

@lgierth max-age is still to low to get properly preloaded :)

The max-age must be at least 31536000 seconds (≈ 1 year), but the header currently only has max-age=15768000.

https://hstspreload.org/?domain=ipfs.io

ghost commented 7 years ago

Fixed in #345 :+1: