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

Add option to disable HSTS for nginx (Bug #6650) #3856

Closed
wants to merge 4 commits into from

Conversation

@doktornotor
Copy link
Contributor

doktornotor commented Oct 22, 2017

As far as webconfigurator is concerned, current default behavior remains unchanged (HSTS enabled with HTTPS) if the toggle is unset.

If HSTS is desired for CP, I can add a toggle there as well. IMNSHO, it's pretty much never desired since it makes testing or changing your mind very much impossible as you have no control over the CP clients in general.

@NOYB

This comment has been minimized.

Copy link
Contributor

NOYB commented Oct 22, 2017

If HSTS is desired for CP, I can add a toggle there as well. IMNSHO, it's pretty much never desired since it makes testing or changing your mind very much impossible as you have no control over the CP clients in general.

Once a client has received the HSTS cookie you're still out of luck to change your mind. Until client clears that cookie the associated host is going to https. Regardless of any changes made to the server.
As for not meeting testing needs, customize the test environment.

For HSTS enabled clients, the proper solution is to use https services, or use a different host name for accessing non-https services.

HSTS is performing it's job very well. Better to be inconvenienced to setup services properly than to disable security features and some day find out have been going along with stuff inadvertently in the clear.

There are appropriate viable solutions to this perceived HSTS issue without this feature.

if (security.value < convenience.value) { fail(); }
@miken32

This comment has been minimized.

Copy link
Contributor

miken32 commented Oct 22, 2017

The “proper solution” is to allow users to customize the firewall as they see fit, which this patch does, and not to impose an onerous security system on users without any choice in the matter.

@NOYB

This comment has been minimized.

Copy link
Contributor

NOYB commented Oct 22, 2017

HSTS enabled clients using a different host name than the router for non-https services is hardly onerous. Plus it actually does solve the issue of changing your mind. Where as this feature does not. Just change host name.

Users can customize the firewall all they want. It's open source. Mine is highly customized with features I like and find useful.

This patch does not solve the stated problem. It merely alters and shifts it to some degree in some circumstances.

This feature to disable HSTS for sake of convenience when there are easy, appropriate, viable capabilities to set up services appropriately such that they are not affected by the routers HSTS, is misguided as a solution.

A service that is undesirably affected by the router GUI access HSTS is a poorly designed service. Use a different host name. Not the router host name.

@doktornotor

This comment has been minimized.

Copy link
Contributor Author

doktornotor commented Oct 23, 2017

There is a number of issues caused by hard-coded HSTS mentioned on the linked bug, which include captive portals, undesirable issues with pfSense packages such as HAproxy (you cannot enable HSTS globally there because you end up sending the header twice) or bandwidthd (does not support HTTPS at all, it's completely broken out-of-the-box with workarounds involving complex HAproxy setup.)

Use a different hostname doesn't cut it. The hardcoded HSTS with unconfigurable max-age of 1 year is just broken design, sorry.

On another note, HSTS does really pretty much nothing meaningful for security here. If you wanted a redirect, there's another checkbox for that already, and if you want to force people to HTTPS, there's nothing stopping you from choosing HTTPS as protocol without any redirect and move on. Actually meaningful things would include things such as nginx not binding to a wildcard (notably bad when the firewall rules get no-op - see #6028 - or 2FA for the webGUI.

@jim-p

This comment has been minimized.

Copy link
Contributor

jim-p commented Oct 23, 2017

Given the special way that HSTS gets handled by browsers, it can be too much of a good thing in certain cases. Enough cases that I think the option is worth including.

@jim-p
jim-p approved these changes Oct 23, 2017
@jim-p jim-p requested review from rbgarga and sbeaver-netgate Oct 23, 2017
@NOYB

This comment has been minimized.

Copy link
Contributor

NOYB commented Oct 24, 2017

What is broken is those packages. Accommodating broken packages prolongs their broken state and all but ensures they will never be fixed. They should not be relying on the router hostname if they cannot handle how it may be configured. Allowing them to dictate and prevent the use of HSTS by the router's GUI is broken design. Properly designed they should not be dependent on this nor require that it be disabled. They are broken! Sorry!

Ah ain't that just terrible that security feature would require complex setup. Such a shame. Any network engineer worth their salt can handle it.

If using a different hostname that the client has not associated with an HSTS cookie doesn't cut it then HSTS is not the issue.

But apparently the tradition of brokenness accommodation and convenience at expense of security is continuing, alive and well.

@jim-p

This comment has been minimized.

Copy link
Contributor

jim-p commented Oct 24, 2017

No, we merely acknowledge that we do not live in a perfect world. Outside of the lab, there are real requirements that can't always be that strictly enforced. The good thing is that you don't have to touch a thing to keep the same behavior it has now. The option doesn't hurt you, so just ignore it and move on.

@NOYB

This comment has been minimized.

Copy link
Contributor

NOYB commented Oct 24, 2017

Actually, in a way it does potentially hurt me, and others. Maybe I, and others, would like to make use of some of those broken packages if they were fixed rather than requiring HSTS to be disabled for the router's GUI. Accommodating them in their broken state practically ensures they will never be fixed. Yes that is an impact to those who would like to use them, but don't want to disable HSTS on the router's GUI to do so.

This is a like a bacteria laced band-aid that will ensure the wound doesn't heal.

@doktornotor doktornotor deleted the doktornotor:patch-11 branch Oct 27, 2017
@pfsense pfsense deleted a comment from doktornotor Oct 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.