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

Support HTTPS redirect #4641

Closed
agjohnson opened this issue Sep 17, 2018 · 24 comments · Fixed by #6905
Closed

Support HTTPS redirect #4641

agjohnson opened this issue Sep 17, 2018 · 24 comments · Fixed by #6905
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Milestone

Comments

@agjohnson
Copy link
Contributor

agjohnson commented Sep 17, 2018

This is a placeholder issue, we've had a few requests to enable HTTPS always on, and hey why not HSTS as well, when HTTPS is checked on the project domain admin dashboard page.

Edit: for HSTS, see #4135 instead of this issue.

@agjohnson agjohnson added Improvement Minor improvement to code Accepted Accepted issue on our roadmap labels Sep 17, 2018
@agjohnson agjohnson added this to the 2.8 milestone Sep 17, 2018
@davidfischer
Copy link
Contributor

I think we could turn on HSTS on readthedocs.org and readthedocs.io without issues.

I'm not sure we should do this for custom domains at all. I'm not strictly opposed to it, but I'm not sure we should do it, either. Turning on HSTS with a long max-age (a short max-age isn't useful) is a one-way street. You can't really go back. Once somebody's browser has received a response with HSTS for docs.example.com, that browser will never request that site over plain HTTP again. Clearing the HSTS settings for a browser is non-trivial.

@agjohnson
Copy link
Contributor Author

Yup, we've certainly hit this before. Let's focus on https first, we can come back to HSTS.

@ba11b0y
Copy link
Contributor

ba11b0y commented Oct 1, 2018

@agjohnson Would like to take this up. Any pointers as to where to start?

@stsewd
Copy link
Member

stsewd commented Oct 1, 2018

@invinciblycool I think this can be solved only for the core team, as they have access to the servers

@humitos
Copy link
Member

humitos commented Feb 12, 2019

What's the status of this issue? It seems the only missing thing is enabling HSTS.

In another PR, @davidfischer implemented PUBLIC_DOMAIN_USES_HTTPS = True and https on custom domains, so we are maybe ready for HSTS?

@davidfischer
Copy link
Contributor

In another PR, @davidfischer implemented PUBLIC_DOMAIN_USES_HTTPS = True and https on custom domains, so we are maybe ready for HSTS?

I think we may never do HSTS for custom domains. HSTS for readthedocs.io can roll out anytime. For .org, it is nearly ready (see the private ops issue https://github.com/rtfd/readthedocs-ops/issues/294)

The harder one is redirecting custom domain traffic HTTP -> HTTPS. To do this, we need to know that the domain is active and the certificate is deployed. Many custom domains are not setup correctly and don't have a certificate. Redirecting those would be bad.

I see two ways to do HTTP -> HTTPS for custom domains:

  • Write a file (.https or something like it) and nginx can redirect if it sees that file
  • Write a database entry when the certificate is setup correctly (or just repurpose the https flag). Then we would need to change our logic such custom domains always hit django rather than just hitting nginx. We might be able to only hit django when the domain is requested over HTTP only and HTTPS doesn't hit django.

Do you see any other way?

@JLLeitschuh
Copy link

This should be considered a security vulnerability in RTFD.

Developers utilizing RTFD sometimes link to their releases which could be maliciously MITM to link to compromised artifacts.

Additionally, there are vulnerabilities that can be executed by abusing the Copy-Paste functionality in browsers to hide unexpected payloads, for example when copying shell commands.

Here's an example:
https://thejh.net/misc/website-terminal-copy-paste

@davidfischer
Copy link
Contributor

@JLLeitschuh There's a couple things going on here:

This should be considered a security vulnerability in RTFD.

I'm all for running the entire site over HTTPS and I do believe that would increase security and privacy. However, I would not call running a static website over HTTP a security vulnerability.

Developers utilizing RTFD sometimes link to their releases which could be maliciously MITM to link to compromised artifacts.

Can you go into a little more details here?

Additionally, there are vulnerabilities that can be executed by abusing the Copy-Paste functionality in browsers to hide unexpected payloads, for example when copying shell commands.

What does this have to do with an HTTPS redirect?

@humitos
Copy link
Member

humitos commented Mar 11, 2019

HSTS for readthedocs.io can roll out anytime. For .org, it is nearly ready (see the private ops issue rtfd/readthedocs-ops#294)

That private issue is closed and it seems that it's already active.

I see two ways to do HTTP -> HTTPS for custom domains:

I think the solution at nginx level is better because of performance, but I'd probably like to have the data on the db also as a "trustable source".

@davidfischer
Copy link
Contributor

That private issue is closed and it seems that it's already active.

HSTS is not active. We just had a few remnants of the dashboard that were served over plain HTTP and if we enable HSTS none of the dashboard can be served over HTTP.

@AustinShalit
Copy link

Hi folks,

Just looking to see if this issue made it onto the roadmap for an upcoming release.

Thank you!

@davidfischer
Copy link
Contributor

@AustinShalit, this issue sort of has a few things going on with it. Specifically are you asking about HTTP -> HTTPS redirects for custom domains?

@AustinShalit
Copy link

Sorry about not being clear. Yes, I am talking about HTTP -> HTTPS redirects for custom domains.

@AustinShalit
Copy link

@davidfischer It has been a week. I just wanted to make sure I didn't get lost in the notification noise.

@davidfischer
Copy link
Contributor

Sorry I missed this. For custom domains, this is a somewhat complicated feature and will require an architectural change on our side. Currently serving docs on a custom domain only serves static files. However, this change necessitates making sure that the cert for the domain is successfully deployed before doing the redirect. We have hundreds of custom domains that "work" but are not configured such that we can issue a certificate for them so we can't just do a redirect for all of them without breaking lots of docs.

This is on the roadmap and should be completed this year, but it is not likely to be in a release in the next week or two.

@mrocklin
Copy link

For others, we ended up working around this by adding a JavaScript redirect from http -> https in our sphinx theme. It's not 100% perfect, but pragmatically it has been working out great.

https://github.com/dask/dask-sphinx-theme/blob/master/dask_sphinx_theme/static/js/custom.js

https://github.com/dask/dask-sphinx-theme/blob/8e25fe28390b28df7ab1c3f0e0c647f834c8a844/dask_sphinx_theme/layout.html#L6

@JLLeitschuh
Copy link

@mrocklin This still leaves you vulnerable to a downgrade attack where a potential attacker can strip our your custom JS logic and leave them in a downgraded state. Unfortunately, I'd argue that your solution gives a false sense of security when really, none is offered there.

@Fiahblade
Copy link

Any new updates on this?

@Daltz333
Copy link

Daltz333 commented Jan 3, 2020

Just an issue bump, since it was talked about being done (now last year).

@davidfischer
Copy link
Contributor

To update here, #6905 is scheduled for release next week and will solve this issue. In order to redirect, you need to make sure that the HTTPS flag is set on your custom domain.

Screen Shot 2020-04-22 at 3 29 16 PM

@AustinShalit
Copy link

Just out of curiosity, what kind of redirect was implemented?

@mrocklin
Copy link

mrocklin commented Apr 24, 2020 via email

@ericholscher
Copy link
Member

Just out of curiosity, what kind of redirect was implemented?

@AustinShalit We currently have it as a 302 to be safe. Once we're confident in the code, we'll likely switch it to a 301.

@ryanpitts
Copy link

Yesssss thank you for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Projects
None yet