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.

Sign up for GitHub

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

Jump to bottom

block-all-mixed-content directive is deprecated #372

Closed
fcrozatier opened this issue Jul 6, 2022 · 15 comments
Closed

block-all-mixed-content directive is deprecated #372

fcrozatier opened this issue Jul 6, 2022 · 15 comments
Milestone
v6.0.0

Comments

@fcrozatier
Copy link
Contributor

The block-all-mixed-content directive is deprecated and not recommended

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/block-all-mixed-content

Should we remove it ? PR#371
@EvanHahn EvanHahn added this to the v6.0.0 milestone Jul 6, 2022
@EvanHahn
Copy link
Member

EvanHahn commented Jul 6, 2022

Thanks for reporting! I'll look into this for the next major version.

@fcrozatier
Copy link
Contributor Author

Ok let me know your thoughts on this !

@EvanHahn
Copy link
Member

EvanHahn commented Jul 7, 2022

Will do. I'm busy this week but will take a look sometime after that.

@EvanHahn
Copy link
Member

I spent a little time thinking about this.

The directive is deprecated, so we should eventually remove it. But some old browsers likely still respect it, and there's probably still some security benefit there.

I'm thinking of leaving it in and revisiting this in a year.

What do you think?

@fcrozatier
Copy link
Contributor Author

Well this directive is not supported by ie so there is no benefits for really old browsers.

Also the upgrade-insecure-requests which effectively blocks the mixed content by redirecting over HTTPS has much better support and all browsers supporting the former support this one so there is no security hole.

The upgrade-insecure-requests is evaluated before the block-all-mixed-content and when set the latter is a no-op. So we should only set one of these directives.

I think that since we have set the upgrade-insecure-requests, the block-all-mixed-content directive is already ineffective so we can safely remove it.

@fcrozatier
Copy link
Contributor Author

Forgot the links :
block-all-mixed-contentnot supported by ie: https://caniuse.com/?search=block-all-mixed-content
upgrade-insecure-requests has much better support: https://caniuse.com/?search=upgrade-insecure-requests

@EvanHahn
Copy link
Member

It doesn't benefit really old browsers, but it does seem to benefit some. For example, according to Can I Use, Firefox 48–101 supports this directive and has 2.85% usage globally. Maybe it makes sense to continue to support it for awhile? Does it hurt anything, other than bandwidth?

@fcrozatier
Copy link
Contributor Author

Indeed Firefox 48-101 supports it but it also supports upgrade-insecure-requests so in this case setting both would make block-all-mixed-content a no-op according to the note here.

It doesn't hurt to keep it, but a no-op has no benefits doesn't it ? I haven't seen a browser supporting block-all-mixed-content and not upgrade-insecure-requests so it may have no effect whatsoever if both are set ?
Does it make sense to keep supporting it if it's a no-op ?

@EvanHahn
Copy link
Member

EvanHahn commented Jul 13, 2022 via email

@fcrozatier
Copy link
Contributor Author

The MDN says

The upgrade-insecure-requests directive is evaluated before block-all-mixed-content. If the former is set, the latter does nothing, so set one directive or the other – not both, unless you want to force HTTPS on older browsers that do not force it after a redirect to HTTP.

So the block-all-mixed-content directive does nothing in conjunction with upgrade-insecure-requests. The last bit is obscure to me.
What do you think ?

@EvanHahn
Copy link
Member

I'll take a look. If it's completely useless for all browsers, I want to remove it. If it's useful for some browsers, then it's a harder question.

@fcrozatier
Copy link
Contributor Author

What is your final decision ?

@EvanHahn
Copy link
Member

Sorry, I haven't had time to look into this. It's still on my list.

@EvanHahn
Copy link
Member

EvanHahn commented Aug 3, 2022

Took a look at this. We should remove the header. I'll review #371 now.

Sorry for the delay!

EvanHahn pushed a commit that referenced this issue Aug 3, 2022
@fcrozatier @EvanHahn
Content-Security-Policy: remove block-all-mixed-content
1245693 
To quote [MDN][mdn]:

> Deprecated: This feature is no longer recommended. Though some
> browsers might still support it, it may have already been removed from
> relevant web standards [...]. Avoid using it, and update existing code
> if possible [...]

See [#371][371] and [#372][372].

[mdn]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/block-all-mixed-content
[371]: #371
[372]: #372
@EvanHahn
Copy link
Member

EvanHahn commented Aug 3, 2022

block-all-mixed-content will be removed in Helmet v6. See more details in this comment: #371 (comment).

@EvanHahn EvanHahn closed this as completed Aug 3, 2022
EvanHahn pushed a commit that referenced this issue Aug 26, 2022
@fcrozatier @EvanHahn
Content-Security-Policy: remove block-all-mixed-content
3874c6b 
To quote [MDN][mdn]:

> Deprecated: This feature is no longer recommended. Though some
> browsers might still support it, it may have already been removed from
> relevant web standards [...]. Avoid using it, and update existing code
> if possible [...]

See [#371][371] and [#372][372].

[mdn]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/block-all-mixed-content
[371]: #371
[372]: #372
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
v6.0.0
Development

No branches or pull requests
2 participants
@EvanHahn @fcrozatier