-
Notifications
You must be signed in to change notification settings - Fork 166
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
Enable HSTS on website #2857
Comments
Do we have a sense of how risky adding this would be? Do we have metrics to determine if traffic that we're handling per subdomain is mostly http or https? |
Do we have any subdomain that does something other than redirecting to https when it's accessed by http? |
AFAIK only http://unencrypted.nodejs.org, but if you go there it says
Looks like the (already past) 2022 date was already an extension from 2020-01-01 #55 (comment) (cc @jbergstroem). There was draft communication around a plan to enable HSTS from 2016 but I don't think it ever got published: #484. |
I'm not quite sure I understand the concerns. AFAIK, HSTS does not prevent clients from using HTTP. In fact, clients should ignore HSTS response headers when making HTTP requests. Only when a client is connected over HTTPS already, HSTS tells the client to keep using HTTPS instead of HTTP in the future. We are already redirecting to HTTPS on the main domain (via |
This is what can be problematic (assuming we enable it for subdomains). After a client has accessed |
That's entirely optional though, and if we have any non-HTTPS subdomains, we can just secure the other subdomains. |
@rvagg and @jbergstroem do either of you two remember why we did not enable HSTS when we last discussed it. |
@nodejs/version-management is there any concern that turning on HSTS (which may affect non-encrypted downloads from the main website)? |
@mhdawson - I can only speak for NVM for Windows, but I can't think of a reason why this shouldn't be done. NVM4W has an optional flag to ignore certificate verification, which is primarily for people hosting their own in-house mirrors. Users cannot download from nodejs.org over a non-encrypted connection, unless they have some sort of custom proxy. |
@mhdawson which past concerns are listed there? |
The only thing I can add is that its hard to cover every scenario you are not fully aware of and HSTS is a one way street. You enable it and there is not a way back. |
Is there any way to enable it in general, but NOT on |
As far as I know you enable it per domain (say, Edit: the idea of HSTS is really to blanket your domain with https. If you need nuance you can always pass HSTS as part of serving requests per domain (instead of having a provider like cloudflare lazy it for you which is probably great for 99% of their users). |
@ljharb from what I remember reading the prior issue and some discussion it was around old versions of nvm that could not handle https for downloads. That was 2016, so those versions would be really old now. |
Yes, the |
@mhdawson due to a zsh bug triggered by the list of node versions getting long enough, many users on versions of nvm that old have already been broken, so i think that particular concern is gone. |
It's unlikely that automated tools keep a database of HSTS-enabled websites, though. I'm not sure the technology really makes sense for clients other than browsers. |
Speaking for |
@targos yeah if curl and wget and friends don’t care about hsts, then there’d be no risk |
Looks like
( |
afaik the reason we've continued to punt on this is the /dist/ access for older tooling; but we've also said we needed to have a limit on that backward compatibility - although we probably didn't invest in the type of comms needed to just pull the plug (I can't recall if we ever published blog posts on this with timelines, we intended to) |
We could do something like I've seen GitHub does: |
it was maybe 4 years ago that we had basically that plan .. disable for a weekend in December, then for a week, then just yank it, but we never pulled the trigger iirc someone has to coordinate and execute this, that was the holdup, we were all too busy with other things to make this a priority |
In a recent discussion with @richardlau, @BethGriggs and @sxa I think we figured out that when we put Cloudflare in front of our downloads we effectively forced SSL. This means that today you cannot download without SSL from nodejs.org. We also discussed that HSTS only adds a header and anything that understands the header will also support SSL. Given those two (and mostly that we already force SSL) I'm thinking we should pretty safe turning on HSTS in Cloudflare for nodejs.org and then later deciding if we do it in the webserver itself as well. I think we could
|
sgtm, how far in advance should we do the blog/banner? |
@mmarchini I was thinking at least 1 week but open to other suggestions. |
|
I don't think a blog post is necessary if we're not turning this on for subdomains. I believe we've already made a redirect for all resources on http go to https, so there's not a way to access anything other than redirects via http (order matters here, there's nothing above rule 4 that would prevent this): It used to be enabled for /dist/ only when we first switched, this is probably still enabled in nginx, but as per those rules in CF we're even doing it for that now, and probably have been for some time:
I say we just turn it on without a blog post, we just don't have much to say ("hey, we're turning this on, it's not going to impact you though, so cool"). |
I agree with @rvagg. However, just to make sure this doesn't get stuck on a back-and-forth about a blog post, I'm going to write one up anyway. I say that whoever is going to actually do the work (which I guess is "check a box" or something like that?) to enable HSTS can make the call on publishing the blog post or else closing the pull request. |
Should we close this? |
This was discussed a while back and we had done what was planned at the time in #516.
However, seems like we stopped short of enabling HSTS. It's been reported to the project and opening this issue to discuss in the build team and see if we can do it now.
The text was updated successfully, but these errors were encountered: