-
Notifications
You must be signed in to change notification settings - Fork 368
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
remove block-all-mixed-content from helmet-csp default directives #460
Comments
I keep this directive around for the legacy browsers that respect it. Do you think it should be removed despite that?
|
Ah I guess I didnt think of legacy scenarios How about adding a way of disabling it instead? I've tried with [], null, '' and it either results in an error or it gets put there anyway |
null should disable the directive. What does your code look like?
|
You are absolutely correct. Maybe I have misspelled it the first time I tried or had an error intersect while I was implementing it. Adding null removes it correctly. Regarding if it should be removed or not I had a few after thoughts: a) If it is to be kept it should be in the default directives in helmet so it matches helmet-csp to avoid any confusion, along with documentation for why it is there so developers can decide if they want to remove it or not, giving them the option to save a few additional bytes each request if they don't care about legacy browsers (which is exactly what I'm doing. Removing to save the bytes, since my website wouldn't work with a legacy browser anyway) b) The standard today is https and if you're fiddling with content security policy chances are you have HSTS enabled as well, which means you will only be vulnerable the first visit to the site anyway, and not at all if preload has been set. c) The policy that replaces block-all-mixed-content, upgrade-insecure-requests, was implemented in all major browser in around 2015-2018 depending on the developer, so the amount of vulnerable browsers shouldn't be that high. Combined with HSTS settings only edge cases should remain. It is up to you to do keep it or not. If I can disable it it doesn't matter to me wether it is there or not. If I read some of your stack overflow replies correctly I understand you want helmet to be as easy to setup and use as possible, so in the end it is a question of would you be removing it to save bandwidth more often than not |
This is helpful context. I'll think about it and consider removing it in the next major version. |
👍 for removing from defaults (for both helmet & helmet-csp) and eventually adding a note to add legacy "block-all-mixed-content" directive |
block-all-mixed-content has been deprecated as detailed here https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/block-all-mixed-content and is basically build into the Content-Security-Policy
while its not doing anything as chrome is ignoring the directive it just bothers me slightly that its there
The text was updated successfully, but these errors were encountered: