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

Keyword {nonce} should cover script-src-elem in J4 System Http-Header plugin #38291

Closed
ghost opened this issue Jul 18, 2022 · 8 comments
Closed

Comments

@ghost
Copy link

ghost commented Jul 18, 2022

At the moment, many third party plugins are still not CSP friendly. They write inline <script> to the html body or use inline event handler sparingly.

Firefox does not support script-src-elem, script-src-atr nor 'unsafe-hashes'. Inline event handler, e.g. onload, requires 'unsafe-hashes' to pass.

The only definition that will work for Firefox is script-src: 'self' 'unsafe-inline'. But this defeats the whole purpose of Content Security Policy and this http-header plugin.

To have the maximum CSP protection across browsers, the following configuration is desired:
script-src: 'self' 'unsafe-inline'
script-src-elem: 'self' {nonce} and 'sha256 hashes' for inline <script>
script-src-atr: 'unsafe-hashes' 'sha256 hashes' for inline event handler.

The definition for script-src is for browsers like Firefox that does not support script-src-atr and 'unsafe-hashes'

The definition for script-src-elem and script-src-atr are for browsers like Chrome and Edge. sha256 can be calculated manually.

Unfortunately, as far as J4 4.1.5 is concerned, {nonce} keyword is applicable to only script-src and style-src.

If we can make {nonce} also applicable to script-src-elem, then we will have a short-term workaround that make the best use of http-header.
script-src-elem
script-src-elem-err

@GHSVS-de
Copy link
Contributor

GHSVS-de commented Jul 18, 2022

I don't know if relevant. Check yourself:
The {nonce} placeholder has been removed in 4.2-dev and more changed/adapted.

Issue: #37461
Pr: #37940
Pr: #37942

@dgrammatiko
Copy link
Contributor

#31485

@ghost
Copy link
Author

ghost commented Jul 19, 2022

I don't know if relevant. Check yourself: The {nonce} placeholder has been removed in 4.2-dev and more changed/adapted.

Issue: #37461 Pr: #37940 Pr: #37942

I have gone through them but I am worry that Firefox will still break because for inline event handler (onclick, onload etc), CSP requires 'unsafe-hashes' which Firefox does not support. So if nonce are generated under "script-src", any 'unsafe-inline' there will be voided and Firefox will refuse to execute those inline event handler.

To work across browsers, it should be selectable to put the nonce under "script-src-elem" for Chrome and Edge, and keep 'unsafe-inline' in "script-src". Otherwise Firefox will probably stop to run properly for J4 sites with CSP.

I have modified the module such that nonce are generated under "script-src-elem" and I do not get any compliant now from Chrome, Edge and Firefox
2022-07-19 17_57_22-Plugins_ System - HTTP Headers - K-Leaders - Administration — Mozilla Firefox
.

@zero-24
Copy link
Member

zero-24 commented Jul 19, 2022

As mentiond in the other issue we are most likely need to add the two directives to the array here and we are good to go, but i have not tested it yet https://github.com/joomla/joomla-cms/blob/4.2-dev/plugins/system/httpheaders/httpheaders.php#L119-L128

@ghost
Copy link
Author

ghost commented Jul 19, 2022

As mentiond in the other issue we are most likely need to add the two directives to the array here and we are good to go, but i have not tested it yet https://github.com/joomla/joomla-cms/blob/4.2-dev/plugins/system/httpheaders/httpheaders.php#L119-L128

That was exactly what I have done in v 4.1.5: change the directive to script-src-elem in the array. Will take a look at the code for 4.2 to see whether that will be as simple as 4.1.5.

@zero-24
Copy link
Member

zero-24 commented Jul 19, 2022

There should not be such major changes with 4.2 when that works please create an PR against 4.2-dev so it can be tested and shipped with an upcomming release.

@ghost
Copy link
Author

ghost commented Jul 20, 2022

There should not be such major changes with 4.2 when that works please create an PR against 4.2-dev so it can be tested and shipped with an upcomming release.

I am embarrassed to say I am not a qualified developer and have little experience working with git. :(

@zero-24
Copy link
Member

zero-24 commented Jul 21, 2022

PR has been created please test it: #38318 will close here so we can have the discussion on the PR.

@zero-24 zero-24 closed this as completed Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants