search

slackhq / csp-html-webpack-plugin

A plugin which, when combined with HTMLWebpackPlugin, adds CSP tags to the HTML output.
MIT License
164 stars 39 forks source link

Generate hashes for external scripts and styles #87

Closed sersorrel closed 2 years ago

sersorrel commented 3 years ago

Summary

CSP 3.0 permits hashes in the CSP to match external scripts. This PR generates those hashes for scripts that Webpack knows about (option 2 in https://github.com/slackhq/csp-html-webpack-plugin/issues/50#issuecomment-570307617).

NB: as I understand it, it's required that external scripts have an integrity attribute in order to be permitted via CSP hash, so this is only useful in conjunction with something like webpack-subresource-integrity.

Fixes #50 (see also #35).

Things I'd particularly like feedback on:

Requirements (place an x in each [ ])

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 3 years ago

Codecov Report

Merging #87 (bbc072a) into master (d4cf1dd) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #87   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          108       154   +46     
  Branches        19        28    +9     
=========================================
+ Hits           108       154   +46
Impacted Files Coverage Δ
test-utils/webpack-helpers.js 100.00% <ø> (ø)
plugin.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d4cf1dd...bbc072a. Read the comment docs.

AnujRNair commented 3 years ago

Thanks for this contribution, and sorry for the delay in getting back to you! I plan on reviewing this over the next week, and will post comments in the PR :)

sersorrel commented 3 years ago

Thanks for the review! It might be a while before I get time to update this PR, sorry – I have a bunch of university deadlines in the next few weeks.

sersorrel commented 3 years ago

still lots of uni work, but here are some changes :)

(argh, now I remember I forgot to run eslint, let me fix those problems now...)

Nantris commented 2 years ago

Any movement on this?

@AnujRNair would this be mergable if the requested changes were made?

@melloware I know you were also interested.

melloware commented 2 years ago

I am 100% interested in this. In fact so interested I am considering forking this lib and applying this patch and releasing it to NPM. I don't want to but if I have no other choice I might do it.

sersorrel commented 2 years ago

it's been a while since I wrote this, but as far as I remember, I addressed all the feedback to the best of my ability. if more changes are needed, I'd prefer to avoid needing to test them myself, if possible – I no longer have a development environment set up for the project I was testing this on. (I'd be entirely happy for someone else to take the existing work and get it into a mergeable state.)

melloware commented 2 years ago

OK I forked this repo and published it to NPM including these changes.

GitHub: https://github.com/melloware/csp-webpack-plugin

NPM: https://www.npmjs.com/package/@melloware/csp-webpack-plugin

@Slapbox I would much appreciate you giving it a try. I am seeing one weird issue where my JS code integrity hash is not the same as Google is Calculating. I am using Create React App so I have a feeling the JS is being changed somehow AFTER the plugin has calculated the integrity hash.

Edit: I fixed the integrity issues by switching internally to this plugin: https://www.npmjs.com/package/webpack-subresource-integrity

Everything seems to be working great for my projects now!

anacierdem commented 2 years ago

I have also found another issue with this change. HtmlWebpackPlugin is using PROCESS_ASSETS_STAGE_OPTIMIZE_INLINE processAssets stage which runs before PROCESS_ASSETS_STAGE_OPTIMIZE_HASH of the RealContentHashPlugin which is enabled by default on production builds. It effectively replaces hashes in content after the csp hashes are generated, making them stale. So this will not work correctly on production builds.

melloware commented 2 years ago

@anacierdem you can try my custom plugin if you want this is all working properly. https://www.npmjs.com/package/@melloware/csp-webpack-plugin

anacierdem commented 2 years ago

We have already developed an alternative fork also fixing Firefox CSP-3 issues, which is mostly specific to our use case at the moment. Thank you for the heads up!

melloware commented 2 years ago

Can you shed some light on your Firefox CSP issues?

anacierdem commented 2 years ago

Basically it behaves like it is unable to calculate the same hash for scripts with src and integrity when strict-dynamic is enabled for script-src. See the long running Firefox bug here. Inline scripts without an integrity work fine so we are using an inline loader script to load external resources as a workaround until it is fixed.

melloware commented 2 years ago

Basically it behaves like it is unable to calculate the same hash for scripts with src and integrity when strict-dynamic is enabled for script-src. See the long running Firefox bug here. Inline scripts without an integrity work fine so we are using an inline loader script to load external resources as a workaround until it is fixed.

@anacierdem that is weird I am using my version of the plugin and Firefox is working fine with the integrity hashes and strict-dynamic??? I am using Firefox 96.0 (64 bit). ???

EDIT: Nevermind I use NONCE's for scripts and Hashes for Integrity so it works fine. Its only a bug if you use HASH's and not NONCE's.

sersorrel commented 2 years ago

I don't have the bandwidth or the desire to make the suggested changes. Anyone should feel free to modify this PR and resubmit it, the code is MIT-licensed.