Skip to content

Optional HSTS support #4135

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

Closed
westurner opened this issue May 23, 2018 · 21 comments · Fixed by #6953
Closed

Optional HSTS support #4135

westurner opened this issue May 23, 2018 · 21 comments · Fixed by #6953
Labels
Accepted Accepted issue on our roadmap Improvement Minor improvement to code
Milestone

Comments

@westurner
Copy link

Details

@westurner
Copy link
Author

django.middleware.security.SecurityMiddleware supports app-wide HSTS
https://docs.djangoproject.com/en/2.0/ref/middleware/#http-strict-transport-security

https://docs.djangoproject.com/en/2.0/topics/security/#ssl-https

https://docs.djangoproject.com/en/2.0/_modules/django/middleware/security/#SecurityMiddleware

SecurityMiddleware.process_response() checks self.sts_seconds, self.sts_include_subdomains, and self.sts_preload; which are set in SecurityMiddleware.__init__.

IIUC, SecurityMiddleware then must be redfined in order to conditionally specify the HSTS HTTP header according to an RTD project's custom settings.

@westurner
Copy link
Author

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/projects/models.py

  • Project > #project features
    • hsts_enabled (Boolean?)
  • forms.py (UpdateProjectForm, ProjectAdvancedForm)
  • admin.py?

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/settings/base.py

  • MIDDLEWARE_CLASSES
    • already includes SecurityMiddleware, which we don't want to modify
      • "PerProjectHSTSMiddleware"?

@westurner
Copy link
Author

https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/core/middleware.py

SubdomainMiddleware could either?

  • include the HSTS functionality
  • pass the config through for a distinct "PerProjectHSTSMiddleware"

@davidfischer davidfischer added the Status: blocked Issue is blocked on another issue label May 23, 2018
@davidfischer
Copy link
Contributor

You have a few different ideas here and I'll attempt to address them:

HSTS on readthedocs.org (not Documentation Sites)

This should be relatively easy to add. It isn't my top priority right now but I will get around to it. This can be rolled out transparently. However, I don't really think this is what you're asking about.

HSTS on *.readthedocs.io (Documentation Sites hosted on our domain)

These typically do not hit Django whatsoever unless there's a redirect. So this would just be an nginx setting. We probably wouldn't do any per-project stuff.

Currently *.readthedocs.io has a valid wildcard certificate. We are in the process of serving all of this traffic over HTTPS. The first step went live on Monday (#3987). There are still a couple more steps here (302 redirects, then 301 redirects) but I believe they will be done in the next couple weeks.

Only once all the issues with permanent redirects are sorted out can we consider HSTS for these domains.

HSTS on Documentation Sites using custom domains

I believe this is what you want. There are a few more steps here and they are laid out in this ticket: #3282 (comment). This will involve setting up Lets Encrypt (probably a custom client) to get/store/serve/refresh certificates for ~2500 domains. It won't be trivial (see #2652). We are actively working toward this but I'd guess it's closer to 1-2 months away.

@westurner
Copy link
Author

westurner commented May 23, 2018

HSTS on *.readthedocs.io (Documentation Sites hosted on our domain)

Is it possible to create per-project nginx configs in order to make HSTS an optional per-project setting?

Why would anyone want HSTS to be optional?

  • At one time, e.g. CloudFlare couldn't proxy to GitHub Pages HTTPS-only sites; IDK if this is still the case.
  • Are there customers which cannot or do not want to support HTTPS?

HSTS on Documentation Sites using custom domains

That could be a paid feature; but people might go to other services where letsencrypt services for custom domains are free.

From @ericholscher https://twitter.com/ericholscher/status/996871554255458305 :

Has anyone written a good @letsencrypt set of Django models and Celery tasks for keeping a set of domains expire dates tracked up to date? I'm going to end up creating it if not, but I can't imagine this hasn't already been solved at this point.

@davidfischer
Copy link
Contributor

That could be a paid feature; but people might go to other services where letsencrypt services for custom domains are free.

I believe the plan is for this to be a free feature.

@davidfischer
Copy link
Contributor

davidfischer commented May 23, 2018

Are there customers which cannot or do not want to support HTTPS?

That's why we are rolling it out slowly:

  • Make all the links point to HTTPS
  • Respond with a 302 redirect when loaded over HTTP -> HTTPS
  • Respond with a 301 redirect when loaded over HTTP -> HTTPS

If there are issues, hopefully we catch them before people's browsers store permanent redirects or HSTS with long durations. Those are harder to undo.

@agjohnson agjohnson changed the title ENH,SEC: Optional HSTS support Optional HSTS support May 31, 2018
@agjohnson agjohnson added this to the 2.6 milestone May 31, 2018
@agjohnson agjohnson added the Accepted Accepted issue on our roadmap label Jun 8, 2018
@davidfischer
Copy link
Contributor

Just to give an update here, we are now redirecting to HTTPS for *.readthedocs.io. We are not yet doing that for custom domains.

@agjohnson agjohnson modified the milestones: 3.2, 3.4 Jan 25, 2019
@stsewd stsewd modified the milestones: 3.4, 3.5 Mar 27, 2019
@davidfischer
Copy link
Contributor

To give an update here:

  • We are going to enable HSTS for readthedocs.org and *.readthedocs.io. I believe there are no downsides to this at all at this point and we've taken care of all the lingering issues of various things served over plain HTTP. I'm looking to have this done by the end of May at the latest.
  • I am leaning toward NOT enabling HSTS for custom domains or if we do, only setting it with a short (sub 1 week) expiry time. The reason for this is that it is mostly irreversible and users may do this and not understand the consequences. I looked at what other hosts do and they mostly do not do this. For example, GH pages does not set HSTS on a custom domain.

@davidfischer
Copy link
Contributor

We have enabled HSTS for readthedocs.org and *.readthedocs.io with short durations. If you encounter issues, please let me know. Otherwise, we are going to increase the durations in a week or two.

@alex
Copy link
Contributor

alex commented May 5, 2019

I see there's a checkbox for " Always use HTTPS for this domain" in the admin now, but it looks like that just controls redirects.

When HSTS finally makes it to the top of the feature queue, pyca/cryptography would be happy to beta test it :-)

@astrojuanlu
Copy link
Contributor

I see there's a checkbox for " Always use HTTPS for this domain" in the admin now, but it looks like that just controls redirects.

As far as I understand, there are no redirects for custom domains yet. Or, at least, I tried with http://docs.poliastro.space and it didn't redirect.

@alex
Copy link
Contributor

alex commented May 6, 2019 via email

@davidfischer
Copy link
Contributor

davidfischer commented May 6, 2019

All that checkbox currently governs is all the "view docs" links in the RTD dashboard link to HTTPS. Eventually (this year) we will use that database entry to do HTTP -> HTTPS redirects on custom domains but it requires an architectural change. Currently, serving docs does not hit a server that connects to a database at all.

For readthedocs.org and *.readthedocs.io we always redirect HTTP -> HTTPS.

When HSTS finally makes it to the top of the feature queue, pyca/cryptography would be happy to beta test it :-)

HSTS is rolling out for readthedocs.org and *.readthedocs.io. In the next deploy the max-age bumps to a week. If there's no issues with that, we will go to a year a few weeks after that.

Currently I'm leaning toward not rolling out HSTS for custom domains at all. I'm worried that it presents users who don't understand HSTS a way to make mistakes that aren't easily reversible. I looked around at different hosting platforms that allow custom domains (eg. GitHub Pages) and they don't support HSTS with a browser header as far as I could see. If you feel there is a compelling case for it, please let me know.

@alex
Copy link
Contributor

alex commented May 6, 2019

So, for cryptography.io we currently have a reverse proxy we put in from of RTD to add TLS (predating RTD's current TLS support), and it adds HSTS. We'd love to be able to drop that in favor of just using RTD end to end, but HSTS is kind of a requirement for us.

@davidfischer
Copy link
Contributor

HSTS is kind of a requirement for us

For cryptography.io I completely understand.

I could see HSTS on custom domains being an option that we don't expose in the dashboard UX and it shouldn't be too complicated once the architectural change I mentioned above is done. I think there are definitely some instances where people really need it. I just worry about users making mistakes with it.

@davidfischer
Copy link
Contributor

Small update here: HSTS on readthedocs.org and *.readthedocs.io has been completely rolled out with a 1 year max age.

@westurner
Copy link
Author

westurner commented Aug 15, 2019 via email

@davidfischer davidfischer removed the Status: blocked Issue is blocked on another issue label Apr 23, 2020
@davidfischer
Copy link
Contributor

Now that our architectural change is in place, this is no longer blocked.

@westurner
Copy link
Author

westurner commented Apr 28, 2020 via email

@davidfischer
Copy link
Contributor

We are currently testing HSTS on certain custom domains. There is not yet a UI flag for setting this but feel free to open an issue for any specific projects/domains that need HSTS configured.

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
Development

Successfully merging a pull request may close this issue.

7 participants