ome / omero-web

Django-based OMERO.web client
https://www.openmicroscopy.org/omero
16 stars 29 forks source link

Add some initial CSP settings to test #527

Open will-moore opened 5 months ago

will-moore commented 5 months ago

Fixes #21.

Based initially on reading docs at https://django-csp.readthedocs.io/en/latest/configuration.html and reading https://blog.sucuri.net/2023/04/how-to-set-up-a-content-security-policy-csp-in-3-steps.html

This picks a handful of the most common settings and adds them to omeroweb.settings allowing them to be configured, and using "'self'" as the default value.

To test - page shouldn't be able to load images, scripts etc from other domains.

will-moore commented 5 months ago

This is currently breaking webclient quite badly:

Screenshot 2024-01-23 at 09 48 59

Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-gPuhVMNOoD9kBeWfIcvhcGP4SqBoRvNdtrvVU3QTY3E='), or a nonce ('nonce-...') is required to enable inline execution.

Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-qec7pPGO104C8XSkN6GBy7gMcsk5nq0j1fbKN5iWB2Q='), or a nonce ('nonce-...') is required to enable inline execution.

jQuery.Deferred exception: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self'".
 EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self'".

Uncaught ReferenceError: WEBCLIENT is not defined
    at HTMLDocument.<anonymous> (ome.tree.js?_5.24.0:432:25)

Closing for now...

will-moore commented 5 months ago

After that change, I'm seeing errors like this on the login page:

login/:90 Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self' unsafe-inline". Either the 'unsafe-inline' keyword, a hash or a nonce ('nonce-...') is required to enable inline execution....

Need to add quotes to 'unsafe-inline' too!...

will-moore commented 5 months ago

With those changes, the webclient appears usable, but we're still seeing some errors in the Console:

jstree.js:1980 Refused to create a worker from 'blob:https://merge-ci.openmicroscopy.org/5141d68a-4f7f-450c-b079-6e1506845bd1' because it violates the following Content Security Policy directive: "script-src 'self' 'unsafe-inline' 'unsafe-eval'". Note that 'worker-src' was not explicitly set, so 'script-src' is used as a fallback.

From line https://github.com/ome/omero-web/blob/9841167a70df2f0174aed2820bcd302873b53df3/omeroweb/webgateway/static/3rdparty/jquery.jstree-3.3.12/jstree.js#L1980 See https://stackoverflow.com/questions/54695310/getting-refused-to-create-worker-from-blob-error-in-video-min-js-when-looking "It means that you need explicitly add blob: data schema to default-src or worker-src" "default-src * data: 'unsafe-eval' 'unsafe-inline' blob:"

Refused to load the image 'data:image/gif;base64,R0lGODlhEAAQAPIAAP///zxKW9H...Ow==' because it violates the following Content Security Policy directive: "img-src 'self'".

Seems from https://stackoverflow.com/questions/18447970/content-security-policy-data-not-working-for-base64-images-in-chrome-28 that we need data: added to img-src

will-moore commented 5 months ago

Still seeing worker-src error message from above, and checking the Headers on merge-ci confirms that worker-src is missing:

Header:

Content-Security-Policy: img-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; default-src 'self'; base-uri 'self'

Need to check on why...?

will-moore commented 2 months ago

Now seeing this Response Header on merge-ci:

Content-Security-Policy: default-src 'self'; worker-src 'self' blob:; base-uri 'self'; img-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'

Not seeing any errors. So this looks like a good starting point for default CSP settings.

@knabar is this something we want to progress with or put on hold for now until we feel more urgency for it? Any feedback on the support for CSP in settings.py (does that look like a reasonable way to allow configuration? Any better way to do this with less code/settings)? Thx

knabar commented 2 months ago

@will-moore This looks like a good starting point, no objections to including it into the next release with the appropriate warnings to developers that certain things might need extra configuration to keep them working.

I initially thought that it could be better to have just one setting with a JSON struct for all CSP settings, but that would require more complex code to get the settings into the appropriate place in settings.py, whereas these individual settings work fine - more verbose, but a lot simpler.

So 👍 from me

will-moore commented 1 month ago

As discussed just now, need to think about docs etc and possibility of breaking users' plugins / modifications - Is this a breaking change etc before merging...

knabar commented 1 month ago

Possible issues:

cc: @sbesson @chris-allan